All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] powerpc/papr_scm: Add support for reporting nvdimm health
@ 2020-02-20  9:57 ` Vaibhav Jain
  0 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:57 UTC (permalink / raw)
  To: linuxppc-dev, linux-nvdimm
  Cc: Vaibhav Jain, Aneesh Kumar K . V, Michael Ellerman, Alastair D'Silva

The PAPR standard[1][3] provides suitable mechanisms to query the health and
performance stats of an NVDIMM via various hcalls as described in Ref[2]. Until
now these stats were never available nor exposed to the user-space tools like
'ndctl'. This is partly due to PAPR platform not having support for ACPI and
NFIT. Hence 'ndctl' is unable to query and report the dimm health status and a
user had no way to determine the current health status of a NDVIMM.

To overcome this limitation this patch-set updates papr_scm kernel module to
query and fetch nvdimm health and performance stats using hcalls described in
Ref[2]. This health and performance stats are then exposed to userspace via
syfs and Dimm-Specific-Methods(DSM) issued by libndctl.

These changes coupled with proposed ndtcl changes located at Ref[4] should
provide a way for the user to retrieve NVDIMM health status using ndtcl. Below
is a sample output using proposed kernel + ndctl for PAPR NVDIMM in an
emulation environment:

 # ndctl list -DH
[
  {
    "dev":"nmem0",
    "health":{
      "health_state":"fatal",
      "shutdown_state":"dirty"
    }
  }
]

PAPR Dimm-Specific-Methods(DSM)
================================

As the name suggests DSMs are used by vendor specific code in libndctl to
execute certain operations or fetch certain information for NVDIMMS. DSMs
can be sent to papr_scm module via libndctl (userspace) and libnvdimm(kernel)
using the ND_CMD_CALL ioctl which can be handled in the dimm control function
papr_scm_ndctl(). For PAPR this patchset proposes two DSMs defined in the newly
introduced uapi header named 'papr_scm_dsm.h', that directly map to hcalls
provided by PHYP to query NVDIMM health and stats. These DSMs are:

* DSM_PAPR_SCM_HEALTH: Which map to hcall H_SCM_HEALTH and returns dimm health.

* DSM_PAPR_SCM_STATS: Which map to hcall H_SCM_PERFORMANCE_STATS and returns
		      dimm performance stats.

P.S: The current patch-set only provides an implementation for servicing
DSM_PAPR_SCM_HEALTH and a future patch will add support for DSM_PAPR_SCM_STATS.

The ioctl ND_CMD_CALL can also transfer data between user-space and kernel via
'envelopes'. The envelop is part of a 'struct nd_cmd_pkg' which in return is
wrapped in a user defined struct which in our case is a newly introduced
'struct nd_papr_scm_cmd_pkg'. Apart from 'envelope header' this struct holds
'payload', 'payload offset', 'payload version' and 'command status'.

The 'payload' field of the envelop holds a struct depending on the DSM method
used and should be one of the structs defined in newly introduced uapi header
'papr_scm_dsm.h'. This makes it possible for libndctl/kernel to share the same
definitions for these DSM structs.

Earlier Work
============

An earlier RFC patch set titled "powerpc/papr_scm: Implement support for
reporting DIMM health and stats" [5] was proposed which tried to achieve
same functionality albeit with a different approach i.e papr_scm module
acted as a pass-through for the DSM calls from libndctl.

This patch-set however departs from that design by decoupling the
libndctl <--> papr_scm and papr_scm <--> phyp interfaces. This provides
more flexibility compared to earlier approach were these two interfaces were
coupled with each other.

Structure of the patch-set
==========================

The initial 3 patches of the patch-set add functionality of issuing necessary
HCALLs to PHYP to retrieve the dimm health/performance stats information and
exposing them to user-space via sysfs attributes.

Subsequent patches deal with defining and implementing support for
NVDIMM_FAMILY_PAPR_SCM DSM command family and implementing the payload
versioning scheme as mentioned above.

References:
[1]: "Power Architecture Platform Reference"
      https://en.wikipedia.org/wiki/Power_Architecture_Platform_Reference
[2]: "[DOC,v2] powerpc: Provide initial documentation for PAPR hcalls"
     https://patchwork.ozlabs.org/patch/1154292/
[3]: "Linux on Power Architecture Platform Reference"
     https://members.openpowerfoundation.org/document/dl/469
[4]: https://github.com/vaibhav92/ndctl/tree/papr_scm_health_v1
[5]: https://lore.kernel.org/linuxppc-dev/20200129152844.71286-1-vaibhav@linux.ibm.com/

Vaibhav Jain (8):
  powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface
  powerpc/papr_scm: Provide support for fetching dimm health information
  powerpc/papr_scm: Fetch dimm performance stats from PHYP
  UAPI: ndctl: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM
    family
  powerpc/uapi: Introduce uapi header 'papr_scm_dsm.h' for papr_scm DSMs
  powerpc/papr_scm: Add support for handling PAPR DSM commands
  powerpc/papr_scm: Re-implement 'papr_flags' using
    'nd_papr_scm_dimm_health_stat'
  powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH

 arch/powerpc/include/asm/papr_scm.h          |  68 ++++
 arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 143 +++++++
 arch/powerpc/platforms/pseries/papr_scm.c    | 399 ++++++++++++++++++-
 include/uapi/linux/ndctl.h                   |   1 +
 4 files changed, 602 insertions(+), 9 deletions(-)
 create mode 100644 arch/powerpc/include/asm/papr_scm.h
 create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h

-- 
2.24.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/7] powerpc/papr_scm: Add support for reporting nvdimm health
@ 2020-02-20  9:57 ` Vaibhav Jain
  0 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:57 UTC (permalink / raw)
  To: linuxppc-dev, linux-nvdimm
  Cc: Alastair D'Silva, Aneesh Kumar K . V, Oliver O'Halloran,
	Vishal Verma, Vaibhav Jain, Michael Ellerman, Dan Williams

The PAPR standard[1][3] provides suitable mechanisms to query the health and
performance stats of an NVDIMM via various hcalls as described in Ref[2]. Until
now these stats were never available nor exposed to the user-space tools like
'ndctl'. This is partly due to PAPR platform not having support for ACPI and
NFIT. Hence 'ndctl' is unable to query and report the dimm health status and a
user had no way to determine the current health status of a NDVIMM.

To overcome this limitation this patch-set updates papr_scm kernel module to
query and fetch nvdimm health and performance stats using hcalls described in
Ref[2]. This health and performance stats are then exposed to userspace via
syfs and Dimm-Specific-Methods(DSM) issued by libndctl.

These changes coupled with proposed ndtcl changes located at Ref[4] should
provide a way for the user to retrieve NVDIMM health status using ndtcl. Below
is a sample output using proposed kernel + ndctl for PAPR NVDIMM in an
emulation environment:

 # ndctl list -DH
[
  {
    "dev":"nmem0",
    "health":{
      "health_state":"fatal",
      "shutdown_state":"dirty"
    }
  }
]

PAPR Dimm-Specific-Methods(DSM)
================================

As the name suggests DSMs are used by vendor specific code in libndctl to
execute certain operations or fetch certain information for NVDIMMS. DSMs
can be sent to papr_scm module via libndctl (userspace) and libnvdimm(kernel)
using the ND_CMD_CALL ioctl which can be handled in the dimm control function
papr_scm_ndctl(). For PAPR this patchset proposes two DSMs defined in the newly
introduced uapi header named 'papr_scm_dsm.h', that directly map to hcalls
provided by PHYP to query NVDIMM health and stats. These DSMs are:

* DSM_PAPR_SCM_HEALTH: Which map to hcall H_SCM_HEALTH and returns dimm health.

* DSM_PAPR_SCM_STATS: Which map to hcall H_SCM_PERFORMANCE_STATS and returns
		      dimm performance stats.

P.S: The current patch-set only provides an implementation for servicing
DSM_PAPR_SCM_HEALTH and a future patch will add support for DSM_PAPR_SCM_STATS.

The ioctl ND_CMD_CALL can also transfer data between user-space and kernel via
'envelopes'. The envelop is part of a 'struct nd_cmd_pkg' which in return is
wrapped in a user defined struct which in our case is a newly introduced
'struct nd_papr_scm_cmd_pkg'. Apart from 'envelope header' this struct holds
'payload', 'payload offset', 'payload version' and 'command status'.

The 'payload' field of the envelop holds a struct depending on the DSM method
used and should be one of the structs defined in newly introduced uapi header
'papr_scm_dsm.h'. This makes it possible for libndctl/kernel to share the same
definitions for these DSM structs.

Earlier Work
============

An earlier RFC patch set titled "powerpc/papr_scm: Implement support for
reporting DIMM health and stats" [5] was proposed which tried to achieve
same functionality albeit with a different approach i.e papr_scm module
acted as a pass-through for the DSM calls from libndctl.

This patch-set however departs from that design by decoupling the
libndctl <--> papr_scm and papr_scm <--> phyp interfaces. This provides
more flexibility compared to earlier approach were these two interfaces were
coupled with each other.

Structure of the patch-set
==========================

The initial 3 patches of the patch-set add functionality of issuing necessary
HCALLs to PHYP to retrieve the dimm health/performance stats information and
exposing them to user-space via sysfs attributes.

Subsequent patches deal with defining and implementing support for
NVDIMM_FAMILY_PAPR_SCM DSM command family and implementing the payload
versioning scheme as mentioned above.

