linux-kernel - RE: [PATCH v6 02/10] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CY5PR11MB62115F7951B6045CF254B6A19542A@CY5PR11MB6211.namprd11.prod.outlook.com>
Date: Fri, 4 Jul 2025 12:21:42 +0000
From: "Gupta, Anshuman" <anshuman.gupta@...el.com>
To: Greg KH <gregkh@...uxfoundation.org>, "Nilawar, Badal"
	<badal.nilawar@...el.com>
CC: "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>, "Vivi,
 Rodrigo" <rodrigo.vivi@...el.com>, "Usyskin, Alexander"
	<alexander.usyskin@...el.com>, "Ceraolo Spurio, Daniele"
	<daniele.ceraolospurio@...el.com>
Subject: RE: [PATCH v6 02/10] mei: late_bind: add late binding component
 driver



> -----Original Message-----
> From: Greg KH <gregkh@...uxfoundation.org>
> Sent: Friday, July 4, 2025 5:31 PM
> To: Nilawar, Badal <badal.nilawar@...el.com>
> Cc: intel-xe@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org; linux-
> kernel@...r.kernel.org; Gupta, Anshuman <anshuman.gupta@...el.com>;
> Vivi, Rodrigo <rodrigo.vivi@...el.com>; Usyskin, Alexander
> <alexander.usyskin@...el.com>; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@...el.com>
> Subject: Re: [PATCH v6 02/10] mei: late_bind: add late binding component
> driver
> 
> On Fri, Jul 04, 2025 at 05:18:46PM +0530, Nilawar, Badal wrote:
> >
> > On 04-07-2025 16:04, Greg KH wrote:
> > > On Fri, Jul 04, 2025 at 03:59:40PM +0530, Nilawar, Badal wrote:
> > > > On 04-07-2025 10:44, Greg KH wrote:
> > > > > On Fri, Jul 04, 2025 at 01:00:58AM +0530, Badal Nilawar wrote:
> > > > > > From: Alexander Usyskin <alexander.usyskin@...el.com>
> > > > > >
> > > > > > Add late binding component driver.
> > > > > > It allows pushing the late binding configuration from, for
> > > > > > example, the Xe graphics driver to the Intel discrete graphics card's
> CSE device.
> > > > > >
> > > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
> > > > > > Signed-off-by: Badal Nilawar <badal.nilawar@...el.com>
> > > > > > Reviewed-by: Anshuman Gupta <anshuman.gupta@...el.com>
> > > > > > ---
> > > > > >    drivers/misc/mei/Kconfig                    |   1 +
> > > > > >    drivers/misc/mei/Makefile                   |   1 +
> > > > > >    drivers/misc/mei/late_bind/Kconfig          |  13 +
> > > > > >    drivers/misc/mei/late_bind/Makefile         |   9 +
> > > > > >    drivers/misc/mei/late_bind/mei_late_bind.c  | 272
> > > > > > ++++++++++++++++++++
> > > > > Why do you have a whole subdir for a single .c file?  What's
> > > > > wrong with just keepign it in drivers/misc/mei/ ?
> > > > There is separate subdir for each component used by i915/xe, so
> > > > one was created for late_bind as well. Should we still drop late_bind
> subdir?
> > > >
> > > > cd drivers/misc/mei/
> > > >        gsc_proxy/ hdcp/      late_bind/ pxp/
> > > For "modules" that are just a single file, yeah, that's silly, don't
> > > do that.
> > Another reason to maintain the sub_dir is to accommodate additional
> > files for future platforms. If you still insist, I'll remove the sub_dir.
> 
> Move files around when it happens, for now, it's silly and not needed.
> 
> > > > > > + * @payload_size: size of the payload data in bytes
> > > > > > + * @payload: data to be sent to the firmware  */ struct
> > > > > > +csc_heci_late_bind_req {
> > > > > > +	struct mkhi_msg_hdr header;
> > > > > > +	u32 type;
> > > > > > +	u32 flags;
> > > > > > +	u32 reserved[2];
> > > > > > +	u32 payload_size;
> > > > > As these cross the kernel boundry, they should be the correct
> > > > > type (__u32), but really, please define the endiness of them
> > > > > (__le32) and use the proper macros for that.
> > > > If we go with __le32 then while populating elements of structure
> > > > csc_heci_late_bind_req  I will be using cpu_to_le32().
> > > >
> > > > When mapping the response buffer from the firmware with struct
> > > > csc_heci_late_bind_rsp, there's no need to use le32_to_cpu() since
> > > > the response will already be in little-endian format.
> > > How do you know?  Where is that defined?  Where did the conversion
> > > happen?
> >
> > Sorry, I got confused. Conversion is needed when assigning the
> > response structure elements.
> >
> > e.g ret = (int)(le32_to_cpu)rsp.status;
> 
> But these are read directly from the hardware?  If not, why are they marked as
> packed?
Yes, these are read from firmware, that is the reason they marked as __packed.
IMHO, don't we need change the explicit endianness of response status to address your comment.
Are we missing something here?

Thanks,
Anshuman
> 
> thanks,
> 
> greg k-h