[PATCH 1/2] mei: me: Add exported function to check ME device availabiliy

[Greg Kroah-Hartman]

On Wed, Jul 16, 2025 at 09:38:09AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 7/15/2025 10:10 PM, Greg Kroah-Hartman wrote:
> > On Tue, Jul 15, 2025 at 04:00:01PM -0700, Daniele Ceraolo Spurio wrote:
> > > The intel GFX drivers (i915/xe) interface with the ME device for some of
> > > their features (e.g. PXP, HDCP) via the component interface. Given that
> > > the MEI device can be hidden by BIOS/Coreboot, the GFX drivers need a
> > > way to check if the device is available before attempting to bind the
> > > component, otherwise they'll go ahead and initialize features that will
> > > never work.
> > > The simplest way to check if the device is available is to check the
> > > available devices against the PCI ID list of the mei_me driver. To avoid
> > > duplication of the list, the function to do such a check is added to
> > > the mei_me driver and exported so that the GFX driver can call it
> > > directly.
> > > 
> > > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > > Cc: Alexander Usyskin <alexander.usyskin at intel.com>
> > > Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> > > ---
> > >   drivers/misc/mei/pci-me.c | 17 +++++++++++++++++
> > >   include/linux/mei_me.h    | 20 ++++++++++++++++++++
> > >   2 files changed, 37 insertions(+)
> > >   create mode 100644 include/linux/mei_me.h
> > > 
> > > diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
> > > index 3f9c60b579ae..16e9a11eb286 100644
> > > --- a/drivers/misc/mei/pci-me.c
> > > +++ b/drivers/misc/mei/pci-me.c
> > > @@ -18,6 +18,7 @@
> > >   #include <linux/pm_runtime.h>
> > >   #include <linux/mei.h>
> > > +#include <linux/mei_me.h>
> > >   #include "mei_dev.h"
> > >   #include "client.h"
> > > @@ -133,6 +134,22 @@ static const struct pci_device_id mei_me_pci_tbl[] = {
> > >   MODULE_DEVICE_TABLE(pci, mei_me_pci_tbl);
> > > +/**
> > > + * mei_me_device_present - check if an ME device is present on the system
> > > + *
> > > + * Other drivers (e.g., i915, xe) interface with the ME device for some of their
> > > + * features (e.g., PXP, HDCP). However, the ME device can be hidden by
> > > + * BIOS/coreboot, so this function offers a way for those drivers to check if
> > > + * the device is available before attempting to interface with it.
> > > + *
> > > + * Return: true if an ME device is available, false otherwise
> > > + */
> > > +bool mei_me_device_present(void)
> > > +{
> > > +	return pci_dev_present(mei_me_pci_tbl);
> > And what happens if the device goes away right after you call this?
> 
> Both intel graphics drivers do already handle the device being missing or
> going away, it's just not very clean. Behavior changes based on when this
> happens:
> 
> - if the device is never there or disappears before the component binds, we
> timeout waiting for the bind
> - if the device disappears after the bind, we handle the case as part of the
> component unbind call
> 
> The timeout is quite long and can therefore impact/delay userspace, so the
> aim here is to mitigate that impact in the case where the device is just
> never there, which is easy to check and more likely to happen compared to
> the device going away after initially being there.
> 
> Should I add a note about the device going away to the function doc?
> Something like "Callers are still expected to handle the case where the
> device goes away after this check is performed".

My point is that calls like this are pointless without a lock, as the
state that you think the device is in (which this function returns), is
just a lie as it could have already changed.

So look into what you are really wanting to do here, as what this
function does is not what you think it is doing :)

> > > +}
> > > +EXPORT_SYMBOL(mei_me_device_present);
> > EXPORT_SYMBOL_GPL()?  I have to ask, sorry.
> 
> The non-GPL version seemed more appropriate to me because I didn't think
> calling this function qualified as "derivative work", but I don't really
> care either way because both i915 and Xe are part of the kernel and
> GPL-compatible, so I can switch to the GPL version.

All other mei_* exports are EXPORT_SYMBOL_GPL(), please don't break that
pattern without a whole lot of documentation and reasons to back it up.

thanks,

greg k-h