References:
[1]: "Power Architecture Platform Reference"
      https://en.wikipedia.org/wiki/Power_Architecture_Platform_Reference
[2]: "[DOC,v2] powerpc: Provide initial documentation for PAPR hcalls"
     https://patchwork.ozlabs.org/patch/1154292/
[3]: "Linux on Power Architecture Platform Reference"
     https://members.openpowerfoundation.org/document/dl/469
[4]: https://github.com/vaibhav92/ndctl/tree/papr_scm_health_v1
[5]: https://lore.kernel.org/linuxppc-dev/20200129152844.71286-1-vaibhav@linux.ibm.com/

Vaibhav Jain (8):
  powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface
  powerpc/papr_scm: Provide support for fetching dimm health information
  powerpc/papr_scm: Fetch dimm performance stats from PHYP
  UAPI: ndctl: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM
    family
  powerpc/uapi: Introduce uapi header 'papr_scm_dsm.h' for papr_scm DSMs
  powerpc/papr_scm: Add support for handling PAPR DSM commands
  powerpc/papr_scm: Re-implement 'papr_flags' using
    'nd_papr_scm_dimm_health_stat'
  powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH

 arch/powerpc/include/asm/papr_scm.h          |  68 ++++
 arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 143 +++++++
 arch/powerpc/platforms/pseries/papr_scm.c    | 399 ++++++++++++++++++-
 include/uapi/linux/ndctl.h                   |   1 +
 4 files changed, 602 insertions(+), 9 deletions(-)
 create mode 100644 arch/powerpc/include/asm/papr_scm.h
 create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h

-- 
2.24.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/8] powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface
  2020-02-20  9:57 ` Vaibhav Jain
  (?)
@ 2020-02-20  9:57 ` Vaibhav Jain
  2020-03-09 10:07   ` Aneesh Kumar K.V
  -1 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:57 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva, Aneesh Kumar K . V

Add a new powerpc specific asm header named 'papr-scm.h' that descibes
the interface between PHYP and guest kernel running as an LPAR.

The HCALLs specific to managing SCM are descibed in Ref[1]. The asm
header introduced by this patch however describes the data structures
exchanged between PHYP and kernel during those HCALLs.

Future patches will use these structures to provide support for
retriving nvdimm health and performance stats in papr_scm kernel
module.

[1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 arch/powerpc/include/asm/papr_scm.h | 68 +++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 arch/powerpc/include/asm/papr_scm.h

diff --git a/arch/powerpc/include/asm/papr_scm.h b/arch/powerpc/include/asm/papr_scm.h
new file mode 100644
index 000000000000..d893621063f3
--- /dev/null
+++ b/arch/powerpc/include/asm/papr_scm.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Structures and defines needed to manage nvdimms for spapr guests.
+ */
+#ifndef _ASM_POWERPC_PAPR_SCM_H_
+#define _ASM_POWERPC_PAPR_SCM_H_
+
+#include <linux/types.h>
+#include <asm/bitsperlong.h>
+#include <linux/stringify.h>
+
+/* DIMM health bitmap bitmap indicators */
+/* SCM device is unable to persist memory contents */
+#define PAPR_SCM_DIMM_UNARMED			PPC_BIT(0)
+/* SCM device failed to persist memory contents */
+#define PAPR_SCM_DIMM_SHUTDOWN_DIRTY		PPC_BIT(1)
+/* SCM device contents are persisted from previous IPL */
+#define PAPR_SCM_DIMM_SHUTDOWN_CLEAN		PPC_BIT(2)
+/* SCM device contents are not persisted from previous IPL */
+#define PAPR_SCM_DIMM_EMPTY			PPC_BIT(3)
+/* SCM device memory life remaining is critically low */
+#define PAPR_SCM_DIMM_HEALTH_CRITICAL		PPC_BIT(4)
+/* SCM device will be garded off next IPL due to failure */
+#define PAPR_SCM_DIMM_HEALTH_FATAL		PPC_BIT(5)
+/* SCM contents cannot persist due to current platform health status */
+#define PAPR_SCM_DIMM_HEALTH_UNHEALTHY		PPC_BIT(6)
+/* SCM device is unable to persist memory contents in certain conditions */
+#define PAPR_SCM_DIMM_HEALTH_NON_CRITICAL	PPC_BIT(7)
+/* SCM device is encrypted */
+#define PAPR_SCM_DIMM_ENCRYPTED			PPC_BIT(8)
+/* SCM device has been scrubbed and locked */
+#define PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED	PPC_BIT(9)
+
+/* Bits status indicators for health bitmap indicating unarmed dimm */
+#define PAPR_SCM_DIMM_UNARMED_MASK (PAPR_SCM_DIMM_UNARMED |	\
+					PAPR_SCM_DIMM_HEALTH_UNHEALTHY | \
+					PAPR_SCM_DIMM_HEALTH_NON_CRITICAL)
+
+/* Bits status indicators for health bitmap indicating unflushed dimm */
+#define PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK (PAPR_SCM_DIMM_SHUTDOWN_DIRTY)
+
+/* Bits status indicators for health bitmap indicating unrestored dimm */
+#define PAPR_SCM_DIMM_BAD_RESTORE_MASK  (PAPR_SCM_DIMM_EMPTY)
+
+/* Bit status indicators for smart event notification */
+#define PAPR_SCM_DIMM_SMART_EVENT_MASK (PAPR_SCM_DIMM_HEALTH_CRITICAL | \
+					   PAPR_SCM_DIMM_HEALTH_FATAL | \
+					   PAPR_SCM_DIMM_HEALTH_UNHEALTHY | \
+					   PAPR_SCM_DIMM_HEALTH_NON_CRITICAL)
+
+#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
+
+/* Struct holding a single performance metric */
+struct papr_scm_perf_stat {
+	__be64 statistic_id;
+	__be64 statistic_value;
+};
+
+/* Struct exchanged between kernel and ndctl reporting drc perf stats */
+struct papr_scm_perf_stats {
+	uint8_t eye_catcher[8];
+	__be32 stats_version;		/* Should be 0x01 */
+	__be32 num_statistics;		/* Number of stats following */
+	/* zero or more performance matrics */
+	struct papr_scm_perf_stat scm_statistics[];
+} __packed;
+
+#endif
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/8] powerpc/papr_scm: Provide support for fetching dimm health information
  2020-02-20  9:57 ` Vaibhav Jain
  (?)
  (?)
@ 2020-02-20  9:57 ` Vaibhav Jain
  2020-03-09 10:24   ` Aneesh Kumar K.V
  -1 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:57 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva, Aneesh Kumar K . V

Implement support for fetching dimm health information via
H_SCM_HEALTH hcall as documented in Ref[1]. The hcall returns a pair of
64-bit big-endian integers which are then stored in 'struct
papr_scm_priv' and subsequently exposed to userspace via dimm
attribute 'papr_flags'.

'papr_flags' sysfs attribute reports space separated string flags
indicating various health state an nvdimm can be. These are:

* "not_armed" 	: Indicating that nvdimm contents wont survive a power
  		  cycle.
* "save_fail" 	: Indicating that nvdimm contents couldn't be flushed
		  during last shutdown event.
* "restore_fail": Indicating that nvdimm contents couldn't be restored
		  during dimm initialization.
* "encrypted" 	: Dimm contents are encrypted.
* "smart_notify": There is health event for the nvdimm.
* "scrubbed" 	: Indicating that contents of the nvdimm have been
  		  scrubbed.
* "locked"	: Indicating that nvdimm contents cant be modified
  		  until next power cycle.

[1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/papr_scm.c | 105 +++++++++++++++++++++-
 1 file changed, 103 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 0b4467e378e5..aaf2e4ab1f75 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -14,6 +14,7 @@
 #include <linux/delay.h>
 
 #include <asm/plpar_wrappers.h>
+#include <asm/papr_scm.h>
 
 #define BIND_ANY_ADDR (~0ul)
 
@@ -39,6 +40,13 @@ struct papr_scm_priv {
 	struct resource res;
 	struct nd_region *region;
 	struct nd_interleave_set nd_set;
+
+	/* Protect dimm data from concurrent access */
+	struct mutex dimm_mutex;
+
+	/* Health information for the dimm */
+	__be64 health_bitmap;
+	__be64 health_bitmap_valid;
 };
 
 static int drc_pmem_bind(struct papr_scm_priv *p)
@@ -144,6 +152,35 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
 	return drc_pmem_bind(p);
 }
 
+static int drc_pmem_query_health(struct papr_scm_priv *p)
+{
+	unsigned long ret[PLPAR_HCALL_BUFSIZE];
+	int64_t rc;
+
+	rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
+	if (rc != H_SUCCESS) {
+		dev_err(&p->pdev->dev,
+			 "Failed to query health information, Err:%lld\n", rc);
+		return -ENXIO;
+	}
+
+	/* Protect modifications to papr_scm_priv with the mutex */
+	rc = mutex_lock_interruptible(&p->dimm_mutex);
+	if (rc)
+		return rc;
+
+	/* Store the retrieved health information in dimm platform data */
+	p->health_bitmap = ret[0];
+	p->health_bitmap_valid = ret[1];
+
+	dev_dbg(&p->pdev->dev,
+		"Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
+		be64_to_cpu(p->health_bitmap),
+		be64_to_cpu(p->health_bitmap_valid));
+
+	mutex_unlock(&p->dimm_mutex);
+	return 0;
+}
 
 static int papr_scm_meta_get(struct papr_scm_priv *p,
 			     struct nd_cmd_get_config_data_hdr *hdr)
