linux-kernel - RE: [PATCH v7 2/5] cdx: Export Symbols for MCDI RPC and Initialization

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:
 <SA1PR12MB89474C0C76A66775B91A20C98170A@SA1PR12MB8947.namprd12.prod.outlook.com>
Date: Mon, 16 Jun 2025 12:20:33 +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: 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.

Will do. However I am planning to compile test the cdx changes.

>
> 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
> >