| Message ID | 20230426021418.10186-2-dave@stgolabs.net |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/cxl: Add support for Scan Media | expand |
On Fri, 28 Apr 2023, Fan Ni wrote: >> +static CXLRetCode >> +cmd_media_get_scan_media_capabilities(struct cxl_cmd *cmd, >> + CXLDeviceState *cxl_dstate, uint16_t *len) >> +{ >> + struct get_scan_media_capabilities_pl { >> + uint64_t pa; >> + uint64_t length; >> + } QEMU_PACKED; >> + struct get_scan_media_capabilities_out_pl { >> + uint32_t estimated_runtime_ms; >> + } QEMU_PACKED; >> + struct get_scan_media_capabilities_pl *in = (void *)cmd->payload; >> + struct get_scan_media_capabilities_out_pl *out = (void *)cmd->payload; >> + uint64_t query_start; >> + uint64_t query_length; >> + >> + query_start = ldq_le_p(&in->pa); >> + /* 64 byte alignment required */ >> + if (query_start & 0x3f) { >> + return CXL_MBOX_INVALID_INPUT; >> + } >> + query_length = ldq_le_p(&in->length) * 64; >> + >> + if (query_start + query_length > cxl_dstate->mem_size) { >> + return CXL_MBOX_INVALID_PA; >> + } >> + >> + /* >> + * Just use 400 nanosecond access/read latency + 100 ns for >> + * the cost of updating the poison list. For small enough >> + * chunks return at least 1 ms. >> + */ >> + stq_le_p(&out->estimated_runtime_ms, >> + MAX(1, query_length * (0.0005L/64))); > >estimated_runtime_ms is uint32_t, use stl_le_p instead. Yup, thanks.
The 04/25/2023 19:14, Davidlohr Bueso wrote: > Use simple heuristics to determine the cost of scanning any given chunk, > assuming cost is even across the whole device. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > hw/cxl/cxl-mailbox-utils.c | 45 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 44 insertions(+), 1 deletion(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 0849cfbc2a4c..f346aa8ce3b2 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -79,6 +79,7 @@ enum { > #define GET_POISON_LIST 0x0 > #define INJECT_POISON 0x1 > #define CLEAR_POISON 0x2 > + #define GET_SCAN_MEDIA_CAPABILITIES 0x3 > PHYSICAL_SWITCH = 0x51 > #define IDENTIFY_SWITCH_DEVICE 0x0 > }; > @@ -882,7 +883,7 @@ static CXLRetCode cmd_media_clear_poison(struct cxl_cmd *cmd, > return CXL_MBOX_INTERNAL_ERROR; > } > } > - > + > QLIST_FOREACH(ent, poison_list, node) { > /* > * Test for contained in entry. Simpler than general case > @@ -934,6 +935,45 @@ static CXLRetCode cmd_media_clear_poison(struct cxl_cmd *cmd, > return CXL_MBOX_SUCCESS; > } > > +static CXLRetCode > +cmd_media_get_scan_media_capabilities(struct cxl_cmd *cmd, > + CXLDeviceState *cxl_dstate, uint16_t *len) > +{ > + struct get_scan_media_capabilities_pl { > + uint64_t pa; > + uint64_t length; > + } QEMU_PACKED; > + struct get_scan_media_capabilities_out_pl { > + uint32_t estimated_runtime_ms; > + } QEMU_PACKED; > + struct get_scan_media_capabilities_pl *in = (void *)cmd->payload; > + struct get_scan_media_capabilities_out_pl *out = (void *)cmd->payload; > + uint64_t query_start; > + uint64_t query_length; > + > + query_start = ldq_le_p(&in->pa); > + /* 64 byte alignment required */ > + if (query_start & 0x3f) { > + return CXL_MBOX_INVALID_INPUT; > + } > + query_length = ldq_le_p(&in->length) * 64; > + > + if (query_start + query_length > cxl_dstate->mem_size) { > + return CXL_MBOX_INVALID_PA; > + } > + > + /* > + * Just use 400 nanosecond access/read latency + 100 ns for > + * the cost of updating the poison list. For small enough > + * chunks return at least 1 ms. > + */ > + stq_le_p(&out->estimated_runtime_ms, > + MAX(1, query_length * (0.0005L/64))); estimated_runtime_ms is uint32_t, use stl_le_p instead. > + > + *len = sizeof(*out); > + return CXL_MBOX_SUCCESS; > +} > + > #define IMMEDIATE_CONFIG_CHANGE (1 << 1) > #define IMMEDIATE_DATA_CHANGE (1 << 2) > #define IMMEDIATE_POLICY_CHANGE (1 << 3) > @@ -972,6 +1012,9 @@ static struct cxl_cmd cxl_cmd_set[256][256] = { > cmd_media_inject_poison, 8, 0 }, > [MEDIA_AND_POISON][CLEAR_POISON] = { "MEDIA_AND_POISON_CLEAR_POISON", > cmd_media_clear_poison, 72, 0 }, > + [MEDIA_AND_POISON][GET_SCAN_MEDIA_CAPABILITIES] = { "MEDIA_AND_POISON_GET_SCAN_MEDIA_CAPABILITIES", > + cmd_media_get_scan_media_capabilities, 16, 0 }, > + > }; > > static struct cxl_cmd cxl_cmd_set_sw[256][256] = { > -- > 2.40.0 >
On Tue, 25 Apr 2023 19:14:16 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote: > Use simple heuristics to determine the cost of scanning any given chunk, > assuming cost is even across the whole device. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> Hi. A few trivial additions / comments / moans. Obviously it's an RFC but I can't resist pointing things out you'd fix for the v1. Thanks, Jonathan > --- > hw/cxl/cxl-mailbox-utils.c | 45 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 44 insertions(+), 1 deletion(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 0849cfbc2a4c..f346aa8ce3b2 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -79,6 +79,7 @@ enum { > #define GET_POISON_LIST 0x0 > #define INJECT_POISON 0x1 > #define CLEAR_POISON 0x2 > + #define GET_SCAN_MEDIA_CAPABILITIES 0x3 > PHYSICAL_SWITCH = 0x51 > #define IDENTIFY_SWITCH_DEVICE 0x0 > }; > @@ -882,7 +883,7 @@ static CXLRetCode cmd_media_clear_poison(struct cxl_cmd *cmd, > return CXL_MBOX_INTERNAL_ERROR; > } > } > - > + Hmm. Where did that white space sneak in? I'm guessing in something else I'm carrying on the branch you based on (I'm not seeing this locally now - so might already be fixed). Still shouldn't be in here! > QLIST_FOREACH(ent, poison_list, node) { > /* > * Test for contained in entry. Simpler than general case > @@ -934,6 +935,45 @@ static CXLRetCode cmd_media_clear_poison(struct cxl_cmd *cmd, > return CXL_MBOX_SUCCESS; > } > > +static CXLRetCode > +cmd_media_get_scan_media_capabilities(struct cxl_cmd *cmd, > + CXLDeviceState *cxl_dstate, uint16_t *len) > +{ > + struct get_scan_media_capabilities_pl { > + uint64_t pa; > + uint64_t length; > + } QEMU_PACKED; I'd add some blank lines to help readability a little. One here. > + struct get_scan_media_capabilities_out_pl { > + uint32_t estimated_runtime_ms; > + } QEMU_PACKED; One here. > + struct get_scan_media_capabilities_pl *in = (void *)cmd->payload; > + struct get_scan_media_capabilities_out_pl *out = (void *)cmd->payload; > + uint64_t query_start; > + uint64_t query_length; > + > + query_start = ldq_le_p(&in->pa); > + /* 64 byte alignment required */ > + if (query_start & 0x3f) { > + return CXL_MBOX_INVALID_INPUT; > + } > + query_length = ldq_le_p(&in->length) * 64; > + > + if (query_start + query_length > cxl_dstate->mem_size) { > + return CXL_MBOX_INVALID_PA; > + } > + > + /* > + * Just use 400 nanosecond access/read latency + 100 ns for > + * the cost of updating the poison list. For small enough > + * chunks return at least 1 ms. > + */ > + stq_le_p(&out->estimated_runtime_ms, > + MAX(1, query_length * (0.0005L/64))); > + > + *len = sizeof(*out); > + return CXL_MBOX_SUCCESS; > +} > + > #define IMMEDIATE_CONFIG_CHANGE (1 << 1) > #define IMMEDIATE_DATA_CHANGE (1 << 2) > #define IMMEDIATE_POLICY_CHANGE (1 << 3) > @@ -972,6 +1012,9 @@ static struct cxl_cmd cxl_cmd_set[256][256] = { > cmd_media_inject_poison, 8, 0 }, > [MEDIA_AND_POISON][CLEAR_POISON] = { "MEDIA_AND_POISON_CLEAR_POISON", > cmd_media_clear_poison, 72, 0 }, > + [MEDIA_AND_POISON][GET_SCAN_MEDIA_CAPABILITIES] = { "MEDIA_AND_POISON_GET_SCAN_MEDIA_CAPABILITIES", > + cmd_media_get_scan_media_capabilities, 16, 0 }, > + Grumpy maintainer moment. No blank line should be added here ;) > }; > > static struct cxl_cmd cxl_cmd_set_sw[256][256] = {
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 0849cfbc2a4c..f346aa8ce3b2 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -79,6 +79,7 @@ enum { #define GET_POISON_LIST 0x0 #define INJECT_POISON 0x1 #define CLEAR_POISON 0x2 + #define GET_SCAN_MEDIA_CAPABILITIES 0x3 PHYSICAL_SWITCH = 0x51 #define IDENTIFY_SWITCH_DEVICE 0x0 }; @@ -882,7 +883,7 @@ static CXLRetCode cmd_media_clear_poison(struct cxl_cmd *cmd, return CXL_MBOX_INTERNAL_ERROR; } } - + QLIST_FOREACH(ent, poison_list, node) { /* * Test for contained in entry. Simpler than general case @@ -934,6 +935,45 @@ static CXLRetCode cmd_media_clear_poison(struct cxl_cmd *cmd, return CXL_MBOX_SUCCESS; } +static CXLRetCode +cmd_media_get_scan_media_capabilities(struct cxl_cmd *cmd, + CXLDeviceState *cxl_dstate, uint16_t *len) +{ + struct get_scan_media_capabilities_pl { + uint64_t pa; + uint64_t length; + } QEMU_PACKED; + struct get_scan_media_capabilities_out_pl { + uint32_t estimated_runtime_ms; + } QEMU_PACKED; + struct get_scan_media_capabilities_pl *in = (void *)cmd->payload; + struct get_scan_media_capabilities_out_pl *out = (void *)cmd->payload; + uint64_t query_start; + uint64_t query_length; + + query_start = ldq_le_p(&in->pa); + /* 64 byte alignment required */ + if (query_start & 0x3f) { + return CXL_MBOX_INVALID_INPUT; + } + query_length = ldq_le_p(&in->length) * 64; + + if (query_start + query_length > cxl_dstate->mem_size) { + return CXL_MBOX_INVALID_PA; + } + + /* + * Just use 400 nanosecond access/read latency + 100 ns for + * the cost of updating the poison list. For small enough + * chunks return at least 1 ms. + */ + stq_le_p(&out->estimated_runtime_ms, + MAX(1, query_length * (0.0005L/64))); + + *len = sizeof(*out); + return CXL_MBOX_SUCCESS; +} + #define IMMEDIATE_CONFIG_CHANGE (1 << 1) #define IMMEDIATE_DATA_CHANGE (1 << 2) #define IMMEDIATE_POLICY_CHANGE (1 << 3) @@ -972,6 +1012,9 @@ static struct cxl_cmd cxl_cmd_set[256][256] = { cmd_media_inject_poison, 8, 0 }, [MEDIA_AND_POISON][CLEAR_POISON] = { "MEDIA_AND_POISON_CLEAR_POISON", cmd_media_clear_poison, 72, 0 }, + [MEDIA_AND_POISON][GET_SCAN_MEDIA_CAPABILITIES] = { "MEDIA_AND_POISON_GET_SCAN_MEDIA_CAPABILITIES", + cmd_media_get_scan_media_capabilities, 16, 0 }, + }; static struct cxl_cmd cxl_cmd_set_sw[256][256] = {
Use simple heuristics to determine the cost of scanning any given chunk, assuming cost is even across the whole device. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- hw/cxl/cxl-mailbox-utils.c | 45 +++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-)