@@ -304,6 +341,67 @@ static inline int papr_scm_node(int node)
 	return min_node;
 }
 
+static ssize_t papr_flags_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct nvdimm *dimm = to_nvdimm(dev);
+	struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+	__be64 health;
+	int rc;
+
+	rc = drc_pmem_query_health(p);
+	if (rc)
+		return rc;
+
+	/* Protect against modifications to papr_scm_priv with the mutex */
+	rc = mutex_lock_interruptible(&p->dimm_mutex);
+	if (rc)
+		return rc;
+
+	health = p->health_bitmap & p->health_bitmap_valid;
+
+	/* Check for various masks in bitmap and set the buffer */
+	if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+		rc += sprintf(buf, "not_armed ");
+
+	if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+		rc += sprintf(buf + rc, "save_fail ");
+
+	if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+		rc += sprintf(buf + rc, "restore_fail ");
+
+	if (health & PAPR_SCM_DIMM_ENCRYPTED)
+		rc += sprintf(buf + rc, "encrypted ");
+
+	if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
+		rc += sprintf(buf + rc, "smart_notify ");
+
+	if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
+		rc += sprintf(buf + rc, "scrubbed locked ");
+
+	if (rc > 0)
+		rc += sprintf(buf + rc, "\n");
+
+	mutex_unlock(&p->dimm_mutex);
+	return rc;
+}
+DEVICE_ATTR_RO(papr_flags);
+
+/* papr_scm specific dimm attributes */
+static struct attribute *papr_scm_nd_attributes[] = {
+	&dev_attr_papr_flags.attr,
+	NULL,
+};
+
+static struct attribute_group papr_scm_nd_attribute_group = {
+	.attrs = papr_scm_nd_attributes,
+};
+
+static const struct attribute_group *papr_scm_dimm_attr_groups[] = {
+	&papr_scm_nd_attribute_group,
+	NULL,
+};
+
 static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
 {
 	struct device *dev = &p->pdev->dev;
@@ -330,8 +428,8 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
 	dimm_flags = 0;
 	set_bit(NDD_ALIASING, &dimm_flags);
 
-	p->nvdimm = nvdimm_create(p->bus, p, NULL, dimm_flags,
-				  PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
+	p->nvdimm = nvdimm_create(p->bus, p, papr_scm_dimm_attr_groups,
+				  dimm_flags, PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
 	if (!p->nvdimm) {
 		dev_err(dev, "Error creating DIMM object for %pOF\n", p->dn);
 		goto err;
@@ -415,6 +513,9 @@ static int papr_scm_probe(struct platform_device *pdev)
 	if (!p)
 		return -ENOMEM;
 
+	/* Initialize the dimm mutex */
+	mutex_init(&p->dimm_mutex);
+
 	/* optional DT properties */
 	of_property_read_u32(dn, "ibm,metadata-size", &metadata_size);
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/8] powerpc/papr_scm: Fetch dimm performance stats from PHYP
  2020-02-20  9:57 ` Vaibhav Jain
                   ` (2 preceding siblings ...)
  (?)
@ 2020-02-20  9:58 ` Vaibhav Jain
  2020-03-09 10:28   ` Aneesh Kumar K.V
  -1 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:58 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva, Aneesh Kumar K . V

Implement support for fetching dimm performance metrics via
H_SCM_PERFORMANCE_HEALTH hcall as documented in Ref[1]. The hcall
returns a structure as described in Ref[1] and defined as newly
introduced 'struct papr_scm_perf_stats'. The struct has a header
followed by key-value pairs of performance attributes. The 'key' part
is a 8-byte char array naming the attribute encoded as a __be64
integer. This makes the output buffer format for the hcall self
describing and can be easily interpreted.

This patch implements functionality to fetch these performance stats
and reporting them via a nvdimm sysfs attribute named
'papr_perf_stats'.

A new function drc_pmem_query_stats() is implemented that issues hcall
H_SCM_PERFORMANCE_HEALTH ,requesting PHYP to store performance stats
in pre-allocated 'struct papr_scm_perf_stats' buffer. During nvdimm
initialization in papr_scm_nvdimm_init() this function is called with
an empty buffer to know the max buffer size needed for issuing the
H_SCM_PERFORMANCE_HEALTH hcall. The buffer size retrieved is stored in
newly introduced 'struct papc_scm_priv.len_stat_buffer' for later
retrival.

[1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/papr_scm.c | 107 ++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index aaf2e4ab1f75..28143a681aa2 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -47,6 +47,9 @@ struct papr_scm_priv {
 	/* Health information for the dimm */
 	__be64 health_bitmap;
 	__be64 health_bitmap_valid;
+
+	/* length of the stat buffer as expected by phyp */
+	size_t len_stat_buffer;
 };
 
 static int drc_pmem_bind(struct papr_scm_priv *p)
@@ -152,6 +155,50 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
 	return drc_pmem_bind(p);
 }
 
+static int drc_pmem_query_stats(struct papr_scm_priv *p,
+				struct papr_scm_perf_stats *stats,
+				size_t size, uint64_t *out)
+{
+	unsigned long ret[PLPAR_HCALL_BUFSIZE];
+	int64_t rc;
+
+	/* In case of no out buffer ignore the size */
+	if (!stats)
+		size = 0;
+
+	/*
+	 * Do the HCALL asking PHYP for info and if R4 was requested
+	 * return its value in 'out' variable.
+	 */
+	rc = plpar_hcall(H_SCM_PERFORMANCE_STATS, ret, p->drc_index,
+			 __pa(stats), size);
+	if (out)
+		*out =  be64_to_cpu(ret[0]);
+
+	switch (rc) {
+	case H_SUCCESS:
+		/* Handle the case where size of stat buffer was requested */
+		if (size != 0)
+			dev_dbg(&p->pdev->dev,
+				"Performance stats returned %d stats\n",
+				be32_to_cpu(stats->num_statistics));
+		else
+			dev_dbg(&p->pdev->dev,
+				"Performance stats size %lld\n",
+				be64_to_cpu(ret[0]));
+		return 0;
+	case H_PARTIAL:
+		dev_err(&p->pdev->dev,
+			 "Unknown performance stats, Err:0x%016llX\n",
+			be64_to_cpu(ret[0]));
+		return -ENOENT;
+	default:
+		dev_err(&p->pdev->dev,
+			 "Failed to query performance stats, Err:%lld\n", rc);
+		return -ENXIO;
+	}
+}
+
 static int drc_pmem_query_health(struct papr_scm_priv *p)
 {
 	unsigned long ret[PLPAR_HCALL_BUFSIZE];
@@ -341,6 +388,53 @@ static inline int papr_scm_node(int node)
 	return min_node;
 }
 
+static ssize_t papr_perf_stats_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct nvdimm *dimm = to_nvdimm(dev);
+	struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+	struct papr_scm_perf_stats *retbuffer;
+	struct papr_scm_perf_stat *stat;
+	uint64_t statid, val;
+	int rc, i;
+
+	if (!p->len_stat_buffer)
+		return -ENOENT;
+
+	/* Return buffer for phyp where stats are written */
+	retbuffer = kzalloc(p->len_stat_buffer, GFP_KERNEL);
+	if (!retbuffer)
+		return -ENOMEM;
+
+	/* Setup the buffer */
+	memcpy(retbuffer->eye_catcher, PAPR_SCM_PERF_STATS_EYECATCHER,
+	       sizeof(retbuffer->eye_catcher));
+	retbuffer->stats_version = cpu_to_be32(0x1);
+	retbuffer->num_statistics = 0;
+
+	rc = drc_pmem_query_stats(p, retbuffer, p->len_stat_buffer, NULL);
+	if (rc)
+		goto out;
+
+	/*
+	 * Go through the returned output buffer and print stats and values.
+	 * Since statistic_id is essentially a char string of 8 bytes encoded
+	 * as a __be64, simply use the string format specifier to print it.
+	 */
+	for (i = 0, stat = retbuffer->scm_statistics;
+	    i < be32_to_cpu(retbuffer->num_statistics); ++i, ++stat) {
+		statid = be64_to_cpu(stat->statistic_id);
+		val = be64_to_cpu(stat->statistic_value);
+		rc += sprintf(buf + rc, "%.8s => 0x%016llX\n",
+			      (char *) &(statid), val);
+	}
+out:
+	kfree(retbuffer);
+	return rc;
+
+}
+DEVICE_ATTR_RO(papr_perf_stats);
+
 static ssize_t papr_flags_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
@@ -390,6 +484,7 @@ DEVICE_ATTR_RO(papr_flags);
 /* papr_scm specific dimm attributes */
 static struct attribute *papr_scm_nd_attributes[] = {
 	&dev_attr_papr_flags.attr,
+	&dev_attr_papr_perf_stats.attr,
 	NULL,
 };
 
@@ -409,6 +504,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
 	struct nd_region_desc ndr_desc;
 	unsigned long dimm_flags;
 	int target_nid, online_nid;
+	uint64_t stat_size;
 
 	p->bus_desc.ndctl = papr_scm_ndctl;
 	p->bus_desc.module = THIS_MODULE;
@@ -470,6 +566,17 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
 		dev_info(dev, "Region registered with target node %d and online node %d",
 			 target_nid, online_nid);
 
+	/* Try retriving the stat buffer and see if its supported */
+	if (!drc_pmem_query_stats(p, NULL, 0, &stat_size)) {
+		p->len_stat_buffer = (size_t)stat_size;
+		dev_dbg(&p->pdev->dev, "Max dimm perf stats size %ld bytes\n",
+			p->len_stat_buffer);
+	} else {
+		p->len_stat_buffer = 0;
+		dev_dbg(&p->pdev->dev, "Unable to retrieve performace stats\n");
+		dev_info(&p->pdev->dev, "Limited dimm info available\n");
+	}
+
 	return 0;
 
 err:	nvdimm_bus_unregister(p->bus);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/8] UAPI: ndctl: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM family
  2020-02-20  9:57 ` Vaibhav Jain
@ 2020-02-20  9:58   ` Vaibhav Jain
  -1 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:58 UTC (permalink / raw)
  To: linuxppc-dev, linux-nvdimm
  Cc: Vaibhav Jain, Aneesh Kumar K . V, Michael Ellerman, Alastair D'Silva

Add PAPR-scm family of DSM command-set to the white list of NVDIMM
command sets.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 include/uapi/linux/ndctl.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index de5d90212409..99fb60600ef8 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -244,6 +244,7 @@ struct nd_cmd_pkg {
 #define NVDIMM_FAMILY_HPE2 2
 #define NVDIMM_FAMILY_MSFT 3
 #define NVDIMM_FAMILY_HYPERV 4
+#define NVDIMM_FAMILY_PAPR_SCM 5
 
 #define ND_IOCTL_CALL			_IOWR(ND_IOCTL, ND_CMD_CALL,\
 					struct nd_cmd_pkg)
-- 
2.24.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/8] UAPI: ndctl: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM family
@ 2020-02-20  9:58   ` Vaibhav Jain
  0 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:58 UTC (permalink / raw)
  To: linuxppc-dev, linux-nvdimm
  Cc: Alastair D'Silva, Aneesh Kumar K . V, Oliver O'Halloran,
	Vishal Verma, Vaibhav Jain, Michael Ellerman, Dan Williams

Add PAPR-scm family of DSM command-set to the white list of NVDIMM
command sets.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 include/uapi/linux/ndctl.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index de5d90212409..99fb60600ef8 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -244,6 +244,7 @@ struct nd_cmd_pkg {
 #define NVDIMM_FAMILY_HPE2 2
 #define NVDIMM_FAMILY_MSFT 3
 #define NVDIMM_FAMILY_HYPERV 4
+#define NVDIMM_FAMILY_PAPR_SCM 5
 
 #define ND_IOCTL_CALL			_IOWR(ND_IOCTL, ND_CMD_CALL,\
 					struct nd_cmd_pkg)
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/8] powerpc/uapi: Introduce uapi header 'papr_scm_dsm.h' for papr_scm DSMs
  2020-02-20  9:57 ` Vaibhav Jain
                   ` (4 preceding siblings ...)
  (?)
@ 2020-02-20  9:58 ` Vaibhav Jain
  -1 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:58 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva, Aneesh Kumar K . V

