diff mbox series

[v3,15/27] powerpc/powernv/pmem: Add support for near storage commands

Message ID 20200221032720.33893-16-alastair@au1.ibm.com
State New, archived
Headers show
Series Add support for OpenCAPI Persistent Memory devices | expand

Commit Message

Alastair D'Silva Feb. 21, 2020, 3:27 a.m. UTC
From: Alastair D'Silva <alastair@d-silva.org>

Similar to the previous patch, this adds support for near storage commands.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/platforms/powernv/pmem/ocxl.c    |  6 +++
 .../platforms/powernv/pmem/ocxl_internal.c    | 41 +++++++++++++++++++
 .../platforms/powernv/pmem/ocxl_internal.h    | 37 +++++++++++++++++
 3 files changed, 84 insertions(+)

Comments

Andrew Donnellan Feb. 27, 2020, 8:30 a.m. UTC | #1
On 21/2/20 2:27 pm, Alastair D'Silva wrote:> +int 
ns_response_handled(const struct ocxlpmem *ocxlpmem)
> +{
> +	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIC,
> +				      OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_CHI_NSCRA);
> +}

Same comment as on the last patch - I think we're meant to be clearing 
this bit, not setting it to 1,
Dan Williams Feb. 27, 2020, 5:02 p.m. UTC | #2
On Thu, Feb 20, 2020 at 7:28 PM Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Similar to the previous patch, this adds support for near storage commands.

Similar comment as the last patch. This changelog does not give the
reviewer any frame of reference to review the patch.
Alastair D'Silva Feb. 27, 2020, 11:56 p.m. UTC | #3
On Thu, 2020-02-27 at 19:30 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:> +int 
> ns_response_handled(const struct ocxlpmem *ocxlpmem)
> > +{
> > +	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_CHIC,
> > +				      OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_CHI_NSCRA);
> > +}
> 
> Same comment as on the last patch - I think we're meant to be
> clearing 
> this bit, not setting it to 1,
> 

Same reply :) Writing to the CHIC register clears the bit in CHI.

>
Frederic Barrat March 2, 2020, 5:58 p.m. UTC | #4
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Similar to the previous patch, this adds support for near storage commands.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---


Is any of these new functions ever called?

   Fred


