On Fri, 22 Dec 2023 18:00:49 +0900
Hyeonggon Yoo <[email protected]> wrote:
> In the current mdev_reg_read() implementation, it consistently returns
> that the Media Status is Ready (01b). This was fine until commit
> 25a52959f99d ("hw/cxl: Add support for device sanitation") because the
> media was presumed to be ready.
>
> However, as per the CXL 3.0 spec "8.2.9.8.5.1 Sanitize (Opcode 4400h)",
> during sanitation, the Media State should be set to Disabled (11b). The
> mentioned commit correctly sets it to Disabled, but mdev_reg_read()
> still returns Media Status as Ready.
>
> To address this, update mdev_reg_read() to read register values instead
> of returning dummy values.
>
> Fixes: commit 25a52959f99d ("hw/cxl: Add support for device sanitation")
> Reviewed-by: Davidlohr Bueso <[email protected]>
> Signed-off-by: Hyeonggon Yoo <[email protected]>
I've applied this one to my tree. (I'll push that out in a day or two after
tidying up some other outstanding stuff).
Sometime in next week or so I'll send out a set bundling together various
fixes and cleanup with the intent for getting it applied.
Thanks,
Jonathan
> ---
> hw/cxl/cxl-device-utils.c | 17 +++++++++++------
> include/hw/cxl/cxl_device.h | 4 +++-
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index 29de298117..ba3f80e6e7 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -229,12 +229,9 @@ static void mailbox_reg_write(void *opaque, hwaddr
> offset, uint64_t value,
>
> static uint64_t mdev_reg_read(void *opaque, hwaddr offset, unsigned size)
> {
> - uint64_t retval = 0;
> -
> - retval = FIELD_DP64(retval, CXL_MEM_DEV_STS, MEDIA_STATUS, 1);
> - retval = FIELD_DP64(retval, CXL_MEM_DEV_STS, MBOX_READY, 1);
> + CXLDeviceState *cxl_dstate = opaque;
>
> - return retval;
> + return cxl_dstate->mbox_reg_state64[R_CXL_MEM_DEV_STS];
> }
>
> static void ro_reg_write(void *opaque, hwaddr offset, uint64_t value,
> @@ -371,7 +368,15 @@ static void mailbox_reg_init_common(CXLDeviceState
> *cxl_dstate)
> cxl_dstate->mbox_msi_n = msi_n;
> }
>
> -static void memdev_reg_init_common(CXLDeviceState *cxl_dstate) { }
> +static void memdev_reg_init_common(CXLDeviceState *cxl_dstate)
> +{
> + uint64_t memdev_status_reg;
> +
> + memdev_status_reg = FIELD_DP64(0, CXL_MEM_DEV_STS, MEDIA_STATUS, 1);
> + memdev_status_reg = FIELD_DP64(memdev_status_reg, CXL_MEM_DEV_STS,
> + MBOX_READY, 1);
> + cxl_dstate->mbox_reg_state64[R_CXL_MEM_DEV_STS] = memdev_status_reg;
> +}
>
> void cxl_device_register_init_t3(CXLType3Dev *ct3d)
> {
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index b2cb280e16..b318d94b36 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -408,7 +408,9 @@ static inline void __toggle_media(CXLDeviceState
> *cxl_dstate, int val)
> {
> uint64_t dev_status_reg;
>
> - dev_status_reg = FIELD_DP64(0, CXL_MEM_DEV_STS, MEDIA_STATUS, val);
> + dev_status_reg = cxl_dstate->mbox_reg_state64[R_CXL_MEM_DEV_STS];
> + dev_status_reg = FIELD_DP64(dev_status_reg, CXL_MEM_DEV_STS,
> MEDIA_STATUS,
> + val);
> cxl_dstate->mbox_reg_state64[R_CXL_MEM_DEV_STS] = dev_status_reg;
> }
> #define cxl_dev_disable_media(cxlds) \
