Re: [PATCH 3/5] cxl/device: Handle Media Disabled state

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Tue, 18 Apr 2023 10:23:35 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> Ensure that both memory device read/writes and mailbox commands
> can deal with the device media being disabled - per CXL 3.0 specs
> 8.2.9.8.5.1 - Sanitize. Disabled semantics here are strictly 11b,
> however noting that if the media is not in the ready state (01b),
> user data is not accessible.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  hw/cxl/cxl-mailbox-utils.c  | 15 +++++++++++++++
>  hw/mem/cxl_type3.c          | 10 ++++++++++
>  include/hw/cxl/cxl_device.h | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 224bfdb4bfca..e0e20bc3a2bb 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -891,6 +891,21 @@ void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
>          if (len == cxl_cmd->in || cxl_cmd->in == ~0) {
>              cxl_cmd->payload = cxl_dstate->mbox_reg_state +
>                  A_CXL_DEV_CMD_PAYLOAD;
> +
> +            if (cxl_dev_media_disabled(cxl_dstate)) {
> +                if (h == cmd_events_get_records ||
> +                    h == cmd_logs_get_log ||
> +                    h == cmd_ccls_get_partition_info ||
> +                    h == cmd_ccls_get_lsa ||
> +                    h == cmd_ccls_set_lsa ||
> +                    h == cmd_media_get_poison_list ||
> +                    h == cmd_media_inject_poison ||
> +                    h == cmd_media_clear_poison) {
> +                        ret = CXL_MBOX_MEDIA_DISABLED;
> +                        goto done;
> +                }
> +            }
> +
>              /* Only one bg command at a time */
>              if ((cxl_cmd->effect & BACKGROUND_OPERATION) &&
>                  cxl_dstate->bg.runtime > 0) {
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 801f72c5fcae..707bdc263f03 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -12,6 +12,7 @@
>  #include "qemu/pmem.h"
>  #include "qemu/range.h"
>  #include "qemu/rcu.h"
> +#include "qemu/guest-random.h"
>  #include "sysemu/hostmem.h"
>  #include "sysemu/numa.h"
>  #include "hw/cxl/cxl.h"
> @@ -989,6 +990,11 @@ MemTxResult cxl_type3_read(PCIDevice *d, hwaddr host_addr, uint64_t *data,
>      if (res) {
>          return MEMTX_ERROR;
>      }
> +    /* all memory reads will return random values */

Reference for this would be good.  Random is expensive if we can get
away with a constant value.

> +    if (cxl_dev_media_disabled(&CXL_TYPE3(d)->cxl_dstate)) {
> +        qemu_guest_getrandom_nofail(data, size);
> +        return MEMTX_OK;
> +    }
>  
>      return address_space_read(as, dpa_offset, attrs, data, size);
>  }
> @@ -1005,6 +1011,10 @@ MemTxResult cxl_type3_write(PCIDevice *d, hwaddr host_addr, uint64_t data,
>      if (res) {
>          return MEMTX_ERROR;
>      }
> +    /* memory writes to the device will have no effect */
> +    if (cxl_dev_media_disabled(&CXL_TYPE3(d)->cxl_dstate)) {
> +        return MEMTX_OK;
> +    }
>  
>      return address_space_write(as, dpa_offset, attrs, &data, size);
>  }
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index f986651b6ead..3ef7a7cded95 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -346,6 +346,39 @@ REG64(CXL_MEM_DEV_STS, 0)
>      FIELD(CXL_MEM_DEV_STS, MBOX_READY, 4, 1)
>      FIELD(CXL_MEM_DEV_STS, RESET_NEEDED, 5, 3)
>  
> +static inline void cxl_dev_media_toggle(CXLDeviceState *cxl_dstate, int val)
> +{
> +    uint64_t reg;
> +
> +    reg = ldq_le_p(cxl_dstate->mbox_reg_state64 + R_CXL_MEM_DEV_STS);
> +    if (FIELD_EX64(reg, CXL_MEM_DEV_STS, MEDIA_STATUS) == val) {
> +        return;
> +    }
> +
> +    reg = FIELD_DP64(0, CXL_MEM_DEV_STS, MEDIA_STATUS, val);
> +    reg = FIELD_DP64(reg, CXL_MEM_DEV_STS, MBOX_READY, 0x1);
> +
> +    stq_le_p(&cxl_dstate->mbox_reg_state64 + R_CXL_MEM_DEV_STS, reg);

Why & ?

> +}
> +
> +static inline void cxl_dev_media_enable(CXLDeviceState *cxl_dstate)
> +{
> +    cxl_dev_media_toggle(cxl_dstate, 0x1);
> +}
> +
> +static inline void cxl_dev_media_disable(CXLDeviceState *cxl_dstate)
> +{
> +    cxl_dev_media_toggle(cxl_dstate, 0x3);
> +}
> +
> +static inline bool cxl_dev_media_disabled(CXLDeviceState *cxl_dstate)
> +{
> +    uint64_t reg;
> +
> +    reg = ldq_le_p(cxl_dstate->mbox_reg_state64 + R_CXL_MEM_DEV_STS);
> +    return unlikely(FIELD_EX64(reg, CXL_MEM_DEV_STS, MEDIA_STATUS) == 0x3);
> +}
> +
>  typedef struct CXLError {
>      QTAILQ_ENTRY(CXLError) node;
>      int type; /* Error code as per FE definition */