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:
 <SA1PR12MB8947279143D868E39D30F616817CA@SA1PR12MB8947.namprd12.prod.outlook.com>
Date: Fri, 20 Jun 2025 11:03:02 +0000
From: "Datta, Shubhrajyoti" <shubhrajyoti.datta@....com>
To: "Ghannam, Yazen" <Yazen.Ghannam@....com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>, "git (AMD-Xilinx)"
	<git@....com>, Krzysztof Kozlowski <krzk@...nel.org>, Rob Herring
	<robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Borislav Petkov
	<bp@...en8.de>, Tony Luck <tony.luck@...el.com>, James Morse
	<james.morse@....com>, Mauro Carvalho Chehab <mchehab@...nel.org>, Robert
 Richter <rric@...nel.org>, "Gupta, Nipun" <Nipun.Gupta@....com>, "Agarwal,
 Nikhil" <nikhil.agarwal@....com>
Subject: RE: [PATCH v7 2/5] cdx: Export Symbols for MCDI RPC and
 Initialization

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Datta, Shubhrajyoti
> Sent: Monday, June 16, 2025 5:51 PM
> To: Ghannam, Yazen <Yazen.Ghannam@....com>
> Cc: linux-kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-
> edac@...r.kernel.org; git (AMD-Xilinx) <git@....com>; Krzysztof Kozlowski
> <krzk@...nel.org>; Rob Herring <robh@...nel.org>; Conor Dooley
> <conor+dt@...nel.org>; Borislav Petkov <bp@...en8.de>; Tony Luck
> <tony.luck@...el.com>; James Morse <james.morse@....com>; Mauro
> Carvalho Chehab <mchehab@...nel.org>; Robert Richter <rric@...nel.org>;
> Gupta, Nipun <Nipun.Gupta@....com>; Agarwal, Nikhil
> <nikhil.agarwal@....com>
> Subject: RE: [PATCH v7 2/5] cdx: Export Symbols for MCDI RPC and Initialization
>
>
>
> > -----Original Message-----
> > From: Ghannam, Yazen <Yazen.Ghannam@....com>
> > Sent: Saturday, June 14, 2025 1:41 AM
> > To: Datta, Shubhrajyoti <shubhrajyoti.datta@....com>
> > Cc: linux-kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-
> > edac@...r.kernel.org; git (AMD-Xilinx) <git@....com>; Krzysztof
> > Kozlowski <krzk@...nel.org>; Rob Herring <robh@...nel.org>; Conor
> > Dooley <conor+dt@...nel.org>; Borislav Petkov <bp@...en8.de>; Tony
> > Luck <tony.luck@...el.com>; James Morse <james.morse@....com>; Mauro
> > Carvalho Chehab <mchehab@...nel.org>; Robert Richter
> > <rric@...nel.org>; Gupta, Nipun <Nipun.Gupta@....com>; Agarwal, Nikhil
> > <nikhil.agarwal@....com>
> > Subject: Re: [PATCH v7 2/5] cdx: Export Symbols for MCDI RPC and
> > Initialization
> >
> > On Thu, May 29, 2025 at 12:30:14PM +0530, Shubhrajyoti Datta wrote:
> > > The cdx_mcdi_init, cdx_mcdi_process_cmd, and cdx_mcdi_rpc functions
> > > are needed by VersalNET EDAC modules that interact with the MCDI
> > > (Management Controller Direct Interface) framework. These functions
> > > facilitate communication between different hardware components by
> > > enabling command execution and status management.
> > >
> > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@....com>
> > > ---
> > >
> > > Changes in v7:
> > > - Add the kernel doc description
> > > - Add the prototype from first patch to here
> > >
> > > Changes in v6:
> > > - Update commit description
> > >
> > > Changes in v2:
> > > - Export the symbols for module compilation
> > >
> > >  drivers/cdx/controller/mcdi.c | 29 +++++++++++++++++++++++++++++
> > >  include/linux/cdx/mcdi.h      |  6 ++++++
> >
> > You've added the function prototypes to this new global header.
> >
> > But you didn't remove them from the local header.
> >   drivers/cdx/controller/mcdi.h
> >
> > Also, you haven't included the new global header in the cdx/controller code.
> >
> > Even though this does compile, it doesn't seem proper.
> >
> > I expect you would want to do the following:
> >
> > 1) Add the common code to the global header.
> > 2) Remove the common code from the local header.
> > 3) Include the global header everywhere the common code is needed.
> >
> > Keeping the diff below for reference.
>

I plan to have the current (in the cdx) mcdi.h to mcdid.h and move the common parts to
Include/linux/cdx/mcdi.h

And for the bitfield.h will move to include/linux/cdx/bitfield.h as most of it is needed
by others


I plan to rename the existing mcdi.h under cdx/controller/ to mcdid.h.
The common prototypes and definitions will be moved to the new global header at
include/linux/cdx/mcdi.h.

