| Message ID | 20240324-dcd-type2-upstream-v1-1-b7b00d623625@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | DCD: Add support for Dynamic Capacity Devices (DCD) | expand |
On Sun, 24 Mar 2024 16:18:04 -0700 ira.weiny@intel.com 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. > > Flag DC Device (DCD) commands in a device if they are supported. > 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> This aligns with similar existing code etc and the opcodes are right so There is a vague argument that maybe we shouldn't claim to support the command in the debug message at that point in the patch set, but I don't think we really care either way as expectation is this set should go in all together. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
On Sun, Mar 24, 2024 at 04:18:04PM -0700, ira.weiny@intel.com 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. > > Flag DC Device (DCD) commands in a device if they are supported. > 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> > --- Reviewed-by: Fan Ni <fan.ni@samsung.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(+) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 9adda4795eb7..ed4131c6f50b 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -161,6 +161,34 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security, > } > } > > +static bool cxl_is_dcd_command(u16 opcode) > +{ > +#define CXL_MBOX_OP_DCD_CMDS 0x48 > + > + return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS; > +} > + > +static void cxl_set_dcd_cmd_enabled(struct cxl_memdev_state *mds, > + u16 opcode) > +{ > + switch (opcode) { > + case CXL_MBOX_OP_GET_DC_CONFIG: > + set_bit(CXL_DCD_ENABLED_GET_CONFIG, mds->dcd_cmds); > + break; > + case CXL_MBOX_OP_GET_DC_EXTENT_LIST: > + set_bit(CXL_DCD_ENABLED_GET_EXTENT_LIST, mds->dcd_cmds); > + break; > + case CXL_MBOX_OP_ADD_DC_RESPONSE: > + set_bit(CXL_DCD_ENABLED_ADD_RESPONSE, mds->dcd_cmds); > + break; > + case CXL_MBOX_OP_RELEASE_DC: > + set_bit(CXL_DCD_ENABLED_RELEASE, mds->dcd_cmds); > + break; > + default: > + break; > + } > +} > + > static bool cxl_is_poison_command(u16 opcode) > { > #define CXL_MBOX_OP_POISON_CMDS 0x43 > @@ -733,6 +761,11 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel) > enabled++; > } > > + if (cxl_is_dcd_command(opcode)) { > + cxl_set_dcd_cmd_enabled(mds, opcode); > + enabled++; > + } > + > dev_dbg(dev, "Opcode 0x%04x %s\n", opcode, > enabled ? "enabled" : "unsupported by driver"); > } > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 20fb3b35e89e..79a67cff9143 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -238,6 +238,15 @@ struct cxl_event_state { > struct mutex log_lock; > }; > > +/* Device enabled DCD commands */ > +enum dcd_cmd_enabled_bits { > + CXL_DCD_ENABLED_GET_CONFIG, > + CXL_DCD_ENABLED_GET_EXTENT_LIST, > + CXL_DCD_ENABLED_ADD_RESPONSE, > + CXL_DCD_ENABLED_RELEASE, > + CXL_DCD_ENABLED_MAX > +}; > + > /* 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 > * @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 >
On Sun, 24 Mar 2024, ira.weiny@intel.com 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. I vote to fold this into patch 3, favoring reduced patch count in the series to trvially enlarging that particular patch. >Flag DC Device (DCD) commands in a device if they are supported. >Subsequent patches will key off these bits to configure DCD. It would be good to mention these here explicitly (if this patch will live on). For example, that config will be the driver's way of telling if dcd is enabled or disabled - we could have cases of that zeroed bit but the rest enabled. lgtm otherwise. Reviewed-by: Davidlohr Bueso <dave@stgolabs.net> >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(+) > >diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c >index 9adda4795eb7..ed4131c6f50b 100644 >--- a/drivers/cxl/core/mbox.c >+++ b/drivers/cxl/core/mbox.c >@@ -161,6 +161,34 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security, > } > } > >+static bool cxl_is_dcd_command(u16 opcode) >+{ >+#define CXL_MBOX_OP_DCD_CMDS 0x48 >+ >+ return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS; >+} >+ >+static void cxl_set_dcd_cmd_enabled(struct cxl_memdev_state *mds, >+ u16 opcode) >+{ >+ switch (opcode) { >+ case CXL_MBOX_OP_GET_DC_CONFIG: >+ set_bit(CXL_DCD_ENABLED_GET_CONFIG, mds->dcd_cmds); >+ break; >+ case CXL_MBOX_OP_GET_DC_EXTENT_LIST: >+ set_bit(CXL_DCD_ENABLED_GET_EXTENT_LIST, mds->dcd_cmds); >+ break; >+ case CXL_MBOX_OP_ADD_DC_RESPONSE: >+ set_bit(CXL_DCD_ENABLED_ADD_RESPONSE, mds->dcd_cmds); >+ break; >+ case CXL_MBOX_OP_RELEASE_DC: >+ set_bit(CXL_DCD_ENABLED_RELEASE, mds->dcd_cmds); >+ break; >+ default: >+ break; >+ } >+} >+ > static bool cxl_is_poison_command(u16 opcode) > { > #define CXL_MBOX_OP_POISON_CMDS 0x43 >@@ -733,6 +761,11 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel) > enabled++; > } > >+ if (cxl_is_dcd_command(opcode)) { >+ cxl_set_dcd_cmd_enabled(mds, opcode); >+ enabled++; >+ } >+ > dev_dbg(dev, "Opcode 0x%04x %s\n", opcode, > enabled ? "enabled" : "unsupported by driver"); > } >diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h >index 20fb3b35e89e..79a67cff9143 100644 >--- a/drivers/cxl/cxlmem.h >+++ b/drivers/cxl/cxlmem.h >@@ -238,6 +238,15 @@ struct cxl_event_state { > struct mutex log_lock; > }; > >+/* Device enabled DCD commands */ >+enum dcd_cmd_enabled_bits { >+ CXL_DCD_ENABLED_GET_CONFIG, >+ CXL_DCD_ENABLED_GET_EXTENT_LIST, >+ CXL_DCD_ENABLED_ADD_RESPONSE, >+ CXL_DCD_ENABLED_RELEASE, >+ CXL_DCD_ENABLED_MAX >+}; >+ > /* 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 > * @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 >
On 3/24/24 4:18 PM, ira.weiny@intel.com 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. > > Flag DC Device (DCD) commands in a device if they are supported. > 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> Reviewed-by: Dave Jiang <dave.jiang@intel.com> small formatting nit below > --- > 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(+) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 9adda4795eb7..ed4131c6f50b 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -161,6 +161,34 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security, > } > } > > +static bool cxl_is_dcd_command(u16 opcode) > +{ > +#define CXL_MBOX_OP_DCD_CMDS 0x48 > + > + return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS; > +} > + > +static void cxl_set_dcd_cmd_enabled(struct cxl_memdev_state *mds, > + u16 opcode) This seems misaligned. DJ > +{ > + switch (opcode) { > + case CXL_MBOX_OP_GET_DC_CONFIG: > + set_bit(CXL_DCD_ENABLED_GET_CONFIG, mds->dcd_cmds); > + break; > + case CXL_MBOX_OP_GET_DC_EXTENT_LIST: > + set_bit(CXL_DCD_ENABLED_GET_EXTENT_LIST, mds->dcd_cmds); > + break; > + case CXL_MBOX_OP_ADD_DC_RESPONSE: > + set_bit(CXL_DCD_ENABLED_ADD_RESPONSE, mds->dcd_cmds); > + break; > + case CXL_MBOX_OP_RELEASE_DC: > + set_bit(CXL_DCD_ENABLED_RELEASE, mds->dcd_cmds); > + break; > + default: > + break; > + } > +} > + > static bool cxl_is_poison_command(u16 opcode) > { > #define CXL_MBOX_OP_POISON_CMDS 0x43 > @@ -733,6 +761,11 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel) > enabled++; > } > > + if (cxl_is_dcd_command(opcode)) { > + cxl_set_dcd_cmd_enabled(mds, opcode); > + enabled++; > + } > + > dev_dbg(dev, "Opcode 0x%04x %s\n", opcode, > enabled ? "enabled" : "unsupported by driver"); > } > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 20fb3b35e89e..79a67cff9143 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -238,6 +238,15 @@ struct cxl_event_state { > struct mutex log_lock; > }; > > +/* Device enabled DCD commands */ > +enum dcd_cmd_enabled_bits { > + CXL_DCD_ENABLED_GET_CONFIG, > + CXL_DCD_ENABLED_GET_EXTENT_LIST, > + CXL_DCD_ENABLED_ADD_RESPONSE, > + CXL_DCD_ENABLED_RELEASE, > + CXL_DCD_ENABLED_MAX > +}; > + > /* 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 > * @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 > }; > >
Davidlohr Bueso wrote: > On Sun, 24 Mar 2024, ira.weiny@intel.com 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. > > I vote to fold this into patch 3, favoring reduced patch count in the > series to trvially enlarging that particular patch. I'll consider it. I've tried hard to split the original series up into very easily reviewable chunks. So I'm inclined to leave this separate for now. > > >Flag DC Device (DCD) commands in a device if they are supported. > >Subsequent patches will key off these bits to configure DCD. > > It would be good to mention these here explicitly (if this patch will > live on). For example, that config will be the driver's way of telling > if dcd is enabled or disabled - we could have cases of that zeroed bit > but the rest enabled. It took me a bit to parse this but I see what you mean. Yes the GET_CONFIG command is the one used specifically to determine if DCD is supported. I'll clean up the commit message. In retrospect it was wrong for me to mention subsequent patches here. I'm going to move this final detail to patch 3. > > lgtm otherwise. > > Reviewed-by: Davidlohr Bueso <dave@stgolabs.net> Thanks! :-D Ira > > >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(+) > > > >diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > >index 9adda4795eb7..ed4131c6f50b 100644 > >--- a/drivers/cxl/core/mbox.c > >+++ b/drivers/cxl/core/mbox.c > >@@ -161,6 +161,34 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security, > > } > > } > > > >+static bool cxl_is_dcd_command(u16 opcode) > >+{ > >+#define CXL_MBOX_OP_DCD_CMDS 0x48 > >+ > >+ return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS; > >+} > >+ > >+static void cxl_set_dcd_cmd_enabled(struct cxl_memdev_state *mds, > >+ u16 opcode) > >+{ > >+ switch (opcode) { > >+ case CXL_MBOX_OP_GET_DC_CONFIG: > >+ set_bit(CXL_DCD_ENABLED_GET_CONFIG, mds->dcd_cmds); > >+ break; > >+ case CXL_MBOX_OP_GET_DC_EXTENT_LIST: > >+ set_bit(CXL_DCD_ENABLED_GET_EXTENT_LIST, mds->dcd_cmds); > >+ break; > >+ case CXL_MBOX_OP_ADD_DC_RESPONSE: > >+ set_bit(CXL_DCD_ENABLED_ADD_RESPONSE, mds->dcd_cmds); > >+ break; > >+ case CXL_MBOX_OP_RELEASE_DC: > >+ set_bit(CXL_DCD_ENABLED_RELEASE, mds->dcd_cmds); > >+ break; > >+ default: > >+ break; > >+ } > >+} > >+ > > static bool cxl_is_poison_command(u16 opcode) > > { > > #define CXL_MBOX_OP_POISON_CMDS 0x43 > >@@ -733,6 +761,11 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel) > > enabled++; > > } > > > >+ if (cxl_is_dcd_command(opcode)) { > >+ cxl_set_dcd_cmd_enabled(mds, opcode); > >+ enabled++; > >+ } > >+ > > dev_dbg(dev, "Opcode 0x%04x %s\n", opcode, > > enabled ? "enabled" : "unsupported by driver"); > > } > >diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > >index 20fb3b35e89e..79a67cff9143 100644 > >--- a/drivers/cxl/cxlmem.h > >+++ b/drivers/cxl/cxlmem.h > >@@ -238,6 +238,15 @@ struct cxl_event_state { > > struct mutex log_lock; > > }; > > > >+/* Device enabled DCD commands */ > >+enum dcd_cmd_enabled_bits { > >+ CXL_DCD_ENABLED_GET_CONFIG, > >+ CXL_DCD_ENABLED_GET_EXTENT_LIST, > >+ CXL_DCD_ENABLED_ADD_RESPONSE, > >+ CXL_DCD_ENABLED_RELEASE, > >+ CXL_DCD_ENABLED_MAX > >+}; > >+ > > /* 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 > > * @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 > >
Dave Jiang wrote: > > > On 3/24/24 4:18 PM, ira.weiny@intel.com 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. > > > > Flag DC Device (DCD) commands in a device if they are supported. > > 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> > > Reviewed-by: Dave Jiang <dave.jiang@intel.com> > > small formatting nit below > > > --- > > 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(+) > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > index 9adda4795eb7..ed4131c6f50b 100644 > > --- a/drivers/cxl/core/mbox.c > > +++ b/drivers/cxl/core/mbox.c > > @@ -161,6 +161,34 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security, > > } > > } > > > > +static bool cxl_is_dcd_command(u16 opcode) > > +{ > > +#define CXL_MBOX_OP_DCD_CMDS 0x48 > > + > > + return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS; > > +} > > + > > +static void cxl_set_dcd_cmd_enabled(struct cxl_memdev_state *mds, > > + u16 opcode) > > This seems misaligned. Fixed, Ira
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 >
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index 9adda4795eb7..ed4131c6f50b 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -161,6 +161,34 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security, } } +static bool cxl_is_dcd_command(u16 opcode) +{ +#define CXL_MBOX_OP_DCD_CMDS 0x48 + + return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS; +} + +static void cxl_set_dcd_cmd_enabled(struct cxl_memdev_state *mds, + u16 opcode) +{ + switch (opcode) { + case CXL_MBOX_OP_GET_DC_CONFIG: + set_bit(CXL_DCD_ENABLED_GET_CONFIG, mds->dcd_cmds); + break; + case CXL_MBOX_OP_GET_DC_EXTENT_LIST: + set_bit(CXL_DCD_ENABLED_GET_EXTENT_LIST, mds->dcd_cmds); + break; + case CXL_MBOX_OP_ADD_DC_RESPONSE: + set_bit(CXL_DCD_ENABLED_ADD_RESPONSE, mds->dcd_cmds); + break; + case CXL_MBOX_OP_RELEASE_DC: + set_bit(CXL_DCD_ENABLED_RELEASE, mds->dcd_cmds); + break; + default: + break; + } +} + static bool cxl_is_poison_command(u16 opcode) { #define CXL_MBOX_OP_POISON_CMDS 0x43 @@ -733,6 +761,11 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel) enabled++; } + if (cxl_is_dcd_command(opcode)) { + cxl_set_dcd_cmd_enabled(mds, opcode); + enabled++; + } + dev_dbg(dev, "Opcode 0x%04x %s\n", opcode, enabled ? "enabled" : "unsupported by driver"); } diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 20fb3b35e89e..79a67cff9143 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -238,6 +238,15 @@ struct cxl_event_state { struct mutex log_lock; }; +/* Device enabled DCD commands */ +enum dcd_cmd_enabled_bits { + CXL_DCD_ENABLED_GET_CONFIG, + CXL_DCD_ENABLED_GET_EXTENT_LIST, + CXL_DCD_ENABLED_ADD_RESPONSE, + CXL_DCD_ENABLED_RELEASE, + CXL_DCD_ENABLED_MAX +}; + /* 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 * @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 };