Re: [RFC PATCH v3 07/31] hw/cxl/device: Add cheap EVENTS implementation (8.2.9.1)

Jonathan Cameron
2021-02-02T20:02:33Z

On Mon, 1 Feb 2021 16:59:24 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> Using the previously implemented stubbed helpers, it is now possible to
> easily add the missing, required commands to the implementation.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
comment inline. Otherwise LGTM.
> ---
> hw/cxl/cxl-mailbox-utils.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 466055b01a..7c939a1851 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -37,6 +37,14 @@
> * a register interface that already deals with it.
> */
>
> +enum {
> + EVENTS = 0x01,
> + #define GET_RECORDS 0x0
> + #define CLEAR_RECORDS 0x1
> + #define GET_INTERRUPT_POLICY 0x2
> + #define SET_INTERRUPT_POLICY 0x3
> +};
> +
> /* 8.2.8.4.5.1 Command Return Codes */
> typedef enum {
> CXL_MBOX_SUCCESS = 0x0,
> @@ -105,10 +113,23 @@ struct cxl_cmd {
> 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);

Ideally add section reference comments for these. Makes my life a tiny
bit easier as a reviewer!

> +define_mailbox_handler_nop(EVENTS_SET_INTERRUPT_POLICY);
> +
> +#define IMMEDIATE_CONFIG_CHANGE (1 << 1)
> +#define IMMEDIATE_LOG_CHANGE (1 << 4)
> +
> #define CXL_CMD(s, c, in, cel_effect) \
> [s][c] = { stringify(s##_##c), cmd_##s##_##c, in, cel_effect }
>
> -static struct cxl_cmd cxl_cmd_set[256][256] = {};
> +static struct cxl_cmd cxl_cmd_set[256][256] = {
> + CXL_CMD(EVENTS, GET_RECORDS, 1, 0),
> + CXL_CMD(EVENTS, CLEAR_RECORDS, ~0, IMMEDIATE_LOG_CHANGE),
> + CXL_CMD(EVENTS, GET_INTERRUPT_POLICY, 0, 0),
> + CXL_CMD(EVENTS, SET_INTERRUPT_POLICY, 4, IMMEDIATE_CONFIG_CHANGE),
> +};
>
> #undef CXL_CMD
>