linux-kernel - Re: [PATCH v8 01/21] cxl/mbox: Flag support for Dynamic Capacity Devices (DCD)

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: <677c7f0f5ada9_dab0d2948f@iweiny-mobl.notmuch>
Date: Mon, 6 Jan 2025 19:10:39 -0600
From: Ira Weiny <ira.weiny@...el.com>
To: Dan Williams <dan.j.williams@...el.com>, Ira Weiny <ira.weiny@...el.com>,
	Dave Jiang <dave.jiang@...el.com>, Fan Ni <fan.ni@...sung.com>, "Jonathan
 Cameron" <Jonathan.Cameron@...wei.com>, Jonathan Corbet <corbet@....net>,
	Andrew Morton <akpm@...ux-foundation.org>, Kees Cook <kees@...nel.org>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>
CC: Dan Williams <dan.j.williams@...el.com>, Davidlohr Bueso
	<dave@...olabs.net>, Alison Schofield <alison.schofield@...el.com>, "Vishal
 Verma" <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
	<linux-cxl@...r.kernel.org>, <linux-doc@...r.kernel.org>,
	<nvdimm@...ts.linux.dev>, <linux-kernel@...r.kernel.org>,
	<linux-hardening@...r.kernel.org>, Li Ming <ming.li@...omail.com>
Subject: Re: [PATCH v8 01/21] cxl/mbox: Flag support for Dynamic Capacity
 Devices (DCD)

Dan Williams wrote:
> Ira Weiny wrote:
> > Per the CXL 3.1 specification software must check the Command Effects
> > Log (CEL) for dynamic capacity command support.
> > 
> > Detect support for the DCD commands while reading the CEL, including:
> > 
> > 	Get DC Config
> > 	Get DC Extent List
> > 	Add DC Response
> > 	Release DC
> > 
> > Based on an original patch by Navneet Singh.
> > 
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> > Reviewed-by: Fan Ni <fan.ni@...sung.com>
> > Reviewed-by: Dave Jiang <dave.jiang@...el.com>
> > Reviewed-by: Davidlohr Bueso <dave@...olabs.net>
> > Reviewed-by: Li Ming <ming.li@...omail.com>
> > Signed-off-by: Ira Weiny <ira.weiny@...el.com>

[snip]

> >  /* Device enabled poison commands */
> >  enum poison_cmd_enabled_bits {
> >  	CXL_POISON_ENABLED_LIST,
> > @@ -461,6 +470,7 @@ static inline struct cxl_dev_state *mbox_to_cxlds(struct cxl_mailbox *cxl_mbox)
> >   * @lsa_size: Size of Label Storage Area
> >   *                (CXL 2.0 8.2.9.5.1.1 Identify Memory Device)
> >   * @firmware_version: Firmware version for the memory device.
> > + * @dcd_cmds: List of DCD commands implemented by memory device
> >   * @enabled_cmds: Hardware commands found enabled in CEL.
> >   * @exclusive_cmds: Commands that are kernel-internal only
> >   * @total_bytes: sum of all possible capacities
> > @@ -485,6 +495,7 @@ struct cxl_memdev_state {
> >  	struct cxl_dev_state cxlds;
> >  	size_t lsa_size;
> >  	char firmware_version[0x10];
> > +	DECLARE_BITMAP(dcd_cmds, CXL_DCD_ENABLED_MAX);
> 
> Can you clarify why cxl_memdev_state needs this bitmap?

Nope.  I think you are right that there is no need for partial support.

> In the case of
> 'security' and 'poison' functionality there is a subset of functionality
> that can be enabled if some of the commands are missing. Like poison
> listing is still possible even if poison injection is missing. In the
> case of DCD it is all or nothing.
> 
> In short, I do not think the cxl_memdev_state object needs to track
> anything more than a single "DCD capable" flag, and cxl_walk_cel() can
> check for all commands locally without carrying that bitmap around
> indefinitely.
> 
> Something simple like:
> 
> cxl_walk_cel()
>     for (...) {
>       if (cxl_is_dcd_command()
>         set_bit(opcode & 0xf, &dcd_commands);
>     }
>     if (dcd_commands == 0xf)
>         mds->dcd_enabled = true;
>     else if (dcd_commands)
>         dev_dbg(...)
> 
> ...otherwise it begs the question why the driver would care about
> anything other than "all" dcd commands?

Yea this could be done.

Ira