Re: [PATCH v4 14/25] nvdimm/ocxl: Add support for Admin commands
On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva wrote:
>
> Admin commands for these devices are the primary means of interacting
> with the device controller to provide functionality beyond the load/store
> capabilities offered via the NPU.
>
> For example, SMART data, firmware update, and device error logs are
> implemented via admin commands.
>
> 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
> ---
> drivers/nvdimm/ocxl/main.c | 65 ++
> drivers/nvdimm/ocxl/ocxlpmem.h | 50 -
> drivers/nvdimm/ocxl/ocxlpmem_internal.c | 261
> 3 files changed, 375 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
> index be76acd33d74..8db573036423 100644
> --- a/drivers/nvdimm/ocxl/main.c
> +++ b/drivers/nvdimm/ocxl/main.c
> @@ -217,6 +217,58 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
> return 0;
> }
>
> +/**
> + * extract_command_metadata() - Extract command data from MMIO & save it for
> further use
> + * @ocxlpmem: the device metadata
> + * @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 extract_command_metadata(struct ocxlpmem *ocxlpmem, u32 offset,
> + struct command_metadata *command_metadata)
How about "struct ocxlpmem *ocp" throughout all these patches? The
full duplication of the type name as the local variable name makes
this look like non-idiomatic Linux code to me. It had not quite hit me
until I saw "struct command_metadata *command_metadata" that just
strikes me as too literal and the person that gets to maintain this
code later will appreciate a smaller amount of typing.
Also, is it really the case that the layout of the admin command
metadata needs to be programmatically determined at runtime? I would
expect it to be a static command definition in the spec.
> +{
> + int rc;
> + u64 tmp;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset,
> +OCXL_LITTLE_ENDIAN, &tmp);
> + if (rc)
> + return rc;
> +
> + command_metadata->request_offset = tmp >> 32;
> + command_metadata->response_offset = tmp & 0x;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset + 8,
> +OCXL_LITTLE_ENDIAN, &tmp);
> + if (rc)
> + return rc;
> +
> + command_metadata->data_offset = tmp >> 32;
> + command_metadata->data_size = tmp & 0x;
> +
> + command_metadata->id = 0;
> +
> + return 0;
> +}
> +
> +/**
> + * setup_command_metadata() - Set up the command metadata
> + * @ocxlpmem: the device metadata
> + */
> +static int setup_command_metadata(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> +
> + mutex_init(&ocxlpmem->admin_command.lock);
> +
> + rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_ACMA_CREQO,
> + &ocxlpmem->admin_command);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> /**
> * allocate_minor() - Allocate a minor number to use for an OpenCAPI pmem
> device
> * @ocxlpmem: the device metadata
> @@ -421,6 +473,14 @@ static int probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
>
> ocxlpmem->pdev = pci_dev_get(pdev);
>
> + ocxlpmem->timeouts[ADMIN_COMMAND_ERRLOG] = 2000; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_HEARTBEAT] = 100; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_SMART] = 100; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_DUMP] = 1000; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_STATS] = 100; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_SHUTDOWN] = 1000; // ms
> + ocxlpmem->timeouts[ADMIN_COMMAND_FW_UPDATE] = 16000; // ms
> +
> pci_set_drvdata(pdev, ocxlpmem);
>
> ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
> @@ -467,6 +527,11 @@ static int probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> goto err;
> }
>
> + if (setup_command_metadata(ocxlpmem)) {
> + dev_err(&pdev->dev, "Could not read command metadata\n");
> + goto err;
> + }
> +
> elapsed = 0;
> timeout = ocxlpmem->readiness_timeout +
> ocxlpmem->memory_available_timeout;
> diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
> index 3eadbe19f6d0..b72b3f909fc3 100644
> --- a/drivers/nvdimm/ocxl/ocxlpmem.h
> +++ b/drivers/nvdimm/ocxl/ocxlpmem.h
> @@ -7,6 +7,7 @@
> #include
>
> #define LABEL_AREA_SIZEBIT_ULL(PA_SECTION_SHIFT)
> +#define DEFAU
[PATCH v4 14/25] nvdimm/ocxl: Add support for Admin commands
Admin commands for these devices are the primary means of interacting
with the device controller to provide functionality beyond the load/store
capabilities offered via the NPU.
For example, SMART data, firmware update, and device error logs are
implemented via admin commands.
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
---
drivers/nvdimm/ocxl/main.c | 65 ++
drivers/nvdimm/ocxl/ocxlpmem.h | 50 -
drivers/nvdimm/ocxl/ocxlpmem_internal.c | 261
3 files changed, 375 insertions(+), 1 deletion(-)
diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
index be76acd33d74..8db573036423 100644
--- a/drivers/nvdimm/ocxl/main.c
+++ b/drivers/nvdimm/ocxl/main.c
@@ -217,6 +217,58 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
return 0;
}
+/**
+ * extract_command_metadata() - Extract command data from MMIO & save it for
further use
+ * @ocxlpmem: the device metadata
+ * @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 extract_command_metadata(struct ocxlpmem *ocxlpmem, u32 offset,
+ struct command_metadata *command_metadata)
+{
+ int rc;
+ u64 tmp;
+
+ rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset,
+OCXL_LITTLE_ENDIAN, &tmp);
+ if (rc)
+ return rc;
+
+ command_metadata->request_offset = tmp >> 32;
+ command_metadata->response_offset = tmp & 0x;
+
+ rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset + 8,
+OCXL_LITTLE_ENDIAN, &tmp);
+ if (rc)
+ return rc;
+
+ command_metadata->data_offset = tmp >> 32;
+ command_metadata->data_size = tmp & 0x;
+
+ command_metadata->id = 0;
+
+ return 0;
+}
+
+/**
+ * setup_command_metadata() - Set up the command metadata
+ * @ocxlpmem: the device metadata
+ */
+static int setup_command_metadata(struct ocxlpmem *ocxlpmem)
+{
+ int rc;
+
+ mutex_init(&ocxlpmem->admin_command.lock);
+
+ rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_ACMA_CREQO,
+ &ocxlpmem->admin_command);
+ if (rc)
+ return rc;
+
+ return 0;
+}
+
/**
* allocate_minor() - Allocate a minor number to use for an OpenCAPI pmem
device
* @ocxlpmem: the device metadata
@@ -421,6 +473,14 @@ static int probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
ocxlpmem->pdev = pci_dev_get(pdev);
+ ocxlpmem->timeouts[ADMIN_COMMAND_ERRLOG] = 2000; // ms
+ ocxlpmem->timeouts[ADMIN_COMMAND_HEARTBEAT] = 100; // ms
+ ocxlpmem->timeouts[ADMIN_COMMAND_SMART] = 100; // ms
+ ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_DUMP] = 1000; // ms
+ ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_STATS] = 100; // ms
+ ocxlpmem->timeouts[ADMIN_COMMAND_SHUTDOWN] = 1000; // ms
+ ocxlpmem->timeouts[ADMIN_COMMAND_FW_UPDATE] = 16000; // ms
+
pci_set_drvdata(pdev, ocxlpmem);
ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
@@ -467,6 +527,11 @@ static int probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
goto err;
}
+ if (setup_command_metadata(ocxlpmem)) {
+ dev_err(&pdev->dev, "Could not read command metadata\n");
+ goto err;
+ }
+
elapsed = 0;
timeout = ocxlpmem->readiness_timeout +
ocxlpmem->memory_available_timeout;
diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
index 3eadbe19f6d0..b72b3f909fc3 100644
--- a/drivers/nvdimm/ocxl/ocxlpmem.h
+++ b/drivers/nvdimm/ocxl/ocxlpmem.h
@@ -7,6 +7,7 @@
#include
#define LABEL_AREA_SIZEBIT_ULL(PA_SECTION_SHIFT)
+#define DEFAULT_TIMEOUT 100
#define GLOBAL_MMIO_CHI0x000
#define GLOBAL_MMIO_CHIC 0x008
@@ -57,6 +58,7 @@
#define GLOBAL_MMIO_HCI_CONTROLLER_DUMP_COLLECTED BIT_ULL(5) // CDC
#define GLOBAL_MMIO_HCI_REQ_HEALTH_PERFBIT_ULL(6) //
CHPD
+// must be maintained with admin_command_names in ocxlpmem_internal.c
#define ADMIN_COMMAND_HEARTBEAT0x00u
#define ADMIN_COMMAND_SHUTDOWN 0x01u
#define ADMIN_COMMAND_FW_UPDATE0x02u
@@ -68,6 +70,13 @@
#define ADMIN_COMMAND_CMD_CAPS 0x08u
#define ADMIN_COMMAND_MAX 0x08u
+#define NS_COMMAND_SECURE_ERASE0x20ull
+
+#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_SUCCESS 0x20
+#define NS_RESPONSE_SECURE_ERASE_ACCESSIBLE_ATTEMPTED 0x28
+#define NS_RESPONSE_SECURE_ERASE_DEFECTIVE_SUCCESS 0x30
+#define NS_RESPONSE_SECURE_ERASE_DE
