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@xxxxxxxxxx>
> > Reviewed-by: Fan Ni <fan.ni@xxxxxxxxxxx>
> > Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> > Reviewed-by: Davidlohr Bueso <dave@xxxxxxxxxxxx>
> > Reviewed-by: Li Ming <ming.li@xxxxxxxxxxxx>
> > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
[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