Define and add a new uapi header for papr_scm describing device
specific methods (DSMs) and structs for libndctl. PAPR-SCM specific
implementation in libndctl will use these commands/structs to interact
with papr_scm kernel module. Currently only DSMs to retrieve health and
performance statistics information of a dimm are defined.

DSM Envelope
=============

The ioctl ND_CMD_CALL transfers data between user-space and kernel via
'envelopes'. An envelope consists of a header and user-defined payload
section. The primary structure describing this envelope is 'struct
nd_papr_scm_cmd_pkg' which expects a payload at the end of the envelop
pointed to by 'nd_papr_scm_cmd_pkg.payload_offset'. Currently two
payloads are defined 'struct nd_papr_scm_dimm_health_stat' and 'struct
nd_papr_scm_perf_stats'. These can be used to retrieve dimm-health and
performance stats respectively.

The header is defined as 'struct nd_cmd_pkg' which in return is
wrapped in a user defined struct called 'struct
nd_papr_scm_cmd_pkg'. This relationship is illustrated below:

     64-Bytes         8-Bytes
 +-------------+-------------------+------------------------------+
 | nd_family   |	       	   |				  |
 |             |                   |				  |
 | nd_size_out | cmd_status        |				  |
 |             |                   |         			  |
 | nd_size_in  | payload_version   |           PAYLOAD      	  |
 |             |                   |           			  |
 | nd_command  | payload_offset    |				  |
 |             |      |            |				  |
 | nd_fw_size  |      +----------> |				  |
 +-------------+-------------------+------------------------------+
 \ nd_cmd_pkg /                   /                              /
  \----------/                   /                              /
   \ 	nd_papr_scm_cmd_pkg     /			       /
    \--------------------------/      	       	       	      /
     \  	 		  Envelope	  	     /
      \-----------------------------------------------------/

Important fields to note in above illustration are:

* 'nd_command'	: DSM command sent by libndctl
* 'nd_family' 	: Id for newly introduced DSM family NVDIMM_FAMILY_PAPR_SCM
* 'nd_fw_size'	: Number of bytes that kernel wanted to copy to the
  payload but may not have copied due to limited size of the envelope.
* 'nd_size_in/out' : Number of bytes that kernel needs to copy from
  user-space (in) and copy-back to user-space (out).
* 'cmd_status' 	: Out variable indicating any error encountered while
  servicing the DSM.
* 'payload_version': Version number associated with the payload.
* 'payload_offset': Offset of the payload from start of the envelope.

libnvdimm enforces a hard limit of 256 bytes on the envelope size,
which leaves around 184 bytes for the envelope payload (ignoring any
padding that the compiler may silently introduce).

Envelope Payload Layout
=======================

The layout of the DSM Payload is defined by various structs defined in
'papr_scm_dsm.h'. Definition of these structs are shared between
papr_scm and libndctl so that contents of payload can be
interpreted. This patch-set introduces two such structs namely
'nd_papr_scm_dimm_health_stat' and 'nd_papr_scm_perf_stats' that can
be used to exchange dimm health and performance stats between papr_scm
and libndctl.

During servicing of a DSM the papr_scm module will read input args
from the payload field by casting its contents to an appropriate
struct pointer based on the DSM command. Similarly the output of
servicing the DSM command will be copied to the payload field using
the same struct.

Payload Version
===============

Since the structs associated with each DSM can evolve over time adding
more data and the definitions of these structs known to papr_scm and
libndctl may differ, hence the version number is associated with each
iteration of the struct. This version number is exchanged between
papr_scm <-> libndctl via the 'payload_version' of the DSM envelope.

When libndctl sends an envelope to papr_scm it populates the
'payload_version' field with the version number of the struct it had
copied and/or expects in the payload area. The papr_scm module when
servicing the DSM envelop checks the 'payload_version', if required
changes it to a different version number that it knows about and then
use the DSM struct associated with the new version number to process
the DSM (i.e read the args and copy the results to the payload
area). Libndctl on receiving the envelop back from papr_scm again
checks the 'payload_version' field and based on it use the appropriate
version dsm struct to parse the results.

Above scheme of exchanging different versioned DSM struct between
libndctl and papr_scm should work until following two assumptions
hold:

Let T(X) = { set of attributes of DSM struct 'T' versioned X }

1. T(X) is a proper subset of T(Y) if X > Y.
   i.e Each new version of DSM struct should retain existing struct
   attributes.

2. If an entity (libndctl or papr_scm) supports a DSM struct T(X) then
   it should also support T(1), T(2)...T(X - 1).
   i.e When adding support for new version of a DSM struct, libndctl
   and papr_scm should retain support of the existing DSM struct
   version they support.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 143 +++++++++++++++++++
 1 file changed, 143 insertions(+)
 create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h

