* Re: [PATCH] hw/cxl: Support aborting background commands
[not found] <20240813221255.179200-1-dave@stgolabs.net>
@ 2024-08-27 15:33 ` Jonathan Cameron via
2024-10-22 3:23 ` Davidlohr Bueso
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron via @ 2024-08-27 15:33 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 13 Aug 2024 15:12:55 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> As of 3.1 spec, background commands can be canceled with a new
> abort command. Implement the support, which is advertised in
> the CEL. No ad-hoc context undoing is necessary as all the
> command logic of the running bg command is done upon completion.
> Arbitrarily, the on-going background cmd will not be aborted if
> already at least 85% done;
>
> A mutex is introduced to stabilize mbox request cancel command vs
> the timer callback being fired scenarios (as well as reading the
> mbox registers). While some operations under critical regions
> may be unnecessary (irq notifying, cmd callbacks), this is not
> a path where performance is important, so simplicity is preferred.
>
> Tested-by: Ajay Joshi <ajay.opensrc@micron.com>
> Reviewed-by: Ajay Joshi <ajay.opensrc@micron.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
+CC qemu-devel
No comments inline and LGTM. I'll queue it on my tree and push
that out on gitlab sometime soonish.
J
> ---
>
> Changes based on the following thread:
> https://lore.kernel.org/linux-cxl/20240729102010.20996-1-ajay.opensrc@micron.com
>
> - Added a mutex (and expanded CCI to have a destroy counterpart).
> An 'initialized' flag is added for correctly handling the reset()
> case.
> - Added the case where cancel is not done.
> - Picked up Ajay's tags but it would be good to re-review/test if
> possible.
>
> hw/cxl/cxl-device-utils.c | 5 ++-
> hw/cxl/cxl-mailbox-utils.c | 63 +++++++++++++++++++++++++++++++++---
> hw/mem/cxl_type3.c | 8 ++++-
> include/hw/cxl/cxl_device.h | 4 +++
> include/hw/cxl/cxl_mailbox.h | 1 +
> 5 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index 035d034f6dd8..1a9779ed8201 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -95,11 +95,14 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
> }
> if (offset == A_CXL_DEV_MAILBOX_STS) {
> uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
> - if (cci->bg.complete_pct) {
> +
> + qemu_mutex_lock(&cci->bg.lock);
> + if (cci->bg.complete_pct == 100 || cci->bg.aborted) {
> status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
> 0);
> cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
> }
> + qemu_mutex_unlock(&cci->bg.lock);
> }
> return cxl_dstate->mbox_reg_state64[offset / size];
> default:
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index b752920ec88a..ff12dfc3dcc4 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -56,6 +56,7 @@ enum {
> INFOSTAT = 0x00,
> #define IS_IDENTIFY 0x1
> #define BACKGROUND_OPERATION_STATUS 0x2
> + #define BACKGROUND_OPERATION_ABORT 0x5
> EVENTS = 0x01,
> #define GET_RECORDS 0x0
> #define CLEAR_RECORDS 0x1
> @@ -624,6 +625,38 @@ static CXLRetCode cmd_infostat_bg_op_sts(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +/* CXL r3.1 Section 8.2.9.1.5: Request Abort Background Operation (Opcode 0005h) */
> +static CXLRetCode cmd_infostat_bg_op_abort(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci)
> +{
> + int bg_set = cci->bg.opcode >> 8;
> + int bg_cmd = cci->bg.opcode & 0xff;
> + const struct cxl_cmd *bg_c = &cci->cxl_cmd_set[bg_set][bg_cmd];
> +
> + if (!(bg_c->effect & CXL_MBOX_BACKGROUND_OPERATION_ABORT)) {
> + return CXL_MBOX_REQUEST_ABORT_NOTSUP;
> + }
> +
> + qemu_mutex_lock(&cci->bg.lock);
> + if (cci->bg.runtime) {
> + /* operation is near complete, let it finish */
> + if (cci->bg.complete_pct < 85) {
> + timer_del(cci->bg.timer);
> + cci->bg.ret_code = CXL_MBOX_ABORTED;
> + cci->bg.starttime = 0;
> + cci->bg.runtime = 0;
> + cci->bg.aborted = true;
> + }
> + }
> + qemu_mutex_unlock(&cci->bg.lock);
> +
> + return CXL_MBOX_SUCCESS;
> +}
> +
> #define CXL_FW_SLOTS 2
> #define CXL_FW_SIZE 0x02000000 /* 32 mb */
>
> @@ -2665,6 +2698,8 @@ static CXLRetCode cmd_dcd_release_dyn_cap(const struct cxl_cmd *cmd,
> }
>
> static const struct cxl_cmd cxl_cmd_set[256][256] = {
> + [INFOSTAT][BACKGROUND_OPERATION_ABORT] = { "BACKGROUND_OPERATION_ABORT",
> + cmd_infostat_bg_op_abort, 0, 0 },
> [EVENTS][GET_RECORDS] = { "EVENTS_GET_RECORDS",
> cmd_events_get_records, 1, 0 },
> [EVENTS][CLEAR_RECORDS] = { "EVENTS_CLEAR_RECORDS",
> @@ -2708,7 +2743,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> [SANITIZE][OVERWRITE] = { "SANITIZE_OVERWRITE", cmd_sanitize_overwrite, 0,
> (CXL_MBOX_IMMEDIATE_DATA_CHANGE |
> CXL_MBOX_SECURITY_STATE_CHANGE |
> - CXL_MBOX_BACKGROUND_OPERATION)},
> + CXL_MBOX_BACKGROUND_OPERATION |
> + CXL_MBOX_BACKGROUND_OPERATION_ABORT)},
> [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE",
> cmd_get_security_state, 0, 0 },
> [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST",
> @@ -2721,7 +2757,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> "MEDIA_AND_POISON_GET_SCAN_MEDIA_CAPABILITIES",
> cmd_media_get_scan_media_capabilities, 16, 0 },
> [MEDIA_AND_POISON][SCAN_MEDIA] = { "MEDIA_AND_POISON_SCAN_MEDIA",
> - cmd_media_scan_media, 17, CXL_MBOX_BACKGROUND_OPERATION },
> + cmd_media_scan_media, 17,
> + (CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT)},
> [MEDIA_AND_POISON][GET_SCAN_MEDIA_RESULTS] = {
> "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
> cmd_media_get_scan_media_results, 0, 0 },
> @@ -2745,6 +2782,8 @@ static const struct cxl_cmd cxl_cmd_set_sw[256][256] = {
> [INFOSTAT][IS_IDENTIFY] = { "IDENTIFY", cmd_infostat_identify, 0, 0 },
> [INFOSTAT][BACKGROUND_OPERATION_STATUS] = { "BACKGROUND_OPERATION_STATUS",
> cmd_infostat_bg_op_sts, 0, 0 },
> + [INFOSTAT][BACKGROUND_OPERATION_ABORT] = { "BACKGROUND_OPERATION_ABORT",
> + cmd_infostat_bg_op_abort, 0, 0 },
> [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set, 8,
> CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
> @@ -2831,6 +2870,7 @@ int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
> cci->bg.opcode = (set << 8) | cmd;
>
> cci->bg.complete_pct = 0;
> + cci->bg.aborted = false;
> cci->bg.ret_code = 0;
>
> now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> @@ -2844,10 +2884,12 @@ int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
> static void bg_timercb(void *opaque)
> {
> CXLCCI *cci = opaque;
> - uint64_t now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> - uint64_t total_time = cci->bg.starttime + cci->bg.runtime;
> + uint64_t now, total_time;
> +
> + qemu_mutex_lock(&cci->bg.lock);
>
> - assert(cci->bg.runtime > 0);
> + now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> + total_time = cci->bg.starttime + cci->bg.runtime;
>
> if (now >= total_time) { /* we are done */
> uint16_t ret = CXL_MBOX_SUCCESS;
> @@ -2899,6 +2941,8 @@ static void bg_timercb(void *opaque)
> msi_notify(pdev, cxl_dstate->mbox_msi_n);
> }
> }
> +
> + qemu_mutex_unlock(&cci->bg.lock);
> }
>
> static void cxl_rebuild_cel(CXLCCI *cci)
> @@ -2927,12 +2971,21 @@ void cxl_init_cci(CXLCCI *cci, size_t payload_max)
> cci->bg.complete_pct = 0;
> cci->bg.starttime = 0;
> cci->bg.runtime = 0;
> + cci->bg.aborted = false;
> cci->bg.timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
> bg_timercb, cci);
> + qemu_mutex_init(&cci->bg.lock);
>
> memset(&cci->fw, 0, sizeof(cci->fw));
> cci->fw.active_slot = 1;
> cci->fw.slot[cci->fw.active_slot - 1] = true;
> + cci->initialized = true;
> +}
> +
> +void cxl_destroy_cci(CXLCCI *cci)
> +{
> + qemu_mutex_destroy(&cci->bg.lock);
> + cci->initialized = false;
> }
>
> static void cxl_copy_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmds)[256])
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 4114163324bd..f04aa58ea85d 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -961,6 +961,7 @@ static void ct3_exit(PCIDevice *pci_dev)
> pcie_aer_exit(pci_dev);
> cxl_doe_cdat_release(cxl_cstate);
> g_free(regs->special_ops);
> + cxl_destroy_cci(&ct3d->cci);
> if (ct3d->dc.host_dc) {
> cxl_destroy_dc_regions(ct3d);
> address_space_destroy(&ct3d->dc.host_dc_as);
> @@ -1209,12 +1210,17 @@ static void ct3d_reset(DeviceState *dev)
> * Bring up an endpoint to target with MCTP over VDM.
> * This device is emulating an MLD with single LD for now.
> */
> + if (ct3d->vdm_fm_owned_ld_mctp_cci.initialized) {
> + cxl_destroy_cci(&ct3d->vdm_fm_owned_ld_mctp_cci);
> + }
> cxl_initialize_t3_fm_owned_ld_mctpcci(&ct3d->vdm_fm_owned_ld_mctp_cci,
> DEVICE(ct3d), DEVICE(ct3d),
> 512); /* Max payload made up */
> + if (ct3d->ld0_cci.initialized) {
> + cxl_destroy_cci(&ct3d->ld0_cci);
> + }
> cxl_initialize_t3_ld_cci(&ct3d->ld0_cci, DEVICE(ct3d), DEVICE(ct3d),
> 512); /* Max payload made up */
> -
> }
>
> static Property ct3_props[] = {
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index e14e56ae4bc2..c0e8d40053db 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -176,10 +176,12 @@ typedef struct CXLCCI {
> uint16_t opcode;
> uint16_t complete_pct;
> uint16_t ret_code; /* Current value of retcode */
> + bool aborted;
> uint64_t starttime;
> /* set by each bg cmd, cleared by the bg_timer when complete */
> uint64_t runtime;
> QEMUTimer *timer;
> + QemuMutex lock; /* serializes mbox abort vs timer cb */
> } bg;
>
> /* firmware update */
> @@ -201,6 +203,7 @@ typedef struct CXLCCI {
> DeviceState *d;
> /* Pointer to the device hosting the protocol conversion */
> DeviceState *intf;
> + bool initialized;
> } CXLCCI;
>
> typedef struct cxl_device_state {
> @@ -316,6 +319,7 @@ void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max);
> void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
> DeviceState *d, size_t payload_max);
> void cxl_init_cci(CXLCCI *cci, size_t payload_max);
> +void cxl_destroy_cci(CXLCCI *cci);
> void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
> size_t payload_max);
> int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
> diff --git a/include/hw/cxl/cxl_mailbox.h b/include/hw/cxl/cxl_mailbox.h
> index beb048052e1b..9008402d1c46 100644
> --- a/include/hw/cxl/cxl_mailbox.h
> +++ b/include/hw/cxl/cxl_mailbox.h
> @@ -14,5 +14,6 @@
> #define CXL_MBOX_IMMEDIATE_LOG_CHANGE (1 << 4)
> #define CXL_MBOX_SECURITY_STATE_CHANGE (1 << 5)
> #define CXL_MBOX_BACKGROUND_OPERATION (1 << 6)
> +#define CXL_MBOX_BACKGROUND_OPERATION_ABORT (1 << 7)
>
> #endif
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/cxl: Support aborting background commands
2024-08-27 15:33 ` [PATCH] hw/cxl: Support aborting background commands Jonathan Cameron via
@ 2024-10-22 3:23 ` Davidlohr Bueso
2024-10-22 17:00 ` Jonathan Cameron via
0 siblings, 1 reply; 6+ messages in thread
From: Davidlohr Bueso @ 2024-10-22 3:23 UTC (permalink / raw)
To: Jonathan Cameron
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
>No comments inline and LGTM. I'll queue it on my tree and push
>that out on gitlab sometime soonish.
I don't see this picked up, which is a good thing atm. While testing
the kernel side, I noticed the following is needed, will send a v2
with it folded in.
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index 1a9779ed8201..0d429b59aafc 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -94,14 +94,15 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
cxl_dstate->mbox_reg_state64[offset / size] = bg_status_reg;
}
if (offset == A_CXL_DEV_MAILBOX_STS) {
+ int bgop;
uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
qemu_mutex_lock(&cci->bg.lock);
- if (cci->bg.complete_pct == 100 || cci->bg.aborted) {
- status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
- 0);
- cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
- }
+ bgop = !(cci->bg.complete_pct == 100 || cci->bg.aborted);
+
+ status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
+ bgop);
+ cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
qemu_mutex_unlock(&cci->bg.lock);
}
return cxl_dstate->mbox_reg_state64[offset / size];
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index d5b084388288..760a8571fda6 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -2731,9 +2731,11 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
[FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
cmd_firmware_update_get_info, 0, 0 },
[FIRMWARE_UPDATE][TRANSFER] = { "FIRMWARE_UPDATE_TRANSFER",
- cmd_firmware_update_transfer, ~0, CXL_MBOX_BACKGROUND_OPERATION },
+ cmd_firmware_update_transfer, ~0,
+ CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
[FIRMWARE_UPDATE][ACTIVATE] = { "FIRMWARE_UPDATE_ACTIVATE",
- cmd_firmware_update_activate, 2, CXL_MBOX_BACKGROUND_OPERATION },
+ cmd_firmware_update_activate, 2,
+ CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
[TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
[TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set,
8, CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
Thanks,
Davidlohr
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/cxl: Support aborting background commands
2024-10-22 3:23 ` Davidlohr Bueso
@ 2024-10-22 17:00 ` Jonathan Cameron via
2025-02-03 15:07 ` Jonathan Cameron via
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron via @ 2024-10-22 17:00 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Mon, 21 Oct 2024 20:23:46 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
> >No comments inline and LGTM. I'll queue it on my tree and push
> >that out on gitlab sometime soonish.
>
> I don't see this picked up, which is a good thing atm. While testing
> the kernel side, I noticed the following is needed, will send a v2
> with it folded in.
Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
I'll pick up your v2 and replace that.
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index 1a9779ed8201..0d429b59aafc 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -94,14 +94,15 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
> cxl_dstate->mbox_reg_state64[offset / size] = bg_status_reg;
> }
> if (offset == A_CXL_DEV_MAILBOX_STS) {
> + int bgop;
> uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
>
> qemu_mutex_lock(&cci->bg.lock);
> - if (cci->bg.complete_pct == 100 || cci->bg.aborted) {
> - status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
> - 0);
> - cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
> - }
> + bgop = !(cci->bg.complete_pct == 100 || cci->bg.aborted);
> +
> + status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
> + bgop);
> + cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
> qemu_mutex_unlock(&cci->bg.lock);
> }
> return cxl_dstate->mbox_reg_state64[offset / size];
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index d5b084388288..760a8571fda6 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -2731,9 +2731,11 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
> cmd_firmware_update_get_info, 0, 0 },
> [FIRMWARE_UPDATE][TRANSFER] = { "FIRMWARE_UPDATE_TRANSFER",
> - cmd_firmware_update_transfer, ~0, CXL_MBOX_BACKGROUND_OPERATION },
> + cmd_firmware_update_transfer, ~0,
> + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> [FIRMWARE_UPDATE][ACTIVATE] = { "FIRMWARE_UPDATE_ACTIVATE",
> - cmd_firmware_update_activate, 2, CXL_MBOX_BACKGROUND_OPERATION },
> + cmd_firmware_update_activate, 2,
> + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set,
> 8, CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
>
> Thanks,
> Davidlohr
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/cxl: Support aborting background commands
2024-10-22 17:00 ` Jonathan Cameron via
@ 2025-02-03 15:07 ` Jonathan Cameron via
2025-02-05 4:47 ` Davidlohr Bueso
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron via @ 2025-02-03 15:07 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 22 Oct 2024 18:00:30 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> On Mon, 21 Oct 2024 20:23:46 -0700
> Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> > On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
> > >No comments inline and LGTM. I'll queue it on my tree and push
> > >that out on gitlab sometime soonish.
> >
> > I don't see this picked up, which is a good thing atm. While testing
> > the kernel side, I noticed the following is needed, will send a v2
> > with it folded in.
>
> Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
> I'll pick up your v2 and replace that.
Hi Davidlohr,
Did I miss v2, or still to send?
Thanks,
Jonathan
> >
> > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> > index 1a9779ed8201..0d429b59aafc 100644
> > --- a/hw/cxl/cxl-device-utils.c
> > +++ b/hw/cxl/cxl-device-utils.c
> > @@ -94,14 +94,15 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
> > cxl_dstate->mbox_reg_state64[offset / size] = bg_status_reg;
> > }
> > if (offset == A_CXL_DEV_MAILBOX_STS) {
> > + int bgop;
> > uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
> >
> > qemu_mutex_lock(&cci->bg.lock);
> > - if (cci->bg.complete_pct == 100 || cci->bg.aborted) {
> > - status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
> > - 0);
> > - cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
> > - }
> > + bgop = !(cci->bg.complete_pct == 100 || cci->bg.aborted);
> > +
> > + status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
> > + bgop);
> > + cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
> > qemu_mutex_unlock(&cci->bg.lock);
> > }
> > return cxl_dstate->mbox_reg_state64[offset / size];
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index d5b084388288..760a8571fda6 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -2731,9 +2731,11 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> > [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
> > cmd_firmware_update_get_info, 0, 0 },
> > [FIRMWARE_UPDATE][TRANSFER] = { "FIRMWARE_UPDATE_TRANSFER",
> > - cmd_firmware_update_transfer, ~0, CXL_MBOX_BACKGROUND_OPERATION },
> > + cmd_firmware_update_transfer, ~0,
> > + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> > [FIRMWARE_UPDATE][ACTIVATE] = { "FIRMWARE_UPDATE_ACTIVATE",
> > - cmd_firmware_update_activate, 2, CXL_MBOX_BACKGROUND_OPERATION },
> > + cmd_firmware_update_activate, 2,
> > + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> > [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> > [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set,
> > 8, CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
> >
> > Thanks,
> > Davidlohr
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/cxl: Support aborting background commands
2025-02-03 15:07 ` Jonathan Cameron via
@ 2025-02-05 4:47 ` Davidlohr Bueso
2025-02-05 17:24 ` Jonathan Cameron via
0 siblings, 1 reply; 6+ messages in thread
From: Davidlohr Bueso @ 2025-02-05 4:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Mon, 03 Feb 2025, Jonathan Cameron wrote:
>On Tue, 22 Oct 2024 18:00:30 +0100
>Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
>
>> On Mon, 21 Oct 2024 20:23:46 -0700
>> Davidlohr Bueso <dave@stgolabs.net> wrote:
>>
>> > On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
>> > >No comments inline and LGTM. I'll queue it on my tree and push
>> > >that out on gitlab sometime soonish.
>> >
>> > I don't see this picked up, which is a good thing atm. While testing
>> > the kernel side, I noticed the following is needed, will send a v2
>> > with it folded in.
>>
>> Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
>> I'll pick up your v2 and replace that.
>
>Hi Davidlohr,
>
>Did I miss v2, or still to send?
Ah, thanks for the reminder, I had forgotten to send it out.
Actually, considering you have been carrying v1 for a while now,
would it not be cleaner/better just rebasing the diff and sending you
that instead of a whole new v2? Otherwise, I'm not sure what to do
when working on your latest 2025-01-24 branch.
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/cxl: Support aborting background commands
2025-02-05 4:47 ` Davidlohr Bueso
@ 2025-02-05 17:24 ` Jonathan Cameron via
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron via @ 2025-02-05 17:24 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 4 Feb 2025 20:47:49 -0800
Davidlohr Bueso <dave@stgolabs.net> wrote:
> On Mon, 03 Feb 2025, Jonathan Cameron wrote:
>
> >On Tue, 22 Oct 2024 18:00:30 +0100
> >Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> >
> >> On Mon, 21 Oct 2024 20:23:46 -0700
> >> Davidlohr Bueso <dave@stgolabs.net> wrote:
> >>
> >> > On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
> >> > >No comments inline and LGTM. I'll queue it on my tree and push
> >> > >that out on gitlab sometime soonish.
> >> >
> >> > I don't see this picked up, which is a good thing atm. While testing
> >> > the kernel side, I noticed the following is needed, will send a v2
> >> > with it folded in.
> >>
> >> Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
> >> I'll pick up your v2 and replace that.
> >
> >Hi Davidlohr,
> >
> >Did I miss v2, or still to send?
>
> Ah, thanks for the reminder, I had forgotten to send it out.
>
> Actually, considering you have been carrying v1 for a while now,
> would it not be cleaner/better just rebasing the diff and sending you
> that instead of a whole new v2? Otherwise, I'm not sure what to do
> when working on your latest 2025-01-24 branch.
Pick point where I'm carrying it and post a patch based on the
next commit up. Just let me know you've done that and all
should be good.
Jonathan
>
> Thanks,
> Davidlohr
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-05 17:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240813221255.179200-1-dave@stgolabs.net>
2024-08-27 15:33 ` [PATCH] hw/cxl: Support aborting background commands Jonathan Cameron via
2024-10-22 3:23 ` Davidlohr Bueso
2024-10-22 17:00 ` Jonathan Cameron via
2025-02-03 15:07 ` Jonathan Cameron via
2025-02-05 4:47 ` Davidlohr Bueso
2025-02-05 17:24 ` 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).