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: <2025091805-sedan-elongated-40cf@gregkh>
Date: Thu, 18 Sep 2025 18:27:16 +0200
From: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To: Lucas De Marchi <lucas.demarchi@...el.com>
Cc: "Usyskin, Alexander" <alexander.usyskin@...el.com>,
	"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>,
	"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 Fri, Sep 12, 2025 at 12:19:21AM -0500, Lucas De Marchi wrote:
> 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?

No objection from me:

Acked-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>