| Date | Wed, 10 Apr 2024 11:15:32 -0700 | From | Alison Schofield <> | Subject | Re: [PATCH 01/26] cxl/mbox: Flag support for Dynamic Capacity Devices (DCD) |
| |
On Sun, Mar 24, 2024 at 04:18:04PM -0700, Ira Weiny wrote: > From: Navneet Singh <navneet.singh@intel.com> > > Per the CXL 3.1 specification software must check the Command Effects > Log (CEL) to know if a device supports dynamic capacity (DC). If the > device does support DC the specifics of the DC Regions (0-7) are read > through the mailbox.
Do I need to know this 'If the device...' piece to understand this patch? I like that below you say 'Subsequent patches will...' That seems enough to set the scene.
> > Flag DC Device (DCD) commands in a device if they are supported. Why be vague w 'Flag'. How about 'Add a bitmap of DCD enabled commands to the driver device state structure.'
> Subsequent patches will key off these bits to configure DCD. > > Signed-off-by: Navneet Singh <navneet.singh@intel.com> > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > Changes for v1 > [iweiny: update to latest master] > [iweiny: update commit message] > [iweiny: Based on the fix: > https://lore.kernel.org/all/20230903-cxl-cel-fix-v1-1-e260c9467be3@intel.com/ > [jonathan: remove unneeded format change] > [jonathan: don't split security code in mbox.c] > --- > drivers/cxl/core/mbox.c | 33 +++++++++++++++++++++++++++++++++ > drivers/cxl/cxlmem.h | 15 +++++++++++++++ > 2 files changed, 48 insertions(+) >
snip
> /* Device enabled poison commands */ > enum poison_cmd_enabled_bits { > CXL_POISON_ENABLED_LIST, > @@ -454,6 +463,7 @@ struct cxl_dev_state { > * (CXL 2.0 8.2.9.5.1.1 Identify Memory Device) > * @mbox_mutex: Mutex to synchronize mailbox access. > * @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
It's not a 'List' it's a bitmap. How about mimicing 'enabled_cmds' description:
* @dcd_cmds: DCD commands found enabled in CEL
> * @total_bytes: sum of all possible capacities > @@ -481,6 +491,7 @@ struct cxl_memdev_state { > size_t lsa_size; > struct mutex mbox_mutex; /* Protects device mailbox and firmware */ > char firmware_version[0x10]; > + DECLARE_BITMAP(dcd_cmds, CXL_DCD_ENABLED_MAX); > DECLARE_BITMAP(enabled_cmds, CXL_MEM_COMMAND_ID_MAX); > DECLARE_BITMAP(exclusive_cmds, CXL_MEM_COMMAND_ID_MAX); > u64 total_bytes; > @@ -551,6 +562,10 @@ enum cxl_opcode { > CXL_MBOX_OP_UNLOCK = 0x4503, > CXL_MBOX_OP_FREEZE_SECURITY = 0x4504, > CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE = 0x4505, > + CXL_MBOX_OP_GET_DC_CONFIG = 0x4800, > + CXL_MBOX_OP_GET_DC_EXTENT_LIST = 0x4801, > + CXL_MBOX_OP_ADD_DC_RESPONSE = 0x4802, > + CXL_MBOX_OP_RELEASE_DC = 0x4803, > CXL_MBOX_OP_MAX = 0x10000 > }; > > > -- > 2.44.0 >
|