diff mbox series

[v6,20/43] hw/cxl/device: Add some trivial commands

Message ID 20220211120747.3074-21-Jonathan.Cameron@huawei.com (mailing list archive)
State New, archived
Headers show
Series CXl 2.0 emulation Support | expand

Commit Message

Jonathan Cameron Feb. 11, 2022, 12:07 p.m. UTC
From: Ben Widawsky <ben.widawsky@intel.com>

GET_FW_INFO and GET_PARTITION_INFO, for this emulation, is equivalent to
info already returned in the IDENTIFY command. To have a more robust
implementation, add those.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 hw/cxl/cxl-mailbox-utils.c | 69 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

Comments

Alex Bennée March 1, 2022, 6:46 p.m. UTC | #1
Jonathan Cameron <Jonathan.Cameron@huawei.com> writes:

> From: Ben Widawsky <ben.widawsky@intel.com>
>
> GET_FW_INFO and GET_PARTITION_INFO, for this emulation, is equivalent to
> info already returned in the IDENTIFY command. To have a more robust
> implementation, add those.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  hw/cxl/cxl-mailbox-utils.c | 69 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 808faec114..d022711b2a 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -44,6 +44,8 @@ enum {
>          #define CLEAR_RECORDS   0x1
>          #define GET_INTERRUPT_POLICY   0x2
>          #define SET_INTERRUPT_POLICY   0x3
> +    FIRMWARE_UPDATE = 0x02,
> +        #define GET_INFO      0x0
>      TIMESTAMP   = 0x03,
>          #define GET           0x0
>          #define SET           0x1
> @@ -52,6 +54,8 @@ enum {
>          #define GET_LOG       0x1
>      IDENTIFY    = 0x40,
>          #define MEMORY_DEVICE 0x0
> +    CCLS        = 0x41,
> +        #define GET_PARTITION_INFO     0x0
>  };
>  
>  /* 8.2.8.4.5.1 Command Return Codes */
> @@ -114,6 +118,39 @@ DEFINE_MAILBOX_HANDLER_NOP(events_clear_records);
>  DEFINE_MAILBOX_HANDLER_ZEROED(events_get_interrupt_policy, 4);
>  DEFINE_MAILBOX_HANDLER_NOP(events_set_interrupt_policy);
>  
> +/* 8.2.9.2.1 */
> +static ret_code cmd_firmware_update_get_info(struct cxl_cmd *cmd,
> +                                             CXLDeviceState *cxl_dstate,
> +                                             uint16_t *len)
> +{
> +    struct {
> +        uint8_t slots_supported;
> +        uint8_t slot_info;
> +        uint8_t caps;
> +        uint8_t rsvd[0xd];
> +        char fw_rev1[0x10];
> +        char fw_rev2[0x10];
> +        char fw_rev3[0x10];
> +        char fw_rev4[0x10];
> +    } __attribute__((packed)) *fw_info;
> +    _Static_assert(sizeof(*fw_info) == 0x50, "Bad firmware info
> size");

note: we have QEMU_PACKED, QEMU_BUILD_BUG_ON and friends in compiler.h which are
preferred for potential compiler portability reasons.

> +
> +    if (cxl_dstate->pmem_size < (256 << 20)) {
> +        return CXL_MBOX_INTERNAL_ERROR;
> +    }
> +
> +    fw_info = (void *)cmd->payload;
> +    memset(fw_info, 0, sizeof(*fw_info));
> +
> +    fw_info->slots_supported = 2;
> +    fw_info->slot_info = BIT(0) | BIT(3);
> +    fw_info->caps = 0;
> +    snprintf(fw_info->fw_rev1, 0x10, "BWFW VERSION %02d", 0);

Given you have a fixed string here could you not:

  pstrcpy(fw_info->fw_rev1, 0x10, "BWFW VERSION 0");
  
> +
> +    *len = sizeof(*fw_info);
> +    return CXL_MBOX_SUCCESS;
> +}
> +
>  /* 8.2.9.3.1 */
>  static ret_code cmd_timestamp_get(struct cxl_cmd *cmd,
>                                    CXLDeviceState *cxl_dstate,
> @@ -260,6 +297,33 @@ static ret_code cmd_identify_memory_device(struct cxl_cmd *cmd,
>      return CXL_MBOX_SUCCESS;
>  }
>  
> +static ret_code cmd_ccls_get_partition_info(struct cxl_cmd *cmd,
> +                                           CXLDeviceState *cxl_dstate,
> +                                           uint16_t *len)
> +{
> +    struct {
> +        uint64_t active_vmem;
> +        uint64_t active_pmem;
> +        uint64_t next_vmem;
> +        uint64_t next_pmem;
> +    } __attribute__((packed)) *part_info = (void *)cmd->payload;
> +    _Static_assert(sizeof(*part_info) == 0x20, "Bad get partition info size");
> +    uint64_t size = cxl_dstate->pmem_size;
> +
> +    if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
> +        return CXL_MBOX_INTERNAL_ERROR;
> +    }
> +
> +    /* PMEM only */
> +    part_info->active_vmem = 0;
> +    part_info->next_vmem = 0;
> +    part_info->active_pmem = size / (256 << 20);
> +    part_info->next_pmem = part_info->active_pmem;
> +
> +    *len = sizeof(*part_info);
> +    return CXL_MBOX_SUCCESS;
> +}
> +
>  #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
>  #define IMMEDIATE_POLICY_CHANGE (1 << 3)
>  #define IMMEDIATE_LOG_CHANGE (1 << 4)
> @@ -273,15 +337,18 @@ static struct cxl_cmd cxl_cmd_set[256][256] = {
>          cmd_events_get_interrupt_policy, 0, 0 },
>      [EVENTS][SET_INTERRUPT_POLICY] = { "EVENTS_SET_INTERRUPT_POLICY",
>          cmd_events_set_interrupt_policy, 4, IMMEDIATE_CONFIG_CHANGE },
> +    [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
> +        cmd_firmware_update_get_info, 0, 0 },
>      [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
>      [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set, 8, IMMEDIATE_POLICY_CHANGE },
>      [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", cmd_logs_get_supported, 0, 0 },
>      [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
>      [IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE",
>          cmd_identify_memory_device, 0, 0 },
> +    [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO",
> +        cmd_ccls_get_partition_info, 0, 0 },
>  };
>  
> -
>  void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
>  {
>      uint16_t ret = CXL_MBOX_SUCCESS;
Jonathan Cameron March 4, 2022, 1:16 p.m. UTC | #2
On Tue, 01 Mar 2022 18:46:30 +0000
Alex Bennée <alex.bennee@linaro.org> wrote:

> Jonathan Cameron <Jonathan.Cameron@huawei.com> writes:
> 
> > From: Ben Widawsky <ben.widawsky@intel.com>
> >
> > GET_FW_INFO and GET_PARTITION_INFO, for this emulation, is equivalent to
> > info already returned in the IDENTIFY command. To have a more robust
> > implementation, add those.
> >
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Hi Alex,

> > ---
> >  hw/cxl/cxl-mailbox-utils.c | 69 +++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index 808faec114..d022711b2a 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -44,6 +44,8 @@ enum {
> >          #define CLEAR_RECORDS   0x1
> >          #define GET_INTERRUPT_POLICY   0x2
> >          #define SET_INTERRUPT_POLICY   0x3
> > +    FIRMWARE_UPDATE = 0x02,
> > +        #define GET_INFO      0x0
> >      TIMESTAMP   = 0x03,
> >          #define GET           0x0
> >          #define SET           0x1
> > @@ -52,6 +54,8 @@ enum {
> >          #define GET_LOG       0x1
> >      IDENTIFY    = 0x40,
> >          #define MEMORY_DEVICE 0x0
> > +    CCLS        = 0x41,
> > +        #define GET_PARTITION_INFO     0x0
> >  };
> >  
> >  /* 8.2.8.4.5.1 Command Return Codes */
> > @@ -114,6 +118,39 @@ DEFINE_MAILBOX_HANDLER_NOP(events_clear_records);
> >  DEFINE_MAILBOX_HANDLER_ZEROED(events_get_interrupt_policy, 4);
> >  DEFINE_MAILBOX_HANDLER_NOP(events_set_interrupt_policy);
> >  
> > +/* 8.2.9.2.1 */
> > +static ret_code cmd_firmware_update_get_info(struct cxl_cmd *cmd,
> > +                                             CXLDeviceState *cxl_dstate,
> > +                                             uint16_t *len)
> > +{
> > +    struct {
> > +        uint8_t slots_supported;
> > +        uint8_t slot_info;
> > +        uint8_t caps;
> > +        uint8_t rsvd[0xd];
> > +        char fw_rev1[0x10];
> > +        char fw_rev2[0x10];
> > +        char fw_rev3[0x10];
> > +        char fw_rev4[0x10];
> > +    } __attribute__((packed)) *fw_info;
> > +    _Static_assert(sizeof(*fw_info) == 0x50, "Bad firmware info
> > size");  
> 
> note: we have QEMU_PACKED, QEMU_BUILD_BUG_ON and friends in compiler.h which are
> preferred for potential compiler portability reasons.

Replaced all instances of alignment, packed and Static_assert in the
patch set with the compiler.h equivalents. Not that has minor
affect on some earlier patch sets but feels trivial enough
that I've kept RBs etc.

> 
> > +
> > +    if (cxl_dstate->pmem_size < (256 << 20)) {
> > +        return CXL_MBOX_INTERNAL_ERROR;
> > +    }
> > +
> > +    fw_info = (void *)cmd->payload;
> > +    memset(fw_info, 0, sizeof(*fw_info));
> > +
> > +    fw_info->slots_supported = 2;
> > +    fw_info->slot_info = BIT(0) | BIT(3);
> > +    fw_info->caps = 0;
> > +    snprintf(fw_info->fw_rev1, 0x10, "BWFW VERSION %02d", 0);  
> 
> Given you have a fixed string here could you not:
> 
>   pstrcpy(fw_info->fw_rev1, 0x10, "BWFW VERSION 0");

Make sense.

>   
> > +
> > +    *len = sizeof(*fw_info);
> > +    return CXL_MBOX_SUCCESS;
> > +}
> > +
> >  /* 8.2.9.3.1 */
> >  static ret_code cmd_timestamp_get(struct cxl_cmd *cmd,
> >                                    CXLDeviceState *cxl_dstate,
> > @@ -260,6 +297,33 @@ static ret_code cmd_identify_memory_device(struct cxl_cmd *cmd,
> >      return CXL_MBOX_SUCCESS;
> >  }
> >  
> > +static ret_code cmd_ccls_get_partition_info(struct cxl_cmd *cmd,
> > +                                           CXLDeviceState *cxl_dstate,
> > +                                           uint16_t *len)
> > +{
> > +    struct {
> > +        uint64_t active_vmem;
> > +        uint64_t active_pmem;
> > +        uint64_t next_vmem;
> > +        uint64_t next_pmem;
> > +    } __attribute__((packed)) *part_info = (void *)cmd->payload;
> > +    _Static_assert(sizeof(*part_info) == 0x20, "Bad get partition info size");
> > +    uint64_t size = cxl_dstate->pmem_size;
> > +
> > +    if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
> > +        return CXL_MBOX_INTERNAL_ERROR;
> > +    }
> > +
> > +    /* PMEM only */
> > +    part_info->active_vmem = 0;
> > +    part_info->next_vmem = 0;
> > +    part_info->active_pmem = size / (256 << 20);
> > +    part_info->next_pmem = part_info->active_pmem;
> > +
> > +    *len = sizeof(*part_info);
> > +    return CXL_MBOX_SUCCESS;
> > +}
> > +
> >  #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
> >  #define IMMEDIATE_POLICY_CHANGE (1 << 3)
> >  #define IMMEDIATE_LOG_CHANGE (1 << 4)
> > @@ -273,15 +337,18 @@ static struct cxl_cmd cxl_cmd_set[256][256] = {
> >          cmd_events_get_interrupt_policy, 0, 0 },
> >      [EVENTS][SET_INTERRUPT_POLICY] = { "EVENTS_SET_INTERRUPT_POLICY",
> >          cmd_events_set_interrupt_policy, 4, IMMEDIATE_CONFIG_CHANGE },
> > +    [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
> > +        cmd_firmware_update_get_info, 0, 0 },
> >      [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> >      [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set, 8, IMMEDIATE_POLICY_CHANGE },
> >      [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", cmd_logs_get_supported, 0, 0 },
> >      [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
> >      [IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE",
> >          cmd_identify_memory_device, 0, 0 },
> > +    [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO",
> > +        cmd_ccls_get_partition_info, 0, 0 },
> >  };
> >  
> > -
Also fixed this bit of rebasing mess up.

Thanks,

Jonathan

> >  void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
> >  {
> >      uint16_t ret = CXL_MBOX_SUCCESS;  
> 
>
diff mbox series

Patch

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 808faec114..d022711b2a 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -44,6 +44,8 @@  enum {
         #define CLEAR_RECORDS   0x1
         #define GET_INTERRUPT_POLICY   0x2
         #define SET_INTERRUPT_POLICY   0x3
+    FIRMWARE_UPDATE = 0x02,
+        #define GET_INFO      0x0
     TIMESTAMP   = 0x03,
         #define GET           0x0
         #define SET           0x1
@@ -52,6 +54,8 @@  enum {
         #define GET_LOG       0x1
     IDENTIFY    = 0x40,
         #define MEMORY_DEVICE 0x0
+    CCLS        = 0x41,
+        #define GET_PARTITION_INFO     0x0
 };
 
 /* 8.2.8.4.5.1 Command Return Codes */
@@ -114,6 +118,39 @@  DEFINE_MAILBOX_HANDLER_NOP(events_clear_records);
 DEFINE_MAILBOX_HANDLER_ZEROED(events_get_interrupt_policy, 4);
 DEFINE_MAILBOX_HANDLER_NOP(events_set_interrupt_policy);
 
+/* 8.2.9.2.1 */
+static ret_code cmd_firmware_update_get_info(struct cxl_cmd *cmd,
+                                             CXLDeviceState *cxl_dstate,
+                                             uint16_t *len)
+{
+    struct {
+        uint8_t slots_supported;
+        uint8_t slot_info;
+        uint8_t caps;
+        uint8_t rsvd[0xd];
+        char fw_rev1[0x10];
+        char fw_rev2[0x10];
+        char fw_rev3[0x10];
+        char fw_rev4[0x10];
+    } __attribute__((packed)) *fw_info;
+    _Static_assert(sizeof(*fw_info) == 0x50, "Bad firmware info size");
+
+    if (cxl_dstate->pmem_size < (256 << 20)) {
+        return CXL_MBOX_INTERNAL_ERROR;
+    }
+
+    fw_info = (void *)cmd->payload;
+    memset(fw_info, 0, sizeof(*fw_info));
+
+    fw_info->slots_supported = 2;
+    fw_info->slot_info = BIT(0) | BIT(3);
+    fw_info->caps = 0;
+    snprintf(fw_info->fw_rev1, 0x10, "BWFW VERSION %02d", 0);
+
+    *len = sizeof(*fw_info);
+    return CXL_MBOX_SUCCESS;
+}
+
 /* 8.2.9.3.1 */
 static ret_code cmd_timestamp_get(struct cxl_cmd *cmd,
                                   CXLDeviceState *cxl_dstate,
@@ -260,6 +297,33 @@  static ret_code cmd_identify_memory_device(struct cxl_cmd *cmd,
     return CXL_MBOX_SUCCESS;
 }
 
+static ret_code cmd_ccls_get_partition_info(struct cxl_cmd *cmd,
+                                           CXLDeviceState *cxl_dstate,
+                                           uint16_t *len)
+{
+    struct {
+        uint64_t active_vmem;
+        uint64_t active_pmem;
+        uint64_t next_vmem;
+        uint64_t next_pmem;
+    } __attribute__((packed)) *part_info = (void *)cmd->payload;
+    _Static_assert(sizeof(*part_info) == 0x20, "Bad get partition info size");
+    uint64_t size = cxl_dstate->pmem_size;
+
+    if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
+        return CXL_MBOX_INTERNAL_ERROR;
+    }
+
+    /* PMEM only */
+    part_info->active_vmem = 0;
+    part_info->next_vmem = 0;
+    part_info->active_pmem = size / (256 << 20);
+    part_info->next_pmem = part_info->active_pmem;
+
+    *len = sizeof(*part_info);
+    return CXL_MBOX_SUCCESS;
+}
+
 #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
 #define IMMEDIATE_POLICY_CHANGE (1 << 3)
 #define IMMEDIATE_LOG_CHANGE (1 << 4)
@@ -273,15 +337,18 @@  static struct cxl_cmd cxl_cmd_set[256][256] = {
         cmd_events_get_interrupt_policy, 0, 0 },
     [EVENTS][SET_INTERRUPT_POLICY] = { "EVENTS_SET_INTERRUPT_POLICY",
         cmd_events_set_interrupt_policy, 4, IMMEDIATE_CONFIG_CHANGE },
+    [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
+        cmd_firmware_update_get_info, 0, 0 },
     [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
     [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set, 8, IMMEDIATE_POLICY_CHANGE },
     [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", cmd_logs_get_supported, 0, 0 },
     [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
     [IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE",
         cmd_identify_memory_device, 0, 0 },
+    [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO",
+        cmd_ccls_get_partition_info, 0, 0 },
 };
 
-
 void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
 {
     uint16_t ret = CXL_MBOX_SUCCESS;