* [PATCH -qemu 0/3] hw/cxl: Support dirty shutdown
@ 2024-12-20 16:00 Davidlohr Bueso
2024-12-20 16:00 ` [PATCH 1/3] cxl: Fix mbox cmd enum order Davidlohr Bueso
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Davidlohr Bueso @ 2024-12-20 16:00 UTC (permalink / raw)
To: jonathan.cameron; +Cc: fan.ni, dan.j.williams, a.manzanares, dave, linux-cxl
Hello,
The following adds support for emulating dirty shutdown semantics.
Patch 1 is a trivial cleanup, patch 2 adds get/set shutdown state
commands, and patch 3 introduces a new 'dirty-shutdown=N' parameter
for type3 memdevs which explicitly sets the dirty shutdown counter
to whatever passed value.
This series aids testing Global Persistent Flush support in the kernel.
Applies against the 'cxl-2024-11-27' branch in Jonathan's tree.
Thanks!
Davidlohr Bueso (3):
cxl: Fix mbox cmd enum order
cxl: Support Get/Set Shutdown State commands
cxl/type3: Add 'dirty-shutdown' parameter
hw/cxl/cxl-mailbox-utils.c | 91 +++++++++++++++++++++++++++++++++++--
hw/mem/cxl_type3.c | 1 +
include/hw/cxl/cxl_device.h | 5 ++
3 files changed, 92 insertions(+), 5 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] cxl: Fix mbox cmd enum order 2024-12-20 16:00 [PATCH -qemu 0/3] hw/cxl: Support dirty shutdown Davidlohr Bueso @ 2024-12-20 16:00 ` Davidlohr Bueso 2024-12-21 19:05 ` Fan Ni 2024-12-20 16:00 ` [PATCH 2/3] cxl: Support Get/Set Shutdown State commands Davidlohr Bueso 2024-12-20 16:00 ` [PATCH 3/3] cxl/type3: Add 'dirty-shutdown' parameter Davidlohr Bueso 2 siblings, 1 reply; 9+ messages in thread From: Davidlohr Bueso @ 2024-12-20 16:00 UTC (permalink / raw) To: jonathan.cameron; +Cc: fan.ni, dan.j.williams, a.manzanares, dave, linux-cxl Move the out of place commands into their proper place. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- hw/cxl/cxl-mailbox-utils.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 9c7ea5bc35b8..f6f993e7bc4f 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -86,11 +86,6 @@ enum { #define GET_PARTITION_INFO 0x0 #define GET_LSA 0x2 #define SET_LSA 0x3 - SANITIZE = 0x44, - #define OVERWRITE 0x0 - #define SECURE_ERASE 0x1 - PERSISTENT_MEM = 0x45, - #define GET_SECURITY_STATE 0x0 MEDIA_AND_POISON = 0x43, #define GET_POISON_LIST 0x0 #define INJECT_POISON 0x1 @@ -98,6 +93,11 @@ enum { #define GET_SCAN_MEDIA_CAPABILITIES 0x3 #define SCAN_MEDIA 0x4 #define GET_SCAN_MEDIA_RESULTS 0x5 + SANITIZE = 0x44, + #define OVERWRITE 0x0 + #define SECURE_ERASE 0x1 + PERSISTENT_MEM = 0x45, + #define GET_SECURITY_STATE 0x0 DCD_CONFIG = 0x48, #define GET_DC_CONFIG 0x0 #define GET_DYN_CAP_EXT_LIST 0x1 -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] cxl: Fix mbox cmd enum order 2024-12-20 16:00 ` [PATCH 1/3] cxl: Fix mbox cmd enum order Davidlohr Bueso @ 2024-12-21 19:05 ` Fan Ni 0 siblings, 0 replies; 9+ messages in thread From: Fan Ni @ 2024-12-21 19:05 UTC (permalink / raw) To: Davidlohr Bueso; +Cc: jonathan.cameron, dan.j.williams, a.manzanares, linux-cxl On Fri, Dec 20, 2024 at 08:00:24AM -0800, Davidlohr Bueso wrote: > Move the out of place commands into their proper place. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- Reviewed-by: Fan Ni <fan.ni@samsung.com> > hw/cxl/cxl-mailbox-utils.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 9c7ea5bc35b8..f6f993e7bc4f 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -86,11 +86,6 @@ enum { > #define GET_PARTITION_INFO 0x0 > #define GET_LSA 0x2 > #define SET_LSA 0x3 > - SANITIZE = 0x44, > - #define OVERWRITE 0x0 > - #define SECURE_ERASE 0x1 > - PERSISTENT_MEM = 0x45, > - #define GET_SECURITY_STATE 0x0 > MEDIA_AND_POISON = 0x43, > #define GET_POISON_LIST 0x0 > #define INJECT_POISON 0x1 > @@ -98,6 +93,11 @@ enum { > #define GET_SCAN_MEDIA_CAPABILITIES 0x3 > #define SCAN_MEDIA 0x4 > #define GET_SCAN_MEDIA_RESULTS 0x5 > + SANITIZE = 0x44, > + #define OVERWRITE 0x0 > + #define SECURE_ERASE 0x1 > + PERSISTENT_MEM = 0x45, > + #define GET_SECURITY_STATE 0x0 > DCD_CONFIG = 0x48, > #define GET_DC_CONFIG 0x0 > #define GET_DYN_CAP_EXT_LIST 0x1 > -- > 2.39.5 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] cxl: Support Get/Set Shutdown State commands 2024-12-20 16:00 [PATCH -qemu 0/3] hw/cxl: Support dirty shutdown Davidlohr Bueso 2024-12-20 16:00 ` [PATCH 1/3] cxl: Fix mbox cmd enum order Davidlohr Bueso @ 2024-12-20 16:00 ` Davidlohr Bueso 2024-12-21 19:18 ` Fan Ni 2024-12-23 20:01 ` Jonathan Cameron 2024-12-20 16:00 ` [PATCH 3/3] cxl/type3: Add 'dirty-shutdown' parameter Davidlohr Bueso 2 siblings, 2 replies; 9+ messages in thread From: Davidlohr Bueso @ 2024-12-20 16:00 UTC (permalink / raw) To: jonathan.cameron; +Cc: fan.ni, dan.j.williams, a.manzanares, dave, linux-cxl As per the latest spec, add mailbox commands 4203h and 4204h. While upon reboot this operation is obviously a nop (flag is cleared but the dirty shutdown count is not increased), it can still serve to test some basic paths. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- hw/cxl/cxl-mailbox-utils.c | 49 +++++++++++++++++++++++++++++++++++++ include/hw/cxl/cxl_device.h | 2 ++ 2 files changed, 51 insertions(+) diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index f6f993e7bc4f..ff1d3f50610c 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -86,6 +86,9 @@ enum { #define GET_PARTITION_INFO 0x0 #define GET_LSA 0x2 #define SET_LSA 0x3 + HEALTH_INFO_ALERTS = 0x42, + #define GET_SHUTDOWN_STATE 0x3 + #define SET_SHUTDOWN_STATE 0x4 MEDIA_AND_POISON = 0x43, #define GET_POISON_LIST 0x0 #define INJECT_POISON 0x1 @@ -1721,6 +1724,48 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, return CXL_MBOX_BG_STARTED; } +/* CXL r3.2 Section 8.2.10.9.3.4: Get Shutdown State (Opcode 4203h) */ +static CXLRetCode cmd_health_get_shutdown_state(const struct cxl_cmd *cmd, + uint8_t *payload_in, + size_t len_in, + uint8_t *payload_out, + size_t *len_out, + CXLCCI *cci) +{ + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; + struct get_shutdown_state_pl { + uint8_t state; + } QEMU_PACKED; + struct get_shutdown_state_pl *out = (void *)payload_out; + + out->state = cxl_dstate->shutdown_state; + *len_out = sizeof(out); + + return CXL_MBOX_SUCCESS; +} + +/* CXL r3.2 Section 8.2.10.9.3.5: Set Shutdown State (Opcode 4204h) */ +static CXLRetCode cmd_health_set_shutdown_state(const struct cxl_cmd *cmd, + uint8_t *payload_in, + size_t len_in, + uint8_t *payload_out, + size_t *len_out, + CXLCCI *cci) +{ + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; + struct set_shutdown_state_pl { + uint8_t state; + } QEMU_PACKED; + struct set_shutdown_state_pl *in = (void *)payload_in; + + cxl_dstate->shutdown_state = in->state; + *len_out = 0; + + return CXL_MBOX_SUCCESS; +} + static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, uint8_t *payload_in, size_t len_in, @@ -2866,6 +2911,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", cmd_get_security_state, 0, 0 }, + [HEALTH_INFO_ALERTS][GET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_GET_SHUTDOWN_STATE", + cmd_health_get_shutdown_state, 0, 0 }, + [HEALTH_INFO_ALERTS][SET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_SET_SHUTDOWN_STATE", + cmd_health_set_shutdown_state, 1, 0 }, [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST", cmd_media_get_poison_list, 16, 0 }, [MEDIA_AND_POISON][INJECT_POISON] = { "MEDIA_AND_POISON_INJECT_POISON", diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h index a64739be25e9..69e6330fe66d 100644 --- a/include/hw/cxl/cxl_device.h +++ b/include/hw/cxl/cxl_device.h @@ -292,6 +292,8 @@ typedef struct cxl_device_state { CPMUState cpmu[CXL_NUM_CPMU_INSTANCES]; CHMUState chmu[1]; CXLEventLog event_logs[CXL_EVENT_TYPE_MAX]; + + uint8_t shutdown_state; } CXLDeviceState; /* Initialize the register block for a device */ -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] cxl: Support Get/Set Shutdown State commands 2024-12-20 16:00 ` [PATCH 2/3] cxl: Support Get/Set Shutdown State commands Davidlohr Bueso @ 2024-12-21 19:18 ` Fan Ni 2024-12-23 19:57 ` Jonathan Cameron 2024-12-23 20:01 ` Jonathan Cameron 1 sibling, 1 reply; 9+ messages in thread From: Fan Ni @ 2024-12-21 19:18 UTC (permalink / raw) To: Davidlohr Bueso; +Cc: jonathan.cameron, dan.j.williams, a.manzanares, linux-cxl On Fri, Dec 20, 2024 at 08:00:25AM -0800, Davidlohr Bueso wrote: > As per the latest spec, add mailbox commands 4203h and 4204h. > While upon reboot this operation is obviously a nop (flag is > cleared but the dirty shutdown count is not increased), it > can still serve to test some basic paths. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> ... > --- > hw/cxl/cxl-mailbox-utils.c | 49 +++++++++++++++++++++++++++++++++++++ > include/hw/cxl/cxl_device.h | 2 ++ > 2 files changed, 51 insertions(+) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index f6f993e7bc4f..ff1d3f50610c 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -86,6 +86,9 @@ enum { > #define GET_PARTITION_INFO 0x0 > #define GET_LSA 0x2 > #define SET_LSA 0x3 > + HEALTH_INFO_ALERTS = 0x42, > + #define GET_SHUTDOWN_STATE 0x3 > + #define SET_SHUTDOWN_STATE 0x4 > MEDIA_AND_POISON = 0x43, > #define GET_POISON_LIST 0x0 > #define INJECT_POISON 0x1 > @@ -1721,6 +1724,48 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +/* CXL r3.2 Section 8.2.10.9.3.4: Get Shutdown State (Opcode 4203h) */ > +static CXLRetCode cmd_health_get_shutdown_state(const struct cxl_cmd *cmd, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; > + struct get_shutdown_state_pl { > + uint8_t state; > + } QEMU_PACKED; > + struct get_shutdown_state_pl *out = (void *)payload_out; > + > + out->state = cxl_dstate->shutdown_state; > + *len_out = sizeof(out); > + > + return CXL_MBOX_SUCCESS; > +} > + > +/* CXL r3.2 Section 8.2.10.9.3.5: Set Shutdown State (Opcode 4204h) */ > +static CXLRetCode cmd_health_set_shutdown_state(const struct cxl_cmd *cmd, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; > + struct set_shutdown_state_pl { > + uint8_t state; > + } QEMU_PACKED; > + struct set_shutdown_state_pl *in = (void *)payload_in; Do we need to check the input size and return invalid input payload size? > + > + cxl_dstate->shutdown_state = in->state; > + *len_out = 0; > + > + return CXL_MBOX_SUCCESS; > +} > + > static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, > uint8_t *payload_in, > size_t len_in, > @@ -2866,6 +2911,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { > CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, > [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", > cmd_get_security_state, 0, 0 }, > + [HEALTH_INFO_ALERTS][GET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_GET_SHUTDOWN_STATE", > + cmd_health_get_shutdown_state, 0, 0 }, > + [HEALTH_INFO_ALERTS][SET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_SET_SHUTDOWN_STATE", > + cmd_health_set_shutdown_state, 1, 0 }, Per the spec, I think the return value should be CXL_MBOX_IMMEDIATE_POLICY_CHANGE. Fan > [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST", > cmd_media_get_poison_list, 16, 0 }, > [MEDIA_AND_POISON][INJECT_POISON] = { "MEDIA_AND_POISON_INJECT_POISON", > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > index a64739be25e9..69e6330fe66d 100644 > --- a/include/hw/cxl/cxl_device.h > +++ b/include/hw/cxl/cxl_device.h > @@ -292,6 +292,8 @@ typedef struct cxl_device_state { > CPMUState cpmu[CXL_NUM_CPMU_INSTANCES]; > CHMUState chmu[1]; > CXLEventLog event_logs[CXL_EVENT_TYPE_MAX]; > + > + uint8_t shutdown_state; > } CXLDeviceState; > > /* Initialize the register block for a device */ > -- > 2.39.5 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] cxl: Support Get/Set Shutdown State commands 2024-12-21 19:18 ` Fan Ni @ 2024-12-23 19:57 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2024-12-23 19:57 UTC (permalink / raw) To: Fan Ni; +Cc: Davidlohr Bueso, dan.j.williams, a.manzanares, linux-cxl On Sat, 21 Dec 2024 11:18:48 -0800 Fan Ni <nifan.cxl@gmail.com> wrote: > On Fri, Dec 20, 2024 at 08:00:25AM -0800, Davidlohr Bueso wrote: > > As per the latest spec, add mailbox commands 4203h and 4204h. > > While upon reboot this operation is obviously a nop (flag is > > cleared but the dirty shutdown count is not increased), it > > can still serve to test some basic paths. > > > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > > ... > > > --- > > hw/cxl/cxl-mailbox-utils.c | 49 +++++++++++++++++++++++++++++++++++++ > > include/hw/cxl/cxl_device.h | 2 ++ > > 2 files changed, 51 insertions(+) > > > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > > index f6f993e7bc4f..ff1d3f50610c 100644 > > --- a/hw/cxl/cxl-mailbox-utils.c > > +++ b/hw/cxl/cxl-mailbox-utils.c > > @@ -86,6 +86,9 @@ enum { > > #define GET_PARTITION_INFO 0x0 > > #define GET_LSA 0x2 > > #define SET_LSA 0x3 > > + HEALTH_INFO_ALERTS = 0x42, > > + #define GET_SHUTDOWN_STATE 0x3 > > + #define SET_SHUTDOWN_STATE 0x4 > > MEDIA_AND_POISON = 0x43, > > #define GET_POISON_LIST 0x0 > > #define INJECT_POISON 0x1 > > @@ -1721,6 +1724,48 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > > return CXL_MBOX_BG_STARTED; > > } > > > > +/* CXL r3.2 Section 8.2.10.9.3.4: Get Shutdown State (Opcode 4203h) */ > > +static CXLRetCode cmd_health_get_shutdown_state(const struct cxl_cmd *cmd, > > + uint8_t *payload_in, > > + size_t len_in, > > + uint8_t *payload_out, > > + size_t *len_out, > > + CXLCCI *cci) > > +{ > > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > > + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; > > + struct get_shutdown_state_pl { > > + uint8_t state; > > + } QEMU_PACKED; > > + struct get_shutdown_state_pl *out = (void *)payload_out; > > + > > + out->state = cxl_dstate->shutdown_state; > > + *len_out = sizeof(out); > > + > > + return CXL_MBOX_SUCCESS; > > +} > > + > > +/* CXL r3.2 Section 8.2.10.9.3.5: Set Shutdown State (Opcode 4204h) */ > > +static CXLRetCode cmd_health_set_shutdown_state(const struct cxl_cmd *cmd, > > + uint8_t *payload_in, > > + size_t len_in, > > + uint8_t *payload_out, > > + size_t *len_out, > > + CXLCCI *cci) > > +{ > > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > > + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; > > + struct set_shutdown_state_pl { > > + uint8_t state; > > + } QEMU_PACKED; > > + struct set_shutdown_state_pl *in = (void *)payload_in; > > Do we need to check the input size and return invalid input payload > size? Not for fixed size commands. See check in cxl_process_cci_message. It's the variable sized commands that trip us up and need explicit checks. > > > + > > + cxl_dstate->shutdown_state = in->state; > > + *len_out = 0; > > + > > + return CXL_MBOX_SUCCESS; > > +} > > + > > static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, > > uint8_t *payload_in, > > size_t len_in, > > @@ -2866,6 +2911,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { > > CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, > > [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", > > cmd_get_security_state, 0, 0 }, > > + [HEALTH_INFO_ALERTS][GET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_GET_SHUTDOWN_STATE", > > + cmd_health_get_shutdown_state, 0, 0 }, > > + [HEALTH_INFO_ALERTS][SET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_SET_SHUTDOWN_STATE", > > + cmd_health_set_shutdown_state, 1, 0 }, > > Per the spec, I think the return value should be CXL_MBOX_IMMEDIATE_POLICY_CHANGE. > > Fan > > [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST", > > cmd_media_get_poison_list, 16, 0 }, > > [MEDIA_AND_POISON][INJECT_POISON] = { "MEDIA_AND_POISON_INJECT_POISON", > > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > > index a64739be25e9..69e6330fe66d 100644 > > --- a/include/hw/cxl/cxl_device.h > > +++ b/include/hw/cxl/cxl_device.h > > @@ -292,6 +292,8 @@ typedef struct cxl_device_state { > > CPMUState cpmu[CXL_NUM_CPMU_INSTANCES]; > > CHMUState chmu[1]; > > CXLEventLog event_logs[CXL_EVENT_TYPE_MAX]; > > + > > + uint8_t shutdown_state; > > } CXLDeviceState; > > > > /* Initialize the register block for a device */ > > -- > > 2.39.5 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] cxl: Support Get/Set Shutdown State commands 2024-12-20 16:00 ` [PATCH 2/3] cxl: Support Get/Set Shutdown State commands Davidlohr Bueso 2024-12-21 19:18 ` Fan Ni @ 2024-12-23 20:01 ` Jonathan Cameron 1 sibling, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2024-12-23 20:01 UTC (permalink / raw) To: Davidlohr Bueso; +Cc: fan.ni, dan.j.williams, a.manzanares, linux-cxl On Fri, 20 Dec 2024 08:00:25 -0800 Davidlohr Bueso <dave@stgolabs.net> wrote: > As per the latest spec, add mailbox commands 4203h and 4204h. > While upon reboot this operation is obviously a nop (flag is > cleared but the dirty shutdown count is not increased), it > can still serve to test some basic paths. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> Hi Davidlohr,, Some trivial stuff inline. Thanks, Jonathan > --- > hw/cxl/cxl-mailbox-utils.c | 49 +++++++++++++++++++++++++++++++++++++ > include/hw/cxl/cxl_device.h | 2 ++ > 2 files changed, 51 insertions(+) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index f6f993e7bc4f..ff1d3f50610c 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -86,6 +86,9 @@ enum { > #define GET_PARTITION_INFO 0x0 > #define GET_LSA 0x2 > #define SET_LSA 0x3 > + HEALTH_INFO_ALERTS = 0x42, > + #define GET_SHUTDOWN_STATE 0x3 > + #define SET_SHUTDOWN_STATE 0x4 > MEDIA_AND_POISON = 0x43, > #define GET_POISON_LIST 0x0 > #define INJECT_POISON 0x1 > @@ -1721,6 +1724,48 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +/* CXL r3.2 Section 8.2.10.9.3.4: Get Shutdown State (Opcode 4203h) */ > +static CXLRetCode cmd_health_get_shutdown_state(const struct cxl_cmd *cmd, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; > + struct get_shutdown_state_pl { > + uint8_t state; > + } QEMU_PACKED; > + struct get_shutdown_state_pl *out = (void *)payload_out; We only need the (void *) for unnamed types. Where we know the type might as well get it right. E.g. see cmd_events_set_interrupt_policy. Mind you, not much point in keeping the type named here. struct { uint8_t state; } QEMU_PACKED = (void *)payload_out; > + > + out->state = cxl_dstate->shutdown_state; > + *len_out = sizeof(out); > + > + return CXL_MBOX_SUCCESS; > +} > + > +/* CXL r3.2 Section 8.2.10.9.3.5: Set Shutdown State (Opcode 4204h) */ > +static CXLRetCode cmd_health_set_shutdown_state(const struct cxl_cmd *cmd, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLDeviceState *cxl_dstate = &ct3d->cxl_dstate; > + struct set_shutdown_state_pl { > + uint8_t state; > + } QEMU_PACKED; As above. Naming the type isn't that useful and if you want to should cast to that not a void * > + struct set_shutdown_state_pl *in = (void *)payload_in; > + > + cxl_dstate->shutdown_state = in->state; > + *len_out = 0; > + > + return CXL_MBOX_SUCCESS; > +} > + > static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, > uint8_t *payload_in, > size_t len_in, > @@ -2866,6 +2911,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { > CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, > [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", > cmd_get_security_state, 0, 0 }, > + [HEALTH_INFO_ALERTS][GET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_GET_SHUTDOWN_STATE", > + cmd_health_get_shutdown_state, 0, 0 }, > + [HEALTH_INFO_ALERTS][SET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_SET_SHUTDOWN_STATE", > + cmd_health_set_shutdown_state, 1, 0 }, > [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST", > cmd_media_get_poison_list, 16, 0 }, > [MEDIA_AND_POISON][INJECT_POISON] = { "MEDIA_AND_POISON_INJECT_POISON", > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > index a64739be25e9..69e6330fe66d 100644 > --- a/include/hw/cxl/cxl_device.h > +++ b/include/hw/cxl/cxl_device.h > @@ -292,6 +292,8 @@ typedef struct cxl_device_state { > CPMUState cpmu[CXL_NUM_CPMU_INSTANCES]; > CHMUState chmu[1]; > CXLEventLog event_logs[CXL_EVENT_TYPE_MAX]; > + > + uint8_t shutdown_state; > } CXLDeviceState; > > /* Initialize the register block for a device */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] cxl/type3: Add 'dirty-shutdown' parameter 2024-12-20 16:00 [PATCH -qemu 0/3] hw/cxl: Support dirty shutdown Davidlohr Bueso 2024-12-20 16:00 ` [PATCH 1/3] cxl: Fix mbox cmd enum order Davidlohr Bueso 2024-12-20 16:00 ` [PATCH 2/3] cxl: Support Get/Set Shutdown State commands Davidlohr Bueso @ 2024-12-20 16:00 ` Davidlohr Bueso 2024-12-23 20:08 ` Jonathan Cameron 2 siblings, 1 reply; 9+ messages in thread From: Davidlohr Bueso @ 2024-12-20 16:00 UTC (permalink / raw) To: jonathan.cameron; +Cc: fan.ni, dan.j.williams, a.manzanares, dave, linux-cxl Add a new parameter for type3 memory devices to set the dirty shutdown count to a specified value. This allows emulating failure paths and informing the admin of such event via the Get Health Info command. For example, upon a failed GPF, users can boot with dirty-shutdown=1 and with the cleared shutdown state, to emulate the hardware behavior. root@cxl:~# cxl list -m mem1 -H { "memdev":"mem1", "pmem_size":2147483648, "health":{ "maintenance_needed":false, "performance_degraded":false, "hw_replacement_needed":false, "media_normal":true, "media_not_ready":false, "media_persistence_lost":false, "media_data_lost":false, "media_powerloss_persistence_loss":false, "media_shutdown_persistence_loss":false, "media_persistence_loss_imminent":false, "media_powerloss_data_loss":false, "media_shutdown_data_loss":false, "media_data_loss_imminent":false, "ext_life_used":"normal", "ext_temperature":"normal", "ext_corrected_volatile":"normal", "ext_corrected_persistent":"normal", "life_used_percent":20, "temperature":30, "dirty_shutdowns":1, "volatile_errors":0, "pmem_errors":0 }, "serial":0, "host":"0000:0e:00.0" } Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- hw/cxl/cxl-mailbox-utils.c | 32 ++++++++++++++++++++++++++++++++ hw/mem/cxl_type3.c | 1 + include/hw/cxl/cxl_device.h | 3 +++ 3 files changed, 36 insertions(+) diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index ff1d3f50610c..85a58ab96bef 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -87,6 +87,7 @@ enum { #define GET_LSA 0x2 #define SET_LSA 0x3 HEALTH_INFO_ALERTS = 0x42, + #define GET_HEALTH_INFO 0x0 #define GET_SHUTDOWN_STATE 0x3 #define SET_SHUTDOWN_STATE 0x4 MEDIA_AND_POISON = 0x43, @@ -1724,6 +1725,35 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, return CXL_MBOX_BG_STARTED; } +/* CXL r3.2 Section 8.2.10.9.3.1: Get Shutdown State (Opcode 4200h) */ +static CXLRetCode cmd_health_get_health_info(const struct cxl_cmd *cmd, + uint8_t *payload_in, + size_t len_in, + uint8_t *payload_out, + size_t *len_out, + CXLCCI *cci) +{ + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); + struct get_health_info_pl { + uint8_t health_status; + uint8_t media_status; + uint8_t additional_status; + uint8_t life_used; + uint16_t device_temperature; + uint32_t dirty_shutdown_count; + uint32_t corrected_volatile_error_count; + uint32_t corrected_persistent_error_count; + } QEMU_PACKED *out = (void *)payload_out; + + /* anything not set explicitly is considered under normal health */ + out->life_used = 20; + out->device_temperature = 30; + out->dirty_shutdown_count = ct3d->dirty_shutdown; + *len_out = sizeof(out); + + return CXL_MBOX_SUCCESS; +} + /* CXL r3.2 Section 8.2.10.9.3.4: Get Shutdown State (Opcode 4203h) */ static CXLRetCode cmd_health_get_shutdown_state(const struct cxl_cmd *cmd, uint8_t *payload_in, @@ -2911,6 +2941,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", cmd_get_security_state, 0, 0 }, + [HEALTH_INFO_ALERTS][GET_HEALTH_INFO] = { "HEALTH_INFO_ALERTS_GET_HEALTH_INFO", + cmd_health_get_health_info, 0, 0 }, [HEALTH_INFO_ALERTS][GET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_GET_SHUTDOWN_STATE", cmd_health_get_shutdown_state, 0, 0 }, [HEALTH_INFO_ALERTS][SET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_SET_SHUTDOWN_STATE", diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c index 5f365afb4dd1..e622eb9101ce 100644 --- a/hw/mem/cxl_type3.c +++ b/hw/mem/cxl_type3.c @@ -1380,6 +1380,7 @@ static Property ct3_props[] = { TYPE_MEMORY_BACKEND, HostMemoryBackend *), DEFINE_PROP_LINK("lsa", CXLType3Dev, lsa, TYPE_MEMORY_BACKEND, HostMemoryBackend *), + DEFINE_PROP_UINT32("dirty-shutdown", CXLType3Dev, dirty_shutdown, 0), DEFINE_PROP_UINT64("sn", CXLType3Dev, sn, UI64_NULL), DEFINE_PROP_STRING("cdat", CXLType3Dev, cxl_cstate.cdat.filename), DEFINE_PROP_UINT8("num-dc-regions", CXLType3Dev, dc.num_regions, 0), diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h index 69e6330fe66d..f756e1a99f33 100644 --- a/include/hw/cxl/cxl_device.h +++ b/include/hw/cxl/cxl_device.h @@ -653,6 +653,9 @@ struct CXLType3Dev { uint8_t num_regions; /* 0-8 regions */ CXLDCRegion regions[DCD_MAX_NUM_REGION]; } dc; + + /* Dirty shutdown count */ + uint32_t dirty_shutdown; }; #define TYPE_CXL_TYPE3 "cxl-type3" -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] cxl/type3: Add 'dirty-shutdown' parameter 2024-12-20 16:00 ` [PATCH 3/3] cxl/type3: Add 'dirty-shutdown' parameter Davidlohr Bueso @ 2024-12-23 20:08 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2024-12-23 20:08 UTC (permalink / raw) To: Davidlohr Bueso; +Cc: fan.ni, dan.j.williams, a.manzanares, linux-cxl On Fri, 20 Dec 2024 08:00:26 -0800 Davidlohr Bueso <dave@stgolabs.net> wrote: > Add a new parameter for type3 memory devices to set the > dirty shutdown count to a specified value. This allows > emulating failure paths and informing the admin of such > event via the Get Health Info command. > > For example, upon a failed GPF, users can boot with > dirty-shutdown=1 and with the cleared shutdown state, > to emulate the hardware behavior. > Just noticed, this isn't +CC to qemu-devel. Please do that even for patches posted for testing. Makes them easier to upstream later if we want to as the discussion is all there. A few comments inline. Jonathan > root@cxl:~# cxl list -m mem1 -H > { > "memdev":"mem1", > "pmem_size":2147483648, > "health":{ > "maintenance_needed":false, > "performance_degraded":false, > "hw_replacement_needed":false, > "media_normal":true, > "media_not_ready":false, > "media_persistence_lost":false, > "media_data_lost":false, > "media_powerloss_persistence_loss":false, > "media_shutdown_persistence_loss":false, > "media_persistence_loss_imminent":false, > "media_powerloss_data_loss":false, > "media_shutdown_data_loss":false, > "media_data_loss_imminent":false, > "ext_life_used":"normal", > "ext_temperature":"normal", > "ext_corrected_volatile":"normal", > "ext_corrected_persistent":"normal", > "life_used_percent":20, > "temperature":30, > "dirty_shutdowns":1, > "volatile_errors":0, > "pmem_errors":0 > }, > "serial":0, > "host":"0000:0e:00.0" > } > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > hw/cxl/cxl-mailbox-utils.c | 32 ++++++++++++++++++++++++++++++++ > hw/mem/cxl_type3.c | 1 + > include/hw/cxl/cxl_device.h | 3 +++ > 3 files changed, 36 insertions(+) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index ff1d3f50610c..85a58ab96bef 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -87,6 +87,7 @@ enum { > #define GET_LSA 0x2 > #define SET_LSA 0x3 > HEALTH_INFO_ALERTS = 0x42, > + #define GET_HEALTH_INFO 0x0 > #define GET_SHUTDOWN_STATE 0x3 > #define SET_SHUTDOWN_STATE 0x4 > MEDIA_AND_POISON = 0x43, > @@ -1724,6 +1725,35 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +/* CXL r3.2 Section 8.2.10.9.3.1: Get Shutdown State (Opcode 4200h) */ > +static CXLRetCode cmd_health_get_health_info(const struct cxl_cmd *cmd, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + struct get_health_info_pl { > + uint8_t health_status; > + uint8_t media_status; > + uint8_t additional_status; > + uint8_t life_used; > + uint16_t device_temperature; > + uint32_t dirty_shutdown_count; > + uint32_t corrected_volatile_error_count; > + uint32_t corrected_persistent_error_count; This duplicates most of CXLEventMemoryModule (which is defined in the spec in terms of this payload. We should factor it out of there an into a header to reuse in two places. Also make sure the data matches for the stuff like device_temperature. > + } QEMU_PACKED *out = (void *)payload_out; > + > + /* anything not set explicitly is considered under normal health */ > + out->life_used = 20; > + out->device_temperature = 30; > + out->dirty_shutdown_count = ct3d->dirty_shutdown; > + *len_out = sizeof(out); > + > + return CXL_MBOX_SUCCESS; > +} > + > /* CXL r3.2 Section 8.2.10.9.3.4: Get Shutdown State (Opcode 4203h) */ > static CXLRetCode cmd_health_get_shutdown_state(const struct cxl_cmd *cmd, > uint8_t *payload_in, > @@ -2911,6 +2941,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { > CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, > [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", > cmd_get_security_state, 0, 0 }, > + [HEALTH_INFO_ALERTS][GET_HEALTH_INFO] = { "HEALTH_INFO_ALERTS_GET_HEALTH_INFO", > + cmd_health_get_health_info, 0, 0 }, > [HEALTH_INFO_ALERTS][GET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_GET_SHUTDOWN_STATE", > cmd_health_get_shutdown_state, 0, 0 }, > [HEALTH_INFO_ALERTS][SET_SHUTDOWN_STATE] = { "HEALTH_INFO_ALERTS_SET_SHUTDOWN_STATE", > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > index 5f365afb4dd1..e622eb9101ce 100644 > --- a/hw/mem/cxl_type3.c > +++ b/hw/mem/cxl_type3.c > @@ -1380,6 +1380,7 @@ static Property ct3_props[] = { > TYPE_MEMORY_BACKEND, HostMemoryBackend *), > DEFINE_PROP_LINK("lsa", CXLType3Dev, lsa, TYPE_MEMORY_BACKEND, > HostMemoryBackend *), > + DEFINE_PROP_UINT32("dirty-shutdown", CXLType3Dev, dirty_shutdown, 0), > DEFINE_PROP_UINT64("sn", CXLType3Dev, sn, UI64_NULL), > DEFINE_PROP_STRING("cdat", CXLType3Dev, cxl_cstate.cdat.filename), > DEFINE_PROP_UINT8("num-dc-regions", CXLType3Dev, dc.num_regions, 0), > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > index 69e6330fe66d..f756e1a99f33 100644 > --- a/include/hw/cxl/cxl_device.h > +++ b/include/hw/cxl/cxl_device.h > @@ -653,6 +653,9 @@ struct CXLType3Dev { > uint8_t num_regions; /* 0-8 regions */ > CXLDCRegion regions[DCD_MAX_NUM_REGION]; > } dc; > + > + /* Dirty shutdown count */ > + uint32_t dirty_shutdown; > }; > > #define TYPE_CXL_TYPE3 "cxl-type3" ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-12-23 20:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-20 16:00 [PATCH -qemu 0/3] hw/cxl: Support dirty shutdown Davidlohr Bueso 2024-12-20 16:00 ` [PATCH 1/3] cxl: Fix mbox cmd enum order Davidlohr Bueso 2024-12-21 19:05 ` Fan Ni 2024-12-20 16:00 ` [PATCH 2/3] cxl: Support Get/Set Shutdown State commands Davidlohr Bueso 2024-12-21 19:18 ` Fan Ni 2024-12-23 19:57 ` Jonathan Cameron 2024-12-23 20:01 ` Jonathan Cameron 2024-12-20 16:00 ` [PATCH 3/3] cxl/type3: Add 'dirty-shutdown' parameter Davidlohr Bueso 2024-12-23 20:08 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox