diff mbox series

[RFC,6/6] hw/cxl/mailbox: Wire up Get/Set Event Interrupt policy

Message ID 20221010222944.3923556-7-ira.weiny@intel.com (mailing list archive)
State New, archived
Headers show
Series QEMU CXL Provide mock CXL events and irq support | expand

Commit Message

Ira Weiny Oct. 10, 2022, 10:29 p.m. UTC 
From: Ira Weiny <ira.weiny@intel.com>

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.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 hw/cxl/cxl-mailbox-utils.c  | 129 ++++++++++++++++++++++++++++++------
 include/hw/cxl/cxl_events.h |  21 ++++++
 2 files changed, 129 insertions(+), 21 deletions(-)

Comments

Jonathan Cameron Oct. 11, 2022, 10:40 a.m. UTC | #1 
On Mon, 10 Oct 2022 15:29:44 -0700
ira.weiny@intel.com wrote:

> From: Ira Weiny <ira.weiny@intel.com>
> 
> 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.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
A few trivial comments inline.

Thanks,

Jonathan

> ---
>  hw/cxl/cxl-mailbox-utils.c  | 129 ++++++++++++++++++++++++++++++------
>  include/hw/cxl/cxl_events.h |  21 ++++++
>  2 files changed, 129 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index df345f23a30c..52e8804c24ed 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -101,25 +101,6 @@ struct cxl_cmd {
>      uint8_t *payload;
>  };
>  
> -#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 ret_code cmd_events_get_records(struct cxl_cmd *cmd,
>                                         CXLDeviceState *cxlds,
>                                         uint16_t *len)
> @@ -218,6 +199,110 @@ static ret_code cmd_events_clear_records(struct cxl_cmd *cmd,
>      return CXL_MBOX_SUCCESS;
>  }
>  
> +static ret_code cmd_events_get_interrupt_policy(struct cxl_cmd *cmd,
> +                                                CXLDeviceState *cxl_dstate,
> +                                                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 = find_event_log(cxl_dstate, CXL_EVENT_TYPE_INFO);

Less obvious than below case, but again, perhaps a little utility function
to cut down on duplication.

> +    if (log->irq_enabled) {
> +        policy->info_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
> +    }
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_WARN);
> +    if (log->irq_enabled) {
> +        policy->warn_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
> +    }
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FAIL);
> +    if (log->irq_enabled) {
> +        policy->failure_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
> +    }
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FATAL);
> +    if (log->irq_enabled) {
> +        policy->fatal_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
> +    }
> +
> +    log = find_event_log(cxl_dstate, 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 *cxl_dstate,
> +                                                uint16_t *len)
> +{
> +    struct cxl_event_interrupt_policy *policy;
> +    struct cxl_event_log *log;
> +
> +    policy = (struct cxl_event_interrupt_policy *)cmd->payload;
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_INFO);
Maybe a utility function?

	set_int_policy(cxl_dstate, CXL_EVENT_TYPE_INFO,
		       policy->info_settings);
	set_int_policy(cxl_dstate, CXL_EVENT_TYPE_WARN,
		       policy->warn_settings);
etc


> +    if ((policy->info_settings & CXL_EVENT_INT_MODE_MASK) ==
> +                                                    CXL_INT_MSI_MSIX) {
> +        log->irq_enabled = true;
> +        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_INFO];
> +    } else {
> +        log->irq_enabled = false;
> +        log->irq_vec = 0;
> +    }
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_WARN);
> +    if ((policy->warn_settings & CXL_EVENT_INT_MODE_MASK) ==
> +                                                    CXL_INT_MSI_MSIX) {
> +        log->irq_enabled = true;
> +        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_WARN];
> +    } else {
> +        log->irq_enabled = false;
> +        log->irq_vec = 0;
> +    }
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FAIL);
> +    if ((policy->failure_settings & CXL_EVENT_INT_MODE_MASK) ==
> +                                                    CXL_INT_MSI_MSIX) {
> +        log->irq_enabled = true;
> +        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_FAIL];
> +    } else {
> +        log->irq_enabled = false;
> +        log->irq_vec = 0;
> +    }
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FATAL);
> +    if ((policy->fatal_settings & CXL_EVENT_INT_MODE_MASK) ==
> +                                                    CXL_INT_MSI_MSIX) {
> +        log->irq_enabled = true;
> +        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_FATAL];
> +    } else {
> +        log->irq_enabled = false;
> +        log->irq_vec = 0;
> +    }
> +
> +    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_DYNAMIC_CAP);
> +    if ((policy->dyn_cap_settings & CXL_EVENT_INT_MODE_MASK) ==
> +                                                    CXL_INT_MSI_MSIX) {
> +        log->irq_enabled = true;
> +        /* Dynamic Capacity borrows the same vector as info */
> +        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_INFO];
> +    } else {
> +        log->irq_enabled = false;
> +        log->irq_vec = 0;
> +    }
> +
> +    *len = sizeof(*policy);
> +    return CXL_MBOX_SUCCESS;
> +}
> +

...