diff --git a/arch/powerpc/include/uapi/asm/papr_scm_dsm.h b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
new file mode 100644
index 000000000000..aacced453579
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
@@ -0,0 +1,143 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * PAPR SCM Device specific methods for libndctl and ndctl
+ *
+ * (C) Copyright IBM 2020
+ *
+ * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_
+#define _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_
+
+#include <linux/types.h>
+
+#ifdef __KERNEL__
+#include <linux/ndctl.h>
+#else
+#include <ndctl.h>
+#endif
+
+/*
+ * Sub commands for ND_CMD_CALL. To prevent overlap from ND_CMD_*, values for
+ * these enums start at 0x10000. These values are then returned from
+ * cmd_to_func() making it easy to implement the switch-case block in
+ * papr_scm_ndctl()
+ */
+enum dsm_papr_scm {
+	DSM_PAPR_SCM_MIN =  0x10000,
+	DSM_PAPR_SCM_HEALTH,
+	DSM_PAPR_SCM_STATS,
+	DSM_PAPR_SCM_MAX,
+};
+
+enum dsm_papr_scm_dimm_health {
+	DSM_PAPR_SCM_DIMM_HEALTHY,
+	DSM_PAPR_SCM_DIMM_UNHEALTHY,
+	DSM_PAPR_SCM_DIMM_CRITICAL,
+	DSM_PAPR_SCM_DIMM_FATAL,
+};
+
+/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
+struct nd_papr_scm_cmd_pkg {
+	struct nd_cmd_pkg hdr;		/* Package header containing sub-cmd */
+	int32_t cmd_status;		/* Out: Sub-cmd status returned back */
+	uint16_t payload_offset;	/* In: offset from start of struct */
+	uint16_t payload_version;	/* In/Out: version of the payload */
+	uint8_t payload[];		/* In/Out: Sub-cmd data buffer */
+};
+
+/* Helpers to evaluate the size of PAPR_SCM envelope */
+/* Calculate the papr_scm-header size */
+#define ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE \
+	(sizeof(struct nd_papr_scm_cmd_pkg) - sizeof(struct nd_cmd_pkg))
+/*
+ * Given a type calculate the envelope size
+ * (nd-header + papr_scm-header + payload)
+ */
+#define ND_PAPR_SCM_ENVELOPE_SIZE(_type_)	\
+	(sizeof(_type_) + sizeof(struct nd_papr_scm_cmd_pkg))
+
+/* Given a type envelope-content size (papr_scm-header + payload) */
+#define ND_PAPR_SCM_ENVELOPE_CONTENT_SIZE(_type_)	\
+	(sizeof(_type_) + ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE)
+
+/*
+ * Struct exchanged between kernel & ndctl in for PAPR_DSM_PAPR_SMART_HEALTH
+ * Various bitflags indicate the health status of the dimm.
+ */
+struct nd_papr_scm_dimm_health_stat_v1 {
+	/* Dimm not armed. So contents wont persist */
+	bool dimm_unarmed;
+	/* Previous shutdown did not persist contents */
+	bool dimm_bad_shutdown;
+	/* Contents from previous shutdown werent restored */
+	bool dimm_bad_restore;
+	/* Contents of the dimm have been scrubbed */
+	bool dimm_scrubbed;
+	/* Contents of the dimm cant be modified until CEC reboot */
+	bool dimm_locked;
+	/* Contents of dimm are encrypted */
+	bool dimm_encrypted;
+
+	enum dsm_papr_scm_dimm_health dimm_health;
+};
+
+/*
+ * Typedef the current struct for dimm_health so that any application
+ * or kernel recompiled after introducing a new version autometically
+ * supports the new version.
+ */
+#define nd_papr_scm_dimm_health_stat nd_papr_scm_dimm_health_stat_v1
+
+/* Current version number for the dimm health struct */
+#define ND_PAPR_SCM_DIMM_HEALTH_VERSION 1
+
+/* Struct holding a single performance metric */
+struct nd_papr_scm_perf_stat {
+	u64 statistic_id;
+	u64 statistic_value;
+};
+
+/* Struct exchanged between kernel and ndctl reporting drc perf stats */
+struct nd_papr_scm_perf_stats_v1 {
+	/* Number of stats following */
+	u32 num_statistics;
+
+	/* zero or more performance matrics */
+	struct nd_papr_scm_perf_stat scm_statistics[];
+};
+
+/*
+ * Typedef the current struct for dimm_stats so that any application
+ * or kernel recompiled after introducing a new version autometically
+ * supports the new version.
+ */
+#define nd_papr_scm_perf_stats nd_papr_scm_perf_stats_v1
+#define ND_PAPR_SCM_DIMM_PERF_STATS_VERSION 1
+
+/* Convert a libnvdimm nd_cmd_pkg to papr_scm specific pkg */
+static struct nd_papr_scm_cmd_pkg *nd_to_papr_cmd_pkg(struct nd_cmd_pkg *cmd)
+{
+	return (struct nd_papr_scm_cmd_pkg *) cmd;
+}
+
+/* Return the payload pointer for a given pcmd */
+static void *papr_scm_pcmd_to_payload(struct nd_papr_scm_cmd_pkg *pcmd)
+{
+	if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
+		return NULL;
+	else
+		return (void *)((u8 *) pcmd + pcmd->payload_offset);
+}
+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/8] powerpc/papr_scm: Add support for handling PAPR DSM commands
  2020-02-20  9:57 ` Vaibhav Jain
                   ` (5 preceding siblings ...)
  (?)
