netdev - Re: [PATCH v8 03/27] cxl: add capabilities field to cxl_dev_state and cxl

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: <20250102124944.0000260e@huawei.com>
Date: Thu, 2 Jan 2025 12:49:44 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Alejandro Lucero Palau <alucerop@....com>
CC: <alejandro.lucero-palau@....com>, <linux-cxl@...r.kernel.org>,
	<netdev@...r.kernel.org>, <dan.j.williams@...el.com>,
	<martin.habets@...inx.com>, <edward.cree@....com>, <davem@...emloft.net>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <edumazet@...gle.com>,
	<dave.jiang@...el.com>
Subject: Re: [PATCH v8 03/27] cxl: add capabilities field to cxl_dev_state
 and cxl_port


> >> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> >> index 19e5d883557a..f656fcd4945f 100644
> >> --- a/include/cxl/cxl.h
> >> +++ b/include/cxl/cxl.h
> >> @@ -12,6 +12,25 @@ enum cxl_resource {
> >>   	CXL_RES_PMEM,
> >>   };
> >>   
> >> +/* Capabilities as defined for:
> >> + *
> >> + *	Component Registers (Table 8-22 CXL 3.1 specification)
> >> + *	Device Registers (8.2.8.2.1 CXL 3.1 specification)
> >> + *
> >> + * and currently being used for kernel CXL support.
> >> + */
> >> +
> >> +enum cxl_dev_cap {
> >> +	/* capabilities from Component Registers */
> >> +	CXL_DEV_CAP_RAS,
> >> +	CXL_DEV_CAP_HDM,
> >> +	/* capabilities from Device Registers */
> >> +	CXL_DEV_CAP_DEV_STATUS,
> >> +	CXL_DEV_CAP_MAILBOX_PRIMARY,
> >> +	CXL_DEV_CAP_MEMDEV,
> >> +	CXL_MAX_CAPS = 64  
> > Why set it to 64?  All the bitmaps etc will autosize so
> > you just need to ensure you use correct set_bit() and test_bit()
> > that are happy dealing with bitmaps of multiple longs.
> >  
> 
> Initially it was set to 32, but DECLARE_BITMAP uses unsigned long, so 
> for initializing/zeroing the locally allocated bitmap in some functions, 
> bitmap_clear had to use sizeof for the size, and I was suggested to 
> define CXL_MAX_CAPS to 64 and use it instead, what seems cleaner.

It should never have been using sizeof() once it was a bitmap.
Just clear what is actually used and make sure no code assumes
any particular length of bitmap.  Then you will never have
to deal with changing it.

Then CXL_MAX_CAP just becomes last entry in this enum.

The only time this is becomes tricky with bitmaps is if you need
to set a bits in a constant bitmap as then you can't use the
set/get functions and have to assume something about the length.

Don't think that applies here.

Jonathan

 
> 
> 
> >> +};
> >> +
> >>   struct cxl_dev_state *cxl_accel_state_create(struct device *dev);
> >>   
> >>   void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);