> diff --git a/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
> index 255111f3dcfb..c121e504a6db 100644
> --- a/include/hw/cxl/cxl_events.h
> +++ b/include/hw/cxl/cxl_events.h
> @@ -170,4 +170,25 @@ struct cxl_event_mem_module {
>      uint8_t reserved[CXL_EVENT_MEM_MOD_RES_SIZE];
>  } QEMU_PACKED;
>  
> +/**
> + * 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,

I guess at somepoint we'll probably want to wire up the INT_FW path.
Job for another day though!

> +    CXL_INT_RES      = 0x03,

Why define the reserved value here?  By definition we won't use it.

> +};
> +#define CXL_EVENT_INT_MODE_MASK 0x3
> +#define CXL_EVENT_INT_SETTING(vector) ((((uint8_t)vector & 0xf) << 4) | CXL_INT_MSI_MSIX)

I probably haven't had enough caffeine yet today, but why the cast given
you are masking to a smaller range?

> +struct cxl_event_interrupt_policy {
> +    uint8_t info_settings;

Can we shorten these to just info, warn, failure, fatal, dyn_cap?
No real loss I think and will help with some of the long lines above.

> +    uint8_t warn_settings;
> +    uint8_t failure_settings;
> +    uint8_t fatal_settings;
> +    uint8_t dyn_cap_settings;
> +} QEMU_PACKED;
> +
>  #endif /* CXL_EVENTS_H */
diff mbox series

Patch

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index df345f23a30c..52e8804c24ed 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -101,25 +101,6 @@  struct cxl_cmd {
     uint8_t *payload;
 };
 
-#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 ret_code cmd_events_get_records(struct cxl_cmd *cmd,
                                        CXLDeviceState *cxlds,
                                        uint16_t *len)
@@ -218,6 +199,110 @@  static ret_code cmd_events_clear_records(struct cxl_cmd *cmd,
     return CXL_MBOX_SUCCESS;
 }
 
+static ret_code cmd_events_get_interrupt_policy(struct cxl_cmd *cmd,
+                                                CXLDeviceState *cxl_dstate,
+                                                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 = find_event_log(cxl_dstate, CXL_EVENT_TYPE_INFO);
+    if (log->irq_enabled) {
+        policy->info_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+    }
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_WARN);
+    if (log->irq_enabled) {
+        policy->warn_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+    }
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FAIL);
+    if (log->irq_enabled) {
+        policy->failure_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+    }
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FATAL);
+    if (log->irq_enabled) {
+        policy->fatal_settings = CXL_EVENT_INT_SETTING(log->irq_vec);
+    }
+
+    log = find_event_log(cxl_dstate, 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 *cxl_dstate,
+                                                uint16_t *len)
+{
+    struct cxl_event_interrupt_policy *policy;
+    struct cxl_event_log *log;
+
+    policy = (struct cxl_event_interrupt_policy *)cmd->payload;
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_INFO);
+    if ((policy->info_settings & CXL_EVENT_INT_MODE_MASK) ==
+                                                    CXL_INT_MSI_MSIX) {
+        log->irq_enabled = true;
+        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_INFO];
+    } else {
+        log->irq_enabled = false;
+        log->irq_vec = 0;
+    }
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_WARN);
+    if ((policy->warn_settings & CXL_EVENT_INT_MODE_MASK) ==
+                                                    CXL_INT_MSI_MSIX) {
+        log->irq_enabled = true;
+        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_WARN];
+    } else {
+        log->irq_enabled = false;
+        log->irq_vec = 0;
+    }
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FAIL);
+    if ((policy->failure_settings & CXL_EVENT_INT_MODE_MASK) ==
+                                                    CXL_INT_MSI_MSIX) {
+        log->irq_enabled = true;
+        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_FAIL];
+    } else {
+        log->irq_enabled = false;
+        log->irq_vec = 0;
+    }
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_FATAL);
+    if ((policy->fatal_settings & CXL_EVENT_INT_MODE_MASK) ==
+                                                    CXL_INT_MSI_MSIX) {
+        log->irq_enabled = true;
+        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_FATAL];
+    } else {
+        log->irq_enabled = false;
+        log->irq_vec = 0;
+    }
+
+    log = find_event_log(cxl_dstate, CXL_EVENT_TYPE_DYNAMIC_CAP);
+    if ((policy->dyn_cap_settings & CXL_EVENT_INT_MODE_MASK) ==
+                                                    CXL_INT_MSI_MSIX) {
+        log->irq_enabled = true;
+        /* Dynamic Capacity borrows the same vector as info */
+        log->irq_vec = cxl_dstate->event_vector[CXL_EVENT_TYPE_INFO];
+    } else {
+        log->irq_enabled = false;
+        log->irq_vec = 0;
+    }
+
+    *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,
@@ -490,9 +575,11 @@  static struct cxl_cmd cxl_cmd_set[256][256] = {
     [EVENTS][CLEAR_RECORDS] = { "EVENTS_CLEAR_RECORDS",
         cmd_events_clear_records, 8, 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,
+                                      sizeof(struct cxl_event_interrupt_policy),
+                                      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/include/hw/cxl/cxl_events.h b/include/hw/cxl/cxl_events.h
index 255111f3dcfb..c121e504a6db 100644
--- a/include/hw/cxl/cxl_events.h
+++ b/include/hw/cxl/cxl_events.h
@@ -170,4 +170,25 @@  struct cxl_event_mem_module {
     uint8_t reserved[CXL_EVENT_MEM_MOD_RES_SIZE];
 } QEMU_PACKED;
 
+/**
+ * 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;
+
 #endif /* CXL_EVENTS_H */