netdev - Re: [RFC PATCH fwctl 2/5] pds_core: add new fwctl auxilary

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: <20250219082405.GC53094@unreal>
Date: Wed, 19 Feb 2025 10:24:05 +0200
From: Leon Romanovsky <leon@...nel.org>
To: "Nelson, Shannon" <shannon.nelson@....com>
Cc: jgg@...dia.com, andrew.gospodarek@...adcom.com,
	aron.silverton@...cle.com, dan.j.williams@...el.com,
	daniel.vetter@...ll.ch, dave.jiang@...el.com, dsahern@...nel.org,
	gospo@...adcom.com, hch@...radead.org, itayavr@...dia.com,
	jiri@...dia.com, Jonathan.Cameron@...wei.com, kuba@...nel.org,
	lbloch@...dia.com, saeedm@...dia.com, linux-cxl@...r.kernel.org,
	linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
	brett.creeley@....com
Subject: Re: [RFC PATCH fwctl 2/5] pds_core: add new fwctl auxilary_device

On Tue, Feb 18, 2025 at 12:00:52PM -0800, Nelson, Shannon wrote:
> On 2/18/2025 11:28 AM, Leon Romanovsky wrote:
> > 
> > On Tue, Feb 11, 2025 at 03:48:51PM -0800, Shannon Nelson wrote:
> > > Add support for a new fwctl-based auxiliary_device for creating a
> > > channel for fwctl support into the AMD/Pensando DSC.
> > > 
> > > Signed-off-by: Shannon Nelson <shannon.nelson@....com>
> > > ---
> > >   drivers/net/ethernet/amd/pds_core/auxbus.c |  3 +--
> > >   drivers/net/ethernet/amd/pds_core/core.c   |  7 +++++++
> > >   drivers/net/ethernet/amd/pds_core/core.h   |  1 +
> > >   drivers/net/ethernet/amd/pds_core/main.c   | 10 ++++++++++
> > >   include/linux/pds/pds_common.h             |  2 ++
> > >   5 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > <...>
> > 
> > My comment is only slightly related to the patch itself, but worth to
> > write it anyway.
> > 
> > > diff --git a/include/linux/pds/pds_common.h b/include/linux/pds/pds_common.h
> > > index 5802e1deef24..b193adbe7cc3 100644
> > > --- a/include/linux/pds/pds_common.h
> > > +++ b/include/linux/pds/pds_common.h
> > > @@ -29,6 +29,7 @@ enum pds_core_vif_types {
> > >        PDS_DEV_TYPE_ETH        = 3,
> > >        PDS_DEV_TYPE_RDMA       = 4,
> > >        PDS_DEV_TYPE_LM         = 5,
> > > +     PDS_DEV_TYPE_FWCTL      = 6,
> > 
> > This enum and defines below should be cleaned from unsupported types.
> > I don't see any code for RDMA, LM and ETH.
> > 
> > Thanks
> 
> I've looked at those a few times over the life of this code, but I continue
> to leave them there because they are part of the firmware interface
> definition, whether we use them or not.

How? You are passing some number which FW is not aware of it. You can
pass any number you want there. Even it is not true, you can
PDS_DEV_TYPE_FWCTL = 6 here, but remove rest of enums and *_STR defines.

> 
> You're right, there is no ETH or RDMA type code, they exist as historical
> artifacts of the early interface design.

This make me wonder why netdev merged this code which has nothing to do
with netdev subsystem at all.

Thanks