On 7/16/2025 10:57 AM, Daniele Ceraolo Spurio wrote:
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.ceraolospu...@intel.com>
Cc: Alexander Usyskin <alexander.usys...@intel.com>
Cc: Greg Kroah-Hartman <gre...@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?
Any more feedback on this? Should I just go ahead and switch to
returning the PCI ID list?
Thanks,
Daniele
+}
+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
