* [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features
@ 2023-11-24 13:53 shiju.jose--- via
2023-11-24 13:53 ` [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6) shiju.jose--- via
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: shiju.jose--- via @ 2023-11-24 13:53 UTC (permalink / raw)
To: qemu-devel, linux-cxl
Cc: jonathan.cameron, tanxiaofei, prime.zeng, linuxarm, shiju.jose
From: Shiju Jose <shiju.jose@huawei.com>
Add support for the feature commands, device patrol scrub control and
DDR5 ECS control features.
CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub control
feature.
CXL spec 3.1 section 8.2.9.9.11.2 describes the DDR5 Error Check Scrub (ECS)
control feature.
The patches are available here,
https://gitlab.com/shiju.jose/qemu/-/tree/cxl-scrub-2023-11-14
and is based on Jonathan's branch
https://gitlab.com/jic23/qemu/-/tree/cxl-2023-10-16
Changes
v1 -> v2
1. Changes for Davidlohr comments. Thanks.
- Changed CXL SET feature data transfer flags as enum.
- Modified pointer supported_feats to get_feats_out.
- Removed an unnecessary branch.
- Use MIN().
- Move setting of hdr.nsuppfeats further down.
- Return CXL_MBOX_UNSUPPORTED if non-zero selection flag is passed.
- Add more IMMEDIATE_*.* flags set_feature.
- Corrected a spelling error.
Shiju Jose (3):
hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)
hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control feature
hw/cxl/cxl-mailbox-utils.c | 360 +++++++++++++++++++++++++++++++++++++
1 file changed, 360 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)
2023-11-24 13:53 [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features shiju.jose--- via
@ 2023-11-24 13:53 ` shiju.jose--- via
2024-02-15 18:05 ` fan
2023-11-24 13:53 ` [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature shiju.jose--- via
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: shiju.jose--- via @ 2023-11-24 13:53 UTC (permalink / raw)
To: qemu-devel, linux-cxl
Cc: jonathan.cameron, tanxiaofei, prime.zeng, linuxarm, shiju.jose
From: Shiju Jose <shiju.jose@huawei.com>
CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
CXL devices supports features with changeable attributes.
Get Supported Features retrieves the list of supported device specific
features. The settings of a feature can be retrieved using Get Feature and
optionally modified using Set Feature.
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
hw/cxl/cxl-mailbox-utils.c | 167 +++++++++++++++++++++++++++++++++++++
1 file changed, 167 insertions(+)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 6184f44339..1bbc9a48a6 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -66,6 +66,10 @@ enum {
LOGS = 0x04,
#define GET_SUPPORTED 0x0
#define GET_LOG 0x1
+ FEATURES = 0x05,
+ #define GET_SUPPORTED 0x0
+ #define GET_FEATURE 0x1
+ #define SET_FEATURE 0x2
IDENTIFY = 0x40,
#define MEMORY_DEVICE 0x0
CCLS = 0x41,
@@ -785,6 +789,157 @@ static CXLRetCode cmd_logs_get_log(const struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
+/* CXL r3.0 section 8.2.9.6: Features */
+typedef struct CXLSupportedFeatureHeader {
+ uint16_t entries;
+ uint16_t nsuppfeats_dev;
+ uint32_t reserved;
+} QEMU_PACKED CXLSupportedFeatureHeader;
+
+typedef struct CXLSupportedFeatureEntry {
+ QemuUUID uuid;
+ uint16_t feat_index;
+ uint16_t get_feat_size;
+ uint16_t set_feat_size;
+ uint32_t attrb_flags;
+ uint8_t get_feat_version;
+ uint8_t set_feat_version;
+ uint16_t set_feat_effects;
+ uint8_t rsvd[18];
+} QEMU_PACKED CXLSupportedFeatureEntry;
+
+enum CXL_SUPPORTED_FEATURES_LIST {
+ CXL_FEATURE_MAX
+};
+
+/* Get Feature CXL 3.0 Spec 8.2.9.6.2 */
+/*
+ * Get Feature input payload
+ * CXL rev 3.0 section 8.2.9.6.2; Table 8-79
+ */
+/* Get Feature : Payload in selection */
+enum CXL_GET_FEATURE_SELECTION {
+ CXL_GET_FEATURE_SEL_CURRENT_VALUE = 0x0,
+ CXL_GET_FEATURE_SEL_DEFAULT_VALUE = 0x1,
+ CXL_GET_FEATURE_SEL_SAVED_VALUE = 0x2,
+ CXL_GET_FEATURE_SEL_MAX
+};
+
+/* Set Feature CXL 3.0 Spec 8.2.9.6.3 */
+/*
+ * Set Feature input payload
+ * CXL rev 3.0 section 8.2.9.6.3; Table 8-81
+ */
+typedef struct CXLSetFeatureInHeader {
+ QemuUUID uuid;
+ uint32_t flags;
+ uint16_t offset;
+ uint8_t version;
+ uint8_t rsvd[9];
+} QEMU_PACKED QEMU_ALIGNED(16) CXLSetFeatureInHeader;
+
+/* Set Feature : Payload in flags */
+#define CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK 0x7
+enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
+ CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER = 0x0,
+ CXL_SET_FEATURE_FLAG_INITIATE_DATA_TRANSFER = 0x1,
+ CXL_SET_FEATURE_FLAG_CONTINUE_DATA_TRANSFER = 0x2,
+ CXL_SET_FEATURE_FLAG_FINISH_DATA_TRANSFER = 0x3,
+ CXL_SET_FEATURE_FLAG_ABORT_DATA_TRANSFER = 0x4,
+ CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
+};
+
+/* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) */
+static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
+ uint8_t *payload_in,
+ size_t len_in,
+ uint8_t *payload_out,
+ size_t *len_out,
+ CXLCCI *cci)
+{
+ struct {
+ uint32_t count;
+ uint16_t start_index;
+ uint16_t reserved;
+ } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_in = (void *)payload_in;
+
+ struct {
+ CXLSupportedFeatureHeader hdr;
+ CXLSupportedFeatureEntry feat_entries[];
+ } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_out = (void *)payload_out;
+ uint16_t index;
+ uint16_t entry, req_entries;
+ uint16_t feat_entries = 0;
+
+ if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
+ get_feats_in->start_index > CXL_FEATURE_MAX) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+ req_entries = (get_feats_in->count -
+ sizeof(CXLSupportedFeatureHeader)) /
+ sizeof(CXLSupportedFeatureEntry);
+ req_entries = MIN(req_entries, CXL_FEATURE_MAX);
+ index = get_feats_in->start_index;
+
+ entry = 0;
+ while (entry < req_entries) {
+ switch (index) {
+ default:
+ break;
+ }
+ index++;
+ entry++;
+ }
+
+ get_feats_out->hdr.nsuppfeats_dev = CXL_FEATURE_MAX;
+ get_feats_out->hdr.entries = feat_entries;
+ *len_out = sizeof(CXLSupportedFeatureHeader) +
+ feat_entries * sizeof(CXLSupportedFeatureEntry);
+
+ return CXL_MBOX_SUCCESS;
+}
+
+/* CXL r3.0 section 8.2.9.6.2: Get Feature (Opcode 0501h) */
+static CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
+ uint8_t *payload_in,
+ size_t len_in,
+ uint8_t *payload_out,
+ size_t *len_out,
+ CXLCCI *cci)
+{
+ struct {
+ QemuUUID uuid;
+ uint16_t offset;
+ uint16_t count;
+ uint8_t selection;
+ } QEMU_PACKED QEMU_ALIGNED(16) * get_feature;
+ uint16_t bytes_to_copy = 0;
+
+ get_feature = (void *)payload_in;
+
+ if (get_feature->selection != CXL_GET_FEATURE_SEL_CURRENT_VALUE) {
+ return CXL_MBOX_UNSUPPORTED;
+ }
+ if (get_feature->offset + get_feature->count > cci->payload_max) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+
+ *len_out = bytes_to_copy;
+
+ return CXL_MBOX_SUCCESS;
+}
+
+/* CXL r3.0 section 8.2.9.6.3: Set Feature (Opcode 0502h) */
+static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
+ uint8_t *payload_in,
+ size_t len_in,
+ uint8_t *payload_out,
+ size_t *len_out,
+ CXLCCI *cci)
+{
+ return CXL_MBOX_SUCCESS;
+}
+
/* 8.2.9.5.1.1 */
static CXLRetCode cmd_identify_memory_device(const struct cxl_cmd *cmd,
uint8_t *payload_in,
@@ -1954,6 +2109,18 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
[LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", cmd_logs_get_supported,
0, 0 },
[LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
+ [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
+ cmd_features_get_supported, 0x8, 0 },
+ [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
+ cmd_features_get_feature, 0x15, 0 },
+ [FEATURES][SET_FEATURE] = { "FEATURES_SET_FEATURE",
+ cmd_features_set_feature,
+ ~0,
+ (CXL_MBOX_IMMEDIATE_CONFIG_CHANGE |
+ CXL_MBOX_IMMEDIATE_DATA_CHANGE |
+ CXL_MBOX_IMMEDIATE_POLICY_CHANGE |
+ CXL_MBOX_IMMEDIATE_LOG_CHANGE |
+ CXL_MBOX_SECURITY_STATE_CHANGE)},
[IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE",
cmd_identify_memory_device, 0, 0 },
[CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO",
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
2023-11-24 13:53 [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features shiju.jose--- via
2023-11-24 13:53 ` [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6) shiju.jose--- via
@ 2023-11-24 13:53 ` shiju.jose--- via
2024-02-15 18:47 ` fan
2024-02-15 20:55 ` Davidlohr Bueso
2023-11-24 13:53 ` [PATCH v2 3/3] hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS " shiju.jose--- via
2024-02-15 18:08 ` [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features fan
3 siblings, 2 replies; 14+ messages in thread
From: shiju.jose--- via @ 2023-11-24 13:53 UTC (permalink / raw)
To: qemu-devel, linux-cxl
Cc: jonathan.cameron, tanxiaofei, prime.zeng, linuxarm, shiju.jose
From: Shiju Jose <shiju.jose@huawei.com>
CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub control
feature. The device patrol scrub proactively locates and makes corrections
to errors in regular cycle. The patrol scrub control allows the request to
configure patrol scrub input configurations.
The patrol scrub control allows the requester to specify the number of
hours for which the patrol scrub cycles must be completed, provided that
the requested number is not less than the minimum number of hours for the
patrol scrub cycle that the device is capable of. In addition, the patrol
scrub controls allow the host to disable and enable the feature in case
disabling of the feature is needed for other purposes such as
performance-aware operations which require the background operations to be
turned off.
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
hw/cxl/cxl-mailbox-utils.c | 97 +++++++++++++++++++++++++++++++++++++-
1 file changed, 96 insertions(+), 1 deletion(-)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 1bbc9a48a6..5a6f4e4029 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry {
} QEMU_PACKED CXLSupportedFeatureEntry;
enum CXL_SUPPORTED_FEATURES_LIST {
+ CXL_FEATURE_PATROL_SCRUB = 0,
CXL_FEATURE_MAX
};
@@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
};
+/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature */
+static const QemuUUID patrol_scrub_uuid = {
+ .data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
+ 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a)
+};
+
+#define CXL_MEMDEV_PS_GET_FEATURE_VERSION 0x01
+#define CXL_MEMDEV_PS_SET_FEATURE_VERSION 0x01
+#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT BIT(0)
+#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT BIT(1)
+#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT 12
+#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT 1
+#define CXL_MEMDEV_PS_ENABLE_DEFAULT 0
+
+/* CXL memdev patrol scrub control attributes */
+struct CXLMemPatrolScrubReadAttrbs {
+ uint8_t scrub_cycle_cap;
+ uint16_t scrub_cycle;
+ uint8_t scrub_flags;
+} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
+
+typedef struct CXLMemPatrolScrubWriteAttrbs {
+ uint8_t scrub_cycle_hr;
+ uint8_t scrub_flags;
+} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
+
+typedef struct CXLMemPatrolScrubSetFeature {
+ CXLSetFeatureInHeader hdr;
+ CXLMemPatrolScrubWriteAttrbs feat_data;
+} QEMU_PACKED QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
+
/* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) */
static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
uint8_t *payload_in,
@@ -872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
uint16_t feat_entries = 0;
if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
- get_feats_in->start_index > CXL_FEATURE_MAX) {
+ get_feats_in->start_index >= CXL_FEATURE_MAX) {
return CXL_MBOX_INVALID_INPUT;
}
req_entries = (get_feats_in->count -
@@ -884,6 +916,31 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
entry = 0;
while (entry < req_entries) {
switch (index) {
+ case CXL_FEATURE_PATROL_SCRUB:
+ /* Fill supported feature entry for device patrol scrub control */
+ get_feats_out->feat_entries[entry] =
+ (struct CXLSupportedFeatureEntry) {
+ .uuid = patrol_scrub_uuid,
+ .feat_index = index,
+ .get_feat_size = sizeof(cxl_memdev_ps_feat_read_attrbs),
+ .set_feat_size = sizeof(CXLMemPatrolScrubWriteAttrbs),
+ /* Bit[0] : 1, feature attributes changeable */
+ .attrb_flags = 0x1,
+ .get_feat_version = CXL_MEMDEV_PS_GET_FEATURE_VERSION,
+ .set_feat_version = CXL_MEMDEV_PS_SET_FEATURE_VERSION,
+ .set_feat_effects = 0,
+ };
+ feat_entries++;
+ /* Set default value for device patrol scrub read attributes */
+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle_cap =
+ CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT |
+ CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT;
+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle =
+ CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT |
+ (CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT << 8);
+ cxl_memdev_ps_feat_read_attrbs.scrub_flags =
+ CXL_MEMDEV_PS_ENABLE_DEFAULT;
+ break;
default:
break;
}
@@ -924,6 +981,21 @@ static CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
return CXL_MBOX_INVALID_INPUT;
}
+ if (qemu_uuid_is_equal(&get_feature->uuid, &patrol_scrub_uuid)) {
+ if (get_feature->offset >= sizeof(cxl_memdev_ps_feat_read_attrbs)) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+ bytes_to_copy = sizeof(cxl_memdev_ps_feat_read_attrbs) -
+ get_feature->offset;
+ bytes_to_copy = (bytes_to_copy > get_feature->count) ?
+ get_feature->count : bytes_to_copy;
+ memcpy(payload_out,
+ &cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
+ bytes_to_copy);
+ } else {
+ return CXL_MBOX_UNSUPPORTED;
+ }
+
*len_out = bytes_to_copy;
return CXL_MBOX_SUCCESS;
@@ -937,6 +1009,29 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
size_t *len_out,
CXLCCI *cci)
{
+ CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
+ CXLMemPatrolScrubSetFeature *ps_set_feature;
+ CXLSetFeatureInHeader *hdr = (void *)payload_in;
+
+ if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
+ if (hdr->version != CXL_MEMDEV_PS_SET_FEATURE_VERSION ||
+ (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
+ CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
+ return CXL_MBOX_UNSUPPORTED;
+ }
+
+ ps_set_feature = (void *)payload_in;
+ ps_write_attrbs = &ps_set_feature->feat_data;
+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle &= ~0xFF;
+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle |=
+ ps_write_attrbs->scrub_cycle_hr & 0xFF;
+ cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
+ cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
+ ps_write_attrbs->scrub_flags & 0x1;
+ } else {
+ return CXL_MBOX_UNSUPPORTED;
+ }
+
return CXL_MBOX_SUCCESS;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control feature
2023-11-24 13:53 [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features shiju.jose--- via
2023-11-24 13:53 ` [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6) shiju.jose--- via
2023-11-24 13:53 ` [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature shiju.jose--- via
@ 2023-11-24 13:53 ` shiju.jose--- via
2024-02-15 19:11 ` fan
2024-02-15 18:08 ` [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features fan
3 siblings, 1 reply; 14+ messages in thread
From: shiju.jose--- via @ 2023-11-24 13:53 UTC (permalink / raw)
To: qemu-devel, linux-cxl
Cc: jonathan.cameron, tanxiaofei, prime.zeng, linuxarm, shiju.jose
From: Shiju Jose <shiju.jose@huawei.com>
CXL spec 3.1 section 8.2.9.9.11.2 describes the DDR5 Error Check Scrub (ECS)
control feature.
The Error Check Scrub (ECS) is a feature defined in JEDEC DDR5 SDRAM
Specification (JESD79-5) and allows the DRAM to internally read, correct
single-bit errors, and write back corrected data bits to the DRAM array
while providing transparency to error counts. The ECS control feature
allows the request to configure ECS input configurations during system
boot or at run-time.
The ECS control allows the requester to change the log entry type, the ECS
threshold count provided that the request is within the definition
specified in DDR5 mode registers, change mode between codeword mode and
row count mode, and reset the ECS counter.
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
hw/cxl/cxl-mailbox-utils.c | 100 ++++++++++++++++++++++++++++++++++++-
1 file changed, 99 insertions(+), 1 deletion(-)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 5a6f4e4029..098b92815a 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -810,6 +810,7 @@ typedef struct CXLSupportedFeatureEntry {
enum CXL_SUPPORTED_FEATURES_LIST {
CXL_FEATURE_PATROL_SCRUB = 0,
+ CXL_FEATURE_DDR5_ECS,
CXL_FEATURE_MAX
};
@@ -881,6 +882,42 @@ typedef struct CXLMemPatrolScrubSetFeature {
CXLMemPatrolScrubWriteAttrbs feat_data;
} QEMU_PACKED QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
+/*
+ * CXL r3.1 section 8.2.9.9.11.2:
+ * DDR5 Error Check Scrub (ECS) Control Feature
+ */
+static const QemuUUID ddr5_ecs_uuid = {
+ .data = UUID(0xe5b13f22, 0x2328, 0x4a14, 0xb8, 0xba,
+ 0xb9, 0x69, 0x1e, 0x89, 0x33, 0x86)
+};
+
+#define CXL_DDR5_ECS_GET_FEATURE_VERSION 0x01
+#define CXL_DDR5_ECS_SET_FEATURE_VERSION 0x01
+#define CXL_DDR5_ECS_LOG_ENTRY_TYPE_DEFAULT 0x01
+#define CXL_DDR5_ECS_REALTIME_REPORT_CAP_DEFAULT 1
+#define CXL_DDR5_ECS_THRESHOLD_COUNT_DEFAULT 3 /* 3: 256, 4: 1024, 5: 4096 */
+#define CXL_DDR5_ECS_MODE_DEFAULT 0
+
+#define CXL_DDR5_ECS_NUM_MEDIA_FRUS 3
+
+/* CXL memdev DDR5 ECS control attributes */
+struct CXLMemECSReadAttrbs {
+ uint8_t ecs_log_cap;
+ uint8_t ecs_cap;
+ uint16_t ecs_config;
+ uint8_t ecs_flags;
+} QEMU_PACKED cxl_ddr5_ecs_feat_read_attrbs[CXL_DDR5_ECS_NUM_MEDIA_FRUS];
+
+typedef struct CXLDDR5ECSWriteAttrbs {
+ uint8_t ecs_log_cap;
+ uint16_t ecs_config;
+} QEMU_PACKED CXLDDR5ECSWriteAttrbs;
+
+typedef struct CXLDDR5ECSSetFeature {
+ CXLSetFeatureInHeader hdr;
+ CXLDDR5ECSWriteAttrbs feat_data[];
+} QEMU_PACKED QEMU_ALIGNED(16) CXLDDR5ECSSetFeature;
+
/* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) */
static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
uint8_t *payload_in,
@@ -899,7 +936,7 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
CXLSupportedFeatureHeader hdr;
CXLSupportedFeatureEntry feat_entries[];
} QEMU_PACKED QEMU_ALIGNED(16) * get_feats_out = (void *)payload_out;
- uint16_t index;
+ uint16_t count, index;
uint16_t entry, req_entries;
uint16_t feat_entries = 0;
@@ -941,6 +978,35 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
cxl_memdev_ps_feat_read_attrbs.scrub_flags =
CXL_MEMDEV_PS_ENABLE_DEFAULT;
break;
+ case CXL_FEATURE_DDR5_ECS:
+ /* Fill supported feature entry for device DDR5 ECS control */
+ get_feats_out->feat_entries[entry] =
+ (struct CXLSupportedFeatureEntry) {
+ .uuid = ddr5_ecs_uuid,
+ .feat_index = index,
+ .get_feat_size = CXL_DDR5_ECS_NUM_MEDIA_FRUS *
+ sizeof(struct CXLMemECSReadAttrbs),
+ .set_feat_size = CXL_DDR5_ECS_NUM_MEDIA_FRUS *
+ sizeof(CXLDDR5ECSWriteAttrbs),
+ .attrb_flags = 0x1,
+ .get_feat_version = CXL_DDR5_ECS_GET_FEATURE_VERSION,
+ .set_feat_version = CXL_DDR5_ECS_SET_FEATURE_VERSION,
+ .set_feat_effects = 0,
+ };
+ feat_entries++;
+ /* Set default value for DDR5 ECS read attributes */
+ for (count = 0; count < CXL_DDR5_ECS_NUM_MEDIA_FRUS; count++) {
+ cxl_ddr5_ecs_feat_read_attrbs[count].ecs_log_cap =
+ CXL_DDR5_ECS_LOG_ENTRY_TYPE_DEFAULT;
+ cxl_ddr5_ecs_feat_read_attrbs[count].ecs_cap =
+ CXL_DDR5_ECS_REALTIME_REPORT_CAP_DEFAULT;
+ cxl_ddr5_ecs_feat_read_attrbs[count].ecs_config =
+ CXL_DDR5_ECS_THRESHOLD_COUNT_DEFAULT |
+ (CXL_DDR5_ECS_MODE_DEFAULT << 3);
+ /* Reserved */
+ cxl_ddr5_ecs_feat_read_attrbs[count].ecs_flags = 0;
+ }
+ break;
default:
break;
}
@@ -992,6 +1058,19 @@ static CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
memcpy(payload_out,
&cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
bytes_to_copy);
+ } else if (qemu_uuid_is_equal(&get_feature->uuid, &ddr5_ecs_uuid)) {
+ if (get_feature->offset >= CXL_DDR5_ECS_NUM_MEDIA_FRUS *
+ sizeof(struct CXLMemECSReadAttrbs)) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+ bytes_to_copy = CXL_DDR5_ECS_NUM_MEDIA_FRUS *
+ sizeof(struct CXLMemECSReadAttrbs) -
+ get_feature->offset;
+ bytes_to_copy = (bytes_to_copy > get_feature->count) ?
+ get_feature->count : bytes_to_copy;
+ memcpy(payload_out,
+ &cxl_ddr5_ecs_feat_read_attrbs + get_feature->offset,
+ bytes_to_copy);
} else {
return CXL_MBOX_UNSUPPORTED;
}
@@ -1009,8 +1088,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
size_t *len_out,
CXLCCI *cci)
{
+ uint16_t count;
CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
+ CXLDDR5ECSWriteAttrbs *ecs_write_attrbs;
CXLMemPatrolScrubSetFeature *ps_set_feature;
+ CXLDDR5ECSSetFeature *ecs_set_feature;
CXLSetFeatureInHeader *hdr = (void *)payload_in;
if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
@@ -1028,6 +1110,22 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
ps_write_attrbs->scrub_flags & 0x1;
+ } else if (qemu_uuid_is_equal(&hdr->uuid,
+ &ddr5_ecs_uuid)) {
+ if (hdr->version != CXL_DDR5_ECS_SET_FEATURE_VERSION ||
+ (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
+ CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
+ return CXL_MBOX_UNSUPPORTED;
+ }
+
+ ecs_set_feature = (void *)payload_in;
+ ecs_write_attrbs = ecs_set_feature->feat_data;
+ for (count = 0; count < CXL_DDR5_ECS_NUM_MEDIA_FRUS; count++) {
+ cxl_ddr5_ecs_feat_read_attrbs[count].ecs_log_cap =
+ ecs_write_attrbs[count].ecs_log_cap;
+ cxl_ddr5_ecs_feat_read_attrbs[count].ecs_config =
+ ecs_write_attrbs[count].ecs_config & 0x1F;
+ }
} else {
return CXL_MBOX_UNSUPPORTED;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)
2023-11-24 13:53 ` [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6) shiju.jose--- via
@ 2024-02-15 18:05 ` fan
2024-02-16 9:20 ` Shiju Jose via
0 siblings, 1 reply; 14+ messages in thread
From: fan @ 2024-02-15 18:05 UTC (permalink / raw)
To: shiju.jose
Cc: qemu-devel, linux-cxl, jonathan.cameron, tanxiaofei, prime.zeng,
linuxarm
On Fri, Nov 24, 2023 at 09:53:35PM +0800, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
>
> CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
> CXL devices supports features with changeable attributes.
> Get Supported Features retrieves the list of supported device specific
> features. The settings of a feature can be retrieved using Get Feature and
> optionally modified using Set Feature.
>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
Updated the references to align with cxl spec r3.1, other than that looks
good to me.
Fan
> hw/cxl/cxl-mailbox-utils.c | 167 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 167 insertions(+)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 6184f44339..1bbc9a48a6 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -66,6 +66,10 @@ enum {
> LOGS = 0x04,
> #define GET_SUPPORTED 0x0
> #define GET_LOG 0x1
> + FEATURES = 0x05,
> + #define GET_SUPPORTED 0x0
> + #define GET_FEATURE 0x1
> + #define SET_FEATURE 0x2
> IDENTIFY = 0x40,
> #define MEMORY_DEVICE 0x0
> CCLS = 0x41,
> @@ -785,6 +789,157 @@ static CXLRetCode cmd_logs_get_log(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +/* CXL r3.0 section 8.2.9.6: Features */
> +typedef struct CXLSupportedFeatureHeader {
> + uint16_t entries;
> + uint16_t nsuppfeats_dev;
> + uint32_t reserved;
> +} QEMU_PACKED CXLSupportedFeatureHeader;
> +
> +typedef struct CXLSupportedFeatureEntry {
> + QemuUUID uuid;
> + uint16_t feat_index;
> + uint16_t get_feat_size;
> + uint16_t set_feat_size;
> + uint32_t attrb_flags;
> + uint8_t get_feat_version;
> + uint8_t set_feat_version;
> + uint16_t set_feat_effects;
> + uint8_t rsvd[18];
> +} QEMU_PACKED CXLSupportedFeatureEntry;
> +
> +enum CXL_SUPPORTED_FEATURES_LIST {
> + CXL_FEATURE_MAX
> +};
> +
> +/* Get Feature CXL 3.0 Spec 8.2.9.6.2 */
> +/*
> + * Get Feature input payload
> + * CXL rev 3.0 section 8.2.9.6.2; Table 8-79
> + */
> +/* Get Feature : Payload in selection */
> +enum CXL_GET_FEATURE_SELECTION {
> + CXL_GET_FEATURE_SEL_CURRENT_VALUE = 0x0,
> + CXL_GET_FEATURE_SEL_DEFAULT_VALUE = 0x1,
> + CXL_GET_FEATURE_SEL_SAVED_VALUE = 0x2,
> + CXL_GET_FEATURE_SEL_MAX
> +};
> +
> +/* Set Feature CXL 3.0 Spec 8.2.9.6.3 */
> +/*
> + * Set Feature input payload
> + * CXL rev 3.0 section 8.2.9.6.3; Table 8-81
> + */
> +typedef struct CXLSetFeatureInHeader {
> + QemuUUID uuid;
> + uint32_t flags;
> + uint16_t offset;
> + uint8_t version;
> + uint8_t rsvd[9];
> +} QEMU_PACKED QEMU_ALIGNED(16) CXLSetFeatureInHeader;
> +
> +/* Set Feature : Payload in flags */
> +#define CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK 0x7
> +enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
> + CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER = 0x0,
> + CXL_SET_FEATURE_FLAG_INITIATE_DATA_TRANSFER = 0x1,
> + CXL_SET_FEATURE_FLAG_CONTINUE_DATA_TRANSFER = 0x2,
> + CXL_SET_FEATURE_FLAG_FINISH_DATA_TRANSFER = 0x3,
> + CXL_SET_FEATURE_FLAG_ABORT_DATA_TRANSFER = 0x4,
> + CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
> +};
> +
> +/* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) */
> +static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci)
> +{
> + struct {
> + uint32_t count;
> + uint16_t start_index;
> + uint16_t reserved;
> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_in = (void *)payload_in;
> +
> + struct {
> + CXLSupportedFeatureHeader hdr;
> + CXLSupportedFeatureEntry feat_entries[];
> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_out = (void *)payload_out;
> + uint16_t index;
> + uint16_t entry, req_entries;
> + uint16_t feat_entries = 0;
> +
> + if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
> + get_feats_in->start_index > CXL_FEATURE_MAX) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
> + req_entries = (get_feats_in->count -
> + sizeof(CXLSupportedFeatureHeader)) /
> + sizeof(CXLSupportedFeatureEntry);
> + req_entries = MIN(req_entries, CXL_FEATURE_MAX);
> + index = get_feats_in->start_index;
> +
> + entry = 0;
> + while (entry < req_entries) {
> + switch (index) {
> + default:
> + break;
> + }
> + index++;
> + entry++;
> + }
> +
> + get_feats_out->hdr.nsuppfeats_dev = CXL_FEATURE_MAX;
> + get_feats_out->hdr.entries = feat_entries;
> + *len_out = sizeof(CXLSupportedFeatureHeader) +
> + feat_entries * sizeof(CXLSupportedFeatureEntry);
> +
> + return CXL_MBOX_SUCCESS;
> +}
> +
> +/* CXL r3.0 section 8.2.9.6.2: Get Feature (Opcode 0501h) */
> +static CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci)
> +{
> + struct {
> + QemuUUID uuid;
> + uint16_t offset;
> + uint16_t count;
> + uint8_t selection;
> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feature;
> + uint16_t bytes_to_copy = 0;
> +
> + get_feature = (void *)payload_in;
> +
> + if (get_feature->selection != CXL_GET_FEATURE_SEL_CURRENT_VALUE) {
> + return CXL_MBOX_UNSUPPORTED;
> + }
> + if (get_feature->offset + get_feature->count > cci->payload_max) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
> +
> + *len_out = bytes_to_copy;
> +
> + return CXL_MBOX_SUCCESS;
> +}
> +
> +/* CXL r3.0 section 8.2.9.6.3: Set Feature (Opcode 0502h) */
> +static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci)
> +{
> + return CXL_MBOX_SUCCESS;
> +}
> +
> /* 8.2.9.5.1.1 */
> static CXLRetCode cmd_identify_memory_device(const struct cxl_cmd *cmd,
> uint8_t *payload_in,
> @@ -1954,6 +2109,18 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", cmd_logs_get_supported,
> 0, 0 },
> [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
> + [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
> + cmd_features_get_supported, 0x8, 0 },
> + [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
> + cmd_features_get_feature, 0x15, 0 },
> + [FEATURES][SET_FEATURE] = { "FEATURES_SET_FEATURE",
> + cmd_features_set_feature,
> + ~0,
> + (CXL_MBOX_IMMEDIATE_CONFIG_CHANGE |
> + CXL_MBOX_IMMEDIATE_DATA_CHANGE |
> + CXL_MBOX_IMMEDIATE_POLICY_CHANGE |
> + CXL_MBOX_IMMEDIATE_LOG_CHANGE |
> + CXL_MBOX_SECURITY_STATE_CHANGE)},
> [IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE",
> cmd_identify_memory_device, 0, 0 },
> [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO",
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features
2023-11-24 13:53 [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features shiju.jose--- via
` (2 preceding siblings ...)
2023-11-24 13:53 ` [PATCH v2 3/3] hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS " shiju.jose--- via
@ 2024-02-15 18:08 ` fan
2024-02-16 9:22 ` Shiju Jose via
3 siblings, 1 reply; 14+ messages in thread
From: fan @ 2024-02-15 18:08 UTC (permalink / raw)
To: shiju.jose
Cc: qemu-devel, linux-cxl, jonathan.cameron, tanxiaofei, prime.zeng,
linuxarm
On Fri, Nov 24, 2023 at 09:53:34PM +0800, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
>
> Add support for the feature commands, device patrol scrub control and
> DDR5 ECS control features.
>
> CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub control
> feature.
> CXL spec 3.1 section 8.2.9.9.11.2 describes the DDR5 Error Check Scrub (ECS)
> control feature.
>
> The patches are available here,
> https://gitlab.com/shiju.jose/qemu/-/tree/cxl-scrub-2023-11-14
> and is based on Jonathan's branch
> https://gitlab.com/jic23/qemu/-/tree/cxl-2023-10-16
>
> Changes
> v1 -> v2
> 1. Changes for Davidlohr comments. Thanks.
> - Changed CXL SET feature data transfer flags as enum.
> - Modified pointer supported_feats to get_feats_out.
> - Removed an unnecessary branch.
> - Use MIN().
> - Move setting of hdr.nsuppfeats further down.
> - Return CXL_MBOX_UNSUPPORTED if non-zero selection flag is passed.
> - Add more IMMEDIATE_*.* flags set_feature.
> - Corrected a spelling error.
>
> Shiju Jose (3):
> hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)
> hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
> hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control feature
>
> hw/cxl/cxl-mailbox-utils.c | 360 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 360 insertions(+)
>
> --
> 2.34.1
>
Recently, Jonathan has updated all the specification references to align
with cxl spec r3.1, so for the next version, we may want to also do
that.
Fan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
2023-11-24 13:53 ` [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature shiju.jose--- via
@ 2024-02-15 18:47 ` fan
2024-02-16 10:16 ` Shiju Jose via
2024-02-15 20:55 ` Davidlohr Bueso
1 sibling, 1 reply; 14+ messages in thread
From: fan @ 2024-02-15 18:47 UTC (permalink / raw)
To: shiju.jose
Cc: qemu-devel, linux-cxl, jonathan.cameron, tanxiaofei, prime.zeng,
linuxarm, fan.ni
On Fri, Nov 24, 2023 at 09:53:36PM +0800, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
>
> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub control
> feature. The device patrol scrub proactively locates and makes corrections
> to errors in regular cycle. The patrol scrub control allows the request to
> configure patrol scrub input configurations.
>
> The patrol scrub control allows the requester to specify the number of
> hours for which the patrol scrub cycles must be completed, provided that
> the requested number is not less than the minimum number of hours for the
> patrol scrub cycle that the device is capable of. In addition, the patrol
> scrub controls allow the host to disable and enable the feature in case
> disabling of the feature is needed for other purposes such as
> performance-aware operations which require the background operations to be
> turned off.
>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> ---
LGTM except for some minor comments inlined.
> hw/cxl/cxl-mailbox-utils.c | 97 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 1bbc9a48a6..5a6f4e4029 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry {
> } QEMU_PACKED CXLSupportedFeatureEntry;
>
> enum CXL_SUPPORTED_FEATURES_LIST {
> + CXL_FEATURE_PATROL_SCRUB = 0,
> CXL_FEATURE_MAX
> };
>
> @@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
> CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
> };
>
> +/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature */
> +static const QemuUUID patrol_scrub_uuid = {
> + .data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
> + 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a)
> +};
> +
> +#define CXL_MEMDEV_PS_GET_FEATURE_VERSION 0x01
> +#define CXL_MEMDEV_PS_SET_FEATURE_VERSION 0x01
> +#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT BIT(0)
> +#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT BIT(1)
> +#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT 12
> +#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT 1
> +#define CXL_MEMDEV_PS_ENABLE_DEFAULT 0
> +
> +/* CXL memdev patrol scrub control attributes */
> +struct CXLMemPatrolScrubReadAttrbs {
> + uint8_t scrub_cycle_cap;
> + uint16_t scrub_cycle;
> + uint8_t scrub_flags;
> +} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
> +
> +typedef struct CXLMemPatrolScrubWriteAttrbs {
> + uint8_t scrub_cycle_hr;
> + uint8_t scrub_flags;
> +} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
> +
> +typedef struct CXLMemPatrolScrubSetFeature {
> + CXLSetFeatureInHeader hdr;
> + CXLMemPatrolScrubWriteAttrbs feat_data;
> +} QEMU_PACKED QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
> +
> /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) */
> static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> uint8_t *payload_in,
> @@ -872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> uint16_t feat_entries = 0;
>
> if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
> - get_feats_in->start_index > CXL_FEATURE_MAX) {
> + get_feats_in->start_index >= CXL_FEATURE_MAX) {
Not totally sure about this, the spec says "...Greater than..." although I
also think it should be >=. Similar things for the offset usage below.
Fan
> return CXL_MBOX_INVALID_INPUT;
> }
> req_entries = (get_feats_in->count -
> @@ -884,6 +916,31 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> entry = 0;
> while (entry < req_entries) {
> switch (index) {
> + case CXL_FEATURE_PATROL_SCRUB:
> + /* Fill supported feature entry for device patrol scrub control */
> + get_feats_out->feat_entries[entry] =
> + (struct CXLSupportedFeatureEntry) {
> + .uuid = patrol_scrub_uuid,
> + .feat_index = index,
> + .get_feat_size = sizeof(cxl_memdev_ps_feat_read_attrbs),
> + .set_feat_size = sizeof(CXLMemPatrolScrubWriteAttrbs),
> + /* Bit[0] : 1, feature attributes changeable */
> + .attrb_flags = 0x1,
> + .get_feat_version = CXL_MEMDEV_PS_GET_FEATURE_VERSION,
> + .set_feat_version = CXL_MEMDEV_PS_SET_FEATURE_VERSION,
> + .set_feat_effects = 0,
> + };
> + feat_entries++;
> + /* Set default value for device patrol scrub read attributes */
> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle_cap =
> + CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT |
> + CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT;
> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle =
> + CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT |
> + (CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT << 8);
> + cxl_memdev_ps_feat_read_attrbs.scrub_flags =
> + CXL_MEMDEV_PS_ENABLE_DEFAULT;
> + break;
> default:
> break;
> }
> @@ -924,6 +981,21 @@ static CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
> return CXL_MBOX_INVALID_INPUT;
> }
>
> + if (qemu_uuid_is_equal(&get_feature->uuid, &patrol_scrub_uuid)) {
> + if (get_feature->offset >= sizeof(cxl_memdev_ps_feat_read_attrbs)) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
> + bytes_to_copy = sizeof(cxl_memdev_ps_feat_read_attrbs) -
> + get_feature->offset;
> + bytes_to_copy = (bytes_to_copy > get_feature->count) ?
> + get_feature->count : bytes_to_copy;
> + memcpy(payload_out,
> + &cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
> + bytes_to_copy);
> + } else {
> + return CXL_MBOX_UNSUPPORTED;
> + }
> +
> *len_out = bytes_to_copy;
>
> return CXL_MBOX_SUCCESS;
> @@ -937,6 +1009,29 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
> size_t *len_out,
> CXLCCI *cci)
> {
> + CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
> + CXLMemPatrolScrubSetFeature *ps_set_feature;
> + CXLSetFeatureInHeader *hdr = (void *)payload_in;
> +
> + if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
> + if (hdr->version != CXL_MEMDEV_PS_SET_FEATURE_VERSION ||
> + (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
> + CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
> + return CXL_MBOX_UNSUPPORTED;
> + }
> +
> + ps_set_feature = (void *)payload_in;
> + ps_write_attrbs = &ps_set_feature->feat_data;
> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle &= ~0xFF;
> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle |=
> + ps_write_attrbs->scrub_cycle_hr & 0xFF;
> + cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
> + cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
> + ps_write_attrbs->scrub_flags & 0x1;
> + } else {
> + return CXL_MBOX_UNSUPPORTED;
> + }
> +
> return CXL_MBOX_SUCCESS;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control feature
2023-11-24 13:53 ` [PATCH v2 3/3] hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS " shiju.jose--- via
@ 2024-02-15 19:11 ` fan
0 siblings, 0 replies; 14+ messages in thread
From: fan @ 2024-02-15 19:11 UTC (permalink / raw)
To: shiju.jose
Cc: qemu-devel, linux-cxl, jonathan.cameron, tanxiaofei, prime.zeng,
linuxarm
On Fri, Nov 24, 2023 at 09:53:37PM +0800, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
>
> CXL spec 3.1 section 8.2.9.9.11.2 describes the DDR5 Error Check Scrub (ECS)
> control feature.
>
> The Error Check Scrub (ECS) is a feature defined in JEDEC DDR5 SDRAM
> Specification (JESD79-5) and allows the DRAM to internally read, correct
> single-bit errors, and write back corrected data bits to the DRAM array
> while providing transparency to error counts. The ECS control feature
> allows the request to configure ECS input configurations during system
> boot or at run-time.
>
> The ECS control allows the requester to change the log entry type, the ECS
> threshold count provided that the request is within the definition
> specified in DDR5 mode registers, change mode between codeword mode and
> row count mode, and reset the ECS counter.
>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
LGTM.
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Fan
> ---
> hw/cxl/cxl-mailbox-utils.c | 100 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 5a6f4e4029..098b92815a 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -810,6 +810,7 @@ typedef struct CXLSupportedFeatureEntry {
>
> enum CXL_SUPPORTED_FEATURES_LIST {
> CXL_FEATURE_PATROL_SCRUB = 0,
> + CXL_FEATURE_DDR5_ECS,
> CXL_FEATURE_MAX
> };
>
> @@ -881,6 +882,42 @@ typedef struct CXLMemPatrolScrubSetFeature {
> CXLMemPatrolScrubWriteAttrbs feat_data;
> } QEMU_PACKED QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
>
> +/*
> + * CXL r3.1 section 8.2.9.9.11.2:
> + * DDR5 Error Check Scrub (ECS) Control Feature
> + */
> +static const QemuUUID ddr5_ecs_uuid = {
> + .data = UUID(0xe5b13f22, 0x2328, 0x4a14, 0xb8, 0xba,
> + 0xb9, 0x69, 0x1e, 0x89, 0x33, 0x86)
> +};
> +
> +#define CXL_DDR5_ECS_GET_FEATURE_VERSION 0x01
> +#define CXL_DDR5_ECS_SET_FEATURE_VERSION 0x01
> +#define CXL_DDR5_ECS_LOG_ENTRY_TYPE_DEFAULT 0x01
> +#define CXL_DDR5_ECS_REALTIME_REPORT_CAP_DEFAULT 1
> +#define CXL_DDR5_ECS_THRESHOLD_COUNT_DEFAULT 3 /* 3: 256, 4: 1024, 5: 4096 */
> +#define CXL_DDR5_ECS_MODE_DEFAULT 0
> +
> +#define CXL_DDR5_ECS_NUM_MEDIA_FRUS 3
> +
> +/* CXL memdev DDR5 ECS control attributes */
> +struct CXLMemECSReadAttrbs {
> + uint8_t ecs_log_cap;
> + uint8_t ecs_cap;
> + uint16_t ecs_config;
> + uint8_t ecs_flags;
> +} QEMU_PACKED cxl_ddr5_ecs_feat_read_attrbs[CXL_DDR5_ECS_NUM_MEDIA_FRUS];
> +
> +typedef struct CXLDDR5ECSWriteAttrbs {
> + uint8_t ecs_log_cap;
> + uint16_t ecs_config;
> +} QEMU_PACKED CXLDDR5ECSWriteAttrbs;
> +
> +typedef struct CXLDDR5ECSSetFeature {
> + CXLSetFeatureInHeader hdr;
> + CXLDDR5ECSWriteAttrbs feat_data[];
> +} QEMU_PACKED QEMU_ALIGNED(16) CXLDDR5ECSSetFeature;
> +
> /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) */
> static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> uint8_t *payload_in,
> @@ -899,7 +936,7 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> CXLSupportedFeatureHeader hdr;
> CXLSupportedFeatureEntry feat_entries[];
> } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_out = (void *)payload_out;
> - uint16_t index;
> + uint16_t count, index;
> uint16_t entry, req_entries;
> uint16_t feat_entries = 0;
>
> @@ -941,6 +978,35 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> cxl_memdev_ps_feat_read_attrbs.scrub_flags =
> CXL_MEMDEV_PS_ENABLE_DEFAULT;
> break;
> + case CXL_FEATURE_DDR5_ECS:
> + /* Fill supported feature entry for device DDR5 ECS control */
> + get_feats_out->feat_entries[entry] =
> + (struct CXLSupportedFeatureEntry) {
> + .uuid = ddr5_ecs_uuid,
> + .feat_index = index,
> + .get_feat_size = CXL_DDR5_ECS_NUM_MEDIA_FRUS *
> + sizeof(struct CXLMemECSReadAttrbs),
> + .set_feat_size = CXL_DDR5_ECS_NUM_MEDIA_FRUS *
> + sizeof(CXLDDR5ECSWriteAttrbs),
> + .attrb_flags = 0x1,
> + .get_feat_version = CXL_DDR5_ECS_GET_FEATURE_VERSION,
> + .set_feat_version = CXL_DDR5_ECS_SET_FEATURE_VERSION,
> + .set_feat_effects = 0,
> + };
> + feat_entries++;
> + /* Set default value for DDR5 ECS read attributes */
> + for (count = 0; count < CXL_DDR5_ECS_NUM_MEDIA_FRUS; count++) {
> + cxl_ddr5_ecs_feat_read_attrbs[count].ecs_log_cap =
> + CXL_DDR5_ECS_LOG_ENTRY_TYPE_DEFAULT;
> + cxl_ddr5_ecs_feat_read_attrbs[count].ecs_cap =
> + CXL_DDR5_ECS_REALTIME_REPORT_CAP_DEFAULT;
> + cxl_ddr5_ecs_feat_read_attrbs[count].ecs_config =
> + CXL_DDR5_ECS_THRESHOLD_COUNT_DEFAULT |
> + (CXL_DDR5_ECS_MODE_DEFAULT << 3);
> + /* Reserved */
> + cxl_ddr5_ecs_feat_read_attrbs[count].ecs_flags = 0;
> + }
> + break;
> default:
> break;
> }
> @@ -992,6 +1058,19 @@ static CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
> memcpy(payload_out,
> &cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
> bytes_to_copy);
> + } else if (qemu_uuid_is_equal(&get_feature->uuid, &ddr5_ecs_uuid)) {
> + if (get_feature->offset >= CXL_DDR5_ECS_NUM_MEDIA_FRUS *
> + sizeof(struct CXLMemECSReadAttrbs)) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
> + bytes_to_copy = CXL_DDR5_ECS_NUM_MEDIA_FRUS *
> + sizeof(struct CXLMemECSReadAttrbs) -
> + get_feature->offset;
> + bytes_to_copy = (bytes_to_copy > get_feature->count) ?
> + get_feature->count : bytes_to_copy;
> + memcpy(payload_out,
> + &cxl_ddr5_ecs_feat_read_attrbs + get_feature->offset,
> + bytes_to_copy);
> } else {
> return CXL_MBOX_UNSUPPORTED;
> }
> @@ -1009,8 +1088,11 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
> size_t *len_out,
> CXLCCI *cci)
> {
> + uint16_t count;
> CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
> + CXLDDR5ECSWriteAttrbs *ecs_write_attrbs;
> CXLMemPatrolScrubSetFeature *ps_set_feature;
> + CXLDDR5ECSSetFeature *ecs_set_feature;
> CXLSetFeatureInHeader *hdr = (void *)payload_in;
>
> if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
> @@ -1028,6 +1110,22 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
> cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
> cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
> ps_write_attrbs->scrub_flags & 0x1;
> + } else if (qemu_uuid_is_equal(&hdr->uuid,
> + &ddr5_ecs_uuid)) {
> + if (hdr->version != CXL_DDR5_ECS_SET_FEATURE_VERSION ||
> + (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
> + CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
> + return CXL_MBOX_UNSUPPORTED;
> + }
> +
> + ecs_set_feature = (void *)payload_in;
> + ecs_write_attrbs = ecs_set_feature->feat_data;
> + for (count = 0; count < CXL_DDR5_ECS_NUM_MEDIA_FRUS; count++) {
> + cxl_ddr5_ecs_feat_read_attrbs[count].ecs_log_cap =
> + ecs_write_attrbs[count].ecs_log_cap;
> + cxl_ddr5_ecs_feat_read_attrbs[count].ecs_config =
> + ecs_write_attrbs[count].ecs_config & 0x1F;
> + }
> } else {
> return CXL_MBOX_UNSUPPORTED;
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
2023-11-24 13:53 ` [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature shiju.jose--- via
2024-02-15 18:47 ` fan
@ 2024-02-15 20:55 ` Davidlohr Bueso
2024-02-16 12:11 ` Shiju Jose via
1 sibling, 1 reply; 14+ messages in thread
From: Davidlohr Bueso @ 2024-02-15 20:55 UTC (permalink / raw)
To: shiju.jose
Cc: qemu-devel, linux-cxl, jonathan.cameron, tanxiaofei, prime.zeng,
linuxarm, fan.ni
On Fri, 24 Nov 2023, shiju.jose@huawei.com wrote:
>From: Shiju Jose <shiju.jose@huawei.com>
>
>CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub control
>feature. The device patrol scrub proactively locates and makes corrections
>to errors in regular cycle. The patrol scrub control allows the request to
>configure patrol scrub input configurations.
>
>The patrol scrub control allows the requester to specify the number of
>hours for which the patrol scrub cycles must be completed, provided that
>the requested number is not less than the minimum number of hours for the
>patrol scrub cycle that the device is capable of. In addition, the patrol
>scrub controls allow the host to disable and enable the feature in case
>disabling of the feature is needed for other purposes such as
>performance-aware operations which require the background operations to be
>turned off.
>
>Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>---
> hw/cxl/cxl-mailbox-utils.c | 97 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 96 insertions(+), 1 deletion(-)
>
>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>index 1bbc9a48a6..5a6f4e4029 100644
>--- a/hw/cxl/cxl-mailbox-utils.c
>+++ b/hw/cxl/cxl-mailbox-utils.c
>@@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry {
> } QEMU_PACKED CXLSupportedFeatureEntry;
>
> enum CXL_SUPPORTED_FEATURES_LIST {
>+ CXL_FEATURE_PATROL_SCRUB = 0,
> CXL_FEATURE_MAX
> };
>
>@@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
> CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
> };
>
>+/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature */
>+static const QemuUUID patrol_scrub_uuid = {
>+ .data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
>+ 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a)
>+};
>+
>+#define CXL_MEMDEV_PS_GET_FEATURE_VERSION 0x01
>+#define CXL_MEMDEV_PS_SET_FEATURE_VERSION 0x01
>+#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT BIT(0)
>+#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT BIT(1)
>+#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT 12
>+#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT 1
>+#define CXL_MEMDEV_PS_ENABLE_DEFAULT 0
>+
>+/* CXL memdev patrol scrub control attributes */
>+struct CXLMemPatrolScrubReadAttrbs {
>+ uint8_t scrub_cycle_cap;
>+ uint16_t scrub_cycle;
>+ uint8_t scrub_flags;
>+} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
>+
>+typedef struct CXLMemPatrolScrubWriteAttrbs {
>+ uint8_t scrub_cycle_hr;
>+ uint8_t scrub_flags;
>+} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
fyi there is an ask, which I certainly agree with, to make
these static here instead of at runtime.
https://lore.kernel.org/linux-cxl/20240119175006.00007f74@Huawei.com/
Also, this series probably needs rebasing per Jonathan's
latest branch with lots of updates.
>+
>+typedef struct CXLMemPatrolScrubSetFeature {
>+ CXLSetFeatureInHeader hdr;
>+ CXLMemPatrolScrubWriteAttrbs feat_data;
>+} QEMU_PACKED QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
>+
> /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) */
> static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> uint8_t *payload_in,
>@@ -872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> uint16_t feat_entries = 0;
>
> if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
>- get_feats_in->start_index > CXL_FEATURE_MAX) {
>+ get_feats_in->start_index >= CXL_FEATURE_MAX) {
> return CXL_MBOX_INVALID_INPUT;
> }
> req_entries = (get_feats_in->count -
>@@ -884,6 +916,31 @@ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> entry = 0;
> while (entry < req_entries) {
> switch (index) {
>+ case CXL_FEATURE_PATROL_SCRUB:
>+ /* Fill supported feature entry for device patrol scrub control */
>+ get_feats_out->feat_entries[entry] =
>+ (struct CXLSupportedFeatureEntry) {
>+ .uuid = patrol_scrub_uuid,
>+ .feat_index = index,
>+ .get_feat_size = sizeof(cxl_memdev_ps_feat_read_attrbs),
>+ .set_feat_size = sizeof(CXLMemPatrolScrubWriteAttrbs),
>+ /* Bit[0] : 1, feature attributes changeable */
>+ .attrb_flags = 0x1,
>+ .get_feat_version = CXL_MEMDEV_PS_GET_FEATURE_VERSION,
>+ .set_feat_version = CXL_MEMDEV_PS_SET_FEATURE_VERSION,
>+ .set_feat_effects = 0,
>+ };
>+ feat_entries++;
>+ /* Set default value for device patrol scrub read attributes */
>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle_cap =
>+ CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT |
>+ CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT;
>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle =
>+ CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT |
>+ (CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT << 8);
>+ cxl_memdev_ps_feat_read_attrbs.scrub_flags =
>+ CXL_MEMDEV_PS_ENABLE_DEFAULT;
>+ break;
> default:
> break;
> }
>@@ -924,6 +981,21 @@ static CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
> return CXL_MBOX_INVALID_INPUT;
> }
>
>+ if (qemu_uuid_is_equal(&get_feature->uuid, &patrol_scrub_uuid)) {
>+ if (get_feature->offset >= sizeof(cxl_memdev_ps_feat_read_attrbs)) {
>+ return CXL_MBOX_INVALID_INPUT;
>+ }
>+ bytes_to_copy = sizeof(cxl_memdev_ps_feat_read_attrbs) -
>+ get_feature->offset;
>+ bytes_to_copy = (bytes_to_copy > get_feature->count) ?
>+ get_feature->count : bytes_to_copy;
min()?
>+ memcpy(payload_out,
>+ &cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
>+ bytes_to_copy);
>+ } else {
>+ return CXL_MBOX_UNSUPPORTED;
>+ }
>+
> *len_out = bytes_to_copy;
>
> return CXL_MBOX_SUCCESS;
>@@ -937,6 +1009,29 @@ static CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
> size_t *len_out,
> CXLCCI *cci)
> {
>+ CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
>+ CXLMemPatrolScrubSetFeature *ps_set_feature;
>+ CXLSetFeatureInHeader *hdr = (void *)payload_in;
>+
>+ if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
>+ if (hdr->version != CXL_MEMDEV_PS_SET_FEATURE_VERSION ||
>+ (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
>+ CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
>+ return CXL_MBOX_UNSUPPORTED;
>+ }
>+
>+ ps_set_feature = (void *)payload_in;
>+ ps_write_attrbs = &ps_set_feature->feat_data;
>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle &= ~0xFF;
>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle |=
>+ ps_write_attrbs->scrub_cycle_hr & 0xFF;
>+ cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
>+ cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
>+ ps_write_attrbs->scrub_flags & 0x1;
>+ } else {
>+ return CXL_MBOX_UNSUPPORTED;
>+ }
>+
> return CXL_MBOX_SUCCESS;
> }
>
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)
2024-02-15 18:05 ` fan
@ 2024-02-16 9:20 ` Shiju Jose via
0 siblings, 0 replies; 14+ messages in thread
From: Shiju Jose via @ 2024-02-16 9:20 UTC (permalink / raw)
To: fan
Cc: qemu-devel@nongnu.org, linux-cxl@vger.kernel.org,
Jonathan Cameron, tanxiaofei, Zengtao (B), Linuxarm
Hi Fan,
Thanks for the feedbacks.
>-----Original Message-----
>From: fan <nifan.cxl@gmail.com>
>Sent: 15 February 2024 18:06
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: qemu-devel@nongnu.org; linux-cxl@vger.kernel.org; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>; Linuxarm <linuxarm@huawei.com>
>Subject: Re: [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature
>commands (8.2.9.6)
>
>On Fri, Nov 24, 2023 at 09:53:35PM +0800, shiju.jose@huawei.com wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
>> CXL devices supports features with changeable attributes.
>> Get Supported Features retrieves the list of supported device specific
>> features. The settings of a feature can be retrieved using Get Feature
>> and optionally modified using Set Feature.
>>
>> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>> ---
>
>Updated the references to align with cxl spec r3.1, other than that looks good to
>me.
I had posted v3 of this series updated for spec r3.1. Please find here,
https://lore.kernel.org/qemu-devel/20240215110146.1444-1-shiju.jose@huawei.com/T/#t
>
>Fan
>
>> hw/cxl/cxl-mailbox-utils.c | 167
>> +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 167 insertions(+)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 6184f44339..1bbc9a48a6 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -66,6 +66,10 @@ enum {
>> LOGS = 0x04,
>> #define GET_SUPPORTED 0x0
>> #define GET_LOG 0x1
>> + FEATURES = 0x05,
>> + #define GET_SUPPORTED 0x0
>> + #define GET_FEATURE 0x1
>> + #define SET_FEATURE 0x2
>> IDENTIFY = 0x40,
>> #define MEMORY_DEVICE 0x0
>> CCLS = 0x41,
>> @@ -785,6 +789,157 @@ static CXLRetCode cmd_logs_get_log(const struct
>cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>> }
>>
>> +/* CXL r3.0 section 8.2.9.6: Features */ typedef struct
>> +CXLSupportedFeatureHeader {
>> + uint16_t entries;
>> + uint16_t nsuppfeats_dev;
>> + uint32_t reserved;
>> +} QEMU_PACKED CXLSupportedFeatureHeader;
>> +
>> +typedef struct CXLSupportedFeatureEntry {
>> + QemuUUID uuid;
>> + uint16_t feat_index;
>> + uint16_t get_feat_size;
>> + uint16_t set_feat_size;
>> + uint32_t attrb_flags;
>> + uint8_t get_feat_version;
>> + uint8_t set_feat_version;
>> + uint16_t set_feat_effects;
>> + uint8_t rsvd[18];
>> +} QEMU_PACKED CXLSupportedFeatureEntry;
>> +
>> +enum CXL_SUPPORTED_FEATURES_LIST {
>> + CXL_FEATURE_MAX
>> +};
>> +
>> +/* Get Feature CXL 3.0 Spec 8.2.9.6.2 */
>> +/*
>> + * Get Feature input payload
>> + * CXL rev 3.0 section 8.2.9.6.2; Table 8-79 */
>> +/* Get Feature : Payload in selection */ enum
>> +CXL_GET_FEATURE_SELECTION {
>> + CXL_GET_FEATURE_SEL_CURRENT_VALUE = 0x0,
>> + CXL_GET_FEATURE_SEL_DEFAULT_VALUE = 0x1,
>> + CXL_GET_FEATURE_SEL_SAVED_VALUE = 0x2,
>> + CXL_GET_FEATURE_SEL_MAX
>> +};
>> +
>> +/* Set Feature CXL 3.0 Spec 8.2.9.6.3 */
>> +/*
>> + * Set Feature input payload
>> + * CXL rev 3.0 section 8.2.9.6.3; Table 8-81 */ typedef struct
>> +CXLSetFeatureInHeader {
>> + QemuUUID uuid;
>> + uint32_t flags;
>> + uint16_t offset;
>> + uint8_t version;
>> + uint8_t rsvd[9];
>> +} QEMU_PACKED QEMU_ALIGNED(16) CXLSetFeatureInHeader;
>> +
>> +/* Set Feature : Payload in flags */
>> +#define CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK 0x7
>> +enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
>> + CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER = 0x0,
>> + CXL_SET_FEATURE_FLAG_INITIATE_DATA_TRANSFER = 0x1,
>> + CXL_SET_FEATURE_FLAG_CONTINUE_DATA_TRANSFER = 0x2,
>> + CXL_SET_FEATURE_FLAG_FINISH_DATA_TRANSFER = 0x3,
>> + CXL_SET_FEATURE_FLAG_ABORT_DATA_TRANSFER = 0x4,
>> + CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
>> +};
>> +
>> +/* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h)
>> +*/ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd
>*cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci) {
>> + struct {
>> + uint32_t count;
>> + uint16_t start_index;
>> + uint16_t reserved;
>> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_in = (void
>> +*)payload_in;
>> +
>> + struct {
>> + CXLSupportedFeatureHeader hdr;
>> + CXLSupportedFeatureEntry feat_entries[];
>> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_out = (void
>*)payload_out;
>> + uint16_t index;
>> + uint16_t entry, req_entries;
>> + uint16_t feat_entries = 0;
>> +
>> + if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
>> + get_feats_in->start_index > CXL_FEATURE_MAX) {
>> + return CXL_MBOX_INVALID_INPUT;
>> + }
>> + req_entries = (get_feats_in->count -
>> + sizeof(CXLSupportedFeatureHeader)) /
>> + sizeof(CXLSupportedFeatureEntry);
>> + req_entries = MIN(req_entries, CXL_FEATURE_MAX);
>> + index = get_feats_in->start_index;
>> +
>> + entry = 0;
>> + while (entry < req_entries) {
>> + switch (index) {
>> + default:
>> + break;
>> + }
>> + index++;
>> + entry++;
>> + }
>> +
>> + get_feats_out->hdr.nsuppfeats_dev = CXL_FEATURE_MAX;
>> + get_feats_out->hdr.entries = feat_entries;
>> + *len_out = sizeof(CXLSupportedFeatureHeader) +
>> + feat_entries *
>> + sizeof(CXLSupportedFeatureEntry);
>> +
>> + return CXL_MBOX_SUCCESS;
>> +}
>> +
>> +/* CXL r3.0 section 8.2.9.6.2: Get Feature (Opcode 0501h) */ static
>> +CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci) {
>> + struct {
>> + QemuUUID uuid;
>> + uint16_t offset;
>> + uint16_t count;
>> + uint8_t selection;
>> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feature;
>> + uint16_t bytes_to_copy = 0;
>> +
>> + get_feature = (void *)payload_in;
>> +
>> + if (get_feature->selection != CXL_GET_FEATURE_SEL_CURRENT_VALUE) {
>> + return CXL_MBOX_UNSUPPORTED;
>> + }
>> + if (get_feature->offset + get_feature->count > cci->payload_max) {
>> + return CXL_MBOX_INVALID_INPUT;
>> + }
>> +
>> + *len_out = bytes_to_copy;
>> +
>> + return CXL_MBOX_SUCCESS;
>> +}
>> +
>> +/* CXL r3.0 section 8.2.9.6.3: Set Feature (Opcode 0502h) */ static
>> +CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci) {
>> + return CXL_MBOX_SUCCESS;
>> +}
>> +
>> /* 8.2.9.5.1.1 */
>> static CXLRetCode cmd_identify_memory_device(const struct cxl_cmd *cmd,
>> uint8_t *payload_in, @@
>> -1954,6 +2109,18 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>> [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED",
>cmd_logs_get_supported,
>> 0, 0 },
>> [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
>> + [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
>> + cmd_features_get_supported, 0x8, 0 },
>> + [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
>> + cmd_features_get_feature, 0x15, 0 },
>> + [FEATURES][SET_FEATURE] = { "FEATURES_SET_FEATURE",
>> + cmd_features_set_feature,
>> + ~0,
>> + (CXL_MBOX_IMMEDIATE_CONFIG_CHANGE |
>> + CXL_MBOX_IMMEDIATE_DATA_CHANGE |
>> + CXL_MBOX_IMMEDIATE_POLICY_CHANGE |
>> + CXL_MBOX_IMMEDIATE_LOG_CHANGE |
>> + CXL_MBOX_SECURITY_STATE_CHANGE)},
>> [IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE",
>> cmd_identify_memory_device, 0, 0 },
>> [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO",
>> --
>> 2.34.1
>>
Thanks,
Shiju
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features
2024-02-15 18:08 ` [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features fan
@ 2024-02-16 9:22 ` Shiju Jose via
0 siblings, 0 replies; 14+ messages in thread
From: Shiju Jose via @ 2024-02-16 9:22 UTC (permalink / raw)
To: fan
Cc: qemu-devel@nongnu.org, linux-cxl@vger.kernel.org,
Jonathan Cameron, tanxiaofei, Zengtao (B), Linuxarm
Hi Fan,
>-----Original Message-----
>From: fan <nifan.cxl@gmail.com>
>Sent: 15 February 2024 18:09
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: qemu-devel@nongnu.org; linux-cxl@vger.kernel.org; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>; Linuxarm <linuxarm@huawei.com>
>Subject: Re: [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands,
>device patrol scrub control and DDR5 ECS control features
>
>On Fri, Nov 24, 2023 at 09:53:34PM +0800, shiju.jose@huawei.com wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> Add support for the feature commands, device patrol scrub control and
>> DDR5 ECS control features.
>>
>> CXL spec 3.0 section 8.2.9.6 describes optional device specific features.
>> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub
>> control feature.
>> CXL spec 3.1 section 8.2.9.9.11.2 describes the DDR5 Error Check Scrub
>> (ECS) control feature.
>>
>> The patches are available here,
>> https://gitlab.com/shiju.jose/qemu/-/tree/cxl-scrub-2023-11-14
>> and is based on Jonathan's branch
>> https://gitlab.com/jic23/qemu/-/tree/cxl-2023-10-16
>>
>> Changes
>> v1 -> v2
>> 1. Changes for Davidlohr comments. Thanks.
>> - Changed CXL SET feature data transfer flags as enum.
>> - Modified pointer supported_feats to get_feats_out.
>> - Removed an unnecessary branch.
>> - Use MIN().
>> - Move setting of hdr.nsuppfeats further down.
>> - Return CXL_MBOX_UNSUPPORTED if non-zero selection flag is passed.
>> - Add more IMMEDIATE_*.* flags set_feature.
>> - Corrected a spelling error.
>>
>> Shiju Jose (3):
>> hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)
>> hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
>> hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS control feature
>>
>> hw/cxl/cxl-mailbox-utils.c | 360
>> +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 360 insertions(+)
>>
>> --
>> 2.34.1
>>
>
>Recently, Jonathan has updated all the specification references to align
>with cxl spec r3.1, so for the next version, we may want to also do
>that.
I had posted recently v3 of this series updated for spec r3.1. Please find,
https://lore.kernel.org/qemu-devel/20240215110146.1444-1-shiju.jose@huawei.com/T/#t
>
>Fan
Thanks,
Shiju
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
2024-02-15 18:47 ` fan
@ 2024-02-16 10:16 ` Shiju Jose via
2024-02-16 17:35 ` fan
0 siblings, 1 reply; 14+ messages in thread
From: Shiju Jose via @ 2024-02-16 10:16 UTC (permalink / raw)
To: fan
Cc: qemu-devel@nongnu.org, linux-cxl@vger.kernel.org,
Jonathan Cameron, tanxiaofei, Zengtao (B), Linuxarm,
fan.ni@samsung.com
Hi Fan,
>-----Original Message-----
>From: fan <nifan.cxl@gmail.com>
>Sent: 15 February 2024 18:47
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: qemu-devel@nongnu.org; linux-cxl@vger.kernel.org; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>; Linuxarm <linuxarm@huawei.com>;
>fan.ni@samsung.com
>Subject: Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub
>control feature
>
>On Fri, Nov 24, 2023 at 09:53:36PM +0800, shiju.jose@huawei.com wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub
>> control feature. The device patrol scrub proactively locates and makes
>> corrections to errors in regular cycle. The patrol scrub control
>> allows the request to configure patrol scrub input configurations.
>>
>> The patrol scrub control allows the requester to specify the number of
>> hours for which the patrol scrub cycles must be completed, provided
>> that the requested number is not less than the minimum number of hours
>> for the patrol scrub cycle that the device is capable of. In addition,
>> the patrol scrub controls allow the host to disable and enable the
>> feature in case disabling of the feature is needed for other purposes
>> such as performance-aware operations which require the background
>> operations to be turned off.
>>
>> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>> ---
>
>LGTM except for some minor comments inlined.
>
>
>> hw/cxl/cxl-mailbox-utils.c | 97
>> +++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 1bbc9a48a6..5a6f4e4029 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry { }
>> QEMU_PACKED CXLSupportedFeatureEntry;
>>
>> enum CXL_SUPPORTED_FEATURES_LIST {
>> + CXL_FEATURE_PATROL_SCRUB = 0,
>> CXL_FEATURE_MAX
>> };
>>
>> @@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
>> CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
>> };
>>
>> +/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature
>> +*/ static const QemuUUID patrol_scrub_uuid = {
>> + .data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
>> + 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a) };
>> +
>> +#define CXL_MEMDEV_PS_GET_FEATURE_VERSION 0x01
>> +#define CXL_MEMDEV_PS_SET_FEATURE_VERSION 0x01
>> +#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT BIT(0)
>> +#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT
>BIT(1)
>> +#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT 12
>> +#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT 1
>> +#define CXL_MEMDEV_PS_ENABLE_DEFAULT 0
>> +
>> +/* CXL memdev patrol scrub control attributes */ struct
>> +CXLMemPatrolScrubReadAttrbs {
>> + uint8_t scrub_cycle_cap;
>> + uint16_t scrub_cycle;
>> + uint8_t scrub_flags;
>> +} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
>> +
>> +typedef struct CXLMemPatrolScrubWriteAttrbs {
>> + uint8_t scrub_cycle_hr;
>> + uint8_t scrub_flags;
>> +} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
>> +
>> +typedef struct CXLMemPatrolScrubSetFeature {
>> + CXLSetFeatureInHeader hdr;
>> + CXLMemPatrolScrubWriteAttrbs feat_data; } QEMU_PACKED
>> +QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
>> +
>> /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h)
>> */ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd
>*cmd,
>> uint8_t *payload_in, @@
>> -872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const
>struct cxl_cmd *cmd,
>> uint16_t feat_entries = 0;
>>
>> if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
>> - get_feats_in->start_index > CXL_FEATURE_MAX) {
>> + get_feats_in->start_index >= CXL_FEATURE_MAX) {
>
>Not totally sure about this, the spec says "...Greater than..." although I also think
>it should be >=. Similar things for the offset usage below.
Spec r3.1 described in Table 8-95. Get Supported Features Input Payload as ,
"Starting Feature Index: Index of the first requested Supported Feature Entry.
Feature index is a zero-based value."
Thus I believe get_feats_in->start_index >= CXL_FEATURE_MAX is correct because
the feature index is zero-based value.
Regarding the offset usage mentioned, can you point which code?
Is it get_feature->offset?
>
>Fan
>
>> return CXL_MBOX_INVALID_INPUT;
>> }
>> req_entries = (get_feats_in->count - @@ -884,6 +916,31 @@ static
>> CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
>> entry = 0;
>> while (entry < req_entries) {
>> switch (index) {
>> + case CXL_FEATURE_PATROL_SCRUB:
>> + /* Fill supported feature entry for device patrol scrub control */
>> + get_feats_out->feat_entries[entry] =
>> + (struct CXLSupportedFeatureEntry) {
>> + .uuid = patrol_scrub_uuid,
>> + .feat_index = index,
>> + .get_feat_size = sizeof(cxl_memdev_ps_feat_read_attrbs),
>> + .set_feat_size = sizeof(CXLMemPatrolScrubWriteAttrbs),
>> + /* Bit[0] : 1, feature attributes changeable */
>> + .attrb_flags = 0x1,
>> + .get_feat_version = CXL_MEMDEV_PS_GET_FEATURE_VERSION,
>> + .set_feat_version = CXL_MEMDEV_PS_SET_FEATURE_VERSION,
>> + .set_feat_effects = 0,
>> + };
>> + feat_entries++;
>> + /* Set default value for device patrol scrub read attributes */
>> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle_cap =
>> +
>CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT |
>> +
>CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT;
>> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle =
>> + CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT |
>> + (CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT << 8);
>> + cxl_memdev_ps_feat_read_attrbs.scrub_flags =
>> + CXL_MEMDEV_PS_ENABLE_DEFAULT;
>> + break;
>> default:
>> break;
>> }
>> @@ -924,6 +981,21 @@ static CXLRetCode cmd_features_get_feature(const
>struct cxl_cmd *cmd,
>> return CXL_MBOX_INVALID_INPUT;
>> }
>>
>> + if (qemu_uuid_is_equal(&get_feature->uuid, &patrol_scrub_uuid)) {
>> + if (get_feature->offset >= sizeof(cxl_memdev_ps_feat_read_attrbs)) {
>> + return CXL_MBOX_INVALID_INPUT;
>> + }
>> + bytes_to_copy = sizeof(cxl_memdev_ps_feat_read_attrbs) -
>> + get_feature->offset;
>> + bytes_to_copy = (bytes_to_copy > get_feature->count) ?
>> + get_feature->count : bytes_to_copy;
>> + memcpy(payload_out,
>> + &cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
>> + bytes_to_copy);
>> + } else {
>> + return CXL_MBOX_UNSUPPORTED;
>> + }
>> +
>> *len_out = bytes_to_copy;
>>
>> return CXL_MBOX_SUCCESS;
>> @@ -937,6 +1009,29 @@ static CXLRetCode cmd_features_set_feature(const
>struct cxl_cmd *cmd,
>> size_t *len_out,
>> CXLCCI *cci) {
>> + CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
>> + CXLMemPatrolScrubSetFeature *ps_set_feature;
>> + CXLSetFeatureInHeader *hdr = (void *)payload_in;
>> +
>> + if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
>> + if (hdr->version != CXL_MEMDEV_PS_SET_FEATURE_VERSION ||
>> + (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
>> + CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
>> + return CXL_MBOX_UNSUPPORTED;
>> + }
>> +
>> + ps_set_feature = (void *)payload_in;
>> + ps_write_attrbs = &ps_set_feature->feat_data;
>> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle &= ~0xFF;
>> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle |=
>> + ps_write_attrbs->scrub_cycle_hr & 0xFF;
>> + cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
>> + cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
>> + ps_write_attrbs->scrub_flags & 0x1;
>> + } else {
>> + return CXL_MBOX_UNSUPPORTED;
>> + }
>> +
>> return CXL_MBOX_SUCCESS;
>> }
>>
>> --
>> 2.34.1
>>
Thanks,
Shiju
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
2024-02-15 20:55 ` Davidlohr Bueso
@ 2024-02-16 12:11 ` Shiju Jose via
0 siblings, 0 replies; 14+ messages in thread
From: Shiju Jose via @ 2024-02-16 12:11 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: qemu-devel@nongnu.org, linux-cxl@vger.kernel.org,
Jonathan Cameron, tanxiaofei, Zengtao (B), Linuxarm,
fan.ni@samsung.com
Hi Davidlohr,
Thanks for the feedback.
Please find reply inline.
>-----Original Message-----
>From: Davidlohr Bueso <dave@stgolabs.net>
>Sent: 15 February 2024 20:56
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: qemu-devel@nongnu.org; linux-cxl@vger.kernel.org; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>; Linuxarm <linuxarm@huawei.com>;
>fan.ni@samsung.com
>Subject: Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub
>control feature
>
>On Fri, 24 Nov 2023, shiju.jose@huawei.com wrote:
>
>>From: Shiju Jose <shiju.jose@huawei.com>
>>
>>CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub
>>control feature. The device patrol scrub proactively locates and makes
>>corrections to errors in regular cycle. The patrol scrub control allows
>>the request to configure patrol scrub input configurations.
>>
>>The patrol scrub control allows the requester to specify the number of
>>hours for which the patrol scrub cycles must be completed, provided
>>that the requested number is not less than the minimum number of hours
>>for the patrol scrub cycle that the device is capable of. In addition,
>>the patrol scrub controls allow the host to disable and enable the
>>feature in case disabling of the feature is needed for other purposes
>>such as performance-aware operations which require the background
>>operations to be turned off.
>>
>>Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>>Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>>---
>> hw/cxl/cxl-mailbox-utils.c | 97 +++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 96 insertions(+), 1 deletion(-)
>>
>>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>>index 1bbc9a48a6..5a6f4e4029 100644
>>--- a/hw/cxl/cxl-mailbox-utils.c
>>+++ b/hw/cxl/cxl-mailbox-utils.c
>>@@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry { }
>>QEMU_PACKED CXLSupportedFeatureEntry;
>>
>> enum CXL_SUPPORTED_FEATURES_LIST {
>>+ CXL_FEATURE_PATROL_SCRUB = 0,
>> CXL_FEATURE_MAX
>> };
>>
>>@@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
>> CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
>> };
>>
>>+/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature
>>+*/ static const QemuUUID patrol_scrub_uuid = {
>>+ .data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
>>+ 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a) };
>>+
>>+#define CXL_MEMDEV_PS_GET_FEATURE_VERSION 0x01
>>+#define CXL_MEMDEV_PS_SET_FEATURE_VERSION 0x01
>>+#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT BIT(0)
>>+#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT
>BIT(1)
>>+#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT 12
>>+#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT 1
>>+#define CXL_MEMDEV_PS_ENABLE_DEFAULT 0
>>+
>>+/* CXL memdev patrol scrub control attributes */ struct
>>+CXLMemPatrolScrubReadAttrbs {
>>+ uint8_t scrub_cycle_cap;
>>+ uint16_t scrub_cycle;
>>+ uint8_t scrub_flags;
>>+} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
>>+
>>+typedef struct CXLMemPatrolScrubWriteAttrbs {
>>+ uint8_t scrub_cycle_hr;
>>+ uint8_t scrub_flags;
>>+} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
>
>fyi there is an ask, which I certainly agree with, to make these static here
>instead of at runtime.
I will make cxl_memdev_ps_feat_read_attrbs static, however can't make const because
cxl_memdev_ps_feat_read_attrbs use to store the attributes in the set_feature.
May be rename cxl_memdev_ps_feat_read_attrbs to cxl_memdev_ps_feat_ attrbs to avoid confusion?
>
>https://lore.kernel.org/linux-cxl/20240119175006.00007f74@Huawei.com/
>
>Also, this series probably needs rebasing per Jonathan's latest branch with lots of
>updates.
The v3 posted recently was rebased to
Jonathan's recent branch
https://gitlab.com/jic23/qemu/-/tree/cxl-2024-02-05-draft
Looks like latest branch is cxl-2024-02-14. I will rebase.
https://lore.kernel.org/qemu-devel/20240215110146.1444-1-shiju.jose@huawei.com/T/#t
>
>>+
>>+typedef struct CXLMemPatrolScrubSetFeature {
>>+ CXLSetFeatureInHeader hdr;
>>+ CXLMemPatrolScrubWriteAttrbs feat_data; } QEMU_PACKED
>>+QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
>>+
>> /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h)
>>*/ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
>> uint8_t *payload_in, @@
>>-872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const
>struct cxl_cmd *cmd,
>> uint16_t feat_entries = 0;
>>
>> if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
>>- get_feats_in->start_index > CXL_FEATURE_MAX) {
>>+ get_feats_in->start_index >= CXL_FEATURE_MAX) {
>> return CXL_MBOX_INVALID_INPUT;
>> }
>> req_entries = (get_feats_in->count - @@ -884,6 +916,31 @@ static
>>CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
>> entry = 0;
>> while (entry < req_entries) {
>> switch (index) {
>>+ case CXL_FEATURE_PATROL_SCRUB:
>>+ /* Fill supported feature entry for device patrol scrub control */
>>+ get_feats_out->feat_entries[entry] =
>>+ (struct CXLSupportedFeatureEntry) {
>>+ .uuid = patrol_scrub_uuid,
>>+ .feat_index = index,
>>+ .get_feat_size = sizeof(cxl_memdev_ps_feat_read_attrbs),
>>+ .set_feat_size = sizeof(CXLMemPatrolScrubWriteAttrbs),
>>+ /* Bit[0] : 1, feature attributes changeable */
>>+ .attrb_flags = 0x1,
>>+ .get_feat_version = CXL_MEMDEV_PS_GET_FEATURE_VERSION,
>>+ .set_feat_version = CXL_MEMDEV_PS_SET_FEATURE_VERSION,
>>+ .set_feat_effects = 0,
>>+ };
>>+ feat_entries++;
>>+ /* Set default value for device patrol scrub read attributes */
>>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle_cap =
>>+
>CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT |
>>+
>CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT;
>>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle =
>>+ CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT |
>>+ (CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT << 8);
>>+ cxl_memdev_ps_feat_read_attrbs.scrub_flags =
>>+ CXL_MEMDEV_PS_ENABLE_DEFAULT;
>>+ break;
>> default:
>> break;
>> }
>>@@ -924,6 +981,21 @@ static CXLRetCode cmd_features_get_feature(const
>struct cxl_cmd *cmd,
>> return CXL_MBOX_INVALID_INPUT;
>> }
>>
>>+ if (qemu_uuid_is_equal(&get_feature->uuid, &patrol_scrub_uuid)) {
>>+ if (get_feature->offset >= sizeof(cxl_memdev_ps_feat_read_attrbs)) {
>>+ return CXL_MBOX_INVALID_INPUT;
>>+ }
>>+ bytes_to_copy = sizeof(cxl_memdev_ps_feat_read_attrbs) -
>>+ get_feature->offset;
>>+ bytes_to_copy = (bytes_to_copy > get_feature->count) ?
>>+ get_feature->count : bytes_to_copy;
>
>min()?
>
>>+ memcpy(payload_out,
>>+ &cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
>>+ bytes_to_copy);
>>+ } else {
>>+ return CXL_MBOX_UNSUPPORTED;
>>+ }
>>+
>> *len_out = bytes_to_copy;
>>
>> return CXL_MBOX_SUCCESS;
>>@@ -937,6 +1009,29 @@ static CXLRetCode cmd_features_set_feature(const
>struct cxl_cmd *cmd,
>> size_t *len_out,
>> CXLCCI *cci) {
>>+ CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
>>+ CXLMemPatrolScrubSetFeature *ps_set_feature;
>>+ CXLSetFeatureInHeader *hdr = (void *)payload_in;
>>+
>>+ if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
>>+ if (hdr->version != CXL_MEMDEV_PS_SET_FEATURE_VERSION ||
>>+ (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
>>+ CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
>>+ return CXL_MBOX_UNSUPPORTED;
>>+ }
>>+
>>+ ps_set_feature = (void *)payload_in;
>>+ ps_write_attrbs = &ps_set_feature->feat_data;
>>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle &= ~0xFF;
>>+ cxl_memdev_ps_feat_read_attrbs.scrub_cycle |=
>>+ ps_write_attrbs->scrub_cycle_hr & 0xFF;
>>+ cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
>>+ cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
>>+ ps_write_attrbs->scrub_flags & 0x1;
>>+ } else {
>>+ return CXL_MBOX_UNSUPPORTED;
>>+ }
>>+
>> return CXL_MBOX_SUCCESS;
>> }
>>
>>--
>>2.34.1
>>
Thanks,
Shiju
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature
2024-02-16 10:16 ` Shiju Jose via
@ 2024-02-16 17:35 ` fan
0 siblings, 0 replies; 14+ messages in thread
From: fan @ 2024-02-16 17:35 UTC (permalink / raw)
To: Shiju Jose
Cc: fan, qemu-devel@nongnu.org, linux-cxl@vger.kernel.org,
Jonathan Cameron, tanxiaofei, Zengtao (B), Linuxarm,
fan.ni@samsung.com
On Fri, Feb 16, 2024 at 10:16:12AM +0000, Shiju Jose wrote:
> Hi Fan,
>
> >-----Original Message-----
> >From: fan <nifan.cxl@gmail.com>
> >Sent: 15 February 2024 18:47
> >To: Shiju Jose <shiju.jose@huawei.com>
> >Cc: qemu-devel@nongnu.org; linux-cxl@vger.kernel.org; Jonathan Cameron
> ><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
> >Zengtao (B) <prime.zeng@hisilicon.com>; Linuxarm <linuxarm@huawei.com>;
> >fan.ni@samsung.com
> >Subject: Re: [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub
> >control feature
> >
> >On Fri, Nov 24, 2023 at 09:53:36PM +0800, shiju.jose@huawei.com wrote:
> >> From: Shiju Jose <shiju.jose@huawei.com>
> >>
> >> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub
> >> control feature. The device patrol scrub proactively locates and makes
> >> corrections to errors in regular cycle. The patrol scrub control
> >> allows the request to configure patrol scrub input configurations.
> >>
> >> The patrol scrub control allows the requester to specify the number of
> >> hours for which the patrol scrub cycles must be completed, provided
> >> that the requested number is not less than the minimum number of hours
> >> for the patrol scrub cycle that the device is capable of. In addition,
> >> the patrol scrub controls allow the host to disable and enable the
> >> feature in case disabling of the feature is needed for other purposes
> >> such as performance-aware operations which require the background
> >> operations to be turned off.
> >>
> >> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> >> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> >> ---
> >
> >LGTM except for some minor comments inlined.
> >
> >
> >> hw/cxl/cxl-mailbox-utils.c | 97
> >> +++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 96 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> >> index 1bbc9a48a6..5a6f4e4029 100644
> >> --- a/hw/cxl/cxl-mailbox-utils.c
> >> +++ b/hw/cxl/cxl-mailbox-utils.c
> >> @@ -809,6 +809,7 @@ typedef struct CXLSupportedFeatureEntry { }
> >> QEMU_PACKED CXLSupportedFeatureEntry;
> >>
> >> enum CXL_SUPPORTED_FEATURES_LIST {
> >> + CXL_FEATURE_PATROL_SCRUB = 0,
> >> CXL_FEATURE_MAX
> >> };
> >>
> >> @@ -849,6 +850,37 @@ enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER {
> >> CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX
> >> };
> >>
> >> +/* CXL r3.1 section 8.2.9.9.11.1: Device Patrol Scrub Control Feature
> >> +*/ static const QemuUUID patrol_scrub_uuid = {
> >> + .data = UUID(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33,
> >> + 0x75, 0x77, 0x4e, 0x06, 0xdb, 0x8a) };
> >> +
> >> +#define CXL_MEMDEV_PS_GET_FEATURE_VERSION 0x01
> >> +#define CXL_MEMDEV_PS_SET_FEATURE_VERSION 0x01
> >> +#define CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT BIT(0)
> >> +#define CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT
> >BIT(1)
> >> +#define CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT 12
> >> +#define CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT 1
> >> +#define CXL_MEMDEV_PS_ENABLE_DEFAULT 0
> >> +
> >> +/* CXL memdev patrol scrub control attributes */ struct
> >> +CXLMemPatrolScrubReadAttrbs {
> >> + uint8_t scrub_cycle_cap;
> >> + uint16_t scrub_cycle;
> >> + uint8_t scrub_flags;
> >> +} QEMU_PACKED cxl_memdev_ps_feat_read_attrbs;
> >> +
> >> +typedef struct CXLMemPatrolScrubWriteAttrbs {
> >> + uint8_t scrub_cycle_hr;
> >> + uint8_t scrub_flags;
> >> +} QEMU_PACKED CXLMemPatrolScrubWriteAttrbs;
> >> +
> >> +typedef struct CXLMemPatrolScrubSetFeature {
> >> + CXLSetFeatureInHeader hdr;
> >> + CXLMemPatrolScrubWriteAttrbs feat_data; } QEMU_PACKED
> >> +QEMU_ALIGNED(16) CXLMemPatrolScrubSetFeature;
> >> +
> >> /* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h)
> >> */ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd
> >*cmd,
> >> uint8_t *payload_in, @@
> >> -872,7 +904,7 @@ static CXLRetCode cmd_features_get_supported(const
> >struct cxl_cmd *cmd,
> >> uint16_t feat_entries = 0;
> >>
> >> if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) ||
> >> - get_feats_in->start_index > CXL_FEATURE_MAX) {
> >> + get_feats_in->start_index >= CXL_FEATURE_MAX) {
> >
> >Not totally sure about this, the spec says "...Greater than..." although I also think
> >it should be >=. Similar things for the offset usage below.
>
> Spec r3.1 described in Table 8-95. Get Supported Features Input Payload as ,
> "Starting Feature Index: Index of the first requested Supported Feature Entry.
> Feature index is a zero-based value."
> Thus I believe get_feats_in->start_index >= CXL_FEATURE_MAX is correct because
> the feature index is zero-based value.
That is also my understanding.
>
> Regarding the offset usage mentioned, can you point which code?
> Is it get_feature->offset?
>
Yea. Maybe not an issue as long as we keep all consistent.o
Fan
> >
> >Fan
> >
> >> return CXL_MBOX_INVALID_INPUT;
> >> }
> >> req_entries = (get_feats_in->count - @@ -884,6 +916,31 @@ static
> >> CXLRetCode cmd_features_get_supported(const struct cxl_cmd *cmd,
> >> entry = 0;
> >> while (entry < req_entries) {
> >> switch (index) {
> >> + case CXL_FEATURE_PATROL_SCRUB:
> >> + /* Fill supported feature entry for device patrol scrub control */
> >> + get_feats_out->feat_entries[entry] =
> >> + (struct CXLSupportedFeatureEntry) {
> >> + .uuid = patrol_scrub_uuid,
> >> + .feat_index = index,
> >> + .get_feat_size = sizeof(cxl_memdev_ps_feat_read_attrbs),
> >> + .set_feat_size = sizeof(CXLMemPatrolScrubWriteAttrbs),
> >> + /* Bit[0] : 1, feature attributes changeable */
> >> + .attrb_flags = 0x1,
> >> + .get_feat_version = CXL_MEMDEV_PS_GET_FEATURE_VERSION,
> >> + .set_feat_version = CXL_MEMDEV_PS_SET_FEATURE_VERSION,
> >> + .set_feat_effects = 0,
> >> + };
> >> + feat_entries++;
> >> + /* Set default value for device patrol scrub read attributes */
> >> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle_cap =
> >> +
> >CXL_MEMDEV_PS_SCRUB_CYCLE_CHANGE_CAP_DEFAULT |
> >> +
> >CXL_MEMDEV_PS_SCRUB_REALTIME_REPORT_CAP_DEFAULT;
> >> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle =
> >> + CXL_MEMDEV_PS_CUR_SCRUB_CYCLE_DEFAULT |
> >> + (CXL_MEMDEV_PS_MIN_SCRUB_CYCLE_DEFAULT << 8);
> >> + cxl_memdev_ps_feat_read_attrbs.scrub_flags =
> >> + CXL_MEMDEV_PS_ENABLE_DEFAULT;
> >> + break;
> >> default:
> >> break;
> >> }
> >> @@ -924,6 +981,21 @@ static CXLRetCode cmd_features_get_feature(const
> >struct cxl_cmd *cmd,
> >> return CXL_MBOX_INVALID_INPUT;
> >> }
> >>
> >> + if (qemu_uuid_is_equal(&get_feature->uuid, &patrol_scrub_uuid)) {
> >> + if (get_feature->offset >= sizeof(cxl_memdev_ps_feat_read_attrbs)) {
> >> + return CXL_MBOX_INVALID_INPUT;
> >> + }
> >> + bytes_to_copy = sizeof(cxl_memdev_ps_feat_read_attrbs) -
> >> + get_feature->offset;
> >> + bytes_to_copy = (bytes_to_copy > get_feature->count) ?
> >> + get_feature->count : bytes_to_copy;
> >> + memcpy(payload_out,
> >> + &cxl_memdev_ps_feat_read_attrbs + get_feature->offset,
> >> + bytes_to_copy);
> >> + } else {
> >> + return CXL_MBOX_UNSUPPORTED;
> >> + }
> >> +
> >> *len_out = bytes_to_copy;
> >>
> >> return CXL_MBOX_SUCCESS;
> >> @@ -937,6 +1009,29 @@ static CXLRetCode cmd_features_set_feature(const
> >struct cxl_cmd *cmd,
> >> size_t *len_out,
> >> CXLCCI *cci) {
> >> + CXLMemPatrolScrubWriteAttrbs *ps_write_attrbs;
> >> + CXLMemPatrolScrubSetFeature *ps_set_feature;
> >> + CXLSetFeatureInHeader *hdr = (void *)payload_in;
> >> +
> >> + if (qemu_uuid_is_equal(&hdr->uuid, &patrol_scrub_uuid)) {
> >> + if (hdr->version != CXL_MEMDEV_PS_SET_FEATURE_VERSION ||
> >> + (hdr->flags & CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK) !=
> >> + CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER) {
> >> + return CXL_MBOX_UNSUPPORTED;
> >> + }
> >> +
> >> + ps_set_feature = (void *)payload_in;
> >> + ps_write_attrbs = &ps_set_feature->feat_data;
> >> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle &= ~0xFF;
> >> + cxl_memdev_ps_feat_read_attrbs.scrub_cycle |=
> >> + ps_write_attrbs->scrub_cycle_hr & 0xFF;
> >> + cxl_memdev_ps_feat_read_attrbs.scrub_flags &= ~0x1;
> >> + cxl_memdev_ps_feat_read_attrbs.scrub_flags |=
> >> + ps_write_attrbs->scrub_flags & 0x1;
> >> + } else {
> >> + return CXL_MBOX_UNSUPPORTED;
> >> + }
> >> +
> >> return CXL_MBOX_SUCCESS;
> >> }
> >>
> >> --
> >> 2.34.1
> >>
>
> Thanks,
> Shiju
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-02-16 17:36 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-24 13:53 [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features shiju.jose--- via
2023-11-24 13:53 ` [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6) shiju.jose--- via
2024-02-15 18:05 ` fan
2024-02-16 9:20 ` Shiju Jose via
2023-11-24 13:53 ` [PATCH v2 2/3] hw/cxl/cxl-mailbox-utils: Add device patrol scrub control feature shiju.jose--- via
2024-02-15 18:47 ` fan
2024-02-16 10:16 ` Shiju Jose via
2024-02-16 17:35 ` fan
2024-02-15 20:55 ` Davidlohr Bueso
2024-02-16 12:11 ` Shiju Jose via
2023-11-24 13:53 ` [PATCH v2 3/3] hw/cxl/cxl-mailbox-utils: Add device DDR5 ECS " shiju.jose--- via
2024-02-15 19:11 ` fan
2024-02-15 18:08 ` [PATCH v2 0/3] hw/cxl/cxl-mailbox-utils: Add feature commands, device patrol scrub control and DDR5 ECS control features fan
2024-02-16 9:22 ` Shiju Jose via
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).