@ 2020-02-20  9:58 ` Vaibhav Jain
  -1 siblings, 0 replies; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:58 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva, Aneesh Kumar K . V

Implement support for handling PAPR DSM commands in papr_scm
module. We advertise support for ND_CMD_CALL for the dimm command mask
and implement necessary scaffolding in the module to handle ND_CMD_CALL
ioctl and DSM commands that we receive.

The layout of the DSM commands as we expect from libnvdimm/libndctl is
defined in 'struct nd_pkg_papr_scm' which contains a 'struct
nd_cmd_pkg' as header. This header is used to communicate the DSM
command via 'nd_pkg_papr_scm->nd_command' and size of payload that
need to be sent/received for servicing the DSM.

The PAPR DSM commands are assigned indexes started from 0x10000 to
prevent them from overlapping ND_CMD_* values and also makes handling
dimm commands in papr_scm_ndctl() easier via a simplified switch-case
block. For this a new function cmd_to_func() is implemented that reads
the args to papr_scm_ndctl() , performs sanity tests on them and
converts them to PAPR DSM commands which can then be handled via the
switch-case block.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Change-log:
* Added a 'reserved' field in 'struct nd_pkg_papr_scm' to ensure
  'payload' falls on a 8-Byte aligned boundary.
---
 arch/powerpc/platforms/pseries/papr_scm.c | 87 +++++++++++++++++++++--
 1 file changed, 80 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 28143a681aa2..d5eea2f38fa6 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -15,13 +15,15 @@
 
 #include <asm/plpar_wrappers.h>
 #include <asm/papr_scm.h>
+#include <asm/papr_scm_dsm.h>
 
 #define BIND_ANY_ADDR (~0ul)
 
 #define PAPR_SCM_DIMM_CMD_MASK \
 	((1ul << ND_CMD_GET_CONFIG_SIZE) | \
 	 (1ul << ND_CMD_GET_CONFIG_DATA) | \
-	 (1ul << ND_CMD_SET_CONFIG_DATA))
+	 (1ul << ND_CMD_SET_CONFIG_DATA) | \
+	 (1ul << ND_CMD_CALL))
 
 struct papr_scm_priv {
 	struct platform_device *pdev;
@@ -330,19 +332,82 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,
 	return 0;
 }
 
+/*
+ * Validate the input to dimm-control function and return papr_scm specific
+ * commands. This does sanity validation to ND_CMD_CALL sub-command packages.
+ */
+static int cmd_to_func(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
+		       unsigned int buf_len)
+{
+	unsigned long cmd_mask = PAPR_SCM_DIMM_CMD_MASK;
+	struct nd_papr_scm_cmd_pkg *pkg = nd_to_papr_cmd_pkg(buf);
+
+	/* Only dimm-specific calls are supported atm */
+	if (!nvdimm)
+		return -EINVAL;
+
+	if (!test_bit(cmd, &cmd_mask)) {
+		pr_debug("%s: Unsupported cmd=%u\n", __func__, cmd);
+		return -EINVAL;
+	} else if (cmd != ND_CMD_CALL) {
+		return cmd;
+	}
+
+	/* cmd == ND_CMD_CALL so verify the envelop package */
+
+	if (!buf || buf_len < sizeof(struct nd_papr_scm_cmd_pkg)) {
+		pr_debug("%s: Invalid pkg size=%u\n", __func__, buf_len);
+		return -EINVAL;
+	}
+
+	if (pkg->hdr.nd_family != NVDIMM_FAMILY_PAPR_SCM) {
+		pr_debug("%s: Invalid pkg family=0x%llx\n", __func__,
+			 pkg->hdr.nd_family);
+		return -EINVAL;
+
+	}
+
+	if (pkg->hdr.nd_command <= DSM_PAPR_SCM_MIN ||
+	    pkg->hdr.nd_command >= DSM_PAPR_SCM_MAX) {
+
+		/* for unknown subcommands return ND_CMD_CALL */
+		pr_debug("%s: Unknown sub-command=0x%llx\n", __func__,
+			 pkg->hdr.nd_command);
+		return ND_CMD_CALL;
+	}
+
+	/* We except a payload with all DSM commands */
+	if (papr_scm_pcmd_to_payload(pkg) == NULL) {
+		pr_debug("%s: Empty patload for sub-command=0x%llx\n", __func__,
+			 pkg->hdr.nd_command);
+		return -EINVAL;
+	}
+
+	/* Return the DSM_PAPR_SCM_* command */
+	return pkg->hdr.nd_command;
+}
+
 int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
 {
 	struct nd_cmd_get_config_size *get_size_hdr;
 	struct papr_scm_priv *p;
+	struct nd_papr_scm_cmd_pkg *call_pkg = NULL;
+	int cmd_in, rc;
 
-	/* Only dimm-specific calls are supported atm */
-	if (!nvdimm)
-		return -EINVAL;
+	/* Use a local variable in case cmd_rc pointer is NULL */
+	if (cmd_rc == NULL)
+		cmd_rc = &rc;
+
+	cmd_in = cmd_to_func(nvdimm, cmd, buf, buf_len);
+	if (cmd_in < 0) {
+		pr_debug("%s: Invalid cmd=%u. Err=%d\n", __func__, cmd, cmd_in);
+		return cmd_in;
+	}
 
 	p = nvdimm_provider_data(nvdimm);
 
-	switch (cmd) {
+	switch (cmd_in) {
 	case ND_CMD_GET_CONFIG_SIZE:
 		get_size_hdr = buf;
 
@@ -360,13 +425,21 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		*cmd_rc = papr_scm_meta_set(p, buf);
 		break;
 
+	case ND_CMD_CALL:
+		/* This happens if subcommand package sanity fails */
+		call_pkg = nd_to_papr_cmd_pkg(buf);
+		call_pkg->cmd_status = -ENOENT;
+		*cmd_rc = 0;
+		break;
+
 	default:
-		return -EINVAL;
+		dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd_in);
+		*cmd_rc = -EINVAL;
 	}
 
 	dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
 
-	return 0;
+	return *cmd_rc;
 }
 
 static inline int papr_scm_node(int node)
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/8] powerpc/papr_scm: Re-implement 'papr_flags' using 'nd_papr_scm_dimm_health_stat'
  2020-02-20  9:57 ` Vaibhav Jain
                   ` (6 preceding siblings ...)
  (?)
@ 2020-02-20  9:58 ` Vaibhav Jain
  2020-03-09 10:27   ` Aneesh Kumar K.V
  -1 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:58 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva, Aneesh Kumar K . V

Previous commit [1] introduced 'struct nd_papr_scm_dimm_health_stat' for
communicating health status of an nvdimm to libndctl. This struct
however can also be used to cache the nvdimm health information in
'struct papr_scm_priv' instead of two '__be64' values. Benefit of this
re-factoring will be apparent when support for libndctl being able to
request nvdimm health stats is implemented where we can simply memcpy
this struct over to the user-space provided payload envelope.

Hence this patch introduces a new member 'struct papr_scm_priv.health'
that caches the health information of a dimm. This member is populated
inside drc_pmem_query_health() which checks for the various bit flags
returned from H_SCM_HEALTH and sets them in this struct. We also
re-factor 'papr_flags' sysfs attribute show function papr_flags_show()
to use the flags in 'struct papr_scm_priv.health' to return
appropriate status strings pertaining to dimm health.

This patch shouldn't introduce any behavioral change.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/papr_scm.c | 61 ++++++++++++++++-------
 1 file changed, 44 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index d5eea2f38fa6..29f38246c59f 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -47,8 +47,7 @@ struct papr_scm_priv {
 	struct mutex dimm_mutex;
 
 	/* Health information for the dimm */
-	__be64 health_bitmap;
-	__be64 health_bitmap_valid;
+	struct nd_papr_scm_dimm_health_stat health;
 
 	/* length of the stat buffer as expected by phyp */
 	size_t len_stat_buffer;
@@ -205,6 +204,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
 {
 	unsigned long ret[PLPAR_HCALL_BUFSIZE];
 	int64_t rc;
+	__be64 health;
 
 	rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
 	if (rc != H_SUCCESS) {
@@ -219,13 +219,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
 		return rc;
 
 	/* Store the retrieved health information in dimm platform data */
-	p->health_bitmap = ret[0];
-	p->health_bitmap_valid = ret[1];
+	health = ret[0] & ret[1];
 
 	dev_dbg(&p->pdev->dev,
 		"Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
-		be64_to_cpu(p->health_bitmap),
-		be64_to_cpu(p->health_bitmap_valid));
+		be64_to_cpu(ret[0]),
+		be64_to_cpu(ret[1]));
+
+	memset(&p->health, 0, sizeof(p->health));
+
+	/* Check for various masks in bitmap and set the buffer */
+	if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+		p->health.dimm_unarmed = true;
+
+	if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+		p->health.dimm_bad_shutdown = true;
+
+	if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+		p->health.dimm_bad_restore = true;
+
+	if (health & PAPR_SCM_DIMM_ENCRYPTED)
+		p->health.dimm_encrypted = true;
+
+	if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED) {
+		p->health.dimm_locked = true;
+		p->health.dimm_scrubbed = true;
+	}
+
+	if (health & PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
+		p->health.dimm_health = DSM_PAPR_SCM_DIMM_UNHEALTHY;
+
+	if (health & PAPR_SCM_DIMM_HEALTH_CRITICAL)
+		p->health.dimm_health = DSM_PAPR_SCM_DIMM_CRITICAL;
+
+	if (health & PAPR_SCM_DIMM_HEALTH_FATAL)
+		p->health.dimm_health = DSM_PAPR_SCM_DIMM_FATAL;
 
 	mutex_unlock(&p->dimm_mutex);
 	return 0;
@@ -513,7 +541,6 @@ static ssize_t papr_flags_show(struct device *dev,
 {
 	struct nvdimm *dimm = to_nvdimm(dev);
 	struct papr_scm_priv *p = nvdimm_provider_data(dimm);
-	__be64 health;
 	int rc;
 
 	rc = drc_pmem_query_health(p);
@@ -525,26 +552,26 @@ static ssize_t papr_flags_show(struct device *dev,
 	if (rc)
 		return rc;
 
-	health = p->health_bitmap & p->health_bitmap_valid;
-
-	/* Check for various masks in bitmap and set the buffer */
-	if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+	if (p->health.dimm_unarmed)
 		rc += sprintf(buf, "not_armed ");
 
-	if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+	if (p->health.dimm_bad_shutdown)
 		rc += sprintf(buf + rc, "save_fail ");
 
-	if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+	if (p->health.dimm_bad_restore)
 		rc += sprintf(buf + rc, "restore_fail ");
 
-	if (health & PAPR_SCM_DIMM_ENCRYPTED)
+	if (p->health.dimm_encrypted)
 		rc += sprintf(buf + rc, "encrypted ");
 
-	if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
+	if (p->health.dimm_health)
 		rc += sprintf(buf + rc, "smart_notify ");
 
-	if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
-		rc += sprintf(buf + rc, "scrubbed locked ");
+	if (p->health.dimm_scrubbed)
+		rc += sprintf(buf + rc, "scrubbed ");
+
+	if (p->health.dimm_locked)
+		rc += sprintf(buf + rc, "locked ");
 
 	if (rc > 0)
 		rc += sprintf(buf + rc, "\n");
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 8/8] powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH
  2020-02-20  9:57 ` Vaibhav Jain
                   ` (7 preceding siblings ...)
  (?)
@ 2020-02-20  9:58 ` Vaibhav Jain
  2020-03-09 10:58   ` Aneesh Kumar K.V
  -1 siblings, 1 reply; 16+ messages in thread
From: Vaibhav Jain @ 2020-02-20  9:58 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva, Aneesh Kumar K . V

The DSM 'DSM_PAPR_SCM_HEALTH' should return a 'struct
nd_papr_scm_dimm_health_stat' containing information in dimm health back
to user space in response to ND_CMD_CALL. We implement this DSM by
implementing a new function papr_scm_get_health() that queries the
DIMM health information and then copies these bitmaps to the package
payload whose layout is defined by 'struct papr_scm_ndctl_health'.

The patch also handle cases where in future versions of 'struct
papr_scm_ndctl_health' may want to return more health
information. Such payload envelops will contain appropriate version
information in 'struct nd_papr_scm_cmd_pkg.payload_version'. The patch
takes care of only returning the sub-data corresponding to the payload
version requested. Please see the comments in papr_scm_get_health()
for how this is done.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/papr_scm.c | 73 +++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 29f38246c59f..bf81acb0bf3f 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -415,6 +415,74 @@ static int cmd_to_func(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
 	return pkg->hdr.nd_command;
 }
 
+/*
+ * Fetch the DIMM health info and populate it in provided papr_scm package.
+ * Since the caller can request a different version of payload and each new
+ * version of struct nd_papr_scm_dimm_health_stat is a proper-subset of
+ * previous version hence we return a subset of the cached 'struct
+ * nd_papr_scm_dimm_health_stat' depending on the payload version requested.
+ */
+static int papr_scm_get_health(struct papr_scm_priv *p,
+			       struct nd_papr_scm_cmd_pkg *pkg)
+{
+	int rc;
+	size_t copysize;
+	/* Map version to number of bytes to be copied to payload */
+	const size_t copysizes[] = {
+		[1] =
+		sizeof(struct nd_papr_scm_dimm_health_stat_v1),
+
+		/*  This should always be preset */
+		[ND_PAPR_SCM_DIMM_HEALTH_VERSION] =
+		sizeof(struct nd_papr_scm_dimm_health_stat),
+	};
+
+	rc = drc_pmem_query_health(p);
+	if (rc)
+		goto out;
+	/*
+	 * If the requested payload version is greater than one we know
+	 * aboute, return the payload version we know about and let
+	 * caller/userspace handle the mess.
+	 */
+	if (pkg->payload_version > ND_PAPR_SCM_DIMM_HEALTH_VERSION)
+		pkg->payload_version = ND_PAPR_SCM_DIMM_HEALTH_VERSION;
+
+	copysize = copysizes[pkg->payload_version];
+	if (!copysize) {
+		dev_dbg(&p->pdev->dev, "%s Unsupported payload version=0x%x\n",
+			__func__, pkg->payload_version);
+		rc = -ENOSPC;
+		goto out;
+	}
+
+	if (pkg->hdr.nd_size_out < copysize) {
+		dev_dbg(&p->pdev->dev, "%s Payload not large enough\n",
+			__func__);
+		dev_dbg(&p->pdev->dev, "%s Expected %lu, available %u\n",
+			__func__, copysize, pkg->hdr.nd_size_out);
+		rc = -ENOSPC;
+		goto out;
+	}
+
+	dev_dbg(&p->pdev->dev, "%s Copying payload size=%lu version=0x%x\n",
+		__func__, copysize, pkg->payload_version);
+
+	/* Copy a subset of health struct based on copysize */
+	memcpy(papr_scm_pcmd_to_payload(pkg), &p->health, copysize);
+	pkg->hdr.nd_fw_size = copysize;
+
+out:
+	/*
+	 * Put the error in out package and return success from function
+	 * so that errors if any are propogated back to userspace.
+	 */
+	pkg->cmd_status = rc;
+	dev_dbg(&p->pdev->dev, "%s completion code = %d\n", __func__, rc);
+
+	return 0;
+}
+
 int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
 {
@@ -460,6 +528,11 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		*cmd_rc = 0;
 		break;
 
+	case DSM_PAPR_SCM_HEALTH:
+		call_pkg = nd_to_papr_cmd_pkg(buf);
+		*cmd_rc = papr_scm_get_health(p, call_pkg);
+		break;
+
 	default:
 		dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd_in);
 		*cmd_rc = -EINVAL;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/8] powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface
  2020-02-20  9:57 ` [PATCH 1/8] powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface Vaibhav Jain