>   arch/powerpc/platforms/powernv/pmem/ocxl.c    |  6 +++
>   .../platforms/powernv/pmem/ocxl_internal.c    | 41 +++++++++++++++++++
>   .../platforms/powernv/pmem/ocxl_internal.h    | 37 +++++++++++++++++
>   3 files changed, 84 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index 4e782d22605b..b8bd7e703b19 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -259,12 +259,18 @@ static int setup_command_metadata(struct ocxlpmem *ocxlpmem)
>   	int rc;
>   
>   	mutex_init(&ocxlpmem->admin_command.lock);
> +	mutex_init(&ocxlpmem->ns_command.lock);
>   
>   	rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_ACMA_CREQO,
>   				      &ocxlpmem->admin_command);
>   	if (rc)
>   		return rc;
>   
> +	rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_NSCMA_CREQO,
> +					  &ocxlpmem->ns_command);
> +	if (rc)
> +		return rc;
> +
>   	return 0;
>   }
>   
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> index 583f48023025..3e0b133feddf 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> @@ -133,6 +133,47 @@ int admin_response_handled(const struct ocxlpmem *ocxlpmem)
>   				      OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_CHI_ACRA);
>   }
>   
> +int ns_command_request(struct ocxlpmem *ocxlpmem, u8 op_code)
> +{
> +	u64 val;
> +	int rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHI,
> +					 OCXL_LITTLE_ENDIAN, &val);
> +	if (rc)
> +		return rc;
> +
> +	if (!(val & GLOBAL_MMIO_CHI_NSCRA))
> +		return -EBUSY;
> +
> +	return scm_command_request(ocxlpmem, &ocxlpmem->ns_command, op_code);
> +}
> +
> +int ns_response(const struct ocxlpmem *ocxlpmem)
> +{
> +	return command_response(ocxlpmem, &ocxlpmem->ns_command);
> +}
> +
> +int ns_command_execute(const struct ocxlpmem *ocxlpmem)
> +{
> +	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI,
> +				      OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_HCI_NSCRW);
> +}
> +
> +bool ns_command_complete(const struct ocxlpmem *ocxlpmem)
> +{
> +	u64 val = 0;
> +	int rc = ocxlpmem_chi(ocxlpmem, &val);
> +
> +	WARN_ON(rc);
> +
> +	return (val & GLOBAL_MMIO_CHI_NSCRA) != 0;
> +}
> +
> +int ns_response_handled(const struct ocxlpmem *ocxlpmem)
> +{
> +	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIC,
> +				      OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_CHI_NSCRA);
> +}
> +
>   void warn_status(const struct ocxlpmem *ocxlpmem, const char *message,
>   		     u8 status)
>   {
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> index 2fef68c71271..28e2020f6355 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -107,6 +107,7 @@ struct ocxlpmem {
>   	struct ocxl_context *ocxl_context;
>   	void *metadata_addr;
>   	struct command_metadata admin_command;
> +	struct command_metadata ns_command;
>   	struct resource pmem_res;
>   	struct nd_region *nd_region;
>   	char fw_version[8+1];
> @@ -175,6 +176,42 @@ int admin_command_complete_timeout(const struct ocxlpmem *ocxlpmem,
>    */
>   int admin_response_handled(const struct ocxlpmem *ocxlpmem);
>   
> +/**
> + * ns_command_request() - Issue a near storage command request
> + * @ocxlpmem: the device metadata
> + * @op_code: The op-code for the command
> + * Returns an identifier for the command, or negative on error
> + */
> +int ns_command_request(struct ocxlpmem *ocxlpmem, u8 op_code);
> +
> +/**
> + * ns_response() - Validate a near storage response
> + * @ocxlpmem: the device metadata
> + * Returns the status code of the command, or negative on error
> + */
> +int ns_response(const struct ocxlpmem *ocxlpmem);
> +
> +/**
> + * ns_command_execute() - Notify the controller to start processing a pending near storage command
> + * @ocxlpmem: the device metadata
> + * Returns 0 on success, negative on error
> + */
> +int ns_command_execute(const struct ocxlpmem *ocxlpmem);
> +
> +/**
> + * ns_command_complete() - Is a near storage command executing
> + * @ocxlpmem: the device metadata
> + * Returns true if the previous admin command has completed
> + */
> +bool ns_command_complete(const struct ocxlpmem *ocxlpmem);
> +
> +/**
> + * ns_response_handled() - Notify the controller that the near storage response has been handled
> + * @ocxlpmem: the device metadata
> + * Returns 0 on success, negative on failure
> + */
> +int ns_response_handled(const struct ocxlpmem *ocxlpmem);
> +
>   /**
>    * warn_status() - Emit a kernel warning showing a command status.
>    * @ocxlpmem: the device metadata
>
Dan Williams March 2, 2020, 6:42 p.m. UTC | #5
On Mon, Mar 2, 2020 at 9:59 AM Frederic Barrat <fbarrat@linux.ibm.com> wrote:
>
>
>
> Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Similar to the previous patch, this adds support for near storage commands.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
>
>
> Is any of these new functions ever called?

This is my concern as well. The libnvdimm command support is limited
to the commands that Linux will use. Other passthrough commands are
supported through a passthrough interface. However, that passthrough
interface is explicitly limited to publicly documented command sets so
that the kernel has an opportunity to constrain and consolidate
command implementations across vendors.
Alastair D'Silva March 4, 2020, 4:42 a.m. UTC | #6
On Mon, 2020-03-02 at 10:42 -0800, Dan Williams wrote:
> On Mon, Mar 2, 2020 at 9:59 AM Frederic Barrat <fbarrat@linux.ibm.com
> > wrote:
> > 
> > 
> > Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > > 
> > > Similar to the previous patch, this adds support for near storage
> > > commands.
> > > 
> > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > ---
> > 
> > Is any of these new functions ever called?
> 
> This is my concern as well. The libnvdimm command support is limited
> to the commands that Linux will use. Other passthrough commands are
> supported through a passthrough interface. However, that passthrough
> interface is explicitly limited to publicly documented command sets
> so
> that the kernel has an opportunity to constrain and consolidate
> command implementations across vendors.


It will be in the patch that implements overwrite. I moved that patch
out of this series, as it needs more testing, so I guess I can submit
this alongside it.
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
index 4e782d22605b..b8bd7e703b19 100644
--- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
+++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
@@ -259,12 +259,18 @@  static int setup_command_metadata(struct ocxlpmem *ocxlpmem)
 	int rc;
 
 	mutex_init(&ocxlpmem->admin_command.lock);
+	mutex_init(&ocxlpmem->ns_command.lock);
 
 	rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_ACMA_CREQO,
 				      &ocxlpmem->admin_command);
 	if (rc)
 		return rc;
 
+	rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_NSCMA_CREQO,
+					  &ocxlpmem->ns_command);
+	if (rc)
+		return rc;
+
 	return 0;
 }
 
diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
index 583f48023025..3e0b133feddf 100644
--- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
+++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
@@ -133,6 +133,47 @@  int admin_response_handled(const struct ocxlpmem *ocxlpmem)
 				      OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_CHI_ACRA);
 }
 
+int ns_command_request(struct ocxlpmem *ocxlpmem, u8 op_code)
+{
+	u64 val;
+	int rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHI,
+					 OCXL_LITTLE_ENDIAN, &val);
+	if (rc)
+		return rc;
+
+	if (!(val & GLOBAL_MMIO_CHI_NSCRA))
+		return -EBUSY;
+
+	return scm_command_request(ocxlpmem, &ocxlpmem->ns_command, op_code);
+}
+
+int ns_response(const struct ocxlpmem *ocxlpmem)
+{
+	return command_response(ocxlpmem, &ocxlpmem->ns_command);
+}
+
+int ns_command_execute(const struct ocxlpmem *ocxlpmem)
+{
+	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI,
+				      OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_HCI_NSCRW);
+}
+
+bool ns_command_complete(const struct ocxlpmem *ocxlpmem)
+{
+	u64 val = 0;
+	int rc = ocxlpmem_chi(ocxlpmem, &val);
+
+	WARN_ON(rc);
+
+	return (val & GLOBAL_MMIO_CHI_NSCRA) != 0;
+}
+
+int ns_response_handled(const struct ocxlpmem *ocxlpmem)
+{
+	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIC,
+				      OCXL_LITTLE_ENDIAN, GLOBAL_MMIO_CHI_NSCRA);
+}
+
 void warn_status(const struct ocxlpmem *ocxlpmem, const char *message,
 		     u8 status)
 {
diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
index 2fef68c71271..28e2020f6355 100644
--- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
+++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
@@ -107,6 +107,7 @@  struct ocxlpmem {
 	struct ocxl_context *ocxl_context;
 	void *metadata_addr;
 	struct command_metadata admin_command;
+	struct command_metadata ns_command;
 	struct resource pmem_res;
 	struct nd_region *nd_region;
 	char fw_version[8+1];
@@ -175,6 +176,42 @@  int admin_command_complete_timeout(const struct ocxlpmem *ocxlpmem,
  */
 int admin_response_handled(const struct ocxlpmem *ocxlpmem);
 
+/**
+ * ns_command_request() - Issue a near storage command request
+ * @ocxlpmem: the device metadata
+ * @op_code: The op-code for the command
+ * Returns an identifier for the command, or negative on error
+ */
+int ns_command_request(struct ocxlpmem *ocxlpmem, u8 op_code);
+
+/**
+ * ns_response() - Validate a near storage response
+ * @ocxlpmem: the device metadata
+ * Returns the status code of the command, or negative on error
+ */
+int ns_response(const struct ocxlpmem *ocxlpmem);
+
+/**
+ * ns_command_execute() - Notify the controller to start processing a pending near storage command
+ * @ocxlpmem: the device metadata
+ * Returns 0 on success, negative on error
+ */
+int ns_command_execute(const struct ocxlpmem *ocxlpmem);
+
+/**
+ * ns_command_complete() - Is a near storage command executing
+ * @ocxlpmem: the device metadata
+ * Returns true if the previous admin command has completed
+ */
+bool ns_command_complete(const struct ocxlpmem *ocxlpmem);
+
+/**
+ * ns_response_handled() - Notify the controller that the near storage response has been handled
+ * @ocxlpmem: the device metadata
+ * Returns 0 on success, negative on failure
+ */
+int ns_response_handled(const struct ocxlpmem *ocxlpmem);
+
 /**
  * warn_status() - Emit a kernel warning showing a command status.
  * @ocxlpmem: the device metadata