czw., 8 cze 2023 o 00:27 Alex Williamson <alex.williamson@redhat.com> napisał(a):
On Wed, 7 Jun 2023 22:22:12 +0200 Grzegorz Jaszczyk <jaz@semihalf.com> wrote:
Can we drop the NTFY and just use VFIO_PCI_ACPI_IRQ_INDEX?
ACPI_IRQ at first glance could be confused with SCI, which is e.g. registered as "acpi" irq seen in /proc/interrupts, maybe it is worth keeping NTFY here to emphasise the "Notify" part?
Please let me know if you prefer VFIO_PCI_ACPI_IRQ_INDEX or VFIO_PCI_ACPI_NTFY_IRQ_INDEX taking into account the above.
This is a device level ACPI interrupt, so it doesn't seem like it would be confused with SCI. What other ACPI related interrupts would a device have? I'm still partial to dropping the NTFY but if you're attached to it, let's not abbreviate it, make it NOTIFY and do the same for function names.
Ok, I will use VFIO_PCI_ACPI_IRQ_INDEX then.
...
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) { + u32 notification_val; + + if (!count) + return -EINVAL; + + notification_val = *(u32 *)data;
DATA_BOOL is defined as a u8, and of course also as a bool, so we expect only zero/non-zero. I think a valid interpretation would be any non-zero value generates a device check notification value.
Maybe it would be helpful and ease testing if we could use u8 as a notification value placeholder so it would be more flexible? Notification values from 0x80 to 0xBF are device-specific, 0xC0 and above are reserved for definition by hardware vendors for hardware specific notifications and BTW in practice I didn't see notification values that do not fit in u8 but even if exist we can limit to u8 and gain some flexibility anyway. Please let me know what you think.
Does the above seem ok for you?
The data type is only a u8 for practicality, it's still labeled as a bool which suggests it's interpreted as either zero or non-zero. We also need to reconcile DATA_NONE, which should trigger the interrupt, but with an implicit notification value. I see the utility in what you're proposing, but it logically implies an extension of the SET_IRQS ioctl for a new data type which has hardly any practical value. Thanks,
Ok I will generate device check notification value as you earlier suggested. Thank you, Grzegorz