From: Jonathan Cameron via <qemu-devel@nongnu.org> To: <qemu-devel@nongnu.org>, <linux-cxl@vger.kernel.org>, Michael Tsirkin <mst@redhat.com>, Michael Tokarev <mjt@tls.msk.ru> Cc: linuxarm@huawei.com, "Fan Ni" <fan.ni@samsung.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Gregory Price" <gregory.price@memverge.com>, "Davidlohr Bueso" <dave@stgolabs.net>, "Klaus Jensen" <its@irrelevant.dk>, "Corey Minyard" <cminyard@mvista.com> Subject: [PATCH v2 04/17] hw/cxl/mbox: Generalize the CCI command processing Date: Mon, 23 Oct 2023 17:07:53 +0100 [thread overview] Message-ID: <20231023160806.13206-5-Jonathan.Cameron@huawei.com> (raw) In-Reply-To: <20231023160806.13206-1-Jonathan.Cameron@huawei.com> By moving the parts of the mailbox command handling that are CCI type specific out to the caller, make the main handling code generic. Rename it to cxl_process_cci_message() to reflect this new generality. Change the type3 mailbox handling (reused shortly for the switch mailbox CCI) to take a snapshot of the mailbox input data rather than operating on it in place. This reduces the chance of bugs due to aliasing going forwars. Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- v2: Drop Fan RB as significant changes as follow on from the aliasing avoidance change. Also cleared out some bg operation stuff that was here as result of a misshandled patch reorder. --- include/hw/cxl/cxl_device.h | 5 +++- hw/cxl/cxl-device-utils.c | 44 +++++++++++++++++++++++++++++++- hw/cxl/cxl-mailbox-utils.c | 51 ++++++++----------------------------- 3 files changed, 57 insertions(+), 43 deletions(-) diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h index 779ca85319..6f8040b5ff 100644 --- a/include/hw/cxl/cxl_device.h +++ b/include/hw/cxl/cxl_device.h @@ -260,7 +260,10 @@ CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MEMORY_DEVICE, void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max); void cxl_init_cci(CXLCCI *cci, size_t payload_max); -void cxl_process_mailbox(CXLCCI *cci); +int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd, + size_t len_in, uint8_t *pl_in, + size_t *len_out, uint8_t *pl_out, + bool *bg_started); #define cxl_device_cap_init(dstate, reg, cap_id, ver) \ do { \ diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c index 327949a805..eb86634250 100644 --- a/hw/cxl/cxl-device-utils.c +++ b/hw/cxl/cxl-device-utils.c @@ -157,7 +157,49 @@ static void mailbox_reg_write(void *opaque, hwaddr offset, uint64_t value, if (ARRAY_FIELD_EX32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL, DOORBELL)) { - cxl_process_mailbox(cci); + uint64_t command_reg = + cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD]; + uint8_t cmd_set = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, + COMMAND_SET); + uint8_t cmd = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND); + size_t len_in = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, LENGTH); + uint8_t *pl = cxl_dstate->mbox_reg_state + A_CXL_DEV_CMD_PAYLOAD; + /* + * Copy taken to avoid need for individual command handlers to care + * about aliasing. + */ + g_autofree uint8_t *pl_in_copy = NULL; + size_t len_out = 0; + uint64_t status_reg; + bool bg_started = false; + int rc; + + pl_in_copy = g_memdup2(pl, len_in); + if (len_in == 0 || pl_in_copy) { + /* Avoid stale data - including from earlier cmds */ + memset(pl, 0, CXL_MAILBOX_MAX_PAYLOAD_SIZE); + rc = cxl_process_cci_message(cci, cmd_set, cmd, len_in, pl_in_copy, + &len_out, pl, &bg_started); + } else { + rc = CXL_MBOX_INTERNAL_ERROR; + } + + /* Set bg and the return code */ + status_reg = FIELD_DP64(0, CXL_DEV_MAILBOX_STS, BG_OP, + bg_started ? 1 : 0); + status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, ERRNO, rc); + /* Set the return length */ + command_reg = FIELD_DP64(0, CXL_DEV_MAILBOX_CMD, COMMAND_SET, cmd_set); + command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, + COMMAND, cmd); + command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, + LENGTH, len_out); + + cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD] = command_reg; + cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_STS] = status_reg; + /* Tell the host we're done */ + ARRAY_FIELD_DP32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL, + DOORBELL, 0); } } diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 5484dfbbf1..239acc659d 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -754,58 +754,27 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { cmd_media_clear_poison, 72, 0 }, }; -void cxl_process_mailbox(CXLCCI *cci) +int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd, + size_t len_in, uint8_t *pl_in, size_t *len_out, + uint8_t *pl_out, bool *bg_started) { - uint16_t ret = CXL_MBOX_SUCCESS; const struct cxl_cmd *cxl_cmd; - uint64_t status_reg = 0; opcode_handler h; - CXLDeviceState *cxl_dstate = &CXL_TYPE3(cci->d)->cxl_dstate; - uint64_t command_reg = cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD]; - - uint8_t set = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND_SET); - uint8_t cmd = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND); - uint16_t len_in = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, LENGTH); - uint8_t *pl = cxl_dstate->mbox_reg_state + A_CXL_DEV_CMD_PAYLOAD; - /* - * Copy taken to avoid need for individual command handlers to care - * about aliasing. - */ - g_autofree uint8_t *pl_in_copy = NULL; - size_t len_out = 0; - pl_in_copy = g_memdup2(pl, len_in); - /* Avoid stale data - including from earlier commands */ - memset(pl, 0, CXL_MAILBOX_MAX_PAYLOAD_SIZE); + *len_out = 0; cxl_cmd = &cci->cxl_cmd_set[set][cmd]; h = cxl_cmd->handler; - if (h) { - if (len_in == cxl_cmd->in || cxl_cmd->in == ~0) { - ret = (*h)(cxl_cmd, pl, len_in, pl, &len_out, cci); - assert(len_out <= cci->payload_max); - } else { - ret = CXL_MBOX_INVALID_PAYLOAD_LENGTH; - } - } else { + if (!h) { qemu_log_mask(LOG_UNIMP, "Command %04xh not implemented\n", set << 8 | cmd); - ret = CXL_MBOX_UNSUPPORTED; + return CXL_MBOX_UNSUPPORTED; } - /* Set the return code */ - status_reg = FIELD_DP64(0, CXL_DEV_MAILBOX_STS, ERRNO, ret); - - /* Set the return length */ - command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND_SET, 0); - command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND, 0); - command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, LENGTH, len_out); - - cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD] = command_reg; - cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_STS] = status_reg; + if (len_in != cxl_cmd->in && cxl_cmd->in != ~0) { + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; + } - /* Tell the host we're done */ - ARRAY_FIELD_DP32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL, - DOORBELL, 0); + return (*h)(cxl_cmd, pl_in, len_in, pl_out, len_out, cci); } void cxl_init_cci(CXLCCI *cci, size_t payload_max) -- 2.39.2
WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <Jonathan.Cameron@huawei.com> To: <qemu-devel@nongnu.org>, <linux-cxl@vger.kernel.org>, Michael Tsirkin <mst@redhat.com>, Michael Tokarev <mjt@tls.msk.ru> Cc: linuxarm@huawei.com, "Fan Ni" <fan.ni@samsung.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Gregory Price" <gregory.price@memverge.com>, "Davidlohr Bueso" <dave@stgolabs.net>, "Klaus Jensen" <its@irrelevant.dk>, "Corey Minyard" <cminyard@mvista.com> Subject: [PATCH v2 04/17] hw/cxl/mbox: Generalize the CCI command processing Date: Mon, 23 Oct 2023 17:07:53 +0100 [thread overview] Message-ID: <20231023160806.13206-5-Jonathan.Cameron@huawei.com> (raw) Message-ID: <20231023160753._0iB2I-SShJWzmwhil-mhh7GXcmiaQLiRZXzhjZtHS8@z> (raw) In-Reply-To: <20231023160806.13206-1-Jonathan.Cameron@huawei.com> By moving the parts of the mailbox command handling that are CCI type specific out to the caller, make the main handling code generic. Rename it to cxl_process_cci_message() to reflect this new generality. Change the type3 mailbox handling (reused shortly for the switch mailbox CCI) to take a snapshot of the mailbox input data rather than operating on it in place. This reduces the chance of bugs due to aliasing going forwars. Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- v2: Drop Fan RB as significant changes as follow on from the aliasing avoidance change. Also cleared out some bg operation stuff that was here as result of a misshandled patch reorder. --- include/hw/cxl/cxl_device.h | 5 +++- hw/cxl/cxl-device-utils.c | 44 +++++++++++++++++++++++++++++++- hw/cxl/cxl-mailbox-utils.c | 51 ++++++++----------------------------- 3 files changed, 57 insertions(+), 43 deletions(-) diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h index 779ca85319..6f8040b5ff 100644 --- a/include/hw/cxl/cxl_device.h +++ b/include/hw/cxl/cxl_device.h @@ -260,7 +260,10 @@ CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MEMORY_DEVICE, void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max); void cxl_init_cci(CXLCCI *cci, size_t payload_max); -void cxl_process_mailbox(CXLCCI *cci); +int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd, + size_t len_in, uint8_t *pl_in, + size_t *len_out, uint8_t *pl_out, + bool *bg_started); #define cxl_device_cap_init(dstate, reg, cap_id, ver) \ do { \ diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c index 327949a805..eb86634250 100644 --- a/hw/cxl/cxl-device-utils.c +++ b/hw/cxl/cxl-device-utils.c @@ -157,7 +157,49 @@ static void mailbox_reg_write(void *opaque, hwaddr offset, uint64_t value, if (ARRAY_FIELD_EX32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL, DOORBELL)) { - cxl_process_mailbox(cci); + uint64_t command_reg = + cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD]; + uint8_t cmd_set = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, + COMMAND_SET); + uint8_t cmd = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND); + size_t len_in = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, LENGTH); + uint8_t *pl = cxl_dstate->mbox_reg_state + A_CXL_DEV_CMD_PAYLOAD; + /* + * Copy taken to avoid need for individual command handlers to care + * about aliasing. + */ + g_autofree uint8_t *pl_in_copy = NULL; + size_t len_out = 0; + uint64_t status_reg; + bool bg_started = false; + int rc; + + pl_in_copy = g_memdup2(pl, len_in); + if (len_in == 0 || pl_in_copy) { + /* Avoid stale data - including from earlier cmds */ + memset(pl, 0, CXL_MAILBOX_MAX_PAYLOAD_SIZE); + rc = cxl_process_cci_message(cci, cmd_set, cmd, len_in, pl_in_copy, + &len_out, pl, &bg_started); + } else { + rc = CXL_MBOX_INTERNAL_ERROR; + } + + /* Set bg and the return code */ + status_reg = FIELD_DP64(0, CXL_DEV_MAILBOX_STS, BG_OP, + bg_started ? 1 : 0); + status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, ERRNO, rc); + /* Set the return length */ + command_reg = FIELD_DP64(0, CXL_DEV_MAILBOX_CMD, COMMAND_SET, cmd_set); + command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, + COMMAND, cmd); + command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, + LENGTH, len_out); + + cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD] = command_reg; + cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_STS] = status_reg; + /* Tell the host we're done */ + ARRAY_FIELD_DP32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL, + DOORBELL, 0); } } diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 5484dfbbf1..239acc659d 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -754,58 +754,27 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { cmd_media_clear_poison, 72, 0 }, }; -void cxl_process_mailbox(CXLCCI *cci) +int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd, + size_t len_in, uint8_t *pl_in, size_t *len_out, + uint8_t *pl_out, bool *bg_started) { - uint16_t ret = CXL_MBOX_SUCCESS; const struct cxl_cmd *cxl_cmd; - uint64_t status_reg = 0; opcode_handler h; - CXLDeviceState *cxl_dstate = &CXL_TYPE3(cci->d)->cxl_dstate; - uint64_t command_reg = cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD]; - - uint8_t set = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND_SET); - uint8_t cmd = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND); - uint16_t len_in = FIELD_EX64(command_reg, CXL_DEV_MAILBOX_CMD, LENGTH); - uint8_t *pl = cxl_dstate->mbox_reg_state + A_CXL_DEV_CMD_PAYLOAD; - /* - * Copy taken to avoid need for individual command handlers to care - * about aliasing. - */ - g_autofree uint8_t *pl_in_copy = NULL; - size_t len_out = 0; - pl_in_copy = g_memdup2(pl, len_in); - /* Avoid stale data - including from earlier commands */ - memset(pl, 0, CXL_MAILBOX_MAX_PAYLOAD_SIZE); + *len_out = 0; cxl_cmd = &cci->cxl_cmd_set[set][cmd]; h = cxl_cmd->handler; - if (h) { - if (len_in == cxl_cmd->in || cxl_cmd->in == ~0) { - ret = (*h)(cxl_cmd, pl, len_in, pl, &len_out, cci); - assert(len_out <= cci->payload_max); - } else { - ret = CXL_MBOX_INVALID_PAYLOAD_LENGTH; - } - } else { + if (!h) { qemu_log_mask(LOG_UNIMP, "Command %04xh not implemented\n", set << 8 | cmd); - ret = CXL_MBOX_UNSUPPORTED; + return CXL_MBOX_UNSUPPORTED; } - /* Set the return code */ - status_reg = FIELD_DP64(0, CXL_DEV_MAILBOX_STS, ERRNO, ret); - - /* Set the return length */ - command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND_SET, 0); - command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, COMMAND, 0); - command_reg = FIELD_DP64(command_reg, CXL_DEV_MAILBOX_CMD, LENGTH, len_out); - - cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_CMD] = command_reg; - cxl_dstate->mbox_reg_state64[R_CXL_DEV_MAILBOX_STS] = status_reg; + if (len_in != cxl_cmd->in && cxl_cmd->in != ~0) { + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; + } - /* Tell the host we're done */ - ARRAY_FIELD_DP32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL, - DOORBELL, 0); + return (*h)(cxl_cmd, pl_in, len_in, pl_out, len_out, cci); } void cxl_init_cci(CXLCCI *cci, size_t payload_max) -- 2.39.2
next prev parent reply other threads:[~2023-10-23 16:10 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-10-23 16:07 [PATCH v2 00/17] QEMU: CXL mailbox rework and features (Part 1) Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 01/17] hw/cxl/mbox: Pull the payload out of struct cxl_cmd and make instances constant Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 02/17] hw/cxl/mbox: Split mailbox command payload into separate input and output Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-24 16:52 ` fan 2023-10-23 16:07 ` [PATCH v2 03/17] hw/cxl/mbox: Pull the CCI definition out of the CXLDeviceState Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-24 17:02 ` fan 2023-10-23 16:07 ` Jonathan Cameron via [this message] 2023-10-23 16:07 ` [PATCH v2 04/17] hw/cxl/mbox: Generalize the CCI command processing Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 05/17] hw/pci-bridge/cxl_upstream: Move defintion of device to header Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 06/17] hw/cxl: Add a switch mailbox CCI function Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 07/17] hw/cxl/mbox: Add Information and Status / Identify command Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 08/17] hw/cxl/mbox: Add Physical Switch " Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 09/17] hw/pci-bridge/cxl_downstream: Set default link width and link speed Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:07 ` [PATCH v2 10/17] hw/cxl: Implement Physical Ports status retrieval Jonathan Cameron via 2023-10-23 16:07 ` Jonathan Cameron 2023-10-23 16:08 ` [PATCH v2 11/17] hw/cxl/mbox: Add support for background operations Jonathan Cameron via 2023-10-23 16:08 ` Jonathan Cameron 2023-10-23 16:08 ` [PATCH v2 12/17] hw/cxl/mbox: Wire up interrupts for background completion Jonathan Cameron via 2023-10-23 16:08 ` Jonathan Cameron 2023-10-23 16:08 ` [PATCH v2 13/17] hw/cxl: Add support for device sanitation Jonathan Cameron via 2023-10-23 16:08 ` Jonathan Cameron 2023-11-13 23:13 ` Hyeonggon Yoo 2023-10-23 16:08 ` [PATCH v2 14/17] hw/cxl/mbox: Add Get Background Operation Status Command Jonathan Cameron via 2023-10-23 16:08 ` Jonathan Cameron 2023-10-23 16:08 ` [PATCH v2 15/17] hw/cxl/type3: Cleanup multiple CXL_TYPE3() calls in read/write functions Jonathan Cameron via 2023-10-23 16:08 ` Jonathan Cameron 2023-10-23 16:08 ` [PATCH v2 16/17] hw/cxl: Add dummy security state get Jonathan Cameron via 2023-10-23 16:08 ` Jonathan Cameron 2023-10-23 16:08 ` [PATCH v2 17/17] hw/cxl: Add tunneled command support to mailbox for switch cci Jonathan Cameron via 2023-10-23 16:08 ` Jonathan Cameron 2023-11-07 10:08 ` [PATCH v2 00/17] QEMU: CXL mailbox rework and features (Part 1) Michael S. Tsirkin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231023160806.13206-5-Jonathan.Cameron@huawei.com \ --to=qemu-devel@nongnu.org \ --cc=Jonathan.Cameron@huawei.com \ --cc=cminyard@mvista.com \ --cc=dave@stgolabs.net \ --cc=fan.ni@samsung.com \ --cc=gregory.price@memverge.com \ --cc=its@irrelevant.dk \ --cc=linux-cxl@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=mjt@tls.msk.ru \ --cc=mst@redhat.com \ --cc=philmd@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).