I am using most of (close to 90/95 percent) the bitfield.h, I'll move it to include/linux/cdx/bitfield.h to
 make it accessible for other subsystems.

Let me know if that sounds good.
>
> >
> > Thanks,
> > Yazen
> >
> > >  2 files changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/cdx/controller/mcdi.c
> > > b/drivers/cdx/controller/mcdi.c index e760f8d347cc..f3cca4c884ff
> > > 100644
> > > --- a/drivers/cdx/controller/mcdi.c
> > > +++ b/drivers/cdx/controller/mcdi.c
> > > @@ -99,6 +99,19 @@ static unsigned long cdx_mcdi_rpc_timeout(struct
> > cdx_mcdi *cdx, unsigned int cmd
> > >           return cdx->mcdi_ops->mcdi_rpc_timeout(cdx, cmd);  }
> > >
> > > +/**
> > > + * cdx_mcdi_init - Initialize MCDI (Management Controller Driver
> > > +Interface) state
> > > + * @cdx: NIC through which to issue the command
> > > + *
> > > + * This function allocates and initializes internal MCDI structures
> > > +and resources
> > > + * for the CDX device, including the workqueue, locking primitives,
> > > +and command
> > > + * tracking mechanisms. It sets the initial operating mode and
> > > +prepares the device
> > > + * for MCDI operations.
> > > + *
> > > + * Return:
> > > + * * 0        - on success
> > > + * * -ENOMEM  - if memory allocation or workqueue creation fails
> > > +*/
> > >  int cdx_mcdi_init(struct cdx_mcdi *cdx)  {
> > >   struct cdx_mcdi_iface *mcdi;
> > > @@ -128,6 +141,7 @@ int cdx_mcdi_init(struct cdx_mcdi *cdx)
> > >  fail:
> > >   return rc;
> > >  }
> > > +EXPORT_SYMBOL_GPL(cdx_mcdi_init);
> > >
> > >  void cdx_mcdi_finish(struct cdx_mcdi *cdx)  { @@ -553,6 +567,19 @@
> > > static void cdx_mcdi_start_or_queue(struct cdx_mcdi_iface *mcdi,
> > >                   cdx_mcdi_cmd_start_or_queue(mcdi, cmd);  }
> > >
> > > +/**
> > > + * cdx_mcdi_process_cmd - Process an incoming MCDI response
> > > + * @cdx: NIC through which to issue the command
> > > + * @outbuf:  Pointer to the response buffer received from the
> > > +management
> > controller
> > > + * @len:     Length of the response buffer in bytes
> > > + *
> > > + * This function handles a response from the management controller.
> > > +It locates the
> > > + * corresponding command using the sequence number embedded in the
> > > +header,
> > > + * completes the command if it is still pending, and initiates any
> > > +necessary
> > cleanup.
> > > + *
> > > + * The function assumes that the response buffer is well-formed and
> > > +at least one
> > > + * dword in size.
> > > + */
> > >  void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword
> > > *outbuf, int len)  {
> > >   struct cdx_mcdi_iface *mcdi;
> > > @@ -590,6 +617,7 @@ void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx,
> > > struct cdx_dword *outbuf, int le
> > >
> > >   cdx_mcdi_process_cleanup_list(mcdi->cdx, &cleanup_list);  }
> > > +EXPORT_SYMBOL_GPL(cdx_mcdi_process_cmd);
> > >
> > >  static void cdx_mcdi_cmd_work(struct work_struct *context)  { @@
> > > -757,6 +785,7 @@ int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,
> > >   return cdx_mcdi_rpc_sync(cdx, cmd, inbuf, inlen, outbuf, outlen,
> > >                            outlen_actual, false);
> > >  }
> > > +EXPORT_SYMBOL_GPL(cdx_mcdi_rpc);
> > >
> > >  /**
> > >   * cdx_mcdi_rpc_async - Schedule an MCDI command to run
> > > asynchronously diff --git a/include/linux/cdx/mcdi.h
> > > b/include/linux/cdx/mcdi.h index 46e3f63b062a..1344119e9a2c 100644
> > > --- a/include/linux/cdx/mcdi.h
> > > +++ b/include/linux/cdx/mcdi.h
> > > @@ -169,6 +169,12 @@ struct cdx_mcdi_data {
> > >   u32 fn_flags;
> > >  };
> > >
> > > +int cdx_mcdi_init(struct cdx_mcdi *cdx); void
> > > +cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword
> > > +*outbuf, int len); int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,
> > > +          const struct cdx_dword *inbuf, size_t inlen,
> > > +          struct cdx_dword *outbuf, size_t outlen, size_t *outlen_actual);
> > > +
> > >  /*
> > >   * We expect that 16- and 32-bit fields in MCDI requests and responses
> > >   * are appropriately aligned, but 64-bit fields are only
> > > --
> > > 2.34.1
> > >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