| Message ID | 20220318150635.24600-20-Jonathan.Cameron@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | CXl 2.0 emulation Support | expand |
On Fri, Mar 18, 2022 at 03:06:08PM +0000, Jonathan Cameron wrote: > 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, 69 insertions(+) > snip > > +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; > + } QEMU_PACKED *part_info = (void *)cmd->payload; > + QEMU_BUILD_BUG_ON(sizeof(*part_info) != 0x20); > + 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; Setting next like this is logical, but it's not per the CXL spec: 8.2.9.5.2.1 "Next Persistent Capacity: If non-zero, this value shall become the Active Persistent Capacity on the next cold reset. If both this field and the Next Volatile Capacity field are zero, there is no pending change to the partitioning." next_(vmem|pmem) should start as zero and only change as the result of a successful set_partition_info command. From your cover letter: * Volatile memory devices (easy but it's more code so left for now). Wondering if this is something I could do, and follow that with set_partition support. Does that sound reasonable? Alison > + > + *len = sizeof(*part_info); > + return CXL_MBOX_SUCCESS; > +} > + snip
On Fri, 18 Mar 2022 09:56:22 -0700 Alison Schofield <alison.schofield@intel.com> wrote: > On Fri, Mar 18, 2022 at 03:06:08PM +0000, Jonathan Cameron wrote: > > 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, 69 insertions(+) > > > > snip > > > > > +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; > > + } QEMU_PACKED *part_info = (void *)cmd->payload; > > + QEMU_BUILD_BUG_ON(sizeof(*part_info) != 0x20); > > + 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; > > Setting next like this is logical, but it's not per the CXL spec: > > 8.2.9.5.2.1 > "Next Persistent Capacity: If non-zero, this value shall become the > Active Persistent Capacity on the next cold reset. If both this field and the > Next Volatile Capacity field are zero, there is no pending change to the > partitioning." > > next_(vmem|pmem) should start as zero and only change as the result > of a successful set_partition_info command. Ah. Good point and now fixed up. > > From your cover letter: > * Volatile memory devices (easy but it's more code so left for now). > Wondering if this is something I could do, and follow that with > set_partition support. Does that sound reasonable? Sure, that would be great. It raises an interesting question for what the volatile / non volatile backend will look like. So what is the command line? So far I'd been assuming we'd do it as separate memdev devices for volatile from those for non volatile because chances are someone testing would back the volatile with RAM the non volatile with a file. If we enable the partitioning control that approach won't make much sense. Various options come to mind. 1) Just back with one memdev and present that as persistent or not but actually always have it persistent under the hood. 2) Allow backing with 2 memdevs and don't support the set_partition command. Lazy approach I was planing on doing eventualy. 3) Support 3 memdevs. The middle one of which is the part we allow to be repartitioned if it is provided Also, to actually enable this we'd probably want more HDM decoders than currently supported and flags for CFMWS which aren't there yet (Ben asked for them but I've left it for a future patch set). Lots still to do once the initial patch set is moving forwards. I don't mind queuing stuff up it gitlab though and then we can send suitable series to the list once the earlier part has been applied. In the meantime we would have a moderately stable based to build on top of. Thanks, Jonathan > > Alison > > > + > > + *len = sizeof(*part_info); > > + return CXL_MBOX_SUCCESS; > > +} > > + > > snip > >
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 7ed9606c6b..fcd41d9a9d 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -10,6 +10,7 @@ #include "qemu/osdep.h" #include "hw/cxl/cxl.h" #include "hw/pci/pci.h" +#include "qemu/cutils.h" #include "qemu/log.h" #include "qemu/uuid.h" @@ -44,6 +45,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 +55,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 +119,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]; + } QEMU_PACKED *fw_info; + QEMU_BUILD_BUG_ON(sizeof(*fw_info) != 0x50); + + 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; + pstrcpy(fw_info->fw_rev1, sizeof(fw_info->fw_rev1), "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, @@ -258,6 +296,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; + } QEMU_PACKED *part_info = (void *)cmd->payload; + QEMU_BUILD_BUG_ON(sizeof(*part_info) != 0x20); + 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) @@ -271,12 +336,16 @@ 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)