@ 2020-03-09 10:07   ` Aneesh Kumar K.V
  0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2020-03-09 10:07 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> Add a new powerpc specific asm header named 'papr-scm.h' that descibes
> the interface between PHYP and guest kernel running as an LPAR.
>
> The HCALLs specific to managing SCM are descibed in Ref[1]. The asm
> header introduced by this patch however describes the data structures
> exchanged between PHYP and kernel during those HCALLs.
>
> Future patches will use these structures to provide support for
> retriving nvdimm health and performance stats in papr_scm kernel
> module.
>
> [1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
> PAPR hcalls")
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/papr_scm.h | 68 +++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 arch/powerpc/include/asm/papr_scm.h
>
> diff --git a/arch/powerpc/include/asm/papr_scm.h b/arch/powerpc/include/asm/papr_scm.h
> new file mode 100644
> index 000000000000..d893621063f3
> --- /dev/null
> +++ b/arch/powerpc/include/asm/papr_scm.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Structures and defines needed to manage nvdimms for spapr guests.
> + */
> +#ifndef _ASM_POWERPC_PAPR_SCM_H_
> +#define _ASM_POWERPC_PAPR_SCM_H_
> +
> +#include <linux/types.h>
> +#include <asm/bitsperlong.h>
> +#include <linux/stringify.h>
> +
> +/* DIMM health bitmap bitmap indicators */
> +/* SCM device is unable to persist memory contents */
> +#define PAPR_SCM_DIMM_UNARMED			PPC_BIT(0)
> +/* SCM device failed to persist memory contents */
> +#define PAPR_SCM_DIMM_SHUTDOWN_DIRTY		PPC_BIT(1)
> +/* SCM device contents are persisted from previous IPL */
> +#define PAPR_SCM_DIMM_SHUTDOWN_CLEAN		PPC_BIT(2)
> +/* SCM device contents are not persisted from previous IPL */
> +#define PAPR_SCM_DIMM_EMPTY			PPC_BIT(3)
> +/* SCM device memory life remaining is critically low */
> +#define PAPR_SCM_DIMM_HEALTH_CRITICAL		PPC_BIT(4)
> +/* SCM device will be garded off next IPL due to failure */
> +#define PAPR_SCM_DIMM_HEALTH_FATAL		PPC_BIT(5)
> +/* SCM contents cannot persist due to current platform health status */
> +#define PAPR_SCM_DIMM_HEALTH_UNHEALTHY		PPC_BIT(6)
> +/* SCM device is unable to persist memory contents in certain conditions */
> +#define PAPR_SCM_DIMM_HEALTH_NON_CRITICAL	PPC_BIT(7)

That is not really a bad health condition. That is an indication of
vpmem where we won't persist memory contents on CEC reboot.



> +/* SCM device is encrypted */
> +#define PAPR_SCM_DIMM_ENCRYPTED			PPC_BIT(8)
> +/* SCM device has been scrubbed and locked */
> +#define PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED	PPC_BIT(9)
> +
> +/* Bits status indicators for health bitmap indicating unarmed dimm */
> +#define PAPR_SCM_DIMM_UNARMED_MASK (PAPR_SCM_DIMM_UNARMED |	\
> +					PAPR_SCM_DIMM_HEALTH_UNHEALTHY | \
> +					PAPR_SCM_DIMM_HEALTH_NON_CRITICAL)

Based on the above, I guess you should remove PAPR_SCM_DIMM_HEALTH_NON_CRITICAL
from the above?

> +
> +/* Bits status indicators for health bitmap indicating unflushed dimm */
> +#define PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK (PAPR_SCM_DIMM_SHUTDOWN_DIRTY)
> +
> +/* Bits status indicators for health bitmap indicating unrestored dimm */
> +#define PAPR_SCM_DIMM_BAD_RESTORE_MASK  (PAPR_SCM_DIMM_EMPTY)
> +
> +/* Bit status indicators for smart event notification */
> +#define PAPR_SCM_DIMM_SMART_EVENT_MASK (PAPR_SCM_DIMM_HEALTH_CRITICAL | \
> +					   PAPR_SCM_DIMM_HEALTH_FATAL | \
> +					   PAPR_SCM_DIMM_HEALTH_UNHEALTHY | \
> +					   PAPR_SCM_DIMM_HEALTH_NON_CRITICAL)
> +

-aneesh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/8] powerpc/papr_scm: Provide support for fetching dimm health information
  2020-02-20  9:57 ` [PATCH 2/8] powerpc/papr_scm: Provide support for fetching dimm health information Vaibhav Jain
@ 2020-03-09 10:24   ` Aneesh Kumar K.V
  0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2020-03-09 10:24 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> Implement support for fetching dimm health information via
> H_SCM_HEALTH hcall as documented in Ref[1]. The hcall returns a pair of
> 64-bit big-endian integers which are then stored in 'struct
> papr_scm_priv' and subsequently exposed to userspace via dimm
> attribute 'papr_flags'.

Can you  add this as part of PATCH 1. While reviewing i had to go back
and forth between 1 and 2 to find different flags. IMHO it would be nice
to introduce new flags and users of the flag together. 
>
> 'papr_flags' sysfs attribute reports space separated string flags
> indicating various health state an nvdimm can be. These are:
>
> * "not_armed" 	: Indicating that nvdimm contents wont survive a power
>   		  cycle.
> * "save_fail" 	: Indicating that nvdimm contents couldn't be flushed
> 		  during last shutdown event.
> * "restore_fail": Indicating that nvdimm contents couldn't be restored
> 		  during dimm initialization.
> * "encrypted" 	: Dimm contents are encrypted.
> * "smart_notify": There is health event for the nvdimm.

Can you explain this more? This is suppose to be set when there is a
smart event? Currently you derive this based on

+/* Bit status indicators for smart event notification */
+#define PAPR_SCM_DIMM_SMART_EVENT_MASK (PAPR_SCM_DIMM_HEALTH_CRITICAL | \
+					   PAPR_SCM_DIMM_HEALTH_FATAL | \
+					   PAPR_SCM_DIMM_HEALTH_UNHEALTHY | \
+


> * "scrubbed" 	: Indicating that contents of the nvdimm have been
>   		  scrubbed.
> * "locked"	: Indicating that nvdimm contents cant be modified
>   		  until next power cycle.
>
> [1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
> PAPR hcalls")
>


-aneesh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 7/8] powerpc/papr_scm: Re-implement 'papr_flags' using 'nd_papr_scm_dimm_health_stat'
  2020-02-20  9:58 ` [PATCH 7/8] powerpc/papr_scm: Re-implement 'papr_flags' using 'nd_papr_scm_dimm_health_stat' Vaibhav Jain
