linux-kernel - Re: [PATCH v9 2/9] mei: late_bind: add late binding component driver

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: <7l4emr357ugep2brz67d6mxgudxqpgfkf3gyhr5cp6i6uhku42@jhgdf4cujbis>
Date: Fri, 12 Sep 2025 00:19:21 -0500
From: Lucas De Marchi <lucas.demarchi@...el.com>
To: "Usyskin, Alexander" <alexander.usyskin@...el.com>
CC: "Nilawar, Badal" <badal.nilawar@...el.com>,
	"intel-xe@...ts.freedesktop.org" <intel-xe@...ts.freedesktop.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Gupta,
 Anshuman" <anshuman.gupta@...el.com>, "Vivi, Rodrigo"
	<rodrigo.vivi@...el.com>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>, "Ceraolo Spurio, Daniele"
	<daniele.ceraolospurio@...el.com>, "mika.westerberg@...ux.intel.com"
	<mika.westerberg@...ux.intel.com>, "Poosa, Karthik" <karthik.poosa@...el.com>
Subject: Re: [PATCH v9 2/9] mei: late_bind: add late binding component driver

On Tue, Sep 09, 2025 at 09:43:02AM -0500, Lucas De Marchi wrote:
>On Tue, Sep 09, 2025 at 04:50:41AM +0000, Usyskin, Alexander wrote:
>>>>+static int mei_lb_component_match(struct device *dev, int subcomponent,
>>>>+				  void *data)
>>>>+{
>>>>+	/*
>>>>+	 * This function checks if requester is Intel %PCI_CLASS_DISPLAY_VGA
>>>or
>>>>+	 * %PCI_CLASS_DISPLAY_OTHER device, and checks if the requester is
>>>the
>>>>+	 * grand parent of mei_if i.e. late bind MEI device
>>>>+	 */
>>>>+	struct device *base = data;
>>>>+	struct pci_dev *pdev;
>>>>+
>>>>+	if (!dev)
>>>>+		return 0;
>>>>+
>>>>+	if (!dev_is_pci(dev))
>>>>+		return 0;
>>>>+
>>>>+	pdev = to_pci_dev(dev);
>>>>+
>>>>+	if (pdev->vendor != PCI_VENDOR_ID_INTEL)
>>>>+		return 0;
>>>>+
>>>>+	if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) &&
>>>>+	    pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
>>>
>>>this doesn't seem right, we should allow other PCI classes. AFAICS this
>>>check could just be removed and just leave the INTEL_COMPONENT_LB below
>>>to protect for component match
>>>
>>>Lucas De Marchi
>>>
>>
>>The subcomponent is unique only in its own instance of the component framework.
>>Or I'm wrong here?
>>We have to ensure that we have Intel display device, not any other device to
>>subcomponent check to work correctly.
>
>We are matching by child-parent relationship + the component id. So you
>have both the mei device and another pci device that added that specific
>subcomponent and both need to have a common parent. Thinking about
>another device that would match the parent-child relationship:  audio,
>but audio doesn't add that.
>
>what scenario would cause a false match that I'm not seeing?

so, I doesn't seem it would fail any, but it's fine as a sanity check.
This is in fact very similar to mei_pxp_component_match(). If we are
going to remove the display check, it could be done later on top, making
sure not to match what it shouldn't.

So, this looks good to me. I tested this on a Battlemage card
it's correclty loading the firmware:

	xe 0000:03:00.0: [drm:xe_late_bind_init [xe]] Request late binding firmware xe/fan_control_8086_e20b_8086_1100.bin
	xe 0000:03:00.0: [drm] Using fan_control firmware from xe/fan_control_8086_e20b_8086_1100.bin version 203.0.0.0
	...
	mei_lb xe.mei-gscfi.768-e2c2afa2-3817-4d19-9d95-06b16b588a5d: bound 0000:03:00.0 (ops xe_late_bind_component_ops [xe])
	xe 0000:03:00.0: [drm:xe_late_bind_work [xe]] Load fan_control firmware
	xe 0000:03:00.0: [drm:xe_late_bind_work [xe]] Load fan_control firmware successful


Reviewed-by: Lucas De Marchi <lucas.demarchi@...el.com>

Greg, does this look ok to you now for us to merge through drm?

thanks
Lucas De Marchi

>
>Lucas De Marchi