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

[Daniele Ceraolo Spurio]

On 7/16/2025 9:49 AM, Greg Kroah-Hartman wrote:
> 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 :)

What we want to do from the GFX driver POV can be summarized as follows: 
if at the time we try to initialize PXP the CSME PCI device is not 
present, we don't initialize PXP and continue without it.

We don't want to support hot (re-)plug of CSME, so we don't care if the 
state changes after we've checked (or it has already changed by the time 
we act on it):
- if CSME was not there at check-time and re-appears later, we'll still 
continue without PXP support.
- if CSME was there at check-time and then disappears, we'll handle it 
as described above.

IMO this function covers this use-case, but maybe it's too narrow a 
use-case for it to be a generic export in the mei driver?
Would it be cleaner if I replaced this with a function to export the PCI 
ID list, and then ran the pci_dev_present check inside i915, so it 
doesn't matter if it only covers our narrow use-case?

>
>>>> +}
>>>> +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.

Sure, will switch.

Thanks,
Daniele

>
> thanks,
>
> greg k-h