linux-kernel - Re: [PATCH v4] vfio/pci: Propagate ACPI notifications to user-space via eventfd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH76GKN+hC0w_KZjmHkPfYjp+32ZmqWR3yRZasCjygEHPT8Puw@mail.gmail.com>
Date:   Fri, 9 Jun 2023 14:32:50 +0200
From:   Grzegorz Jaszczyk <jaz@...ihalf.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     linux-kernel@...r.kernel.org, dmy@...ihalf.com, tn@...ihalf.com,
        dbehr@...gle.com, dbehr@...omium.org, upstream@...ihalf.com,
        dtor@...gle.com, jgg@...pe.ca, kevin.tian@...el.com,
        cohuck@...hat.com, abhsahu@...dia.com, yishaih@...dia.com,
        yi.l.liu@...el.com, kvm@...r.kernel.org, libvir-list@...hat.com,
        pmorel@...ux.ibm.com, borntraeger@...ux.ibm.com,
        mjrosato@...ux.ibm.com
Subject: Re: [PATCH v4] vfio/pci: Propagate ACPI notifications to user-space
 via eventfd

czw., 8 cze 2023 o 00:27 Alex Williamson <alex.williamson@...hat.com>
napisał(a):
>
> On Wed, 7 Jun 2023 22:22:12 +0200
> Grzegorz Jaszczyk <jaz@...ihalf.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