RE: [PATCH v2 13/27] nvdimm/ocxl: Add support for Admin commands
From: Alastair D'Silva
Date: Wed Feb 19 2020 - 00:01:20 EST
On Mon, 2020-02-03 at 14:18 +0000, Jonathan Cameron wrote:
> On Tue, 3 Dec 2019 14:46:41 +1100
> Alastair D'Silva <alastair@xxxxxxxxxxx> wrote:
>
> > From: Alastair D'Silva <alastair@xxxxxxxxxxx>
> >
> > This patch requests the metadata required to issue admin commands,
> > as well
> > as some helper functions to construct and check the completion of
> > the
> > commands.
> >
> > Signed-off-by: Alastair D'Silva <alastair@xxxxxxxxxxx>
>
> A few trivial bits inline.
>
> Jonathan
>
> > ---
> > drivers/nvdimm/ocxl/scm.c | 67 +++++++++++++
> > drivers/nvdimm/ocxl/scm_internal.c | 152
> > +++++++++++++++++++++++++++++
> > drivers/nvdimm/ocxl/scm_internal.h | 62 ++++++++++++
> > 3 files changed, 281 insertions(+)
> >
> > diff --git a/drivers/nvdimm/ocxl/scm.c b/drivers/nvdimm/ocxl/scm.c
> > index 8088f65c289e..1e175f3c3cf2 100644
> > --- a/drivers/nvdimm/ocxl/scm.c
> > +++ b/drivers/nvdimm/ocxl/scm.c
> > @@ -267,6 +267,58 @@ static int scm_register_lpc_mem(struct
> > scm_data *scm_data)
> > return 0;
> > }
> >
> > +/**
> > + * scm_extract_command_metadata() - Extract command data from MMIO
> > & save it for further use
> > + * @scm_data: a pointer to the SCM device data
> > + * @offset: The base address of the command data structures
> > (address of CREQO)
> > + * @command_metadata: A pointer to the command metadata to
> > populate
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int scm_extract_command_metadata(struct scm_data *scm_data,
> > u32 offset,
> > + struct command_metadata
> > *command_metadata)
> > +{
> > + int rc;
> > + u64 tmp;
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu, offset,
> > OCXL_LITTLE_ENDIAN,
> > + &tmp);
> > + if (rc)
> > + return rc;
> > +
> > + command_metadata->request_offset = tmp >> 32;
> > + command_metadata->response_offset = tmp & 0xFFFFFFFF;
> > +
> > + rc = ocxl_global_mmio_read64(scm_data->ocxl_afu, offset + 8,
> > OCXL_LITTLE_ENDIAN,
> > + &tmp);
> > + if (rc)
> > + return rc;
> > +
> > + command_metadata->data_offset = tmp >> 32;
> > + command_metadata->data_size = tmp & 0xFFFFFFFF;
> > +
> > + command_metadata->id = 0;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * scm_setup_command_metadata() - Set up the command metadata
> > + * @scm_data: a pointer to the SCM device data
> > + */
> > +static int scm_setup_command_metadata(struct scm_data *scm_data)
> > +{
> > + int rc;
> > +
> > + mutex_init(&scm_data->admin_command.lock);
> > +
> > + rc = scm_extract_command_metadata(scm_data,
> > GLOBAL_MMIO_ACMA_CREQO,
> > + &scm_data->admin_command);
> > + if (rc)
> > + return rc;
>
> Unless you are adding to this later in the series.
>
Ignored
> return scm_extract_command_metadata(scm_data,...)
>
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * scm_is_usable() - Is a controller usable?
> > * @scm_data: a pointer to the SCM device data
> > @@ -276,6 +328,8 @@ static bool scm_is_usable(const struct scm_data
> > *scm_data)
> > {
> > u64 chi = 0;
> > int rc = scm_chi(scm_data, &chi);
> > + if (rc)
> > + return false;
> >
> > if (!(chi & GLOBAL_MMIO_CHI_CRDY)) {
> > dev_err(&scm_data->dev, "SCM controller is not
> > ready.\n");
> > @@ -502,6 +556,14 @@ static int scm_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> > }
> > scm_data->pdev = pdev;
> >
> > + scm_data->timeouts[ADMIN_COMMAND_ERRLOG] = 2000; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_HEARTBEAT] = 100; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_SMART] = 100; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_CONTROLLER_DUMP] = 1000; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_CONTROLLER_STATS] = 100; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_SHUTDOWN] = 1000; // ms
> > + scm_data->timeouts[ADMIN_COMMAND_FW_UPDATE] = 16000; // ms
> > +
> > pci_set_drvdata(pdev, scm_data);
> >
> > scm_data->ocxl_fn = ocxl_function_open(pdev);
> > @@ -543,6 +605,11 @@ static int scm_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (scm_setup_command_metadata(scm_data)) {
> > + dev_err(&pdev->dev, "Could not read OCXL command
> > matada\n");
> > + goto err;
> > + }
> > +
> > elapsed = 0;
> > timeout = scm_data->readiness_timeout + scm_data-
> > >memory_available_timeout;
> > while (!scm_is_usable(scm_data)) {
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.c
> > b/drivers/nvdimm/ocxl/scm_internal.c
> > index 72d3c0e7d846..7b11b56863fb 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.c
> > +++ b/drivers/nvdimm/ocxl/scm_internal.c
> > @@ -17,3 +17,155 @@ int scm_chi(const struct scm_data *scm_data,
> > u64 *chi)
> >
> > return 0;
> > }
> > +
> > +static int scm_command_request(const struct scm_data *scm_data,
> > + struct command_metadata *cmd, u8
> > op_code)
> > +{
> > + u64 val = op_code;
> > + int rc;
> > + u8 i;
> > +
> > + cmd->op_code = op_code;
> > + cmd->id++;
> > +
> > + val |= ((u64)cmd->id) << 16;
> > +
> > + rc = ocxl_global_mmio_write64(scm_data->ocxl_afu, cmd-
> > >request_offset,
> > + OCXL_LITTLE_ENDIAN, val);
> > + if (rc)
> > + return rc;
> > +
> > + for (i = 0x08; i <= 0x38; i += 0x08) {
>
> perhaps use sizeof(u64) to explain where the 0x08s come from.
> For the 0x38, might be worth a define.
Ok
>
> > + rc = ocxl_global_mmio_write64(scm_data->ocxl_afu,
> > + cmd->request_offset + i,
> > + OCXL_LITTLE_ENDIAN, 0);
> > + if (rc)
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int scm_admin_command_request(struct scm_data *scm_data, u8
> > op_code)
> > +{
> > + u64 val;
> > + int rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CHI,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + return scm_command_request(scm_data, &scm_data->admin_command,
> > op_code);
> > +}
> > +
> > +static int scm_command_response(const struct scm_data *scm_data,
> > + const struct command_metadata *cmd)
> > +{
> > + u64 val;
> > + u16 id;
> > + u8 status;
> > + int rc = ocxl_global_mmio_read64(scm_data->ocxl_afu,
> > + cmd->response_offset,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + status = val & 0xff;
> > + id = (val >> 16) & 0xffff;
> > +
> > + if (id != cmd->id) {
> > + dev_warn(&scm_data->dev,
> > + "Expected response for command %d, but
> > received response for command %d instead.\n",
> > + cmd->id, id);
> > + }
> > +
> > + return status;
> > +}
> > +
> > +int scm_admin_response(const struct scm_data *scm_data)
> > +{
> > + return scm_command_response(scm_data, &scm_data-
> > >admin_command);
> > +}
> > +
> > +
> > +int scm_admin_command_execute(const struct scm_data *scm_data)
> > +{
> > + return ocxl_global_mmio_set64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_HCI,
> > + OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_HCI_ACRW);
> > +}
> > +
> > +static bool scm_admin_command_complete(const struct scm_data
> > *scm_data)
> > +{
> > + u64 val = 0;
> > +
> > + int rc = scm_chi(scm_data, &val);
> > +
> > + WARN_ON(rc);
> > +
> > + return (val & GLOBAL_MMIO_CHI_ACRA) != 0;
> > +}
> > +
> > +int scm_admin_command_complete_timeout(const struct scm_data
> > *scm_data,
> > + int command)
> > +{
> > + u32 timeout = scm_data->timeouts[command];
> > + // 32 is the next power of 2 greater than the 20ms minimum for
> > msleep
> > +#define TIMEOUT_SLEEP_MILLIS 32
> > + timeout /= TIMEOUT_SLEEP_MILLIS;
> > + if (!timeout)
> > + timeout = SCM_DEFAULT_TIMEOUT / TIMEOUT_SLEEP_MILLIS;
> > +
> > + while (timeout-- > 0) {
> > + if (scm_admin_command_complete(scm_data))
> > + return 0;
> > + msleep(TIMEOUT_SLEEP_MILLIS);
> > + }
> > +
> > + if (scm_admin_command_complete(scm_data))
> > + return 0;
> > +
> > + return -EBUSY;
> > +}
> > +
> > +int scm_admin_response_handled(const struct scm_data *scm_data)
> > +{
> > + return ocxl_global_mmio_set64(scm_data->ocxl_afu,
> > GLOBAL_MMIO_CHIC,
> > + OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_CHI_ACRA);
> > +}
> > +
> > +void scm_warn_status(const struct scm_data *scm_data, const char
> > *message,
> > + u8 status)
> > +{
> > + const char *text = "Unknown";
> > +
> > + switch (status) {
> > + case STATUS_SUCCESS:
> > + text = "Success";
> > + break;
> > +
> > + case STATUS_MEM_UNAVAILABLE:
> > + text = "Persistent memory unavailable";
> > + break;
> > +
> > + case STATUS_BAD_OPCODE:
> > + text = "Bad opcode";
> > + break;
> > +
> > + case STATUS_BAD_REQUEST_PARM:
> > + text = "Bad request parameter";
> > + break;
> > +
> > + case STATUS_BAD_DATA_PARM:
> > + text = "Bad data parameter";
> > + break;
> > +
> > + case STATUS_DEBUG_BLOCKED:
> > + text = "Debug action blocked";
> > + break;
> > +
> > + case STATUS_FAIL:
> > + text = "Failed";
> > + break;
> > + }
> > +
> > + dev_warn(&scm_data->dev, "%s: %s (%x)\n", message, text,
> > status);
> > +}
> > diff --git a/drivers/nvdimm/ocxl/scm_internal.h
> > b/drivers/nvdimm/ocxl/scm_internal.h
> > index 584450f55e30..9bff684cd069 100644
> > --- a/drivers/nvdimm/ocxl/scm_internal.h
> > +++ b/drivers/nvdimm/ocxl/scm_internal.h
> > @@ -6,6 +6,8 @@
> > #include <linux/libnvdimm.h>
> > #include <linux/mm.h>
> >
> > +#define SCM_DEFAULT_TIMEOUT 100
> > +
> > #define GLOBAL_MMIO_CHI 0x000
> > #define GLOBAL_MMIO_CHIC 0x008
> > #define GLOBAL_MMIO_CHIE 0x010
> > @@ -80,6 +82,16 @@
> >
> > #define SCM_LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> >
> > +struct command_metadata {
> > + u32 request_offset;
> > + u32 response_offset;
> > + u32 data_offset;
> > + u32 data_size;
> > + struct mutex lock;
> > + u16 id;
> > + u8 op_code;
> > +};
> > +
> > struct scm_function_0 {
> > struct pci_dev *pdev;
> > struct ocxl_fn *ocxl_fn;
> > @@ -95,9 +107,11 @@ struct scm_data {
> > struct ocxl_afu *ocxl_afu;
> > struct ocxl_context *ocxl_context;
> > void *metadata_addr;
> > + struct command_metadata admin_command;
> > struct resource scm_res;
> > struct nd_region *nd_region;
> > char fw_version[8+1];
> > + u32 timeouts[ADMIN_COMMAND_MAX+1];
> >
> > u32 max_controller_dump_size;
> > u16 scm_revision; // major/minor
> > @@ -122,3 +136,51 @@ struct scm_data {
> > * Returns 0 on success, negative on error
> > */
> > int scm_chi(const struct scm_data *scm_data, u64 *chi);
> > +
> > +/**
> > + * scm_admin_command_request() - Issue an admin command request
> > + * @scm_data: a pointer to the SCM device data
> > + * @op_code: The op-code for the command
> > + *
> > + * Returns an identifier for the command, or negative on error
> > + */
> > +int scm_admin_command_request(struct scm_data *scm_data, u8
> > op_code);
> > +
> > +/**
> > + * scm_admin_response() - Validate an admin response
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns the status code of the command, or negative on error
> > + */
> > +int scm_admin_response(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_admin_command_execute() - Notify the controller to start
> > processing a pending admin command
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns 0 on success, negative on error
> > + */
> > +int scm_admin_command_execute(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_admin_command_complete_timeout() - Wait for an admin
> > command to finish executing
> > + * @scm_data: a pointer to the SCM device data
> > + * @command: the admin command to wait for completion (determines
> > the timeout)
> > + * Returns 0 on success, -EBUSY on timeout
> > + */
> > +int scm_admin_command_complete_timeout(const struct scm_data
> > *scm_data,
> > + int command);
> > +
> > +/**
> > + * scm_admin_response_handled() - Notify the controller that the
> > admin response has been handled
> > + * @scm_data: a pointer to the SCM device data
> > + * Returns 0 on success, negative on failure
> > + */
> > +int scm_admin_response_handled(const struct scm_data *scm_data);
> > +
> > +/**
> > + * scm_warn_status() - Emit a kernel warning showing a command
> > status.
> > + * @scm_data: a pointer to the SCM device data
> > + * @message: A message to accompany the warning
> > + * @status: The command status
> > + */
> > +void scm_warn_status(const struct scm_data *scm_data, const char
> > *message,
> > + u8 status);
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819