* [PATCH v2 1/8] qemu/bswap: Add const_le64()
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2022-12-22 4:24 ` [PATCH v2 2/8] qemu/uuid: Add UUID static initializer Ira Weiny
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
Gcc requires constant versions of cpu_to_le* calls.
Add a 64 bit version.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes from RFC:
Peter
Change order of the definitions, 64-32-16
---
include/qemu/bswap.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index 346d05f2aab3..e1eca22f2548 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -187,6 +187,15 @@ CPU_CONVERT(le, 64, uint64_t)
* used to initialize static variables.
*/
#if HOST_BIG_ENDIAN
+# define const_le64(_x) \
+ ((((_x) & 0x00000000000000ffU) << 56) | \
+ (((_x) & 0x000000000000ff00U) << 40) | \
+ (((_x) & 0x0000000000ff0000U) << 24) | \
+ (((_x) & 0x00000000ff000000U) << 8) | \
+ (((_x) & 0x000000ff00000000U) >> 8) | \
+ (((_x) & 0x0000ff0000000000U) >> 24) | \
+ (((_x) & 0x00ff000000000000U) >> 40) | \
+ (((_x) & 0xff00000000000000U) >> 56))
# define const_le32(_x) \
((((_x) & 0x000000ffU) << 24) | \
(((_x) & 0x0000ff00U) << 8) | \
@@ -196,6 +205,7 @@ CPU_CONVERT(le, 64, uint64_t)
((((_x) & 0x00ff) << 8) | \
(((_x) & 0xff00) >> 8))
#else
+# define const_le64(_x) (_x)
# define const_le32(_x) (_x)
# define const_le16(_x) (_x)
#endif
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/8] qemu/uuid: Add UUID static initializer
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
2022-12-22 4:24 ` [PATCH v2 1/8] qemu/bswap: Add const_le64() Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2022-12-22 4:24 ` [PATCH v2 3/8] hw/cxl/mailbox: Use new UUID network order define for cel_uuid Ira Weiny
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
UUID's are defined as network byte order fields. No static initializer
was available for UUID's in their standard big endian format.
Define a big endian initializer for UUIDs.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
include/qemu/uuid.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
index 9925febfa54d..dc40ee1fc998 100644
--- a/include/qemu/uuid.h
+++ b/include/qemu/uuid.h
@@ -61,6 +61,18 @@ typedef struct {
(clock_seq_hi_and_reserved), (clock_seq_low), (node0), (node1), (node2),\
(node3), (node4), (node5) }
+/* Normal (network byte order) UUID */
+#define UUID(time_low, time_mid, time_hi_and_version, \
+ clock_seq_hi_and_reserved, clock_seq_low, node0, node1, node2, \
+ node3, node4, node5) \
+ { ((time_low) >> 24) & 0xff, ((time_low) >> 16) & 0xff, \
+ ((time_low) >> 8) & 0xff, (time_low) & 0xff, \
+ ((time_mid) >> 8) & 0xff, (time_mid) & 0xff, \
+ ((time_hi_and_version) >> 8) & 0xff, (time_hi_and_version) & 0xff, \
+ (clock_seq_hi_and_reserved), (clock_seq_low), \
+ (node0), (node1), (node2), (node3), (node4), (node5) \
+ }
+
#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
"%02hhx%02hhx-%02hhx%02hhx-" \
"%02hhx%02hhx-" \
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/8] hw/cxl/mailbox: Use new UUID network order define for cel_uuid
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
2022-12-22 4:24 ` [PATCH v2 1/8] qemu/bswap: Add const_le64() Ira Weiny
2022-12-22 4:24 ` [PATCH v2 2/8] qemu/uuid: Add UUID static initializer Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2023-01-03 16:30 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 4/8] hw/cxl/events: Add event status register Ira Weiny
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
The cel_uuid was programatically generated previously because there was
no static initializer for network order UUIDs.
Use the new network order initializer for cel_uuid. Adjust
cxl_initialize_mailbox() because it can't fail now.
Update specification reference.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes from RFC:
New patch.
---
hw/cxl/cxl-device-utils.c | 4 ++--
hw/cxl/cxl-mailbox-utils.c | 14 +++++++-------
include/hw/cxl/cxl_device.h | 2 +-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index 21845dbfd050..34697064714e 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -267,7 +267,7 @@ void cxl_device_register_init_common(CXLDeviceState *cxl_dstate)
cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000);
memdev_reg_init_common(cxl_dstate);
- assert(cxl_initialize_mailbox(cxl_dstate, false) == 0);
+ cxl_initialize_mailbox(cxl_dstate, false);
}
void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate)
@@ -289,5 +289,5 @@ void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate)
cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000);
memdev_reg_init_common(cxl_dstate);
- assert(cxl_initialize_mailbox(cxl_dstate, true) == 0);
+ cxl_initialize_mailbox(cxl_dstate, true);
}
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index c1183614b9a4..157c01255ee3 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -321,7 +321,11 @@ static ret_code cmd_timestamp_set(struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
-static QemuUUID cel_uuid;
+/* CXL 3.0 8.2.9.5.2.1 Command Effects Log (CEL) */
+static QemuUUID cel_uuid = {
+ .data = UUID(0x0da9c0b5, 0xbf41, 0x4b78, 0x8f, 0x79,
+ 0x96, 0xb1, 0x62, 0x3b, 0x3f, 0x17)
+};
/* 8.2.9.4.1 */
static ret_code cmd_logs_get_supported(struct cxl_cmd *cmd,
@@ -684,16 +688,14 @@ void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
DOORBELL, 0);
}
-int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
+void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
{
- /* CXL 2.0: Table 169 Get Supported Logs Log Entry */
- const char *cel_uuidstr = "0da9c0b5-bf41-4b78-8f79-96b1623b3f17";
-
if (!switch_cci) {
cxl_dstate->cxl_cmd_set = cxl_cmd_set;
} else {
cxl_dstate->cxl_cmd_set = cxl_cmd_set_sw;
}
+
for (int set = 0; set < 256; set++) {
for (int cmd = 0; cmd < 256; cmd++) {
if (cxl_dstate->cxl_cmd_set[set][cmd].handler) {
@@ -707,6 +709,4 @@ int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
}
}
}
-
- return qemu_uuid_parse(cel_uuidstr, &cel_uuid);
}
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 1b366b739c62..3be2e37b3e4c 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -238,7 +238,7 @@ CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MEMORY_DEVICE,
CXL_DEVICE_CAP_HDR1_OFFSET +
CXL_DEVICE_CAP_REG_SIZE * 2)
-int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci);
+void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci);
void cxl_process_mailbox(CXLDeviceState *cxl_dstate);
#define cxl_device_cap_init(dstate, reg, cap_id) \
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 4/8] hw/cxl/events: Add event status register
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
` (2 preceding siblings ...)
2022-12-22 4:24 ` [PATCH v2 3/8] hw/cxl/mailbox: Use new UUID network order define for cel_uuid Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2023-01-03 16:36 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands Ira Weiny
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
The device status register block was defined. However, there were no
individual registers nor any data wired up.
Define the event status register [CXL 3.0; 8.2.8.3.1] as part of the
device status register block. Wire up the register and initialize the
event status for each log.
To support CXL 3.0 the version of the device status register block needs
to be 2. Change the macro to allow for setting the version.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes from RFC:
New patch to cover this register which was not being used
before.
---
hw/cxl/cxl-device-utils.c | 50 +++++++++++++++++++++++++++++++++++++--------
include/hw/cxl/cxl_device.h | 23 ++++++++++++++++++---
include/hw/cxl/cxl_events.h | 28 +++++++++++++++++++++++++
3 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index 34697064714e..7f29d40be04a 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -41,7 +41,20 @@ static uint64_t caps_reg_read(void *opaque, hwaddr offset, unsigned size)
static uint64_t dev_reg_read(void *opaque, hwaddr offset, unsigned size)
{
- return 0;
+ CXLDeviceState *cxl_dstate = opaque;
+
+ switch (size) {
+ case 1:
+ return cxl_dstate->dev_reg_state[offset];
+ case 2:
+ return cxl_dstate->dev_reg_state16[offset / size];
+ case 4:
+ return cxl_dstate->dev_reg_state32[offset / size];
+ case 8:
+ return cxl_dstate->dev_reg_state64[offset / size];
+ default:
+ g_assert_not_reached();
+ }
}
static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
@@ -236,7 +249,28 @@ void cxl_device_register_block_init(Object *obj, CXLDeviceState *cxl_dstate)
&cxl_dstate->memory_device);
}
-static void device_reg_init_common(CXLDeviceState *cxl_dstate) { }
+void cxl_event_set_status(CXLDeviceState *cxl_dstate,
+ enum cxl_event_log_type log_type,
+ bool available)
+{
+ if (available) {
+ cxl_dstate->event_status |= (1 << log_type);
+ } else {
+ cxl_dstate->event_status &= ~(1 << log_type);
+ }
+
+ ARRAY_FIELD_DP64(cxl_dstate->dev_reg_state64, CXL_DEV_EVENT_STATUS,
+ EVENT_STATUS, cxl_dstate->event_status);
+}
+
+static void device_reg_init_common(CXLDeviceState *cxl_dstate)
+{
+ enum cxl_event_log_type log;
+
+ for (log = 0; log < CXL_EVENT_TYPE_MAX; log++) {
+ cxl_event_set_status(cxl_dstate, log, false);
+ }
+}
static void mailbox_reg_init_common(CXLDeviceState *cxl_dstate)
{
@@ -258,13 +292,13 @@ void cxl_device_register_init_common(CXLDeviceState *cxl_dstate)
ARRAY_FIELD_DP64(cap_hdrs, CXL_DEV_CAP_ARRAY, CAP_VERSION, 1);
ARRAY_FIELD_DP64(cap_hdrs, CXL_DEV_CAP_ARRAY, CAP_COUNT, cap_count);
- cxl_device_cap_init(cxl_dstate, DEVICE_STATUS, 1);
+ cxl_device_cap_init(cxl_dstate, DEVICE_STATUS, 1, 2);
device_reg_init_common(cxl_dstate);
- cxl_device_cap_init(cxl_dstate, MAILBOX, 2);
+ cxl_device_cap_init(cxl_dstate, MAILBOX, 2, 1);
mailbox_reg_init_common(cxl_dstate);
- cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000);
+ cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000, 1);
memdev_reg_init_common(cxl_dstate);
cxl_initialize_mailbox(cxl_dstate, false);
@@ -280,13 +314,13 @@ void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate)
ARRAY_FIELD_DP64(cap_hdrs, CXL_DEV_CAP_ARRAY, CAP_VERSION, 1);
ARRAY_FIELD_DP64(cap_hdrs, CXL_DEV_CAP_ARRAY, CAP_COUNT, cap_count);
- cxl_device_cap_init(cxl_dstate, DEVICE_STATUS, 1);
+ cxl_device_cap_init(cxl_dstate, DEVICE_STATUS, 1, 2);
device_reg_init_common(cxl_dstate);
- cxl_device_cap_init(cxl_dstate, MAILBOX, 2);
+ cxl_device_cap_init(cxl_dstate, MAILBOX, 2, 1);
mailbox_reg_init_common(cxl_dstate);
- cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000);
+ cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000, 1);
memdev_reg_init_common(cxl_dstate);
cxl_initialize_mailbox(cxl_dstate, true);
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 3be2e37b3e4c..7180fc225e29 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -147,7 +147,16 @@ typedef struct cxl_device_state {
MemoryRegion cpmu_registers[CXL_NUM_CPMU_INSTANCES];
/* mmio for device capabilities array - 8.2.8.2 */
- MemoryRegion device;
+ struct {
+ MemoryRegion device;
+ union {
+ uint8_t dev_reg_state[CXL_DEVICE_STATUS_REGISTERS_LENGTH];
+ uint16_t dev_reg_state16[CXL_DEVICE_STATUS_REGISTERS_LENGTH / 2];
+ uint32_t dev_reg_state32[CXL_DEVICE_STATUS_REGISTERS_LENGTH / 4];
+ uint64_t dev_reg_state64[CXL_DEVICE_STATUS_REGISTERS_LENGTH / 8];
+ };
+ uint64_t event_status;
+ };
MemoryRegion memory_device;
struct {
MemoryRegion caps;
@@ -197,6 +206,10 @@ void cxl_device_register_block_init(Object *obj, CXLDeviceState *dev);
void cxl_device_register_init_common(CXLDeviceState *dev);
void cxl_device_register_init_swcci(CXLDeviceState *dev);
+void cxl_event_set_status(CXLDeviceState *cxl_dstate,
+ enum cxl_event_log_type log_type,
+ bool available);
+
/*
* CXL 2.0 - 8.2.8.1 including errata F4
* Documented as a 128 bit register, but 64 bit accesses and the second
@@ -241,7 +254,7 @@ CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MEMORY_DEVICE,
void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci);
void cxl_process_mailbox(CXLDeviceState *cxl_dstate);
-#define cxl_device_cap_init(dstate, reg, cap_id) \
+#define cxl_device_cap_init(dstate, reg, cap_id, ver) \
do { \
uint32_t *cap_hdrs = dstate->caps_reg_state32; \
int which = R_CXL_DEV_##reg##_CAP_HDR0; \
@@ -249,7 +262,7 @@ void cxl_process_mailbox(CXLDeviceState *cxl_dstate);
FIELD_DP32(cap_hdrs[which], CXL_DEV_##reg##_CAP_HDR0, \
CAP_ID, cap_id); \
cap_hdrs[which] = FIELD_DP32( \
- cap_hdrs[which], CXL_DEV_##reg##_CAP_HDR0, CAP_VERSION, 1); \
+ cap_hdrs[which], CXL_DEV_##reg##_CAP_HDR0, CAP_VERSION, ver); \
cap_hdrs[which + 1] = \
FIELD_DP32(cap_hdrs[which + 1], CXL_DEV_##reg##_CAP_HDR1, \
CAP_OFFSET, CXL_##reg##_REGISTERS_OFFSET); \
@@ -258,6 +271,10 @@ void cxl_process_mailbox(CXLDeviceState *cxl_dstate);
CAP_LENGTH, CXL_##reg##_REGISTERS_LENGTH); \
} while (0)
+/* CXL 3.0 8.2.8.3.1 Event Status Register */
+REG64(CXL_DEV_EVENT_STATUS, 0)
+ FIELD(CXL_DEV_EVENT_STATUS, EVENT_STATUS, 0, 32)
+
/* CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register */
REG32(CXL_DEV_MAILBOX_CAP, 0)
FIELD(CXL_DEV_MAILBOX_CAP, PAYLOAD_SIZE, 0, 5)
diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
new file mode 100644
index 000000000000..7e0647ffb0e3
--- /dev/null
+++ b/include/hw/cxl/cxl_events.h
@@ -0,0 +1,28 @@
+/*
+ * QEMU CXL Events
+ *
+ * Copyright (c) 2022 Intel
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef CXL_EVENTS_H
+#define CXL_EVENTS_H
+
+/*
+ * CXL rev 3.0 section 8.2.9.2.2; Table 8-49
+ *
+ * Define these as the bit position for the event status register for ease of
+ * setting the status.
+ */
+enum cxl_event_log_type {
+ CXL_EVENT_TYPE_INFO = 0,
+ CXL_EVENT_TYPE_WARN = 1,
+ CXL_EVENT_TYPE_FAIL = 2,
+ CXL_EVENT_TYPE_FATAL = 3,
+ CXL_EVENT_TYPE_DYNAMIC_CAP = 4,
+ CXL_EVENT_TYPE_MAX
+};
+
+#endif /* CXL_EVENTS_H */
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
` (3 preceding siblings ...)
2022-12-22 4:24 ` [PATCH v2 4/8] hw/cxl/events: Add event status register Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2023-01-04 18:07 ` Jonathan Cameron via
2023-01-24 17:04 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 6/8] hw/cxl/events: Add event interrupt support Ira Weiny
` (2 subsequent siblings)
7 siblings, 2 replies; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
CXL testing is benefited from an artificial event log injection
mechanism.
Add an event log infrastructure to insert, get, and clear events from
the various logs available on a device.
Replace the stubbed out CXL Get/Clear Event mailbox commands with
commands that operate on the new infrastructure.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Change from RFC:
Process multiple records per Get/Set per the spec
Rework all the calls to be within events.c
Add locking around the event logs to ensure that the log
integrity is maintained
---
hw/cxl/cxl-events.c | 221 ++++++++++++++++++++++++++++++++++++++++++++
hw/cxl/cxl-mailbox-utils.c | 40 +++++++-
hw/cxl/meson.build | 1 +
hw/mem/cxl_type3.c | 1 +
include/hw/cxl/cxl_device.h | 28 ++++++
include/hw/cxl/cxl_events.h | 55 +++++++++++
6 files changed, 344 insertions(+), 2 deletions(-)
diff --git a/hw/cxl/cxl-events.c b/hw/cxl/cxl-events.c
new file mode 100644
index 000000000000..f40c9372704e
--- /dev/null
+++ b/hw/cxl/cxl-events.c
@@ -0,0 +1,221 @@
+/*
+ * CXL Event processing
+ *
+ * Copyright(C) 2022 Intel Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See the
+ * COPYING file in the top-level directory.
+ */
+
+#include <stdint.h>
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "qemu/typedefs.h"
+#include "qemu/error-report.h"
+#include "hw/cxl/cxl.h"
+#include "hw/cxl/cxl_events.h"
+
+/* Artificial limit on the number of events a log can hold */
+#define CXL_TEST_EVENT_OVERFLOW 8
+
+static void reset_overflow(struct cxl_event_log *log)
+{
+ log->overflow_err_count = 0;
+ log->first_overflow_timestamp = 0;
+ log->last_overflow_timestamp = 0;
+}
+
+void cxl_event_init(CXLDeviceState *cxlds)
+{
+ struct cxl_event_log *log;
+ int i;
+
+ for (i = 0; i < CXL_EVENT_TYPE_MAX; i++) {
+ log = &cxlds->event_logs[i];
+ log->next_handle = 1;
+ log->overflow_err_count = 0;
+ log->first_overflow_timestamp = 0;
+ log->last_overflow_timestamp = 0;
+ qemu_mutex_init(&log->lock);
+ QSIMPLEQ_INIT(&log->events);
+ }
+}
+
+static CXLEvent *cxl_event_get_head(struct cxl_event_log *log)
+{
+ return QSIMPLEQ_FIRST(&log->events);
+}
+
+static CXLEvent *cxl_event_get_next(CXLEvent *entry)
+{
+ return QSIMPLEQ_NEXT(entry, node);
+}
+
+static int cxl_event_count(struct cxl_event_log *log)
+{
+ CXLEvent *event;
+ int rc = 0;
+
+ QSIMPLEQ_FOREACH(event, &log->events, node) {
+ rc++;
+ }
+
+ return rc;
+}
+
+static bool cxl_event_empty(struct cxl_event_log *log)
+{
+ return QSIMPLEQ_EMPTY(&log->events);
+}
+
+static void cxl_event_delete_head(CXLDeviceState *cxlds,
+ enum cxl_event_log_type log_type,
+ struct cxl_event_log *log)
+{
+ CXLEvent *entry = cxl_event_get_head(log);
+
+ reset_overflow(log);
+ QSIMPLEQ_REMOVE_HEAD(&log->events, node);
+ if (cxl_event_empty(log)) {
+ cxl_event_set_status(cxlds, log_type, false);
+ }
+ g_free(entry);
+}
+
+/*
+ * return if an interrupt should be generated as a result of inserting this
+ * event.
+ */
+bool cxl_event_insert(CXLDeviceState *cxlds,
+ enum cxl_event_log_type log_type,
+ struct cxl_event_record_raw *event)
+{
+ uint64_t time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ struct cxl_event_log *log;
+ CXLEvent *entry;
+
+ if (log_type >= CXL_EVENT_TYPE_MAX) {
+ return false;
+ }
+
+ log = &cxlds->event_logs[log_type];
+
+ QEMU_LOCK_GUARD(&log->lock);
+
+ if (cxl_event_count(log) >= CXL_TEST_EVENT_OVERFLOW) {
+ if (log->overflow_err_count == 0) {
+ log->first_overflow_timestamp = time;
+ }
+ log->overflow_err_count++;
+ log->last_overflow_timestamp = time;
+ return false;
+ }
+
+ entry = g_new0(CXLEvent, 1);
+ if (!entry) {
+ error_report("Failed to allocate memory for event log entry");
+ return false;
+ }
+
+ memcpy(&entry->data, event, sizeof(*event));
+
+ entry->data.hdr.handle = cpu_to_le16(log->next_handle);
+ log->next_handle++;
+ /* 0 handle is never valid */
+ if (log->next_handle == 0) {
+ log->next_handle++;
+ }
+ entry->data.hdr.timestamp = cpu_to_le64(time);
+
+ QSIMPLEQ_INSERT_TAIL(&log->events, entry, node);
+ cxl_event_set_status(cxlds, log_type, true);
+
+ /* Count went from 0 to 1 */
+ return cxl_event_count(log) == 1;
+}
+
+ret_code cxl_event_get_records(CXLDeviceState *cxlds,
+ struct cxl_get_event_payload *pl,
+ uint8_t log_type, int max_recs,
+ uint16_t *len)
+{
+ struct cxl_event_log *log;
+ CXLEvent *entry;
+ uint16_t nr;
+
+ if (log_type >= CXL_EVENT_TYPE_MAX) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+
+ log = &cxlds->event_logs[log_type];
+
+ QEMU_LOCK_GUARD(&log->lock);
+
+ entry = cxl_event_get_head(log);
+ for (nr = 0; entry && nr < max_recs; nr++) {
+ memcpy(&pl->records[nr], &entry->data, CXL_EVENT_RECORD_SIZE);
+ entry = cxl_event_get_next(entry);
+ }
+
+ if (!cxl_event_empty(log)) {
+ pl->flags |= CXL_GET_EVENT_FLAG_MORE_RECORDS;
+ }
+
+ if (log->overflow_err_count) {
+ pl->flags |= CXL_GET_EVENT_FLAG_OVERFLOW;
+ pl->overflow_err_count = cpu_to_le16(log->overflow_err_count);
+ pl->first_overflow_timestamp = cpu_to_le64(log->first_overflow_timestamp);
+ pl->last_overflow_timestamp = cpu_to_le64(log->last_overflow_timestamp);
+ }
+
+ pl->record_count = cpu_to_le16(nr);
+ *len = CXL_EVENT_PAYLOAD_HDR_SIZE + (CXL_EVENT_RECORD_SIZE * nr);
+ return CXL_MBOX_SUCCESS;
+}
+
+ret_code cxl_event_clear_records(CXLDeviceState *cxlds,
+ struct cxl_clear_event_payload *pl)
+{
+ struct cxl_event_log *log;
+ uint8_t log_type;
+ CXLEvent *entry;
+ int nr;
+
+ log_type = pl->event_log;
+
+ if (log_type >= CXL_EVENT_TYPE_MAX) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+
+ log = &cxlds->event_logs[log_type];
+
+ QEMU_LOCK_GUARD(&log->lock);
+ /*
+ * Must itterate the queue twice.
+ * "The device shall verify the event record handles specified in the input
+ * payload are in temporal order. If the device detects an older event
+ * record that will not be cleared when Clear Event Records is executed,
+ * the device shall return the Invalid Handle return code and shall not
+ * clear any of the specified event records."
+ * -- CXL 3.0 8.2.9.2.3
+ */
+ entry = cxl_event_get_head(log);
+ for (nr = 0; entry && nr < pl->nr_recs; nr++) {
+ uint16_t handle = pl->handle[nr];
+
+ /* NOTE: Both handles are little endian. */
+ if (handle == 0 || entry->data.hdr.handle != handle) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+ entry = cxl_event_get_next(entry);
+ }
+
+ entry = cxl_event_get_head(log);
+ for (nr = 0; entry && nr < pl->nr_recs; nr++) {
+ cxl_event_delete_head(cxlds, log_type, log);
+ entry = cxl_event_get_head(log);
+ }
+
+ return CXL_MBOX_SUCCESS;
+}
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 157c01255ee3..97cf6db8582d 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -9,6 +9,7 @@
#include "qemu/osdep.h"
#include "hw/cxl/cxl.h"
+#include "hw/cxl/cxl_events.h"
#include "hw/pci/pci.h"
#include "hw/pci-bridge/cxl_upstream_port.h"
#include "qemu/cutils.h"
@@ -89,8 +90,6 @@ enum {
return CXL_MBOX_SUCCESS; \
}
-DEFINE_MAILBOX_HANDLER_ZEROED(events_get_records, 0x20);
-DEFINE_MAILBOX_HANDLER_NOP(events_clear_records);
DEFINE_MAILBOX_HANDLER_ZEROED(events_get_interrupt_policy, 4);
DEFINE_MAILBOX_HANDLER_NOP(events_set_interrupt_policy);
@@ -252,6 +251,43 @@ static ret_code cmd_infostat_bg_op_sts(struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
+static ret_code cmd_events_get_records(struct cxl_cmd *cmd,
+ CXLDeviceState *cxlds,
+ uint16_t *len)
+{
+ struct cxl_get_event_payload *pl;
+ uint8_t log_type;
+ int max_recs;
+
+ if (cmd->in < sizeof(log_type)) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+
+ log_type = *((uint8_t *)cmd->payload);
+
+ pl = (struct cxl_get_event_payload *)cmd->payload;
+ memset(pl, 0, sizeof(*pl));
+
+ max_recs = (cxlds->payload_size - CXL_EVENT_PAYLOAD_HDR_SIZE) /
+ CXL_EVENT_RECORD_SIZE;
+ if (max_recs > 0xFFFF) {
+ max_recs = 0xFFFF;
+ }
+
+ return cxl_event_get_records(cxlds, pl, log_type, max_recs, len);
+}
+
+static ret_code cmd_events_clear_records(struct cxl_cmd *cmd,
+ CXLDeviceState *cxlds,
+ uint16_t *len)
+{
+ struct cxl_clear_event_payload *pl;
+
+ pl = (struct cxl_clear_event_payload *)cmd->payload;
+ *len = 0;
+ return cxl_event_clear_records(cxlds, pl);
+}
+
/* 8.2.9.2.1 */
static ret_code cmd_firmware_update_get_info(struct cxl_cmd *cmd,
CXLDeviceState *cxl_dstate,
diff --git a/hw/cxl/meson.build b/hw/cxl/meson.build
index 6e370a32fae9..053058034a53 100644
--- a/hw/cxl/meson.build
+++ b/hw/cxl/meson.build
@@ -7,6 +7,7 @@ softmmu_ss.add(when: 'CONFIG_CXL',
'cxl-cdat.c',
'cxl-cpmu.c',
'switch-mailbox-cci.c',
+ 'cxl-events.c',
),
if_false: files(
'cxl-host-stubs.c',
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 21e866dcaf52..e74ef237dfa9 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -697,6 +697,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
/* CXL RAS uses AER correct INTERNAL erorrs - so enable by default */
pci_set_long(pci_dev->config + 0x200 + PCI_ERR_COR_MASK,
PCI_ERR_COR_MASK_DEFAULT & ~PCI_ERR_COR_INTERNAL);
+ cxl_event_init(&ct3d->cxl_dstate);
return;
err_free_spdm_socket:
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 7180fc225e29..d7b43e74c05c 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -11,6 +11,7 @@
#define CXL_DEVICE_H
#include "hw/register.h"
+#include "hw/cxl/cxl_events.h"
#include "hw/cxl/cxl_cpmu.h"
/*
@@ -142,6 +143,20 @@ struct cxl_cmd {
uint8_t *payload;
};
+typedef struct CXLEvent {
+ struct cxl_event_record_raw data;
+ QSIMPLEQ_ENTRY(CXLEvent) node;
+} CXLEvent;
+
+struct cxl_event_log {
+ uint16_t next_handle;
+ uint16_t overflow_err_count;
+ uint64_t first_overflow_timestamp;
+ uint64_t last_overflow_timestamp;
+ QemuMutex lock;
+ QSIMPLEQ_HEAD(, CXLEvent) events;
+};
+
typedef struct cxl_device_state {
MemoryRegion device_registers;
@@ -197,6 +212,8 @@ typedef struct cxl_device_state {
struct cxl_cmd (*cxl_cmd_set)[256];
/* Move me later */
CPMUState cpmu[CXL_NUM_CPMU_INSTANCES];
+
+ struct cxl_event_log event_logs[CXL_EVENT_TYPE_MAX];
} CXLDeviceState;
/* Initialize the register block for a device */
@@ -381,4 +398,15 @@ struct CSWMBCCIDev {
CXLDeviceState cxl_dstate;
};
+void cxl_event_init(CXLDeviceState *cxlds);
+bool cxl_event_insert(CXLDeviceState *cxlds,
+ enum cxl_event_log_type log_type,
+ struct cxl_event_record_raw *event);
+ret_code cxl_event_get_records(CXLDeviceState *cxlds,
+ struct cxl_get_event_payload *pl,
+ uint8_t log_type, int max_recs,
+ uint16_t *len);
+ret_code cxl_event_clear_records(CXLDeviceState *cxlds,
+ struct cxl_clear_event_payload *pl);
+
#endif
diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
index 7e0647ffb0e3..1798c4502cb3 100644
--- a/include/hw/cxl/cxl_events.h
+++ b/include/hw/cxl/cxl_events.h
@@ -10,6 +10,8 @@
#ifndef CXL_EVENTS_H
#define CXL_EVENTS_H
+#include "qemu/uuid.h"
+
/*
* CXL rev 3.0 section 8.2.9.2.2; Table 8-49
*
@@ -25,4 +27,57 @@ enum cxl_event_log_type {
CXL_EVENT_TYPE_MAX
};
+/*
+ * Common Event Record Format
+ * CXL rev 3.0 section 8.2.9.2.1; Table 8-42
+ */
+#define CXL_EVENT_REC_HDR_RES_LEN 0xf
+struct cxl_event_record_hdr {
+ QemuUUID id;
+ uint8_t length;
+ uint8_t flags[3];
+ uint16_t handle;
+ uint16_t related_handle;
+ uint64_t timestamp;
+ uint8_t maint_op_class;
+ uint8_t reserved[CXL_EVENT_REC_HDR_RES_LEN];
+} QEMU_PACKED;
+
+#define CXL_EVENT_RECORD_DATA_LENGTH 0x50
+struct cxl_event_record_raw {
+ struct cxl_event_record_hdr hdr;
+ uint8_t data[CXL_EVENT_RECORD_DATA_LENGTH];
+} QEMU_PACKED;
+#define CXL_EVENT_RECORD_SIZE (sizeof(struct cxl_event_record_raw))
+
+/*
+ * Get Event Records output payload
+ * CXL rev 3.0 section 8.2.9.2.2; Table 8-50
+ */
+#define CXL_GET_EVENT_FLAG_OVERFLOW BIT(0)
+#define CXL_GET_EVENT_FLAG_MORE_RECORDS BIT(1)
+struct cxl_get_event_payload {
+ uint8_t flags;
+ uint8_t reserved1;
+ uint16_t overflow_err_count;
+ uint64_t first_overflow_timestamp;
+ uint64_t last_overflow_timestamp;
+ uint16_t record_count;
+ uint8_t reserved2[0xa];
+ struct cxl_event_record_raw records[];
+} QEMU_PACKED;
+#define CXL_EVENT_PAYLOAD_HDR_SIZE (sizeof(struct cxl_get_event_payload))
+
+/*
+ * Clear Event Records input payload
+ * CXL rev 3.0 section 8.2.9.2.3; Table 8-51
+ */
+struct cxl_clear_event_payload {
+ uint8_t event_log; /* enum cxl_event_log_type */
+ uint8_t clear_flags;
+ uint8_t nr_recs;
+ uint8_t reserved[3];
+ uint16_t handle[];
+};
+
#endif /* CXL_EVENTS_H */
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 6/8] hw/cxl/events: Add event interrupt support
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
` (4 preceding siblings ...)
2022-12-22 4:24 ` [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2022-12-22 4:24 ` [PATCH v2 7/8] bswap: Add the ability to store to an unaligned 24 bit field Ira Weiny
2022-12-22 4:24 ` [PATCH v2 8/8] hw/cxl/events: Add in inject general media event Ira Weiny
7 siblings, 0 replies; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
Replace the stubbed out CXL Get/Set Event interrupt policy mailbox
commands. Enable those commands to control interrupts for each of the
event log types.
Skip the standard input mailbox length on the Set command due to DCD
being optional. Perform the checks separately.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
NOTE As the spec changes it may be wise to change the standard mailbox
processing to allow for various input length checks. But I'm not going
try and tackle that in this series.
Changes from RFC:
Squashed mailbox and irq patches together to support event
interrupts as a whole
Remove redundant event_vector array
---
hw/cxl/cxl-events.c | 33 +++++++++++++-
hw/cxl/cxl-mailbox-utils.c | 106 +++++++++++++++++++++++++++++++++++---------
hw/mem/cxl_type3.c | 4 +-
include/hw/cxl/cxl_device.h | 5 ++-
include/hw/cxl/cxl_events.h | 23 ++++++++++
5 files changed, 146 insertions(+), 25 deletions(-)
diff --git a/hw/cxl/cxl-events.c b/hw/cxl/cxl-events.c
index f40c9372704e..53ec8447236e 100644
--- a/hw/cxl/cxl-events.c
+++ b/hw/cxl/cxl-events.c
@@ -13,6 +13,8 @@
#include "qemu/bswap.h"
#include "qemu/typedefs.h"
#include "qemu/error-report.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
#include "hw/cxl/cxl.h"
#include "hw/cxl/cxl_events.h"
@@ -26,7 +28,7 @@ static void reset_overflow(struct cxl_event_log *log)
log->last_overflow_timestamp = 0;
}
-void cxl_event_init(CXLDeviceState *cxlds)
+void cxl_event_init(CXLDeviceState *cxlds, int start_msg_num)
{
struct cxl_event_log *log;
int i;
@@ -37,9 +39,16 @@ void cxl_event_init(CXLDeviceState *cxlds)
log->overflow_err_count = 0;
log->first_overflow_timestamp = 0;
log->last_overflow_timestamp = 0;
+ log->irq_enabled = false;
+ log->irq_vec = start_msg_num++;
qemu_mutex_init(&log->lock);
QSIMPLEQ_INIT(&log->events);
}
+
+ /* Override -- Dynamic Capacity uses the same vector as info */
+ cxlds->event_logs[CXL_EVENT_TYPE_DYNAMIC_CAP].irq_vec =
+ cxlds->event_logs[CXL_EVENT_TYPE_INFO].irq_vec;
+
}
static CXLEvent *cxl_event_get_head(struct cxl_event_log *log)
@@ -219,3 +228,25 @@ ret_code cxl_event_clear_records(CXLDeviceState *cxlds,
return CXL_MBOX_SUCCESS;
}
+
+void cxl_event_irq_assert(CXLType3Dev *ct3d)
+{
+ CXLDeviceState *cxlds = &ct3d->cxl_dstate;
+ PCIDevice *pdev = &ct3d->parent_obj;
+ int i;
+
+ for (i = 0; i < CXL_EVENT_TYPE_MAX; i++) {
+ struct cxl_event_log *log = &cxlds->event_logs[i];
+
+ if (!log->irq_enabled || cxl_event_empty(log)) {
+ continue;
+ }
+
+ /* Notifies interrupt, legacy IRQ is not supported */
+ if (msix_enabled(pdev)) {
+ msix_notify(pdev, log->irq_vec);
+ } else if (msi_enabled(pdev)) {
+ msi_notify(pdev, log->irq_vec);
+ }
+ }
+}
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 97cf6db8582d..ff94191a956a 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -74,25 +74,6 @@ enum {
#define IDENTIFY_SWITCH_DEVICE 0x0
};
-#define DEFINE_MAILBOX_HANDLER_ZEROED(name, size) \
- uint16_t __zero##name = size; \
- static ret_code cmd_##name(struct cxl_cmd *cmd, \
- CXLDeviceState *cxl_dstate, uint16_t *len) \
- { \
- *len = __zero##name; \
- memset(cmd->payload, 0, *len); \
- return CXL_MBOX_SUCCESS; \
- }
-#define DEFINE_MAILBOX_HANDLER_NOP(name) \
- static ret_code cmd_##name(struct cxl_cmd *cmd, \
- CXLDeviceState *cxl_dstate, uint16_t *len) \
- { \
- return CXL_MBOX_SUCCESS; \
- }
-
-DEFINE_MAILBOX_HANDLER_ZEROED(events_get_interrupt_policy, 4);
-DEFINE_MAILBOX_HANDLER_NOP(events_set_interrupt_policy);
-
static void find_cxl_usp(PCIBus *b, PCIDevice *d, void *opaque)
{
PCIDevice **found_dev = opaque;
@@ -288,6 +269,88 @@ static ret_code cmd_events_clear_records(struct cxl_cmd *cmd,
return cxl_event_clear_records(cxlds, pl);
}
+static ret_code cmd_events_get_interrupt_policy(struct cxl_cmd *cmd,
+ CXLDeviceState *cxlds,
+ uint16_t *len)
+{
+ struct cxl_event_interrupt_policy *policy;
+ struct cxl_event_log *log;
+
+ policy = (struct cxl_event_interrupt_policy *)cmd->payload;
+ memset(policy, 0, sizeof(*policy));
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_INFO];
+ if (log->irq_enabled) {
+ policy->info_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+ }
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_WARN];
+ if (log->irq_enabled) {
+ policy->warn_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+ }
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_FAIL];
+ if (log->irq_enabled) {
+ policy->failure_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+ }
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_FATAL];
+ if (log->irq_enabled) {
+ policy->fatal_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+ }
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_DYNAMIC_CAP];
+ if (log->irq_enabled) {
+ /* Dynamic Capacity borrows the same vector as info */
+ policy->dyn_cap_settings = CXL_INT_MSI_MSIX;
+ }
+
+ *len = sizeof(*policy);
+ return CXL_MBOX_SUCCESS;
+}
+
+static ret_code cmd_events_set_interrupt_policy(struct cxl_cmd *cmd,
+ CXLDeviceState *cxlds,
+ uint16_t *len)
+{
+ struct cxl_event_interrupt_policy *policy;
+ struct cxl_event_log *log;
+
+ if (*len < CXL_EVENT_INT_SETTING_MIN_LEN) {
+ return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+ }
+
+ policy = (struct cxl_event_interrupt_policy *)cmd->payload;
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_INFO];
+ log->irq_enabled = (policy->info_settings & CXL_EVENT_INT_MODE_MASK) ==
+ CXL_INT_MSI_MSIX;
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_WARN];
+ log->irq_enabled = (policy->warn_settings & CXL_EVENT_INT_MODE_MASK) ==
+ CXL_INT_MSI_MSIX;
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_FAIL];
+ log->irq_enabled = (policy->failure_settings & CXL_EVENT_INT_MODE_MASK) ==
+ CXL_INT_MSI_MSIX;
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_FATAL];
+ log->irq_enabled = (policy->fatal_settings & CXL_EVENT_INT_MODE_MASK) ==
+ CXL_INT_MSI_MSIX;
+
+ /* DCD is optional */
+ if (*len < sizeof(*policy)) {
+ return CXL_MBOX_SUCCESS;
+ }
+
+ log = &cxlds->event_logs[CXL_EVENT_TYPE_DYNAMIC_CAP];
+ log->irq_enabled = (policy->dyn_cap_settings & CXL_EVENT_INT_MODE_MASK) ==
+ CXL_INT_MSI_MSIX;
+
+ *len = sizeof(*policy);
+ return CXL_MBOX_SUCCESS;
+}
+
/* 8.2.9.2.1 */
static ret_code cmd_firmware_update_get_info(struct cxl_cmd *cmd,
CXLDeviceState *cxl_dstate,
@@ -644,9 +707,10 @@ static struct cxl_cmd cxl_cmd_set[256][256] = {
[EVENTS][CLEAR_RECORDS] = { "EVENTS_CLEAR_RECORDS",
cmd_events_clear_records, ~0, IMMEDIATE_LOG_CHANGE },
[EVENTS][GET_INTERRUPT_POLICY] = { "EVENTS_GET_INTERRUPT_POLICY",
- cmd_events_get_interrupt_policy, 0, 0 },
+ cmd_events_get_interrupt_policy, 0, 0 },
[EVENTS][SET_INTERRUPT_POLICY] = { "EVENTS_SET_INTERRUPT_POLICY",
- cmd_events_set_interrupt_policy, 4, IMMEDIATE_CONFIG_CHANGE },
+ cmd_events_set_interrupt_policy,
+ ~0, IMMEDIATE_CONFIG_CHANGE },
[FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
cmd_firmware_update_get_info, 0, 0 },
[TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index e74ef237dfa9..a43949cab120 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -626,7 +626,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
ComponentRegisters *regs = &cxl_cstate->crb;
MemoryRegion *mr = ®s->component_registers;
uint8_t *pci_conf = pci_dev->config;
- unsigned short msix_num = 4;
+ unsigned short msix_num = 8;
int i, rc;
if (!cxl_setup_memory(ct3d, errp)) {
@@ -697,7 +697,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
/* CXL RAS uses AER correct INTERNAL erorrs - so enable by default */
pci_set_long(pci_dev->config + 0x200 + PCI_ERR_COR_MASK,
PCI_ERR_COR_MASK_DEFAULT & ~PCI_ERR_COR_INTERNAL);
- cxl_event_init(&ct3d->cxl_dstate);
+ cxl_event_init(&ct3d->cxl_dstate, 4);
return;
err_free_spdm_socket:
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index d7b43e74c05c..586377607c57 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -153,6 +153,8 @@ struct cxl_event_log {
uint16_t overflow_err_count;
uint64_t first_overflow_timestamp;
uint64_t last_overflow_timestamp;
+ bool irq_enabled;
+ int irq_vec;
QemuMutex lock;
QSIMPLEQ_HEAD(, CXLEvent) events;
};
@@ -398,7 +400,7 @@ struct CSWMBCCIDev {
CXLDeviceState cxl_dstate;
};
-void cxl_event_init(CXLDeviceState *cxlds);
+void cxl_event_init(CXLDeviceState *cxlds, int start_msg_num);
bool cxl_event_insert(CXLDeviceState *cxlds,
enum cxl_event_log_type log_type,
struct cxl_event_record_raw *event);
@@ -408,5 +410,6 @@ ret_code cxl_event_get_records(CXLDeviceState *cxlds,
uint16_t *len);
ret_code cxl_event_clear_records(CXLDeviceState *cxlds,
struct cxl_clear_event_payload *pl);
+void cxl_event_irq_assert(CXLType3Dev *ct3d);
#endif
diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
index 1798c4502cb3..2df40720320a 100644
--- a/include/hw/cxl/cxl_events.h
+++ b/include/hw/cxl/cxl_events.h
@@ -80,4 +80,27 @@ struct cxl_clear_event_payload {
uint16_t handle[];
};
+/**
+ * Event Interrupt Policy
+ *
+ * CXL rev 3.0 section 8.2.9.2.4; Table 8-52
+ */
+enum cxl_event_int_mode {
+ CXL_INT_NONE = 0x00,
+ CXL_INT_MSI_MSIX = 0x01,
+ CXL_INT_FW = 0x02,
+ CXL_INT_RES = 0x03,
+};
+#define CXL_EVENT_INT_MODE_MASK 0x3
+#define CXL_EVENT_INT_SETTING(vector) ((((uint8_t)vector & 0xf) << 4) | CXL_INT_MSI_MSIX)
+struct cxl_event_interrupt_policy {
+ uint8_t info_settings;
+ uint8_t warn_settings;
+ uint8_t failure_settings;
+ uint8_t fatal_settings;
+ uint8_t dyn_cap_settings;
+} QEMU_PACKED;
+/* DCD is optional but other fields are not */
+#define CXL_EVENT_INT_SETTING_MIN_LEN 4
+
#endif /* CXL_EVENTS_H */
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 7/8] bswap: Add the ability to store to an unaligned 24 bit field
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
` (5 preceding siblings ...)
2022-12-22 4:24 ` [PATCH v2 6/8] hw/cxl/events: Add event interrupt support Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2023-01-03 17:14 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 8/8] hw/cxl/events: Add in inject general media event Ira Weiny
7 siblings, 1 reply; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
CXL has 24 bit unaligned fields which need to be stored to. CXL is
specified as little endian.
Define st24_le_p() and the supporting functions to store such a field
from a 32 bit host native value.
The use of b, w, l, q as the size specifier is limiting. So "24" was
used for the size part of the function name.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
include/qemu/bswap.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
index e1eca22f2548..8af4d4a75eb6 100644
--- a/include/qemu/bswap.h
+++ b/include/qemu/bswap.h
@@ -25,6 +25,13 @@ static inline uint16_t bswap16(uint16_t x)
return bswap_16(x);
}
+static inline uint32_t bswap24(uint32_t x)
+{
+ return (((x & 0x000000ffU) << 16) |
+ ((x & 0x0000ff00U) << 0) |
+ ((x & 0x00ff0000U) >> 16));
+}
+
static inline uint32_t bswap32(uint32_t x)
{
return bswap_32(x);
@@ -43,6 +50,13 @@ static inline uint16_t bswap16(uint16_t x)
((x & 0xff00) >> 8));
}
+static inline uint32_t bswap24(uint32_t x)
+{
+ return (((x & 0x000000ffU) << 16) |
+ ((x & 0x0000ff00U) << 0) |
+ ((x & 0x00ff0000U) >> 16));
+}
+
static inline uint32_t bswap32(uint32_t x)
{
return (((x & 0x000000ffU) << 24) |
@@ -72,6 +86,11 @@ static inline void bswap16s(uint16_t *s)
*s = bswap16(*s);
}
+static inline void bswap24s(uint32_t *s)
+{
+ *s = bswap24(*s);
+}
+
static inline void bswap32s(uint32_t *s)
{
*s = bswap32(*s);
@@ -233,6 +252,7 @@ CPU_CONVERT(le, 64, uint64_t)
* size is:
* b: 8 bits
* w: 16 bits
+ * 24: 24 bits
* l: 32 bits
* q: 64 bits
*
@@ -305,6 +325,11 @@ static inline void stw_he_p(void *ptr, uint16_t v)
__builtin_memcpy(ptr, &v, sizeof(v));
}
+static inline void st24_he_p(void *ptr, uint32_t v)
+{
+ __builtin_memcpy(ptr, &v, 3);
+}
+
static inline int ldl_he_p(const void *ptr)
{
int32_t r;
@@ -354,6 +379,11 @@ static inline void stw_le_p(void *ptr, uint16_t v)
stw_he_p(ptr, le_bswap(v, 16));
}
+static inline void st24_le_p(void *ptr, uint32_t v)
+{
+ st24_he_p(ptr, le_bswap(v, 24));
+}
+
static inline void stl_le_p(void *ptr, uint32_t v)
{
stl_he_p(ptr, le_bswap(v, 32));
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 8/8] hw/cxl/events: Add in inject general media event
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
` (6 preceding siblings ...)
2022-12-22 4:24 ` [PATCH v2 7/8] bswap: Add the ability to store to an unaligned 24 bit field Ira Weiny
@ 2022-12-22 4:24 ` Ira Weiny
2023-01-03 18:07 ` Jonathan Cameron via
2023-01-12 15:34 ` Jonathan Cameron via
7 siblings, 2 replies; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
To facilitate testing provide a QMP command to inject a general media
event. The event can be added to the log specified.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes from RFC:
Add all fields for this event
irq happens automatically when log transitions from 0 to 1
---
hw/mem/cxl_type3.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
hw/mem/cxl_type3_stubs.c | 8 ++++
include/hw/cxl/cxl_events.h | 20 ++++++++++
qapi/cxl.json | 25 ++++++++++++
4 files changed, 146 insertions(+)
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index a43949cab120..bedd09e500ba 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -916,6 +916,99 @@ static CXLPoisonList *get_poison_list(CXLType3Dev *ct3d)
return &ct3d->poison_list;
}
+static void cxl_assign_event_header(struct cxl_event_record_hdr *hdr,
+ QemuUUID *uuid, uint8_t flags,
+ uint8_t length)
+{
+ hdr->flags[0] = flags;
+ hdr->length = length;
+ memcpy(&hdr->id, uuid, sizeof(hdr->id));
+ hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+}
+
+QemuUUID gen_media_uuid = {
+ .data = UUID(0xfbcd0a77, 0xc260, 0x417f,
+ 0x85, 0xa9, 0x08, 0x8b, 0x16, 0x21, 0xeb, 0xa6),
+};
+
+#define CXL_GMER_VALID_CHANNEL BIT(0)
+#define CXL_GMER_VALID_RANK BIT(1)
+#define CXL_GMER_VALID_DEVICE BIT(2)
+#define CXL_GMER_VALID_COMPONENT BIT(3)
+
+/*
+ * For channel, rank, and device; any value inside of the fields valid range
+ * will flag that field to be valid. IE pass -1 to mark the field invalid.
+ *
+ * Component ID is device specific. Define this as a string.
+ */
+void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
+ uint8_t flags, uint64_t physaddr,
+ uint8_t descriptor, uint8_t type,
+ uint8_t transaction_type,
+ int16_t channel, int16_t rank,
+ int32_t device,
+ const char *component_id,
+ Error **errp)
+{
+ Object *obj = object_resolve_path(path, NULL);
+ struct cxl_event_gen_media gem;
+ struct cxl_event_record_hdr *hdr = &gem.hdr;
+ CXLDeviceState *cxlds;
+ CXLType3Dev *ct3d;
+ uint16_t valid_flags = 0;
+
+ if (log >= CXL_EVENT_TYPE_MAX) {
+ error_setg(errp, "Invalid log type: %d", log);
+ return;
+ }
+ if (!obj) {
+ error_setg(errp, "Unable to resolve path");
+ return;
+ }
+ if (!object_dynamic_cast(obj, TYPE_CXL_TYPE3)) {
+ error_setg(errp, "Path does not point to a CXL type 3 device");
+ }
+ ct3d = CXL_TYPE3(obj);
+ cxlds = &ct3d->cxl_dstate;
+
+ memset(&gem, 0, sizeof(gem));
+ cxl_assign_event_header(hdr, &gen_media_uuid, flags,
+ sizeof(struct cxl_event_gen_media));
+
+ gem.phys_addr = physaddr;
+ gem.descriptor = descriptor;
+ gem.type = type;
+ gem.transaction_type = transaction_type;
+
+ if (0 <= channel && channel <= 0xFF) {
+ gem.channel = channel;
+ valid_flags |= CXL_GMER_VALID_CHANNEL;
+ }
+
+ if (0 <= rank && rank <= 0xFF) {
+ gem.rank = rank;
+ valid_flags |= CXL_GMER_VALID_RANK;
+ }
+
+ if (0 <= device && device <= 0xFFFFFF) {
+ st24_le_p(gem.device, device);
+ valid_flags |= CXL_GMER_VALID_DEVICE;
+ }
+
+ if (component_id && strcmp(component_id, "")) {
+ strncpy((char *)gem.component_id, component_id,
+ sizeof(gem.component_id) - 1);
+ valid_flags |= CXL_GMER_VALID_COMPONENT;
+ }
+
+ stw_le_p(gem.validity_flags, valid_flags);
+
+ if (cxl_event_insert(cxlds, log, (struct cxl_event_record_raw *)&gem)) {
+ cxl_event_irq_assert(ct3d);
+ }
+}
+
void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
Error **errp)
{
diff --git a/hw/mem/cxl_type3_stubs.c b/hw/mem/cxl_type3_stubs.c
index f2c9f48f4010..62f04d487031 100644
--- a/hw/mem/cxl_type3_stubs.c
+++ b/hw/mem/cxl_type3_stubs.c
@@ -2,6 +2,14 @@
#include "qemu/osdep.h"
#include "qapi/qapi-commands-cxl.h"
+void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
+ uint8_t flags, uint64_t physaddr,
+ uint8_t descriptor, uint8_t type,
+ uint8_t transaction_type,
+ int16_t channel, int16_t rank,
+ int32_t device,
+ char *component_id,
+ Error **errp) {}
void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
Error **errp) {}
void qmp_cxl_inject_uncorrectable_error(const char *path,
diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
index 2df40720320a..3175e9d9866d 100644
--- a/include/hw/cxl/cxl_events.h
+++ b/include/hw/cxl/cxl_events.h
@@ -103,4 +103,24 @@ struct cxl_event_interrupt_policy {
/* DCD is optional but other fields are not */
#define CXL_EVENT_INT_SETTING_MIN_LEN 4
+/*
+ * General Media Event Record
+ * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43
+ */
+#define CXL_EVENT_GEN_MED_COMP_ID_SIZE 0x10
+#define CXL_EVENT_GEN_MED_RES_SIZE 0x2e
+struct cxl_event_gen_media {
+ struct cxl_event_record_hdr hdr;
+ uint64_t phys_addr;
+ uint8_t descriptor;
+ uint8_t type;
+ uint8_t transaction_type;
+ uint8_t validity_flags[2];
+ uint8_t channel;
+ uint8_t rank;
+ uint8_t device[3];
+ uint8_t component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
+ uint8_t reserved[CXL_EVENT_GEN_MED_RES_SIZE];
+} QEMU_PACKED;
+
#endif /* CXL_EVENTS_H */
diff --git a/qapi/cxl.json b/qapi/cxl.json
index b4836bb87f53..56e85a28d7e0 100644
--- a/qapi/cxl.json
+++ b/qapi/cxl.json
@@ -5,6 +5,31 @@
# = CXL devices
##
+##
+# @cxl-inject-gen-media-event:
+#
+# @path: CXL type 3 device canonical QOM path
+#
+# @log: Event Log to add the event to
+# @flags: header flags
+# @physaddr: Physical Address
+# @descriptor: Descriptor
+# @type: Type
+# @transactiontype: Transaction Type
+# @channel: Channel
+# @rank: Rank
+# @device: Device
+# @componentid: Device specific string
+#
+##
+{ 'command': 'cxl-inject-gen-media-event',
+ 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
+ 'physaddr': 'uint64', 'descriptor': 'uint8',
+ 'type': 'uint8', 'transactiontype': 'uint8',
+ 'channel': 'int16', 'rank': 'int16',
+ 'device': 'int32', 'componentid': 'str'
+ }}
+
##
# @cxl-inject-poison:
#
--
2.38.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support
@ 2022-12-22 4:24 Ira Weiny
2022-12-22 4:24 ` [PATCH v2 1/8] qemu/bswap: Add const_le64() Ira Weiny
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Ira Weiny @ 2022-12-22 4:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, Ira Weiny, qemu-devel, linux-cxl,
Peter Maydell
CXL Event records inform the OS of various CXL device events. Thus far CXL
memory devices are emulated and therefore don't naturally generate events.
Add an event infrastructure and mock event injection. Previous versions
included a bulk insertion of lots of events. However, this series focuses on
providing the ability to inject individual events through QMP. Only the
General Media Event is included in this series as an example. Other events can
be added pretty easily once the infrastructure is acceptable.
In addition, this version updates the code to be in line with the
specification based on discussions around the kernel patches.
This series is based on Jonathan's CXL branch here:
https://gitlab.com/jic23/qemu/-/tree/cxl-2022-11-17
Kernel code found here:
https://lore.kernel.org/all/20221212070627.1372402-1-ira.weiny@intel.com/
Previous RFC (V1) version:
https://lore.kernel.org/linux-cxl/20221010222944.3923556-1-ira.weiny@intel.com/
Instructions:
Add qmp option to qemu:
<host> $ qemu-system-x86_64 ... -qmp unix:/tmp/run_qemu_qmp_0,server,nowait ...
OR
<host> $ run_qemu.sh ... --qmp ...
Enable tracing of events within the guest:
<guest> $ echo "" > /sys/kernel/tracing/trace
<guest> $ echo 1 > /sys/kernel/tracing/events/cxl/enable
<guest> $ echo 1 > /sys/kernel/tracing/tracing_on
OPTIONAL: set up ndctl to monitor for events (events will show up as
they are injected)
<guest> $ <path to ndctl>/cxl monitor
Trigger event generation and interrupts in the host:
<host> $ qmpcmd="${qemusrcdir}/build/scripts/qmp/qmp-shell /tmp/run_qemu_qmp_0"
<host> $ echo "cxl-inject-gen-media-event path=cxl-dev0 log=0 flags=1 \
physaddr=1000 descriptor=127 type=3 transactiontype=192 \
channel=3 rank=-1 device=5 componentid='Iras mem'" | $qmpcmd
<Repeat for more events with different values>
View events on the guest:
<guest> $ cat /sys/kernel/tracing/trace
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Michael Tsirkin <mst@redhat.com>
Cc: Ben Widawsky <bwidawsk@kernel.org>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: qemu-devel@nongnu.org
Cc: linux-cxl@vger.kernel.org
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Ira Weiny (8):
qemu/bswap: Add const_le64()
qemu/uuid: Add UUID static initializer
hw/cxl/mailbox: Use new UUID network order define for cel_uuid
hw/cxl/events: Add event status register
hw/cxl/events: Wire up get/clear event mailbox commands
hw/cxl/events: Add event interrupt support
bswap: Add the ability to store to an unaligned 24 bit field
hw/cxl/events: Add in inject general media event
hw/cxl/cxl-device-utils.c | 54 ++++++++--
hw/cxl/cxl-events.c | 252 ++++++++++++++++++++++++++++++++++++++++++++
hw/cxl/cxl-mailbox-utils.c | 160 ++++++++++++++++++++++------
hw/cxl/meson.build | 1 +
hw/mem/cxl_type3.c | 96 ++++++++++++++++-
hw/mem/cxl_type3_stubs.c | 8 ++
include/hw/cxl/cxl_device.h | 56 +++++++++-
include/hw/cxl/cxl_events.h | 126 ++++++++++++++++++++++
include/qemu/bswap.h | 40 +++++++
include/qemu/uuid.h | 12 +++
qapi/cxl.json | 25 +++++
11 files changed, 785 insertions(+), 45 deletions(-)
---
base-commit: 1b4133103d20fc3fea05c7ceca4a242468a5179d
change-id: 20221221-ira-cxl-events-2022-11-17-fef53f9b9ac2
Best regards,
--
Ira Weiny <ira.weiny@intel.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/8] hw/cxl/mailbox: Use new UUID network order define for cel_uuid
2022-12-22 4:24 ` [PATCH v2 3/8] hw/cxl/mailbox: Use new UUID network order define for cel_uuid Ira Weiny
@ 2023-01-03 16:30 ` Jonathan Cameron via
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-03 16:30 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Wed, 21 Dec 2022 20:24:33 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> The cel_uuid was programatically generated previously because there was
> no static initializer for network order UUIDs.
>
> Use the new network order initializer for cel_uuid. Adjust
> cxl_initialize_mailbox() because it can't fail now.
>
> Update specification reference.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
I clearly need a 'pending' branch or tag with just the sane / nearly
ready to send upstream stuff on it. The switch-cci stuff definitely
isn't in that category yet and made this more complex than it needs
to be.
Anyhow, I'll rebase the series to a sensible point in my tree and
push out a new branch for you to sanity check.
This patch and the two before it make sense even without the rest of
the series so I might pull them forwards into a cleanup series I intend
to send out later this week - we'll see how things go.
Assuming any changes to the rest of this series are minor, my thoughts
are we do the following this cycle (and see how far we get)
1) Cleanup series.
2) RAS event series - AER etc.
3) This series
4) Poison series
5) Volatile memory series from Gregory
Might slot the CPMU and ARM64 support in there somewhere, but after this series.
>
> ---
> Changes from RFC:
> New patch.
> ---
> hw/cxl/cxl-device-utils.c | 4 ++--
> hw/cxl/cxl-mailbox-utils.c | 14 +++++++-------
> include/hw/cxl/cxl_device.h | 2 +-
> 3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index 21845dbfd050..34697064714e 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -267,7 +267,7 @@ void cxl_device_register_init_common(CXLDeviceState *cxl_dstate)
> cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000);
> memdev_reg_init_common(cxl_dstate);
>
> - assert(cxl_initialize_mailbox(cxl_dstate, false) == 0);
> + cxl_initialize_mailbox(cxl_dstate, false);
> }
>
> void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate)
> @@ -289,5 +289,5 @@ void cxl_device_register_init_swcci(CXLDeviceState *cxl_dstate)
> cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000);
> memdev_reg_init_common(cxl_dstate);
>
> - assert(cxl_initialize_mailbox(cxl_dstate, true) == 0);
> + cxl_initialize_mailbox(cxl_dstate, true);
> }
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index c1183614b9a4..157c01255ee3 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -321,7 +321,11 @@ static ret_code cmd_timestamp_set(struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> -static QemuUUID cel_uuid;
> +/* CXL 3.0 8.2.9.5.2.1 Command Effects Log (CEL) */
> +static QemuUUID cel_uuid = {
> + .data = UUID(0x0da9c0b5, 0xbf41, 0x4b78, 0x8f, 0x79,
> + 0x96, 0xb1, 0x62, 0x3b, 0x3f, 0x17)
> +};
>
> /* 8.2.9.4.1 */
> static ret_code cmd_logs_get_supported(struct cxl_cmd *cmd,
> @@ -684,16 +688,14 @@ void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
> DOORBELL, 0);
> }
>
> -int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
> +void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
> {
> - /* CXL 2.0: Table 169 Get Supported Logs Log Entry */
> - const char *cel_uuidstr = "0da9c0b5-bf41-4b78-8f79-96b1623b3f17";
> -
> if (!switch_cci) {
> cxl_dstate->cxl_cmd_set = cxl_cmd_set;
> } else {
> cxl_dstate->cxl_cmd_set = cxl_cmd_set_sw;
> }
> +
Trivial but this white space change doesn't belong in a patch doing other stuff.
It became irrelevant anyway when I rebased, but I like to moan.
> for (int set = 0; set < 256; set++) {
> for (int cmd = 0; cmd < 256; cmd++) {
> if (cxl_dstate->cxl_cmd_set[set][cmd].handler) {
> @@ -707,6 +709,4 @@ int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci)
> }
> }
> }
> -
> - return qemu_uuid_parse(cel_uuidstr, &cel_uuid);
> }
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index 1b366b739c62..3be2e37b3e4c 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -238,7 +238,7 @@ CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MEMORY_DEVICE,
> CXL_DEVICE_CAP_HDR1_OFFSET +
> CXL_DEVICE_CAP_REG_SIZE * 2)
>
> -int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci);
> +void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate, bool switch_cci);
> void cxl_process_mailbox(CXLDeviceState *cxl_dstate);
>
> #define cxl_device_cap_init(dstate, reg, cap_id) \
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 4/8] hw/cxl/events: Add event status register
2022-12-22 4:24 ` [PATCH v2 4/8] hw/cxl/events: Add event status register Ira Weiny
@ 2023-01-03 16:36 ` Jonathan Cameron via
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-03 16:36 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Wed, 21 Dec 2022 20:24:34 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> The device status register block was defined. However, there were no
> individual registers nor any data wired up.
>
> Define the event status register [CXL 3.0; 8.2.8.3.1] as part of the
> device status register block. Wire up the register and initialize the
> event status for each log.
>
> To support CXL 3.0 the version of the device status register block needs
> to be 2. Change the macro to allow for setting the version.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
Superficial comment from rebasing (I'll review once I have it all in place).
You haven't included cxl_events.h anywhere until the next patch.
As the enum is used in this patch we have a build problem.
I dragged the include in cxl_device.h forwards to this patch.
Jonathan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 7/8] bswap: Add the ability to store to an unaligned 24 bit field
2022-12-22 4:24 ` [PATCH v2 7/8] bswap: Add the ability to store to an unaligned 24 bit field Ira Weiny
@ 2023-01-03 17:14 ` Jonathan Cameron via
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-03 17:14 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Wed, 21 Dec 2022 20:24:37 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> CXL has 24 bit unaligned fields which need to be stored to. CXL is
> specified as little endian.
>
> Define st24_le_p() and the supporting functions to store such a field
> from a 32 bit host native value.
>
> The use of b, w, l, q as the size specifier is limiting. So "24" was
> used for the size part of the function name.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Hi Ira,
Whilst this seems good to me, it's buried deep in a CXL specific
patch set so I'm thinking it might not get the review it needs.
Perhaps we are better off starting with a local implementation then
posting a follow up series that introduces this an makes use of it
in the CXL code?
One comment inline.
Jonathan
> ---
> include/qemu/bswap.h | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index e1eca22f2548..8af4d4a75eb6 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -25,6 +25,13 @@ static inline uint16_t bswap16(uint16_t x)
> return bswap_16(x);
> }
>
> +static inline uint32_t bswap24(uint32_t x)
> +{
> + return (((x & 0x000000ffU) << 16) |
> + ((x & 0x0000ff00U) << 0) |
> + ((x & 0x00ff0000U) >> 16));
> +}
> +
> static inline uint32_t bswap32(uint32_t x)
> {
> return bswap_32(x);
> @@ -43,6 +50,13 @@ static inline uint16_t bswap16(uint16_t x)
> ((x & 0xff00) >> 8));
> }
>
> +static inline uint32_t bswap24(uint32_t x)
> +{
> + return (((x & 0x000000ffU) << 16) |
> + ((x & 0x0000ff00U) << 0) |
> + ((x & 0x00ff0000U) >> 16));
> +}
Whilst I can see the logic in having two copies to keep it in a sensible
place wrt to the other implementations, neither of these is from byteswap
so I'd just drop it out of the ifdef and have just the one copy.
> +
> static inline uint32_t bswap32(uint32_t x)
> {
> return (((x & 0x000000ffU) << 24) |
> @@ -72,6 +86,11 @@ static inline void bswap16s(uint16_t *s)
> *s = bswap16(*s);
> }
>
> +static inline void bswap24s(uint32_t *s)
> +{
> + *s = bswap24(*s);
> +}
> +
> static inline void bswap32s(uint32_t *s)
> {
> *s = bswap32(*s);
> @@ -233,6 +252,7 @@ CPU_CONVERT(le, 64, uint64_t)
> * size is:
> * b: 8 bits
> * w: 16 bits
> + * 24: 24 bits
> * l: 32 bits
> * q: 64 bits
> *
> @@ -305,6 +325,11 @@ static inline void stw_he_p(void *ptr, uint16_t v)
> __builtin_memcpy(ptr, &v, sizeof(v));
> }
>
> +static inline void st24_he_p(void *ptr, uint32_t v)
> +{
> + __builtin_memcpy(ptr, &v, 3);
> +}
> +
> static inline int ldl_he_p(const void *ptr)
> {
> int32_t r;
> @@ -354,6 +379,11 @@ static inline void stw_le_p(void *ptr, uint16_t v)
> stw_he_p(ptr, le_bswap(v, 16));
> }
>
> +static inline void st24_le_p(void *ptr, uint32_t v)
> +{
> + st24_he_p(ptr, le_bswap(v, 24));
> +}
> +
> static inline void stl_le_p(void *ptr, uint32_t v)
> {
> stl_he_p(ptr, le_bswap(v, 32));
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 8/8] hw/cxl/events: Add in inject general media event
2022-12-22 4:24 ` [PATCH v2 8/8] hw/cxl/events: Add in inject general media event Ira Weiny
@ 2023-01-03 18:07 ` Jonathan Cameron via
2023-01-09 19:15 ` Ira Weiny
2023-01-12 15:34 ` Jonathan Cameron via
1 sibling, 1 reply; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-03 18:07 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Wed, 21 Dec 2022 20:24:38 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> To facilitate testing provide a QMP command to inject a general media
> event. The event can be added to the log specified.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Hi Ira,
Suggestion inline on how to more neatly handle optional arguments using
QMPs inbuilt handling. Short version is stick a * in front of the
argument name in the json and you get a bonus parameter in the callback
bool has_<name> which lets you identify if it is provided or not.
Jonathan
>
> ---
> Changes from RFC:
> Add all fields for this event
> irq happens automatically when log transitions from 0 to 1
> ---
> hw/mem/cxl_type3.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
> hw/mem/cxl_type3_stubs.c | 8 ++++
> include/hw/cxl/cxl_events.h | 20 ++++++++++
> qapi/cxl.json | 25 ++++++++++++
> 4 files changed, 146 insertions(+)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index a43949cab120..bedd09e500ba 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -916,6 +916,99 @@ static CXLPoisonList *get_poison_list(CXLType3Dev *ct3d)
> return &ct3d->poison_list;
> }
>
> +static void cxl_assign_event_header(struct cxl_event_record_hdr *hdr,
> + QemuUUID *uuid, uint8_t flags,
> + uint8_t length)
> +{
> + hdr->flags[0] = flags;
> + hdr->length = length;
> + memcpy(&hdr->id, uuid, sizeof(hdr->id));
> + hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +}
> +
> +QemuUUID gen_media_uuid = {
> + .data = UUID(0xfbcd0a77, 0xc260, 0x417f,
> + 0x85, 0xa9, 0x08, 0x8b, 0x16, 0x21, 0xeb, 0xa6),
> +};
> +
> +#define CXL_GMER_VALID_CHANNEL BIT(0)
> +#define CXL_GMER_VALID_RANK BIT(1)
> +#define CXL_GMER_VALID_DEVICE BIT(2)
> +#define CXL_GMER_VALID_COMPONENT BIT(3)
> +
> +/*
> + * For channel, rank, and device; any value inside of the fields valid range
> + * will flag that field to be valid. IE pass -1 to mark the field invalid.
> + *
> + * Component ID is device specific. Define this as a string.
> + */
> +void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> + uint8_t flags, uint64_t physaddr,
> + uint8_t descriptor, uint8_t type,
> + uint8_t transaction_type,
> + int16_t channel, int16_t rank,
> + int32_t device,
> + const char *component_id,
> + Error **errp)
> +{
> + Object *obj = object_resolve_path(path, NULL);
> + struct cxl_event_gen_media gem;
> + struct cxl_event_record_hdr *hdr = &gem.hdr;
> + CXLDeviceState *cxlds;
> + CXLType3Dev *ct3d;
> + uint16_t valid_flags = 0;
> +
> + if (log >= CXL_EVENT_TYPE_MAX) {
> + error_setg(errp, "Invalid log type: %d", log);
> + return;
> + }
> + if (!obj) {
> + error_setg(errp, "Unable to resolve path");
> + return;
> + }
> + if (!object_dynamic_cast(obj, TYPE_CXL_TYPE3)) {
> + error_setg(errp, "Path does not point to a CXL type 3 device");
> + }
> + ct3d = CXL_TYPE3(obj);
> + cxlds = &ct3d->cxl_dstate;
> +
> + memset(&gem, 0, sizeof(gem));
> + cxl_assign_event_header(hdr, &gen_media_uuid, flags,
> + sizeof(struct cxl_event_gen_media));
> +
> + gem.phys_addr = physaddr;
> + gem.descriptor = descriptor;
> + gem.type = type;
> + gem.transaction_type = transaction_type;
> +
> + if (0 <= channel && channel <= 0xFF) {
> + gem.channel = channel;
> + valid_flags |= CXL_GMER_VALID_CHANNEL;
> + }
> +
> + if (0 <= rank && rank <= 0xFF) {
> + gem.rank = rank;
> + valid_flags |= CXL_GMER_VALID_RANK;
> + }
> +
> + if (0 <= device && device <= 0xFFFFFF) {
> + st24_le_p(gem.device, device);
> + valid_flags |= CXL_GMER_VALID_DEVICE;
> + }
> +
> + if (component_id && strcmp(component_id, "")) {
> + strncpy((char *)gem.component_id, component_id,
> + sizeof(gem.component_id) - 1);
> + valid_flags |= CXL_GMER_VALID_COMPONENT;
> + }
> +
> + stw_le_p(gem.validity_flags, valid_flags);
> +
> + if (cxl_event_insert(cxlds, log, (struct cxl_event_record_raw *)&gem)) {
> + cxl_event_irq_assert(ct3d);
> + }
> +}
> +
> void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> Error **errp)
> {
> diff --git a/hw/mem/cxl_type3_stubs.c b/hw/mem/cxl_type3_stubs.c
> index f2c9f48f4010..62f04d487031 100644
> --- a/hw/mem/cxl_type3_stubs.c
> +++ b/hw/mem/cxl_type3_stubs.c
> @@ -2,6 +2,14 @@
> #include "qemu/osdep.h"
> #include "qapi/qapi-commands-cxl.h"
>
> +void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> + uint8_t flags, uint64_t physaddr,
> + uint8_t descriptor, uint8_t type,
> + uint8_t transaction_type,
> + int16_t channel, int16_t rank,
> + int32_t device,
> + char *component_id,
> + Error **errp) {}
> void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> Error **errp) {}
> void qmp_cxl_inject_uncorrectable_error(const char *path,
> diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
> index 2df40720320a..3175e9d9866d 100644
> --- a/include/hw/cxl/cxl_events.h
> +++ b/include/hw/cxl/cxl_events.h
> @@ -103,4 +103,24 @@ struct cxl_event_interrupt_policy {
> /* DCD is optional but other fields are not */
> #define CXL_EVENT_INT_SETTING_MIN_LEN 4
>
> +/*
> + * General Media Event Record
> + * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43
> + */
> +#define CXL_EVENT_GEN_MED_COMP_ID_SIZE 0x10
> +#define CXL_EVENT_GEN_MED_RES_SIZE 0x2e
> +struct cxl_event_gen_media {
> + struct cxl_event_record_hdr hdr;
> + uint64_t phys_addr;
> + uint8_t descriptor;
> + uint8_t type;
> + uint8_t transaction_type;
> + uint8_t validity_flags[2];
> + uint8_t channel;
> + uint8_t rank;
> + uint8_t device[3];
> + uint8_t component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
> + uint8_t reserved[CXL_EVENT_GEN_MED_RES_SIZE];
> +} QEMU_PACKED;
> +
> #endif /* CXL_EVENTS_H */
> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index b4836bb87f53..56e85a28d7e0 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -5,6 +5,31 @@
> # = CXL devices
> ##
>
> +##
> +# @cxl-inject-gen-media-event:
> +#
> +# @path: CXL type 3 device canonical QOM path
> +#
> +# @log: Event Log to add the event to
> +# @flags: header flags
> +# @physaddr: Physical Address
> +# @descriptor: Descriptor
> +# @type: Type
> +# @transactiontype: Transaction Type
> +# @channel: Channel
> +# @rank: Rank
> +# @device: Device
> +# @componentid: Device specific string
> +#
> +##
> +{ 'command': 'cxl-inject-gen-media-event',
> + 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
> + 'physaddr': 'uint64', 'descriptor': 'uint8',
> + 'type': 'uint8', 'transactiontype': 'uint8',
> + 'channel': 'int16', 'rank': 'int16',
> + 'device': 'int32', 'componentid': 'str'
> + }}
From an interface cleanliness point of view I'd rather see
all the optional fields as optional. That's done by marking them
with a * so
'*channel': 'int16'
Then the signature of the related qmp_cxl_inject_gen_media_event
gains a boolean has_channel parameter (before channel)
Those booleans can be used to set the flags etc.
Very lightly tested:
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 4660a44ef8..cb9bb0b166 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -1203,8 +1203,9 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
uint8_t flags, uint64_t physaddr,
uint8_t descriptor, uint8_t type,
uint8_t transaction_type,
- int16_t channel, int16_t rank,
- int32_t device,
+ bool has_channel, uint8_t channel,
+ bool has_rank, uint8_t rank,
+ bool has_device, uint32_t device,
const char *component_id,
Error **errp)
{
@@ -1238,22 +1239,22 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
gem.type = type;
gem.transaction_type = transaction_type;
- if (0 <= channel && channel <= 0xFF) {
+ if (has_channel) {
gem.channel = channel;
valid_flags |= CXL_GMER_VALID_CHANNEL;
}
- if (0 <= rank && rank <= 0xFF) {
+ if (has_rank) {
gem.rank = rank;
valid_flags |= CXL_GMER_VALID_RANK;
}
- if (0 <= device && device <= 0xFFFFFF) {
+ if (has_device) {
st24_le_p(gem.device, device);
valid_flags |= CXL_GMER_VALID_DEVICE;
}
- if (component_id && strcmp(component_id, "")) {
+ if (component_id) {
strncpy((char *)gem.component_id, component_id,
sizeof(gem.component_id) - 1);
valid_flags |= CXL_GMER_VALID_COMPONENT;
diff --git a/qapi/cxl.json b/qapi/cxl.json
index 56e85a28d7..085f82e7da 100644
--- a/qapi/cxl.json
+++ b/qapi/cxl.json
@@ -26,8 +26,8 @@
'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
'physaddr': 'uint64', 'descriptor': 'uint8',
'type': 'uint8', 'transactiontype': 'uint8',
- 'channel': 'int16', 'rank': 'int16',
- 'device': 'int32', 'componentid': 'str'
+ '*channel': 'uint8', '*rank': 'uint8',
+ '*device': 'uint32', '*componentid': 'str'
}}
##
> +
> ##
> # @cxl-inject-poison:
> #
>
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands
2022-12-22 4:24 ` [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands Ira Weiny
@ 2023-01-04 18:07 ` Jonathan Cameron via
2023-01-24 17:04 ` Jonathan Cameron via
1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-04 18:07 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Wed, 21 Dec 2022 20:24:35 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> CXL testing is benefited from an artificial event log injection
> mechanism.
>
> Add an event log infrastructure to insert, get, and clear events from
> the various logs available on a device.
>
> Replace the stubbed out CXL Get/Clear Event mailbox commands with
> commands that operate on the new infrastructure.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Hi Ira,
The ability to see the ret_code definitions relied on a patch that will
go upstream long after this one. I've dragged that down before this in
my tree, but the naming ret_code is neither compliant with QEMU naming
rules, nor specific enough given that definition is in a header.
Hence I'll also rename it as CXLRetCode.
I'll have a new tree up with that in place in a few days.
I'll carry your patches on that tree, with appropriate changes.
Jonathan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 8/8] hw/cxl/events: Add in inject general media event
2023-01-03 18:07 ` Jonathan Cameron via
@ 2023-01-09 19:15 ` Ira Weiny
2023-01-10 15:38 ` Jonathan Cameron via
0 siblings, 1 reply; 19+ messages in thread
From: Ira Weiny @ 2023-01-09 19:15 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Tue, Jan 03, 2023 at 06:07:19PM +0000, Jonathan Cameron wrote:
> On Wed, 21 Dec 2022 20:24:38 -0800
> Ira Weiny <ira.weiny@intel.com> wrote:
>
> > To facilitate testing provide a QMP command to inject a general media
> > event. The event can be added to the log specified.
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
> Hi Ira,
>
> Suggestion inline on how to more neatly handle optional arguments using
> QMPs inbuilt handling. Short version is stick a * in front of the
> argument name in the json and you get a bonus parameter in the callback
> bool has_<name> which lets you identify if it is provided or not.
>
> Jonathan
>
> >
> > ---
> > Changes from RFC:
> > Add all fields for this event
> > irq happens automatically when log transitions from 0 to 1
> > ---
> > hw/mem/cxl_type3.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
> > hw/mem/cxl_type3_stubs.c | 8 ++++
> > include/hw/cxl/cxl_events.h | 20 ++++++++++
> > qapi/cxl.json | 25 ++++++++++++
> > 4 files changed, 146 insertions(+)
> >
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index a43949cab120..bedd09e500ba 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> > @@ -916,6 +916,99 @@ static CXLPoisonList *get_poison_list(CXLType3Dev *ct3d)
> > return &ct3d->poison_list;
> > }
> >
> > +static void cxl_assign_event_header(struct cxl_event_record_hdr *hdr,
> > + QemuUUID *uuid, uint8_t flags,
> > + uint8_t length)
> > +{
> > + hdr->flags[0] = flags;
> > + hdr->length = length;
> > + memcpy(&hdr->id, uuid, sizeof(hdr->id));
> > + hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > +}
> > +
> > +QemuUUID gen_media_uuid = {
> > + .data = UUID(0xfbcd0a77, 0xc260, 0x417f,
> > + 0x85, 0xa9, 0x08, 0x8b, 0x16, 0x21, 0xeb, 0xa6),
> > +};
> > +
> > +#define CXL_GMER_VALID_CHANNEL BIT(0)
> > +#define CXL_GMER_VALID_RANK BIT(1)
> > +#define CXL_GMER_VALID_DEVICE BIT(2)
> > +#define CXL_GMER_VALID_COMPONENT BIT(3)
> > +
> > +/*
> > + * For channel, rank, and device; any value inside of the fields valid range
> > + * will flag that field to be valid. IE pass -1 to mark the field invalid.
> > + *
> > + * Component ID is device specific. Define this as a string.
> > + */
> > +void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > + uint8_t flags, uint64_t physaddr,
> > + uint8_t descriptor, uint8_t type,
> > + uint8_t transaction_type,
> > + int16_t channel, int16_t rank,
> > + int32_t device,
> > + const char *component_id,
> > + Error **errp)
> > +{
> > + Object *obj = object_resolve_path(path, NULL);
> > + struct cxl_event_gen_media gem;
> > + struct cxl_event_record_hdr *hdr = &gem.hdr;
> > + CXLDeviceState *cxlds;
> > + CXLType3Dev *ct3d;
> > + uint16_t valid_flags = 0;
> > +
> > + if (log >= CXL_EVENT_TYPE_MAX) {
> > + error_setg(errp, "Invalid log type: %d", log);
> > + return;
> > + }
> > + if (!obj) {
> > + error_setg(errp, "Unable to resolve path");
> > + return;
> > + }
> > + if (!object_dynamic_cast(obj, TYPE_CXL_TYPE3)) {
> > + error_setg(errp, "Path does not point to a CXL type 3 device");
> > + }
> > + ct3d = CXL_TYPE3(obj);
> > + cxlds = &ct3d->cxl_dstate;
> > +
> > + memset(&gem, 0, sizeof(gem));
> > + cxl_assign_event_header(hdr, &gen_media_uuid, flags,
> > + sizeof(struct cxl_event_gen_media));
> > +
> > + gem.phys_addr = physaddr;
> > + gem.descriptor = descriptor;
> > + gem.type = type;
> > + gem.transaction_type = transaction_type;
> > +
> > + if (0 <= channel && channel <= 0xFF) {
> > + gem.channel = channel;
> > + valid_flags |= CXL_GMER_VALID_CHANNEL;
> > + }
> > +
> > + if (0 <= rank && rank <= 0xFF) {
> > + gem.rank = rank;
> > + valid_flags |= CXL_GMER_VALID_RANK;
> > + }
> > +
> > + if (0 <= device && device <= 0xFFFFFF) {
> > + st24_le_p(gem.device, device);
> > + valid_flags |= CXL_GMER_VALID_DEVICE;
> > + }
> > +
> > + if (component_id && strcmp(component_id, "")) {
> > + strncpy((char *)gem.component_id, component_id,
> > + sizeof(gem.component_id) - 1);
> > + valid_flags |= CXL_GMER_VALID_COMPONENT;
> > + }
> > +
> > + stw_le_p(gem.validity_flags, valid_flags);
> > +
> > + if (cxl_event_insert(cxlds, log, (struct cxl_event_record_raw *)&gem)) {
> > + cxl_event_irq_assert(ct3d);
> > + }
> > +}
> > +
> > void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> > Error **errp)
> > {
> > diff --git a/hw/mem/cxl_type3_stubs.c b/hw/mem/cxl_type3_stubs.c
> > index f2c9f48f4010..62f04d487031 100644
> > --- a/hw/mem/cxl_type3_stubs.c
> > +++ b/hw/mem/cxl_type3_stubs.c
> > @@ -2,6 +2,14 @@
> > #include "qemu/osdep.h"
> > #include "qapi/qapi-commands-cxl.h"
> >
> > +void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > + uint8_t flags, uint64_t physaddr,
> > + uint8_t descriptor, uint8_t type,
> > + uint8_t transaction_type,
> > + int16_t channel, int16_t rank,
> > + int32_t device,
> > + char *component_id,
> > + Error **errp) {}
> > void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> > Error **errp) {}
> > void qmp_cxl_inject_uncorrectable_error(const char *path,
> > diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
> > index 2df40720320a..3175e9d9866d 100644
> > --- a/include/hw/cxl/cxl_events.h
> > +++ b/include/hw/cxl/cxl_events.h
> > @@ -103,4 +103,24 @@ struct cxl_event_interrupt_policy {
> > /* DCD is optional but other fields are not */
> > #define CXL_EVENT_INT_SETTING_MIN_LEN 4
> >
> > +/*
> > + * General Media Event Record
> > + * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43
> > + */
> > +#define CXL_EVENT_GEN_MED_COMP_ID_SIZE 0x10
> > +#define CXL_EVENT_GEN_MED_RES_SIZE 0x2e
> > +struct cxl_event_gen_media {
> > + struct cxl_event_record_hdr hdr;
> > + uint64_t phys_addr;
> > + uint8_t descriptor;
> > + uint8_t type;
> > + uint8_t transaction_type;
> > + uint8_t validity_flags[2];
> > + uint8_t channel;
> > + uint8_t rank;
> > + uint8_t device[3];
> > + uint8_t component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
> > + uint8_t reserved[CXL_EVENT_GEN_MED_RES_SIZE];
> > +} QEMU_PACKED;
> > +
> > #endif /* CXL_EVENTS_H */
> > diff --git a/qapi/cxl.json b/qapi/cxl.json
> > index b4836bb87f53..56e85a28d7e0 100644
> > --- a/qapi/cxl.json
> > +++ b/qapi/cxl.json
> > @@ -5,6 +5,31 @@
> > # = CXL devices
> > ##
> >
> > +##
> > +# @cxl-inject-gen-media-event:
> > +#
> > +# @path: CXL type 3 device canonical QOM path
> > +#
> > +# @log: Event Log to add the event to
> > +# @flags: header flags
> > +# @physaddr: Physical Address
> > +# @descriptor: Descriptor
> > +# @type: Type
> > +# @transactiontype: Transaction Type
> > +# @channel: Channel
> > +# @rank: Rank
> > +# @device: Device
> > +# @componentid: Device specific string
> > +#
> > +##
> > +{ 'command': 'cxl-inject-gen-media-event',
> > + 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
> > + 'physaddr': 'uint64', 'descriptor': 'uint8',
> > + 'type': 'uint8', 'transactiontype': 'uint8',
> > + 'channel': 'int16', 'rank': 'int16',
> > + 'device': 'int32', 'componentid': 'str'
> > + }}
>
> From an interface cleanliness point of view I'd rather see
> all the optional fields as optional. That's done by marking them
> with a * so
> '*channel': 'int16'
>
> Then the signature of the related qmp_cxl_inject_gen_media_event
> gains a boolean has_channel parameter (before channel)
>
> Those booleans can be used to set the flags etc.
Ah! Ok cool. Yes this would make a lot more sense. I did not realize QMP did
this optional thing. That makes scripting this easier as well!
Did you put all this on a branch or not? I did not see anything new at:
https://gitlab.com/jic23/qemu.git
I can definitely respin but it sounds like you were going to make some changes.
Ira
>
> Very lightly tested:
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 4660a44ef8..cb9bb0b166 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -1203,8 +1203,9 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> uint8_t flags, uint64_t physaddr,
> uint8_t descriptor, uint8_t type,
> uint8_t transaction_type,
> - int16_t channel, int16_t rank,
> - int32_t device,
> + bool has_channel, uint8_t channel,
> + bool has_rank, uint8_t rank,
> + bool has_device, uint32_t device,
> const char *component_id,
> Error **errp)
> {
> @@ -1238,22 +1239,22 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> gem.type = type;
> gem.transaction_type = transaction_type;
>
> - if (0 <= channel && channel <= 0xFF) {
> + if (has_channel) {
> gem.channel = channel;
> valid_flags |= CXL_GMER_VALID_CHANNEL;
> }
>
> - if (0 <= rank && rank <= 0xFF) {
> + if (has_rank) {
> gem.rank = rank;
> valid_flags |= CXL_GMER_VALID_RANK;
> }
>
> - if (0 <= device && device <= 0xFFFFFF) {
> + if (has_device) {
> st24_le_p(gem.device, device);
> valid_flags |= CXL_GMER_VALID_DEVICE;
> }
>
> - if (component_id && strcmp(component_id, "")) {
> + if (component_id) {
> strncpy((char *)gem.component_id, component_id,
> sizeof(gem.component_id) - 1);
> valid_flags |= CXL_GMER_VALID_COMPONENT;
> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 56e85a28d7..085f82e7da 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -26,8 +26,8 @@
> 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
> 'physaddr': 'uint64', 'descriptor': 'uint8',
> 'type': 'uint8', 'transactiontype': 'uint8',
> - 'channel': 'int16', 'rank': 'int16',
> - 'device': 'int32', 'componentid': 'str'
> + '*channel': 'uint8', '*rank': 'uint8',
> + '*device': 'uint32', '*componentid': 'str'
> }}
>
> ##
>
> > +
> > ##
> > # @cxl-inject-poison:
> > #
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 8/8] hw/cxl/events: Add in inject general media event
2023-01-09 19:15 ` Ira Weiny
@ 2023-01-10 15:38 ` Jonathan Cameron via
2023-01-11 14:05 ` Jonathan Cameron via
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-10 15:38 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Mon, 9 Jan 2023 11:15:28 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> On Tue, Jan 03, 2023 at 06:07:19PM +0000, Jonathan Cameron wrote:
> > On Wed, 21 Dec 2022 20:24:38 -0800
> > Ira Weiny <ira.weiny@intel.com> wrote:
> >
> > > To facilitate testing provide a QMP command to inject a general media
> > > event. The event can be added to the log specified.
> > >
> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> >
> > Hi Ira,
> >
> > Suggestion inline on how to more neatly handle optional arguments using
> > QMPs inbuilt handling. Short version is stick a * in front of the
> > argument name in the json and you get a bonus parameter in the callback
> > bool has_<name> which lets you identify if it is provided or not.
> >
> > Jonathan
> >
> > >
> > > ---
> > > Changes from RFC:
> > > Add all fields for this event
> > > irq happens automatically when log transitions from 0 to 1
> > > ---
> > > hw/mem/cxl_type3.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
> > > hw/mem/cxl_type3_stubs.c | 8 ++++
> > > include/hw/cxl/cxl_events.h | 20 ++++++++++
> > > qapi/cxl.json | 25 ++++++++++++
> > > 4 files changed, 146 insertions(+)
> > >
> > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > > index a43949cab120..bedd09e500ba 100644
> > > --- a/hw/mem/cxl_type3.c
> > > +++ b/hw/mem/cxl_type3.c
> > > @@ -916,6 +916,99 @@ static CXLPoisonList *get_poison_list(CXLType3Dev *ct3d)
> > > return &ct3d->poison_list;
> > > }
> > >
> > > +static void cxl_assign_event_header(struct cxl_event_record_hdr *hdr,
> > > + QemuUUID *uuid, uint8_t flags,
> > > + uint8_t length)
> > > +{
> > > + hdr->flags[0] = flags;
> > > + hdr->length = length;
> > > + memcpy(&hdr->id, uuid, sizeof(hdr->id));
> > > + hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > > +}
> > > +
> > > +QemuUUID gen_media_uuid = {
> > > + .data = UUID(0xfbcd0a77, 0xc260, 0x417f,
> > > + 0x85, 0xa9, 0x08, 0x8b, 0x16, 0x21, 0xeb, 0xa6),
> > > +};
> > > +
> > > +#define CXL_GMER_VALID_CHANNEL BIT(0)
> > > +#define CXL_GMER_VALID_RANK BIT(1)
> > > +#define CXL_GMER_VALID_DEVICE BIT(2)
> > > +#define CXL_GMER_VALID_COMPONENT BIT(3)
> > > +
> > > +/*
> > > + * For channel, rank, and device; any value inside of the fields valid range
> > > + * will flag that field to be valid. IE pass -1 to mark the field invalid.
> > > + *
> > > + * Component ID is device specific. Define this as a string.
> > > + */
> > > +void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > > + uint8_t flags, uint64_t physaddr,
> > > + uint8_t descriptor, uint8_t type,
> > > + uint8_t transaction_type,
> > > + int16_t channel, int16_t rank,
> > > + int32_t device,
> > > + const char *component_id,
> > > + Error **errp)
> > > +{
> > > + Object *obj = object_resolve_path(path, NULL);
> > > + struct cxl_event_gen_media gem;
> > > + struct cxl_event_record_hdr *hdr = &gem.hdr;
> > > + CXLDeviceState *cxlds;
> > > + CXLType3Dev *ct3d;
> > > + uint16_t valid_flags = 0;
> > > +
> > > + if (log >= CXL_EVENT_TYPE_MAX) {
> > > + error_setg(errp, "Invalid log type: %d", log);
> > > + return;
> > > + }
> > > + if (!obj) {
> > > + error_setg(errp, "Unable to resolve path");
> > > + return;
> > > + }
> > > + if (!object_dynamic_cast(obj, TYPE_CXL_TYPE3)) {
> > > + error_setg(errp, "Path does not point to a CXL type 3 device");
> > > + }
> > > + ct3d = CXL_TYPE3(obj);
> > > + cxlds = &ct3d->cxl_dstate;
> > > +
> > > + memset(&gem, 0, sizeof(gem));
> > > + cxl_assign_event_header(hdr, &gen_media_uuid, flags,
> > > + sizeof(struct cxl_event_gen_media));
> > > +
> > > + gem.phys_addr = physaddr;
> > > + gem.descriptor = descriptor;
> > > + gem.type = type;
> > > + gem.transaction_type = transaction_type;
> > > +
> > > + if (0 <= channel && channel <= 0xFF) {
> > > + gem.channel = channel;
> > > + valid_flags |= CXL_GMER_VALID_CHANNEL;
> > > + }
> > > +
> > > + if (0 <= rank && rank <= 0xFF) {
> > > + gem.rank = rank;
> > > + valid_flags |= CXL_GMER_VALID_RANK;
> > > + }
> > > +
> > > + if (0 <= device && device <= 0xFFFFFF) {
> > > + st24_le_p(gem.device, device);
> > > + valid_flags |= CXL_GMER_VALID_DEVICE;
> > > + }
> > > +
> > > + if (component_id && strcmp(component_id, "")) {
> > > + strncpy((char *)gem.component_id, component_id,
> > > + sizeof(gem.component_id) - 1);
> > > + valid_flags |= CXL_GMER_VALID_COMPONENT;
> > > + }
> > > +
> > > + stw_le_p(gem.validity_flags, valid_flags);
> > > +
> > > + if (cxl_event_insert(cxlds, log, (struct cxl_event_record_raw *)&gem)) {
> > > + cxl_event_irq_assert(ct3d);
> > > + }
> > > +}
> > > +
> > > void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> > > Error **errp)
> > > {
> > > diff --git a/hw/mem/cxl_type3_stubs.c b/hw/mem/cxl_type3_stubs.c
> > > index f2c9f48f4010..62f04d487031 100644
> > > --- a/hw/mem/cxl_type3_stubs.c
> > > +++ b/hw/mem/cxl_type3_stubs.c
> > > @@ -2,6 +2,14 @@
> > > #include "qemu/osdep.h"
> > > #include "qapi/qapi-commands-cxl.h"
> > >
> > > +void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > > + uint8_t flags, uint64_t physaddr,
> > > + uint8_t descriptor, uint8_t type,
> > > + uint8_t transaction_type,
> > > + int16_t channel, int16_t rank,
> > > + int32_t device,
> > > + char *component_id,
> > > + Error **errp) {}
> > > void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> > > Error **errp) {}
> > > void qmp_cxl_inject_uncorrectable_error(const char *path,
> > > diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
> > > index 2df40720320a..3175e9d9866d 100644
> > > --- a/include/hw/cxl/cxl_events.h
> > > +++ b/include/hw/cxl/cxl_events.h
> > > @@ -103,4 +103,24 @@ struct cxl_event_interrupt_policy {
> > > /* DCD is optional but other fields are not */
> > > #define CXL_EVENT_INT_SETTING_MIN_LEN 4
> > >
> > > +/*
> > > + * General Media Event Record
> > > + * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43
> > > + */
> > > +#define CXL_EVENT_GEN_MED_COMP_ID_SIZE 0x10
> > > +#define CXL_EVENT_GEN_MED_RES_SIZE 0x2e
> > > +struct cxl_event_gen_media {
> > > + struct cxl_event_record_hdr hdr;
> > > + uint64_t phys_addr;
> > > + uint8_t descriptor;
> > > + uint8_t type;
> > > + uint8_t transaction_type;
> > > + uint8_t validity_flags[2];
> > > + uint8_t channel;
> > > + uint8_t rank;
> > > + uint8_t device[3];
> > > + uint8_t component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
> > > + uint8_t reserved[CXL_EVENT_GEN_MED_RES_SIZE];
> > > +} QEMU_PACKED;
> > > +
> > > #endif /* CXL_EVENTS_H */
> > > diff --git a/qapi/cxl.json b/qapi/cxl.json
> > > index b4836bb87f53..56e85a28d7e0 100644
> > > --- a/qapi/cxl.json
> > > +++ b/qapi/cxl.json
> > > @@ -5,6 +5,31 @@
> > > # = CXL devices
> > > ##
> > >
> > > +##
> > > +# @cxl-inject-gen-media-event:
> > > +#
> > > +# @path: CXL type 3 device canonical QOM path
> > > +#
> > > +# @log: Event Log to add the event to
> > > +# @flags: header flags
> > > +# @physaddr: Physical Address
> > > +# @descriptor: Descriptor
> > > +# @type: Type
> > > +# @transactiontype: Transaction Type
> > > +# @channel: Channel
> > > +# @rank: Rank
> > > +# @device: Device
> > > +# @componentid: Device specific string
> > > +#
> > > +##
> > > +{ 'command': 'cxl-inject-gen-media-event',
> > > + 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
> > > + 'physaddr': 'uint64', 'descriptor': 'uint8',
> > > + 'type': 'uint8', 'transactiontype': 'uint8',
> > > + 'channel': 'int16', 'rank': 'int16',
> > > + 'device': 'int32', 'componentid': 'str'
> > > + }}
> >
> > From an interface cleanliness point of view I'd rather see
> > all the optional fields as optional. That's done by marking them
> > with a * so
> > '*channel': 'int16'
> >
> > Then the signature of the related qmp_cxl_inject_gen_media_event
> > gains a boolean has_channel parameter (before channel)
> >
> > Those booleans can be used to set the flags etc.
>
> Ah! Ok cool. Yes this would make a lot more sense. I did not realize QMP did
> this optional thing. That makes scripting this easier as well!
>
> Did you put all this on a branch or not? I did not see anything new at:
>
> https://gitlab.com/jic23/qemu.git
>
> I can definitely respin but it sounds like you were going to make some changes.
I got side tracked and reworked a few more things. Hopefully in have a branch
up in a day or two.
Jonathan
>
> Ira
>
> >
> > Very lightly tested:
> >
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index 4660a44ef8..cb9bb0b166 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> > @@ -1203,8 +1203,9 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > uint8_t flags, uint64_t physaddr,
> > uint8_t descriptor, uint8_t type,
> > uint8_t transaction_type,
> > - int16_t channel, int16_t rank,
> > - int32_t device,
> > + bool has_channel, uint8_t channel,
> > + bool has_rank, uint8_t rank,
> > + bool has_device, uint32_t device,
> > const char *component_id,
> > Error **errp)
> > {
> > @@ -1238,22 +1239,22 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > gem.type = type;
> > gem.transaction_type = transaction_type;
> >
> > - if (0 <= channel && channel <= 0xFF) {
> > + if (has_channel) {
> > gem.channel = channel;
> > valid_flags |= CXL_GMER_VALID_CHANNEL;
> > }
> >
> > - if (0 <= rank && rank <= 0xFF) {
> > + if (has_rank) {
> > gem.rank = rank;
> > valid_flags |= CXL_GMER_VALID_RANK;
> > }
> >
> > - if (0 <= device && device <= 0xFFFFFF) {
> > + if (has_device) {
> > st24_le_p(gem.device, device);
> > valid_flags |= CXL_GMER_VALID_DEVICE;
> > }
> >
> > - if (component_id && strcmp(component_id, "")) {
> > + if (component_id) {
> > strncpy((char *)gem.component_id, component_id,
> > sizeof(gem.component_id) - 1);
> > valid_flags |= CXL_GMER_VALID_COMPONENT;
> > diff --git a/qapi/cxl.json b/qapi/cxl.json
> > index 56e85a28d7..085f82e7da 100644
> > --- a/qapi/cxl.json
> > +++ b/qapi/cxl.json
> > @@ -26,8 +26,8 @@
> > 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
> > 'physaddr': 'uint64', 'descriptor': 'uint8',
> > 'type': 'uint8', 'transactiontype': 'uint8',
> > - 'channel': 'int16', 'rank': 'int16',
> > - 'device': 'int32', 'componentid': 'str'
> > + '*channel': 'uint8', '*rank': 'uint8',
> > + '*device': 'uint32', '*componentid': 'str'
> > }}
> >
> > ##
> >
> > > +
> > > ##
> > > # @cxl-inject-poison:
> > > #
> > >
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 8/8] hw/cxl/events: Add in inject general media event
2023-01-10 15:38 ` Jonathan Cameron via
@ 2023-01-11 14:05 ` Jonathan Cameron via
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-11 14:05 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
> > > From an interface cleanliness point of view I'd rather see
> > > all the optional fields as optional. That's done by marking them
> > > with a * so
> > > '*channel': 'int16'
> > >
> > > Then the signature of the related qmp_cxl_inject_gen_media_event
> > > gains a boolean has_channel parameter (before channel)
> > >
> > > Those booleans can be used to set the flags etc.
> >
> > Ah! Ok cool. Yes this would make a lot more sense. I did not realize QMP did
> > this optional thing. That makes scripting this easier as well!
> >
> > Did you put all this on a branch or not? I did not see anything new at:
> >
> > https://gitlab.com/jic23/qemu.git
> >
> > I can definitely respin but it sounds like you were going to make some changes.
>
> I got side tracked and reworked a few more things. Hopefully in have a branch
> up in a day or two.
Now pushed out as https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-11
My plan is to send the first 8 patches to Michael targetting upstream which includes
your uuid related patches from the start of this series. The changes to RAS error
injection since previous tree are substantial to support multiple header logging.
I've made a few tweaks to your other patches on that branch including the optional
parameters stuff and some reworks I mentioned in this thread.
Note there is a dirty hack on top of that tree to deal with a build issue on my arch-linux
test box that I foolishly upgraded this morning. Might break things on other distros
(version issue with curl).
Jonathan
>
> Jonathan
>
> >
> > Ira
> >
> > >
> > > Very lightly tested:
> > >
> > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > > index 4660a44ef8..cb9bb0b166 100644
> > > --- a/hw/mem/cxl_type3.c
> > > +++ b/hw/mem/cxl_type3.c
> > > @@ -1203,8 +1203,9 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > > uint8_t flags, uint64_t physaddr,
> > > uint8_t descriptor, uint8_t type,
> > > uint8_t transaction_type,
> > > - int16_t channel, int16_t rank,
> > > - int32_t device,
> > > + bool has_channel, uint8_t channel,
> > > + bool has_rank, uint8_t rank,
> > > + bool has_device, uint32_t device,
> > > const char *component_id,
> > > Error **errp)
> > > {
> > > @@ -1238,22 +1239,22 @@ void qmp_cxl_inject_gen_media_event(const char *path, uint8_t log,
> > > gem.type = type;
> > > gem.transaction_type = transaction_type;
> > >
> > > - if (0 <= channel && channel <= 0xFF) {
> > > + if (has_channel) {
> > > gem.channel = channel;
> > > valid_flags |= CXL_GMER_VALID_CHANNEL;
> > > }
> > >
> > > - if (0 <= rank && rank <= 0xFF) {
> > > + if (has_rank) {
> > > gem.rank = rank;
> > > valid_flags |= CXL_GMER_VALID_RANK;
> > > }
> > >
> > > - if (0 <= device && device <= 0xFFFFFF) {
> > > + if (has_device) {
> > > st24_le_p(gem.device, device);
> > > valid_flags |= CXL_GMER_VALID_DEVICE;
> > > }
> > >
> > > - if (component_id && strcmp(component_id, "")) {
> > > + if (component_id) {
> > > strncpy((char *)gem.component_id, component_id,
> > > sizeof(gem.component_id) - 1);
> > > valid_flags |= CXL_GMER_VALID_COMPONENT;
> > > diff --git a/qapi/cxl.json b/qapi/cxl.json
> > > index 56e85a28d7..085f82e7da 100644
> > > --- a/qapi/cxl.json
> > > +++ b/qapi/cxl.json
> > > @@ -26,8 +26,8 @@
> > > 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8',
> > > 'physaddr': 'uint64', 'descriptor': 'uint8',
> > > 'type': 'uint8', 'transactiontype': 'uint8',
> > > - 'channel': 'int16', 'rank': 'int16',
> > > - 'device': 'int32', 'componentid': 'str'
> > > + '*channel': 'uint8', '*rank': 'uint8',
> > > + '*device': 'uint32', '*componentid': 'str'
> > > }}
> > >
> > > ##
> > >
> > > > +
> > > > ##
> > > > # @cxl-inject-poison:
> > > > #
> > > >
> > >
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 8/8] hw/cxl/events: Add in inject general media event
2022-12-22 4:24 ` [PATCH v2 8/8] hw/cxl/events: Add in inject general media event Ira Weiny
2023-01-03 18:07 ` Jonathan Cameron via
@ 2023-01-12 15:34 ` Jonathan Cameron via
1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-12 15:34 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Wed, 21 Dec 2022 20:24:38 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> To facilitate testing provide a QMP command to inject a general media
> event. The event can be added to the log specified.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Hi Ira,
One thing inline that kind of came out of Philippe's review of
the earlier cleanup patches.
>
> ---
> Changes from RFC:
> Add all fields for this event
> irq happens automatically when log transitions from 0 to 1
> ---
> hw/mem/cxl_type3.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
> hw/mem/cxl_type3_stubs.c | 8 ++++
> include/hw/cxl/cxl_events.h | 20 ++++++++++
> qapi/cxl.json | 25 ++++++++++++
> 4 files changed, 146 insertions(+)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index a43949cab120..bedd09e500ba 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -916,6 +916,99 @@ static CXLPoisonList *get_poison_list(CXLType3Dev *ct3d)
> return &ct3d->poison_list;
> }
>
> +static void cxl_assign_event_header(struct cxl_event_record_hdr *hdr,
> + QemuUUID *uuid, uint8_t flags,
make that const QemuUUID *uuid
and ...
> + uint8_t length)
> +{
> + hdr->flags[0] = flags;
> + hdr->length = length;
> + memcpy(&hdr->id, uuid, sizeof(hdr->id));
> + hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +}
> +
static const
> +QemuUUID gen_media_uuid = {
> + .data = UUID(0xfbcd0a77, 0xc260, 0x417f,
> + 0x85, 0xa9, 0x08, 0x8b, 0x16, 0x21, 0xeb, 0xa6),
> +};
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands
2022-12-22 4:24 ` [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands Ira Weiny
2023-01-04 18:07 ` Jonathan Cameron via
@ 2023-01-24 17:04 ` Jonathan Cameron via
1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Cameron via @ 2023-01-24 17:04 UTC (permalink / raw)
To: Ira Weiny
Cc: Michael Tsirkin, Ben Widawsky, qemu-devel, linux-cxl,
Peter Maydell
On Wed, 21 Dec 2022 20:24:35 -0800
Ira Weiny <ira.weiny@intel.com> wrote:
> CXL testing is benefited from an artificial event log injection
> mechanism.
>
> Add an event log infrastructure to insert, get, and clear events from
> the various logs available on a device.
>
> Replace the stubbed out CXL Get/Clear Event mailbox commands with
> commands that operate on the new infrastructure.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Hi Ira,
Before I forget.
In similar fashion to discussion on poison overflow timestamps,
the timestamping in here should take into account the
set timestamp command results so should look like the code
in cmd_timestamp_get()
https://elixir.bootlin.com/qemu/latest/source/hw/cxl/cxl-mailbox-utils.c#L165
One other trivial comment inline.
Not sure if you are going to get back to these. I'm happy to just hack these
changes in if that is easier for you.
Thanks,
Jonathan
>
> ---
> Change from RFC:
> Process multiple records per Get/Set per the spec
> Rework all the calls to be within events.c
> Add locking around the event logs to ensure that the log
> integrity is maintained
> ---
>
> +/*
> + * return if an interrupt should be generated as a result of inserting this
> + * event.
> + */
> +bool cxl_event_insert(CXLDeviceState *cxlds,
> + enum cxl_event_log_type log_type,
> + struct cxl_event_record_raw *event)
> +{
> + uint64_t time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
As noted, this needs to be more complex to take into account that host
and device timestamp base can be different (or it might not
be set at all for the device yet).
> + struct cxl_event_log *log;
> + CXLEvent *entry;
> +
> + if (log_type >= CXL_EVENT_TYPE_MAX) {
> + return false;
> + }
> +
> + log = &cxlds->event_logs[log_type];
> +
> + QEMU_LOCK_GUARD(&log->lock);
> +
> + if (cxl_event_count(log) >= CXL_TEST_EVENT_OVERFLOW) {
> + if (log->overflow_err_count == 0) {
> + log->first_overflow_timestamp = time;
> + }
> + log->overflow_err_count++;
> + log->last_overflow_timestamp = time;
> + return false;
> + }
> +
> + entry = g_new0(CXLEvent, 1);
> + if (!entry) {
No need to check. g_new0 failure results in application termination.
https://libsoup.org/glib/glib-Memory-Allocation.html
(this got pointed out to me the other day in an internal code review!)
> + error_report("Failed to allocate memory for event log entry");
> + return false;
> + }
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-01-24 17:05 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-22 4:24 [PATCH v2 0/8] QEMU CXL Provide mock CXL events and irq support Ira Weiny
2022-12-22 4:24 ` [PATCH v2 1/8] qemu/bswap: Add const_le64() Ira Weiny
2022-12-22 4:24 ` [PATCH v2 2/8] qemu/uuid: Add UUID static initializer Ira Weiny
2022-12-22 4:24 ` [PATCH v2 3/8] hw/cxl/mailbox: Use new UUID network order define for cel_uuid Ira Weiny
2023-01-03 16:30 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 4/8] hw/cxl/events: Add event status register Ira Weiny
2023-01-03 16:36 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 5/8] hw/cxl/events: Wire up get/clear event mailbox commands Ira Weiny
2023-01-04 18:07 ` Jonathan Cameron via
2023-01-24 17:04 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 6/8] hw/cxl/events: Add event interrupt support Ira Weiny
2022-12-22 4:24 ` [PATCH v2 7/8] bswap: Add the ability to store to an unaligned 24 bit field Ira Weiny
2023-01-03 17:14 ` Jonathan Cameron via
2022-12-22 4:24 ` [PATCH v2 8/8] hw/cxl/events: Add in inject general media event Ira Weiny
2023-01-03 18:07 ` Jonathan Cameron via
2023-01-09 19:15 ` Ira Weiny
2023-01-10 15:38 ` Jonathan Cameron via
2023-01-11 14:05 ` Jonathan Cameron via
2023-01-12 15:34 ` Jonathan Cameron 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).