On Sun, 23 Jan 2022 16:31:13 -0800
Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> From: Ben Widawsky <ben.widawsky@xxxxxxxxx>
>
> CXL 2.0 8.1.3.8.2 states:
>
> Memory_Active: When set, indicates that the CXL Range 1 memory is
> fully initialized and available for software use. Must be set within
> Range 1. Memory_Active_Timeout of deassertion of reset to CXL device
> if CXL.mem HwInit Mode=1
>
> Unfortunately, Memory_Active can take quite a long time depending on
> media size (up to 256s per 2.0 spec). Provide a callback for the
> eventual establishment of CXL.mem operations via the 'cxl_mem' driver
> the 'struct cxl_memdev'. The implementation waits for 60s by default for
> now and can be overridden by the mbox_ready_time module parameter.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@xxxxxxxxx>
> [djbw: switch to sleeping wait]
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
Not being a memory device person, I'm not sure whether my query below
is realistic but I worry a little that minimum sleep if not immediately
ready of 1 second is a bit long.
Perhaps that's something to optimize once there are a large number
of implementations to assess if it is worth bothering or not.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 5c43886dc2af..513cb0e2a70a 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -49,7 +49,7 @@
> static unsigned short mbox_ready_timeout = 60;
> module_param(mbox_ready_timeout, ushort, 0600);
> MODULE_PARM_DESC(mbox_ready_timeout,
> - "seconds to wait for mailbox ready status");
> + "seconds to wait for mailbox ready / memory active status");
>
> static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
> {
> @@ -417,6 +417,51 @@ static int wait_for_valid(struct cxl_dev_state *cxlds)
> return -ETIMEDOUT;
> }
>
> +/*
> + * Wait up to @mbox_ready_timeout for the device to report memory
> + * active.
> + */
> +static int wait_for_media_ready(struct cxl_dev_state *cxlds)
> +{
> + struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> + int d = cxlds->cxl_dvsec;
> + bool active = false;
> + u64 md_status;
> + int rc, i;
> +
> + rc = wait_for_valid(cxlds);
> + if (rc)
> + return rc;
> +
> + for (i = mbox_ready_timeout; i; i--) {
> + u32 temp;
> + int rc;
> +
> + rc = pci_read_config_dword(
> + pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &temp);
> + if (rc)
> + return rc;
> +
> + active = FIELD_GET(CXL_DVSEC_MEM_ACTIVE, temp);
> + if (active)
> + break;
> + msleep(1000);
Whilst it can be a while, this seems a bit of an excessive step to me.
If the thing is ready in 10msecs we stil end up waiting a second.
Might be worth checking more often, or doing some sort of fall off
in frequency of checking.
> + }
> +
> + if (!active) {
> + dev_err(&pdev->dev,
> + "timeout awaiting memory active after %d seconds\n",
> + mbox_ready_timeout);
> + return -ETIMEDOUT;
> + }
> +
> + md_status = readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
> + if (!CXLMDEV_READY(md_status))
> + return -EIO;
> +
> + return 0;
> +}
> +