@ 2020-03-09 10:27   ` Aneesh Kumar K.V
  0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2020-03-09 10:27 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> Previous commit [1] introduced 'struct nd_papr_scm_dimm_health_stat' for
> communicating health status of an nvdimm to libndctl. This struct
> however can also be used to cache the nvdimm health information in
> 'struct papr_scm_priv' instead of two '__be64' values. Benefit of this
> re-factoring will be apparent when support for libndctl being able to
> request nvdimm health stats is implemented where we can simply memcpy
> this struct over to the user-space provided payload envelope.
>
> Hence this patch introduces a new member 'struct papr_scm_priv.health'
> that caches the health information of a dimm. This member is populated
> inside drc_pmem_query_health() which checks for the various bit flags
> returned from H_SCM_HEALTH and sets them in this struct. We also
> re-factor 'papr_flags' sysfs attribute show function papr_flags_show()
> to use the flags in 'struct papr_scm_priv.health' to return
> appropriate status strings pertaining to dimm health.
>
> This patch shouldn't introduce any behavioral change.

This patch is undoing what is doing in PATCH 2. If you reorder things,
we could drop either this or PATCH2.?

>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/papr_scm.c | 61 ++++++++++++++++-------
>  1 file changed, 44 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index d5eea2f38fa6..29f38246c59f 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -47,8 +47,7 @@ struct papr_scm_priv {
>  	struct mutex dimm_mutex;
>  
>  	/* Health information for the dimm */
> -	__be64 health_bitmap;
> -	__be64 health_bitmap_valid;
> +	struct nd_papr_scm_dimm_health_stat health;
>  
>  	/* length of the stat buffer as expected by phyp */
>  	size_t len_stat_buffer;
> @@ -205,6 +204,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
>  {
>  	unsigned long ret[PLPAR_HCALL_BUFSIZE];
>  	int64_t rc;
> +	__be64 health;
>  
>  	rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
>  	if (rc != H_SUCCESS) {
> @@ -219,13 +219,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
>  		return rc;
>  
>  	/* Store the retrieved health information in dimm platform data */
> -	p->health_bitmap = ret[0];
> -	p->health_bitmap_valid = ret[1];
> +	health = ret[0] & ret[1];
>  
>  	dev_dbg(&p->pdev->dev,
>  		"Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
> -		be64_to_cpu(p->health_bitmap),
> -		be64_to_cpu(p->health_bitmap_valid));
> +		be64_to_cpu(ret[0]),
> +		be64_to_cpu(ret[1]));
> +
> +	memset(&p->health, 0, sizeof(p->health));
> +
> +	/* Check for various masks in bitmap and set the buffer */
> +	if (health & PAPR_SCM_DIMM_UNARMED_MASK)
> +		p->health.dimm_unarmed = true;
> +
> +	if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
> +		p->health.dimm_bad_shutdown = true;
> +
> +	if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
> +		p->health.dimm_bad_restore = true;
> +
> +	if (health & PAPR_SCM_DIMM_ENCRYPTED)
> +		p->health.dimm_encrypted = true;
> +
> +	if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED) {
> +		p->health.dimm_locked = true;
> +		p->health.dimm_scrubbed = true;
> +	}
> +
> +	if (health & PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
> +		p->health.dimm_health = DSM_PAPR_SCM_DIMM_UNHEALTHY;
> +
> +	if (health & PAPR_SCM_DIMM_HEALTH_CRITICAL)
> +		p->health.dimm_health = DSM_PAPR_SCM_DIMM_CRITICAL;
> +
> +	if (health & PAPR_SCM_DIMM_HEALTH_FATAL)
> +		p->health.dimm_health = DSM_PAPR_SCM_DIMM_FATAL;
>  
>  	mutex_unlock(&p->dimm_mutex);
>  	return 0;
> @@ -513,7 +541,6 @@ static ssize_t papr_flags_show(struct device *dev,
>  {
>  	struct nvdimm *dimm = to_nvdimm(dev);
>  	struct papr_scm_priv *p = nvdimm_provider_data(dimm);
> -	__be64 health;
>  	int rc;
>  
>  	rc = drc_pmem_query_health(p);
> @@ -525,26 +552,26 @@ static ssize_t papr_flags_show(struct device *dev,
>  	if (rc)
>  		return rc;
>  
> -	health = p->health_bitmap & p->health_bitmap_valid;
> -
> -	/* Check for various masks in bitmap and set the buffer */
> -	if (health & PAPR_SCM_DIMM_UNARMED_MASK)
> +	if (p->health.dimm_unarmed)
>  		rc += sprintf(buf, "not_armed ");
>  
> -	if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
> +	if (p->health.dimm_bad_shutdown)
>  		rc += sprintf(buf + rc, "save_fail ");
>  
> -	if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
> +	if (p->health.dimm_bad_restore)
>  		rc += sprintf(buf + rc, "restore_fail ");
>  
> -	if (health & PAPR_SCM_DIMM_ENCRYPTED)
> +	if (p->health.dimm_encrypted)
>  		rc += sprintf(buf + rc, "encrypted ");
>  
> -	if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
> +	if (p->health.dimm_health)
>  		rc += sprintf(buf + rc, "smart_notify ");
>  
> -	if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
> -		rc += sprintf(buf + rc, "scrubbed locked ");
> +	if (p->health.dimm_scrubbed)
> +		rc += sprintf(buf + rc, "scrubbed ");
> +
> +	if (p->health.dimm_locked)
> +		rc += sprintf(buf + rc, "locked ");
>  
>  	if (rc > 0)
>  		rc += sprintf(buf + rc, "\n");
> -- 
> 2.24.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/8] powerpc/papr_scm: Fetch dimm performance stats from PHYP
  2020-02-20  9:58 ` [PATCH 3/8] powerpc/papr_scm: Fetch dimm performance stats from PHYP Vaibhav Jain
@ 2020-03-09 10:28   ` Aneesh Kumar K.V
  0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2020-03-09 10:28 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> Implement support for fetching dimm performance metrics via
> H_SCM_PERFORMANCE_HEALTH hcall as documented in Ref[1]. The hcall
> returns a structure as described in Ref[1] and defined as newly
> introduced 'struct papr_scm_perf_stats'. The struct has a header
> followed by key-value pairs of performance attributes. The 'key' part
> is a 8-byte char array naming the attribute encoded as a __be64
> integer. This makes the output buffer format for the hcall self
> describing and can be easily interpreted.
>
> This patch implements functionality to fetch these performance stats
> and reporting them via a nvdimm sysfs attribute named
> 'papr_perf_stats'.
>
> A new function drc_pmem_query_stats() is implemented that issues hcall
> H_SCM_PERFORMANCE_HEALTH ,requesting PHYP to store performance stats
> in pre-allocated 'struct papr_scm_perf_stats' buffer. During nvdimm
> initialization in papr_scm_nvdimm_init() this function is called with
> an empty buffer to know the max buffer size needed for issuing the
> H_SCM_PERFORMANCE_HEALTH hcall. The buffer size retrieved is stored in
> newly introduced 'struct papc_scm_priv.len_stat_buffer' for later
> retrival.
>

You are not using this hcall in the series? If so can you drop it from
the series and add it when you want to use hcall returned value.

> [1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
> PAPR hcalls")
>


-aneesh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 8/8] powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH
  2020-02-20  9:58 ` [PATCH 8/8] powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH Vaibhav Jain
@ 2020-03-09 10:58   ` Aneesh Kumar K.V
  0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2020-03-09 10:58 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Vaibhav Jain, Michael Ellerman, Oliver O'Halloran,
	Alastair D'Silva

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> The DSM 'DSM_PAPR_SCM_HEALTH' should return a 'struct
> nd_papr_scm_dimm_health_stat' containing information in dimm health back
> to user space in response to ND_CMD_CALL. We implement this DSM by
> implementing a new function papr_scm_get_health() that queries the
> DIMM health information and then copies these bitmaps to the package
> payload whose layout is defined by 'struct papr_scm_ndctl_health'.
>
> The patch also handle cases where in future versions of 'struct
> papr_scm_ndctl_health' may want to return more health
> information. Such payload envelops will contain appropriate version
> information in 'struct nd_papr_scm_cmd_pkg.payload_version'. The patch
> takes care of only returning the sub-data corresponding to the payload
> version requested. Please see the comments in papr_scm_get_health()
> for how this is done.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/papr_scm.c | 73 +++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 29f38246c59f..bf81acb0bf3f 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -415,6 +415,74 @@ static int cmd_to_func(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
>  	return pkg->hdr.nd_command;
>  }
>  
> +/*
> + * Fetch the DIMM health info and populate it in provided papr_scm package.
> + * Since the caller can request a different version of payload and each new
> + * version of struct nd_papr_scm_dimm_health_stat is a proper-subset of
> + * previous version hence we return a subset of the cached 'struct
> + * nd_papr_scm_dimm_health_stat' depending on the payload version requested.
> + */
> +static int papr_scm_get_health(struct papr_scm_priv *p,
> +			       struct nd_papr_scm_cmd_pkg *pkg)
> +{
> +	int rc;
> +	size_t copysize;
> +	/* Map version to number of bytes to be copied to payload */
> +	const size_t copysizes[] = {
> +		[1] =
> +		sizeof(struct nd_papr_scm_dimm_health_stat_v1),
> +
> +		/*  This should always be preset */
> +		[ND_PAPR_SCM_DIMM_HEALTH_VERSION] =
> +		sizeof(struct nd_papr_scm_dimm_health_stat),
> +	};


We will not be able to determine that during build. For performance
hcall to run LPAR should be privileged. ie, even if the kernel supports v2
version of the health information, it may only be able to
return v1 version of the health because LPAR performance stat hcall
failed.

-aneesh

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-03-09 11:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20  9:57 [PATCH 0/7] powerpc/papr_scm: Add support for reporting nvdimm health Vaibhav Jain
2020-02-20  9:57 ` Vaibhav Jain
2020-02-20  9:57 ` [PATCH 1/8] powerpc: Add asm header 'papr_scm.h' describing the papr-scm interface Vaibhav Jain
2020-03-09 10:07   ` Aneesh Kumar K.V
2020-02-20  9:57 ` [PATCH 2/8] powerpc/papr_scm: Provide support for fetching dimm health information Vaibhav Jain
2020-03-09 10:24   ` Aneesh Kumar K.V
2020-02-20  9:58 ` [PATCH 3/8] powerpc/papr_scm: Fetch dimm performance stats from PHYP Vaibhav Jain
2020-03-09 10:28   ` Aneesh Kumar K.V
2020-02-20  9:58 ` [PATCH 4/8] UAPI: ndctl: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM family Vaibhav Jain
2020-02-20  9:58   ` Vaibhav Jain
2020-02-20  9:58 ` [PATCH 5/8] powerpc/uapi: Introduce uapi header 'papr_scm_dsm.h' for papr_scm DSMs Vaibhav Jain
2020-02-20  9:58 ` [PATCH 6/8] powerpc/papr_scm: Add support for handling PAPR DSM commands Vaibhav Jain
2020-02-20  9:58 ` [PATCH 7/8] powerpc/papr_scm: Re-implement 'papr_flags' using 'nd_papr_scm_dimm_health_stat' Vaibhav Jain
2020-03-09 10:27   ` Aneesh Kumar K.V
2020-02-20  9:58 ` [PATCH 8/8] powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH Vaibhav Jain
2020-03-09 10:58   ` Aneesh Kumar K.V

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.