* [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
* [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
* [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 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
* 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
* 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