The PAPR standard[1][3] provides 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 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
a emulation environment:
# ndctl list -DH
[
{
"dev":"nmem0",
"health":{
"health_state":"fatal",
"shutdown_state":"dirty"
}
}
]
Dimm health report output on a pseries guest lpar with vPMEM or HMS
based nvdimms that are in perfectly healthy conditions:
# ndctl list -d nmem0 -H
[
{
"dev":"nmem0",
"health":{
"health_state":"ok",
"shutdown_state":"clean"
}
}
]
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 a single
DSM to retrieve DIMM health, defined in the newly introduced uapi header
named 'papr_scm_dsm.h'. Support for more DSMs will be added in future.
Structure of the patch-set
==========================
The patchset starts with implementing support for fetching nvdimm health
information from PHYP and partially exposing it to user-space via nvdimm
flags.
Second & Third patches deal with implementing support for servicing DSM
commands papr_scm.
Finally the Fourth patch implements support for servicing DSM
'DSM_PAPR_SCM_HEALTH' that returns the nvdimm health information to
libndctl.
Changelog:
==========
v4..v5:
* Fixed a bug in new implementation of papr_scm_ndctl() that was triggering
a false error condition.
v3..v4:
* Restructured papr_scm_ndctl() to dispatch ND_CMD_CALL commands to a new
function named papr_scm_service_dsm() to serivice DSM requests. [Aneesh]
v2..v3:
* Updated the papr_scm_dsm.h header to be more confimant general kernel
guidelines for UAPI headers. [Aneesh]
* Changed the definition of macro PAPR_SCM_DIMM_UNARMED_MASK to not
include case when the nvdimm is unarmed because its a vPMEM
nvdimm. [Aneesh]
v1..v2:
* Restructured the patch-set based on review comments on V1 patch-set to
simplify the patch review. Multiple small patches have been combined into
single patches to reduce cross referencing that was needed in earlier
patch-set. Hence most of the patches in this patch-set as now new. [Aneesh]
* Removed the initial work done for fetch nvdimm performance statistics.
These changes will be re-proposed in a separate patch-set. [Aneesh]
* Simplified handling of versioning of 'struct
nd_papr_scm_dimm_health_stat_v1' as only one version of the structure is
currently in existence.
References:
[1]: "Power Architecture Platform Reference"
https://en.wikipedia.org/wiki/Power_Architecture_Platform_Reference
[2]: commit 58b278f568f0
("powerpc: Provide initial documentation for PAPR hcalls")
[3]: "Linux on Power Architecture Platform Reference"
https://members.openpowerfoundation.org/document/dl/469
[4]: https://patchwork.kernel.org/project/linux-nvdimm/list/?series=244625
Vaibhav Jain (4):
powerpc/papr_scm: Fetch nvdimm health information from PHYP
ndctl/uapi: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM
family
powerpc/papr_scm,uapi: Add support for handling PAPR DSM commands
powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH
arch/powerpc/include/asm/papr_scm.h | 48 ++++
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 201 ++++++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 277 ++++++++++++++++++-
include/uapi/linux/ndctl.h | 1 +
4 files changed, 519 insertions(+), 8 deletions(-)
create mode 100644 arch/powerpc/include/asm/papr_scm.h
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h
--
2.25.1
Implement support for fetching nvdimm 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 partially exposed to user-space via
newly introduced dimm specific attribute 'papr_flags'. Also a new asm
header named 'papr-scm.h' is added that describes the interface
between PHYP and guest kernel.
Following flags are reported via 'papr_flags' sysfs attribute contents
of which are space separated string flags indicating various nvdimm
states:
* "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 <redacted>
---
Changelog:
v4..v5 : None
v3..v4 : None
v2..v3 : Removed PAPR_SCM_DIMM_HEALTH_NON_CRITICAL as a condition for
NVDIMM unarmed [Aneesh]
v1..v2 : New patch in the series.
---
arch/powerpc/include/asm/papr_scm.h | 48 ++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 105 +++++++++++++++++++++-
2 files changed, 151 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/papr_scm.h
@@ -0,0 +1,48 @@+/* SPDX-License-Identifier: GPL-2.0-or-later */+/*+*Structuresanddefinesneededtomanagenvdimmsforspaprguests.+*/+#ifndef _ASM_POWERPC_PAPR_SCM_H_+#define _ASM_POWERPC_PAPR_SCM_H_++#include<linux/types.h>+#include<asm/bitsperlong.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)++/* 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)++#endif
@@ -39,6 +40,13 @@ struct papr_scm_priv {structresourceres;structnd_region*region;structnd_interleave_setnd_set;++/* Protect dimm data from concurrent access */+structmutexdimm_mutex;++/* Health information for the dimm */+__be64health_bitmap;+__be64health_bitmap_valid;};staticintdrc_pmem_bind(structpapr_scm_priv*p)
@@ -144,6 +152,35 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)returndrc_pmem_bind(p);}+staticintdrc_pmem_query_health(structpapr_scm_priv*p)+{+unsignedlongret[PLPAR_HCALL_BUFSIZE];+int64_trc;++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)+returnrc;++/* 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);+return0;+}staticintpapr_scm_meta_get(structpapr_scm_priv*p,structnd_cmd_get_config_data_hdr*hdr)
@@ -304,6 +341,67 @@ static inline int papr_scm_node(int node)returnmin_node;}+staticssize_tpapr_flags_show(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+structnvdimm*dimm=to_nvdimm(dev);+structpapr_scm_priv*p=nvdimm_provider_data(dimm);+__be64health;+intrc;++rc=drc_pmem_query_health(p);+if(rc)+returnrc;++/* Protect against modifications to papr_scm_priv with the mutex */+rc=mutex_lock_interruptible(&p->dimm_mutex);+if(rc)+returnrc;++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);+returnrc;+}+DEVICE_ATTR_RO(papr_flags);++/* papr_scm specific dimm attributes */+staticstructattribute*papr_scm_nd_attributes[]={+&dev_attr_papr_flags.attr,+NULL,+};++staticstructattribute_grouppapr_scm_nd_attribute_group={+.attrs=papr_scm_nd_attributes,+};++staticconststructattribute_group*papr_scm_dimm_attr_groups[]={+&papr_scm_nd_attribute_group,+NULL,+};+staticintpapr_scm_nvdimm_init(structpapr_scm_priv*p){structdevice*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);gotoerr;
@@ -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);
This patch implements support for papr_scm command
'DSM_PAPR_SCM_HEALTH' that returns a newly introduced 'struct
nd_papr_scm_dimm_health_stat' instance containing dimm health
information back to user space in response to ND_CMD_CALL. This
functionality is implemented in newly introduced papr_scm_get_health()
that queries the scm-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 introduces a new member a new member 'struct
papr_scm_priv.health' thats an instance of 'struct
nd_papr_scm_dimm_health_stat' to cache the health information of a
scm-dimm. As a result functions drc_pmem_query_health() and
papr_flags_show() are updated to populate and use this new struct
instead of two be64 integers that we earlier used.
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5: None
v3..v4: Call the DSM_PAPR_SCM_HEALTH service function from
papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
v2..v3: Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx'
types as its exported to the userspace [Aneesh]
Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm
health from enum to #defines [Aneesh]
v1..v2: New patch in the series
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 40 +++++++
arch/powerpc/platforms/pseries/papr_scm.c | 109 ++++++++++++++++---
2 files changed, 132 insertions(+), 17 deletions(-)
@@ -158,4 +159,43 @@ static void *papr_scm_pcmd_to_payload(struct nd_papr_scm_cmd_pkg *pcmd)elsereturn(void*)((__u8*)pcmd+pcmd->payload_offset);}++/* Various scm-dimm health indicators */+#define DSM_PAPR_SCM_DIMM_HEALTHY 0+#define DSM_PAPR_SCM_DIMM_UNHEALTHY 1+#define DSM_PAPR_SCM_DIMM_CRITICAL 2+#define DSM_PAPR_SCM_DIMM_FATAL 3++/*+*Structexchangedbetweenkernel&ndctlinforPAPR_DSM_PAPR_SMART_HEALTH+*Variousbitflagsindicatethehealthstatusofthedimm.+*+*dimm_unarmed:Dimmnotarmed.Socontentswontpersist.+*dimm_bad_shutdown:Previousshutdowndidnotpersistcontents.+*dimm_bad_restore:Contentsfrompreviousshutdownwerentrestored.+*dimm_scrubbed:Contentsofthedimmhavebeenscrubbed.+*dimm_locked:ContentsofthedimmcantbemodifieduntilCECreboot+*dimm_encrypted:Contentsofdimmareencrypted.+*dimm_health:Dimmhealthindicator.+*/+structnd_papr_scm_dimm_health_stat_v1{+__u8dimm_unarmed;+__u8dimm_bad_shutdown;+__u8dimm_bad_restore;+__u8dimm_scrubbed;+__u8dimm_locked;+__u8dimm_encrypted;+__u16dimm_health;+};++/*+*Typedefthecurrentstructfordimm_healthsothatanyapplication+*orkernelrecompiledafterintroducinganewversionautomatically+*supportsthenewversion.+*/+#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+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
@@ -47,8 +47,7 @@ struct papr_scm_priv {structmutexdimm_mutex;/* Health information for the dimm */-__be64health_bitmap;-__be64health_bitmap_valid;+structnd_papr_scm_dimm_health_stathealth;};staticintdrc_pmem_bind(structpapr_scm_priv*p)
@@ -158,6 +157,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p){unsignedlongret[PLPAR_HCALL_BUFSIZE];int64_trc;+__be64health;rc=plpar_hcall(H_SCM_HEALTH,ret,p->drc_index);if(rc!=H_SUCCESS){
@@ -172,13 +172,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)returnrc;/* 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);return0;
@@ -331,6 +359,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,return0;}+/* Fetch the DIMM health info and populate it in provided package. */+staticintpapr_scm_get_health(structpapr_scm_priv*p,+structnd_papr_scm_cmd_pkg*pkg)+{+intrc;+size_tcopysize=sizeof(p->health);++rc=drc_pmem_query_health(p);+if(rc)+gotoout;+/*+*Iftherequestedpayloadversionisgreaterthanoneweknow+*about,returnthepayloadversionweknowaboutandlet+*caller/userspacehandle.+*/+if(pkg->payload_version>ND_PAPR_SCM_DIMM_HEALTH_VERSION)+pkg->payload_version=ND_PAPR_SCM_DIMM_HEALTH_VERSION;++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;+gotoout;+}++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:+/*+*Puttheerrorinoutpackageandreturnsuccessfromfunction+*sothaterrorsifanyarepropogatedbacktouserspace.+*/+pkg->cmd_status=rc;+dev_dbg(&p->pdev->dev,"%s completion code = %d\n",__func__,rc);++return0;+}+staticintpapr_scm_service_dsm(structpapr_scm_priv*p,structnd_papr_scm_cmd_pkg*call_pkg){
@@ -345,6 +418,9 @@ static int papr_scm_service_dsm(struct papr_scm_priv *p,/* Depending on the DSM command call appropriate service routine */switch(call_pkg->hdr.nd_command){+caseDSM_PAPR_SCM_HEALTH:+returnpapr_scm_get_health(p,call_pkg);+default:pr_debug("Unsupported DSM command 0x%llx\n",call_pkg->hdr.nd_command);
@@ -443,26 +518,26 @@ static ssize_t papr_flags_show(struct device *dev,if(rc)returnrc;-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");
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
described in newly introduced uapi header 'papr_scm_dsm.h' which
defines a new 'struct nd_papr_scm_cmd_pkg' 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(). A new function cmd_to_func() is
implemented that reads the args to papr_scm_ndctl() and performs
sanity tests on them. In case of a DSM command being sent via
ND_CMD_CALL a newly introduced function papr_scm_service_dsm() is
called to handle the request.
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5: Fixed a bug in new implementation of papr_scm_ndctl().
v3..v4: Updated papr_scm_ndctl() to delegate DSM command handling to a
different function papr_scm_service_dsm(). [Aneesh]
v2..v3: Updated the nd_papr_scm_cmd_pkg to use __xx types as its
exported to the userspace [Aneesh]
v1..v2: New patch in the series.
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 161 +++++++++++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 97 ++++++++++-
2 files changed, 252 insertions(+), 6 deletions(-)
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h
@@ -0,0 +1,161 @@+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */+/*+*PAPRSCMDevicespecificmethodsandstructforlibndctlandndctl+*+*(C)CopyrightIBM2020+*+*Author:VaibhavJain<vaibhavatlinux.ibm.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2,or(atyouroption)+*anylaterversion.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*/++#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++/*+*DSMEnvelope:+*+*TheioctlND_CMD_CALLtransfersdatabetweenuser-spaceandkernelvia+*'envelopes'whichconsistsofaheaderanduser-definedpayloadsections.+*Theheaderisdescribedby'structnd_papr_scm_cmd_pkg'whichexpectsa+*payloadfollowingitandoffsetofwhichrelativetothestructisprovided+*by'nd_papr_scm_cmd_pkg.payload_offset'.*+*+*+-------------+---------------------+---------------------------++*|64-Bytes|8-Bytes|Max184-Bytes|+*+-------------+---------------------+---------------------------++*|nd_papr_scm_cmd_pkg||+*|-------------+||+*|nd_cmd_pkg|||+*+-------------+---------------------+---------------------------++*|nd_family|||+*|nd_size_out|cmd_status||+*|nd_size_in|payload_version|PAYLOAD|+*|nd_command|payload_offset----->|+*|nd_fw_size|||+*+-------------+---------------------+---------------------------++*+*DSMHeader:+*+*Theheaderisdefinedas'structnd_papr_scm_cmd_pkg'whichembedsa+*'structnd_cmd_pkg'instance.TheDSMcommandisassignedtomember+*'nd_cmd_pkg.nd_command'.Apartfromsizeinformationoftheenvelopwhichis+*containedin'structnd_cmd_pkg',theheaderalsohasmembersfollowing+*members:+*+*'cmd_status':(Out)ErrorsifanyencounteredwhileservicingDSM.+*'payload_version':(In/Out)Versionnumberassociatedwiththepayload.+*'payload_offset':(In)Relativeoffsetofpayloadfromstartofenvelope.+*+*DSMPayload:+*+*ThelayoutoftheDSMPayloadisdefinedbyvariousstructssharedbetween+*papr_scmandlibndctlsothatcontentsofpayloadcanbeinterpreted.During+*servicingofaDSMthepapr_scmmodulewillreadinputargsfromthepayload+*fieldbycastingitscontentstoanappropriatestructpointerbasedonthe+*DSMcommand.SimilarlytheoutputofservicingtheDSMcommandwillbecopied+*tothepayloadfieldusingthesamestruct.+*+*'libnvdimm'enforcesahardlimitof256bytesontheenvelopesize,which+*leavesaround184bytesfortheenvelopepayload(ignoringanypaddingthat+*thecompilermaysilentlyintroduce).+*+*PayloadVersion:+*+*A'payload_version'fieldispresentinDSMheaderthatindicatesaspecific+*versionofthestructurepresentinDSMPayloadforagivenDSMcommand.This+*providesbackwardcompatibilityincasetheDSMPayloadstructureevolves+*anddifferentstructuresaresupportedby'papr_scm'and'libndctl'.+*+*WhensendingaDSMPayloadto'papr_scm','libndctl'shouldsendtheversion+*ofthepayloadstructitsupportsvia'payload_version'field.The'papr_scm'+*modulewhenservicingtheDSMenvelopchecksthe'payload_version'andthen+*uses'payloadstructversion'==MIN('payload_versionfield',+*'maxpayload-struct-versionsupportedbypapr_scm')toservicetheDSM.After+*servicingtheDSM,'papr_scm'putthenegotiatedversionofpayloadstructin+*returned'payload_version'field.+*+*Libndctlonreceivingtheenvelopbackfrompapr_scmagainchecksthe+*'payload_version'fieldandbasedonitusetheappropriateversiondsm+*structtoparsetheresults.+*+*BackwardCompatibility:+*+*AboveschemeofexchangingdifferentversionedDSMstructbetweenlibndctl+*andpapr_scmshouldprovidebackwardcompatibilityuntilfollowingtwo+*assumptions/conditionswhendefiningnewDSMstructshold:+*+*LetT(X)={setofattributesinDSMstruct'T'versionedX}+*+*1.T(X)isapropersubsetofT(Y)ifX>Y.+*i.eEachnewversionofDSMstructshouldretainexistingstruct+*attributesfrompreviousversion+*+*2.Ifanentity(libndctlorpapr_scm)supportsaDSMstructT(X)then+*itshouldalsosupportT(1),T(2)...T(X-1).+*i.eWhenaddingsupportfornewversionofaDSMstruct,libndctl+*andpapr_scmshouldretainsupportoftheexistingDSMstruct+*versiontheysupport.+*/++/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */+structnd_papr_scm_cmd_pkg{+structnd_cmd_pkghdr;/* Package header containing sub-cmd */+__s32cmd_status;/* Out: Sub-cmd status returned back */+__u16payload_offset;/* In: offset from start of struct */+__u16payload_version;/* In/Out: version of the payload */+__u8payload[];/* In/Out: Sub-cmd data buffer */+};++/*+*SubcommandsforND_CMD_CALL.TopreventoverlapfromND_CMD_*,valuesfor+*theseenumsstartat0x10000.Thesevaluesarethenreturnedfrom+*cmd_to_func()makingiteasytoimplementtheswitch-caseblockin+*papr_scm_ndctl().Thesecommandsaresenttothekernelvia+*'nd_papr_scm_cmd_pkg.hdr.nd_command'+*/+enumdsm_papr_scm{+DSM_PAPR_SCM_MIN=0x10000,+DSM_PAPR_SCM_MAX,+};++/* Helpers to evaluate the size of PAPR_SCM envelope */+/* Calculate the papr_scm-header size */+#define ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE \+(sizeof(structnd_papr_scm_cmd_pkg)-sizeof(structnd_cmd_pkg))++/* Given a type calculate envelope-content size (papr_scm-header + payload) */+#define ND_PAPR_SCM_ENVELOPE_CONTENT_SIZE(_type_) \+(sizeof(_type_)+ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE)++/* Convert a libnvdimm nd_cmd_pkg to papr_scm specific pkg */+staticstructnd_papr_scm_cmd_pkg*nd_to_papr_cmd_pkg(structnd_cmd_pkg*cmd)+{+return(structnd_papr_scm_cmd_pkg*)cmd;+}++/* Return the payload pointer for a given pcmd */+staticvoid*papr_scm_pcmd_to_payload(structnd_papr_scm_cmd_pkg*pcmd)+{+if(pcmd->hdr.nd_size_in==0&&pcmd->hdr.nd_size_out==0)+returnNULL;+else+return(void*)((__u8*)pcmd+pcmd->payload_offset);+}+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
@@ -283,15 +285,92 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,return0;}+/*+*Validatetheinputsargstodimm-controlfunctionandreturn'0'ifvalid.+*ThisalsodoesinitialsanityvalidationtoND_CMD_CALLsub-commandpackages.+*/+staticintis_cmd_valid(structnvdimm*nvdimm,unsignedintcmd,void*buf,+unsignedintbuf_len)+{+unsignedlongcmd_mask=PAPR_SCM_DIMM_CMD_MASK;+structnd_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;+}elseif(cmd==ND_CMD_CALL){++/* Verify the envelop package */+if(!buf||buf_len<sizeof(structnd_papr_scm_cmd_pkg)){+pr_debug("%s: Invalid pkg size=%u\n",__func__,+buf_len);+return-EINVAL;+}++/* Verify that the DSM command family is valid */+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;++}++/* We except a payload with all DSM commands */+if(papr_scm_pcmd_to_payload(pkg)==NULL){+pr_debug("%s: Empty payload for sub-command=0x%llx\n",+__func__,pkg->hdr.nd_command);+return-EINVAL;+}+}++/* Command looks valid */+return0;+}++staticintpapr_scm_service_dsm(structpapr_scm_priv*p,+structnd_papr_scm_cmd_pkg*call_pkg)+{+/* unknown subcommands return error in packages */+if(call_pkg->hdr.nd_command<=DSM_PAPR_SCM_MIN||+call_pkg->hdr.nd_command>=DSM_PAPR_SCM_MAX){+pr_debug("Invalid DSM command 0x%llx\n",+call_pkg->hdr.nd_command);+call_pkg->cmd_status=-EINVAL;+return0;+}++/* Depending on the DSM command call appropriate service routine */+switch(call_pkg->hdr.nd_command){+default:+pr_debug("Unsupported DSM command 0x%llx\n",+call_pkg->hdr.nd_command);+call_pkg->cmd_status=-ENOENT;+return0;+}+}+intpapr_scm_ndctl(structnvdimm_bus_descriptor*nd_desc,structnvdimm*nvdimm,unsignedintcmd,void*buf,unsignedintbuf_len,int*cmd_rc){structnd_cmd_get_config_size*get_size_hdr;structpapr_scm_priv*p;+structnd_papr_scm_cmd_pkg*call_pkg=NULL;+intrc;-/* 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_rc=is_cmd_valid(nvdimm,cmd,buf,buf_len);+if(*cmd_rc){+pr_debug("%s: Invalid cmd=0x%x. Err=%d\n",__func__,+cmd,*cmd_rc);+return*cmd_rc;+}p=nvdimm_provider_data(nvdimm);
@@ -313,13 +392,19 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,*cmd_rc=papr_scm_meta_set(p,buf);break;+caseND_CMD_CALL:+call_pkg=nd_to_papr_cmd_pkg(buf);+*cmd_rc=papr_scm_service_dsm(p,call_pkg);+break;+default:-return-EINVAL;+dev_dbg(&p->pdev->dev,"Unknown command = %d\n",cmd);+*cmd_rc=-EINVAL;}dev_dbg(&p->pdev->dev,"returned with cmd_rc = %d\n",*cmd_rc);-return0;+return*cmd_rc;}staticinlineintpapr_scm_node(intnode)
Implement support for fetching nvdimm 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 partially exposed to user-space via
newly introduced dimm specific attribute 'papr_flags'. Also a new asm
header named 'papr-scm.h' is added that describes the interface
between PHYP and guest kernel.
Following flags are reported via 'papr_flags' sysfs attribute contents
of which are space separated string flags indicating various nvdimm
states:
* "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")
Reviewed-by: Aneesh Kumar K.V <redacted>
quoted hunk
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5 : None
v3..v4 : None
v2..v3 : Removed PAPR_SCM_DIMM_HEALTH_NON_CRITICAL as a condition for
NVDIMM unarmed [Aneesh]
v1..v2 : New patch in the series.
---
arch/powerpc/include/asm/papr_scm.h | 48 ++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 105 +++++++++++++++++++++-
2 files changed, 151 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/papr_scm.h
@@ -0,0 +1,48 @@+/* SPDX-License-Identifier: GPL-2.0-or-later */+/*+*Structuresanddefinesneededtomanagenvdimmsforspaprguests.+*/+#ifndef _ASM_POWERPC_PAPR_SCM_H_+#define _ASM_POWERPC_PAPR_SCM_H_++#include<linux/types.h>+#include<asm/bitsperlong.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)++/* 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)++#endif
@@ -39,6 +40,13 @@ struct papr_scm_priv {structresourceres;structnd_region*region;structnd_interleave_setnd_set;++/* Protect dimm data from concurrent access */+structmutexdimm_mutex;++/* Health information for the dimm */+__be64health_bitmap;+__be64health_bitmap_valid;};staticintdrc_pmem_bind(structpapr_scm_priv*p)
@@ -144,6 +152,35 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)returndrc_pmem_bind(p);}+staticintdrc_pmem_query_health(structpapr_scm_priv*p)+{+unsignedlongret[PLPAR_HCALL_BUFSIZE];+int64_trc;++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)+returnrc;++/* 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);+return0;+}staticintpapr_scm_meta_get(structpapr_scm_priv*p,structnd_cmd_get_config_data_hdr*hdr)
@@ -304,6 +341,67 @@ static inline int papr_scm_node(int node)returnmin_node;}+staticssize_tpapr_flags_show(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+structnvdimm*dimm=to_nvdimm(dev);+structpapr_scm_priv*p=nvdimm_provider_data(dimm);+__be64health;+intrc;++rc=drc_pmem_query_health(p);+if(rc)+returnrc;++/* Protect against modifications to papr_scm_priv with the mutex */+rc=mutex_lock_interruptible(&p->dimm_mutex);+if(rc)+returnrc;++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);+returnrc;+}+DEVICE_ATTR_RO(papr_flags);++/* papr_scm specific dimm attributes */+staticstructattribute*papr_scm_nd_attributes[]={+&dev_attr_papr_flags.attr,+NULL,+};++staticstructattribute_grouppapr_scm_nd_attribute_group={+.attrs=papr_scm_nd_attributes,+};++staticconststructattribute_group*papr_scm_dimm_attr_groups[]={+&papr_scm_nd_attribute_group,+NULL,+};+staticintpapr_scm_nvdimm_init(structpapr_scm_priv*p){structdevice*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);gotoerr;
@@ -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.25.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
--
2.25.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
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
described in newly introduced uapi header 'papr_scm_dsm.h' which
defines a new 'struct nd_papr_scm_cmd_pkg' 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(). A new function cmd_to_func() is
implemented that reads the args to papr_scm_ndctl() and performs
sanity tests on them. In case of a DSM command being sent via
ND_CMD_CALL a newly introduced function papr_scm_service_dsm() is
called to handle the request.
Reviewed-by: Aneesh Kumar K.V <redacted>
quoted hunk
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5: Fixed a bug in new implementation of papr_scm_ndctl().
v3..v4: Updated papr_scm_ndctl() to delegate DSM command handling to a
different function papr_scm_service_dsm(). [Aneesh]
v2..v3: Updated the nd_papr_scm_cmd_pkg to use __xx types as its
exported to the userspace [Aneesh]
v1..v2: New patch in the series.
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 161 +++++++++++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 97 ++++++++++-
2 files changed, 252 insertions(+), 6 deletions(-)
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h
@@ -0,0 +1,161 @@+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */+/*+*PAPRSCMDevicespecificmethodsandstructforlibndctlandndctl+*+*(C)CopyrightIBM2020+*+*Author:VaibhavJain<vaibhavatlinux.ibm.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2,or(atyouroption)+*anylaterversion.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*/++#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++/*+*DSMEnvelope:+*+*TheioctlND_CMD_CALLtransfersdatabetweenuser-spaceandkernelvia+*'envelopes'whichconsistsofaheaderanduser-definedpayloadsections.+*Theheaderisdescribedby'structnd_papr_scm_cmd_pkg'whichexpectsa+*payloadfollowingitandoffsetofwhichrelativetothestructisprovided+*by'nd_papr_scm_cmd_pkg.payload_offset'.*+*+*+-------------+---------------------+---------------------------++*|64-Bytes|8-Bytes|Max184-Bytes|+*+-------------+---------------------+---------------------------++*|nd_papr_scm_cmd_pkg||+*|-------------+||+*|nd_cmd_pkg|||+*+-------------+---------------------+---------------------------++*|nd_family|||+*|nd_size_out|cmd_status||+*|nd_size_in|payload_version|PAYLOAD|+*|nd_command|payload_offset----->|+*|nd_fw_size|||+*+-------------+---------------------+---------------------------++*+*DSMHeader:+*+*Theheaderisdefinedas'structnd_papr_scm_cmd_pkg'whichembedsa+*'structnd_cmd_pkg'instance.TheDSMcommandisassignedtomember+*'nd_cmd_pkg.nd_command'.Apartfromsizeinformationoftheenvelopwhichis+*containedin'structnd_cmd_pkg',theheaderalsohasmembersfollowing+*members:+*+*'cmd_status':(Out)ErrorsifanyencounteredwhileservicingDSM.+*'payload_version':(In/Out)Versionnumberassociatedwiththepayload.+*'payload_offset':(In)Relativeoffsetofpayloadfromstartofenvelope.+*+*DSMPayload:+*+*ThelayoutoftheDSMPayloadisdefinedbyvariousstructssharedbetween+*papr_scmandlibndctlsothatcontentsofpayloadcanbeinterpreted.During+*servicingofaDSMthepapr_scmmodulewillreadinputargsfromthepayload+*fieldbycastingitscontentstoanappropriatestructpointerbasedonthe+*DSMcommand.SimilarlytheoutputofservicingtheDSMcommandwillbecopied+*tothepayloadfieldusingthesamestruct.+*+*'libnvdimm'enforcesahardlimitof256bytesontheenvelopesize,which+*leavesaround184bytesfortheenvelopepayload(ignoringanypaddingthat+*thecompilermaysilentlyintroduce).+*+*PayloadVersion:+*+*A'payload_version'fieldispresentinDSMheaderthatindicatesaspecific+*versionofthestructurepresentinDSMPayloadforagivenDSMcommand.This+*providesbackwardcompatibilityincasetheDSMPayloadstructureevolves+*anddifferentstructuresaresupportedby'papr_scm'and'libndctl'.+*+*WhensendingaDSMPayloadto'papr_scm','libndctl'shouldsendtheversion+*ofthepayloadstructitsupportsvia'payload_version'field.The'papr_scm'+*modulewhenservicingtheDSMenvelopchecksthe'payload_version'andthen+*uses'payloadstructversion'==MIN('payload_versionfield',+*'maxpayload-struct-versionsupportedbypapr_scm')toservicetheDSM.After+*servicingtheDSM,'papr_scm'putthenegotiatedversionofpayloadstructin+*returned'payload_version'field.+*+*Libndctlonreceivingtheenvelopbackfrompapr_scmagainchecksthe+*'payload_version'fieldandbasedonitusetheappropriateversiondsm+*structtoparsetheresults.+*+*BackwardCompatibility:+*+*AboveschemeofexchangingdifferentversionedDSMstructbetweenlibndctl+*andpapr_scmshouldprovidebackwardcompatibilityuntilfollowingtwo+*assumptions/conditionswhendefiningnewDSMstructshold:+*+*LetT(X)={setofattributesinDSMstruct'T'versionedX}+*+*1.T(X)isapropersubsetofT(Y)ifX>Y.+*i.eEachnewversionofDSMstructshouldretainexistingstruct+*attributesfrompreviousversion+*+*2.Ifanentity(libndctlorpapr_scm)supportsaDSMstructT(X)then+*itshouldalsosupportT(1),T(2)...T(X-1).+*i.eWhenaddingsupportfornewversionofaDSMstruct,libndctl+*andpapr_scmshouldretainsupportoftheexistingDSMstruct+*versiontheysupport.+*/++/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */+structnd_papr_scm_cmd_pkg{+structnd_cmd_pkghdr;/* Package header containing sub-cmd */+__s32cmd_status;/* Out: Sub-cmd status returned back */+__u16payload_offset;/* In: offset from start of struct */+__u16payload_version;/* In/Out: version of the payload */+__u8payload[];/* In/Out: Sub-cmd data buffer */+};++/*+*SubcommandsforND_CMD_CALL.TopreventoverlapfromND_CMD_*,valuesfor+*theseenumsstartat0x10000.Thesevaluesarethenreturnedfrom+*cmd_to_func()makingiteasytoimplementtheswitch-caseblockin+*papr_scm_ndctl().Thesecommandsaresenttothekernelvia+*'nd_papr_scm_cmd_pkg.hdr.nd_command'+*/+enumdsm_papr_scm{+DSM_PAPR_SCM_MIN=0x10000,+DSM_PAPR_SCM_MAX,+};++/* Helpers to evaluate the size of PAPR_SCM envelope */+/* Calculate the papr_scm-header size */+#define ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE \+(sizeof(structnd_papr_scm_cmd_pkg)-sizeof(structnd_cmd_pkg))++/* Given a type calculate envelope-content size (papr_scm-header + payload) */+#define ND_PAPR_SCM_ENVELOPE_CONTENT_SIZE(_type_) \+(sizeof(_type_)+ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE)++/* Convert a libnvdimm nd_cmd_pkg to papr_scm specific pkg */+staticstructnd_papr_scm_cmd_pkg*nd_to_papr_cmd_pkg(structnd_cmd_pkg*cmd)+{+return(structnd_papr_scm_cmd_pkg*)cmd;+}++/* Return the payload pointer for a given pcmd */+staticvoid*papr_scm_pcmd_to_payload(structnd_papr_scm_cmd_pkg*pcmd)+{+if(pcmd->hdr.nd_size_in==0&&pcmd->hdr.nd_size_out==0)+returnNULL;+else+return(void*)((__u8*)pcmd+pcmd->payload_offset);+}+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
@@ -283,15 +285,92 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,return0;}+/*+*Validatetheinputsargstodimm-controlfunctionandreturn'0'ifvalid.+*ThisalsodoesinitialsanityvalidationtoND_CMD_CALLsub-commandpackages.+*/+staticintis_cmd_valid(structnvdimm*nvdimm,unsignedintcmd,void*buf,+unsignedintbuf_len)+{+unsignedlongcmd_mask=PAPR_SCM_DIMM_CMD_MASK;+structnd_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;+}elseif(cmd==ND_CMD_CALL){++/* Verify the envelop package */+if(!buf||buf_len<sizeof(structnd_papr_scm_cmd_pkg)){+pr_debug("%s: Invalid pkg size=%u\n",__func__,+buf_len);+return-EINVAL;+}++/* Verify that the DSM command family is valid */+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;++}++/* We except a payload with all DSM commands */+if(papr_scm_pcmd_to_payload(pkg)==NULL){+pr_debug("%s: Empty payload for sub-command=0x%llx\n",+__func__,pkg->hdr.nd_command);+return-EINVAL;+}+}++/* Command looks valid */+return0;+}++staticintpapr_scm_service_dsm(structpapr_scm_priv*p,+structnd_papr_scm_cmd_pkg*call_pkg)+{+/* unknown subcommands return error in packages */+if(call_pkg->hdr.nd_command<=DSM_PAPR_SCM_MIN||+call_pkg->hdr.nd_command>=DSM_PAPR_SCM_MAX){+pr_debug("Invalid DSM command 0x%llx\n",+call_pkg->hdr.nd_command);+call_pkg->cmd_status=-EINVAL;+return0;+}++/* Depending on the DSM command call appropriate service routine */+switch(call_pkg->hdr.nd_command){+default:+pr_debug("Unsupported DSM command 0x%llx\n",+call_pkg->hdr.nd_command);+call_pkg->cmd_status=-ENOENT;+return0;+}+}+intpapr_scm_ndctl(structnvdimm_bus_descriptor*nd_desc,structnvdimm*nvdimm,unsignedintcmd,void*buf,unsignedintbuf_len,int*cmd_rc){structnd_cmd_get_config_size*get_size_hdr;structpapr_scm_priv*p;+structnd_papr_scm_cmd_pkg*call_pkg=NULL;+intrc;-/* 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_rc=is_cmd_valid(nvdimm,cmd,buf,buf_len);+if(*cmd_rc){+pr_debug("%s: Invalid cmd=0x%x. Err=%d\n",__func__,+cmd,*cmd_rc);+return*cmd_rc;+}p=nvdimm_provider_data(nvdimm);
@@ -313,13 +392,19 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,*cmd_rc=papr_scm_meta_set(p,buf);break;+caseND_CMD_CALL:+call_pkg=nd_to_papr_cmd_pkg(buf);+*cmd_rc=papr_scm_service_dsm(p,call_pkg);+break;+default:-return-EINVAL;+dev_dbg(&p->pdev->dev,"Unknown command = %d\n",cmd);+*cmd_rc=-EINVAL;}dev_dbg(&p->pdev->dev,"returned with cmd_rc = %d\n",*cmd_rc);-return0;+return*cmd_rc;}staticinlineintpapr_scm_node(intnode)
--
2.25.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
This patch implements support for papr_scm command
'DSM_PAPR_SCM_HEALTH' that returns a newly introduced 'struct
nd_papr_scm_dimm_health_stat' instance containing dimm health
information back to user space in response to ND_CMD_CALL. This
functionality is implemented in newly introduced papr_scm_get_health()
that queries the scm-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 introduces a new member a new member 'struct
papr_scm_priv.health' thats an instance of 'struct
nd_papr_scm_dimm_health_stat' to cache the health information of a
scm-dimm. As a result functions drc_pmem_query_health() and
papr_flags_show() are updated to populate and use this new struct
instead of two be64 integers that we earlier used.
Reviewed-by: Aneesh Kumar K.V <redacted>
quoted hunk
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5: None
v3..v4: Call the DSM_PAPR_SCM_HEALTH service function from
papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
v2..v3: Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx'
types as its exported to the userspace [Aneesh]
Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm
health from enum to #defines [Aneesh]
v1..v2: New patch in the series
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 40 +++++++
arch/powerpc/platforms/pseries/papr_scm.c | 109 ++++++++++++++++---
2 files changed, 132 insertions(+), 17 deletions(-)
@@ -158,4 +159,43 @@ static void *papr_scm_pcmd_to_payload(struct nd_papr_scm_cmd_pkg *pcmd)elsereturn(void*)((__u8*)pcmd+pcmd->payload_offset);}++/* Various scm-dimm health indicators */+#define DSM_PAPR_SCM_DIMM_HEALTHY 0+#define DSM_PAPR_SCM_DIMM_UNHEALTHY 1+#define DSM_PAPR_SCM_DIMM_CRITICAL 2+#define DSM_PAPR_SCM_DIMM_FATAL 3++/*+*Structexchangedbetweenkernel&ndctlinforPAPR_DSM_PAPR_SMART_HEALTH+*Variousbitflagsindicatethehealthstatusofthedimm.+*+*dimm_unarmed:Dimmnotarmed.Socontentswontpersist.+*dimm_bad_shutdown:Previousshutdowndidnotpersistcontents.+*dimm_bad_restore:Contentsfrompreviousshutdownwerentrestored.+*dimm_scrubbed:Contentsofthedimmhavebeenscrubbed.+*dimm_locked:ContentsofthedimmcantbemodifieduntilCECreboot+*dimm_encrypted:Contentsofdimmareencrypted.+*dimm_health:Dimmhealthindicator.+*/+structnd_papr_scm_dimm_health_stat_v1{+__u8dimm_unarmed;+__u8dimm_bad_shutdown;+__u8dimm_bad_restore;+__u8dimm_scrubbed;+__u8dimm_locked;+__u8dimm_encrypted;+__u16dimm_health;+};++/*+*Typedefthecurrentstructfordimm_healthsothatanyapplication+*orkernelrecompiledafterintroducinganewversionautomatically+*supportsthenewversion.+*/+#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+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
@@ -47,8 +47,7 @@ struct papr_scm_priv {structmutexdimm_mutex;/* Health information for the dimm */-__be64health_bitmap;-__be64health_bitmap_valid;+structnd_papr_scm_dimm_health_stathealth;};staticintdrc_pmem_bind(structpapr_scm_priv*p)
@@ -158,6 +157,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p){unsignedlongret[PLPAR_HCALL_BUFSIZE];int64_trc;+__be64health;rc=plpar_hcall(H_SCM_HEALTH,ret,p->drc_index);if(rc!=H_SUCCESS){
@@ -172,13 +172,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)returnrc;/* 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);return0;
@@ -331,6 +359,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,return0;}+/* Fetch the DIMM health info and populate it in provided package. */+staticintpapr_scm_get_health(structpapr_scm_priv*p,+structnd_papr_scm_cmd_pkg*pkg)+{+intrc;+size_tcopysize=sizeof(p->health);++rc=drc_pmem_query_health(p);+if(rc)+gotoout;+/*+*Iftherequestedpayloadversionisgreaterthanoneweknow+*about,returnthepayloadversionweknowaboutandlet+*caller/userspacehandle.+*/+if(pkg->payload_version>ND_PAPR_SCM_DIMM_HEALTH_VERSION)+pkg->payload_version=ND_PAPR_SCM_DIMM_HEALTH_VERSION;++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;+gotoout;+}++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:+/*+*Puttheerrorinoutpackageandreturnsuccessfromfunction+*sothaterrorsifanyarepropogatedbacktouserspace.+*/+pkg->cmd_status=rc;+dev_dbg(&p->pdev->dev,"%s completion code = %d\n",__func__,rc);++return0;+}+staticintpapr_scm_service_dsm(structpapr_scm_priv*p,structnd_papr_scm_cmd_pkg*call_pkg){
@@ -345,6 +418,9 @@ static int papr_scm_service_dsm(struct papr_scm_priv *p,/* Depending on the DSM command call appropriate service routine */switch(call_pkg->hdr.nd_command){+caseDSM_PAPR_SCM_HEALTH:+returnpapr_scm_get_health(p,call_pkg);+default:pr_debug("Unsupported DSM command 0x%llx\n",call_pkg->hdr.nd_command);
@@ -443,26 +518,26 @@ static ssize_t papr_flags_show(struct device *dev,if(rc)returnrc;-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.25.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
From: Dan Williams <hidden> Date: 2020-04-02 03:10:44
On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain [off-list ref] wrote:
Implement support for fetching nvdimm 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 partially exposed to user-space via
newly introduced dimm specific attribute 'papr_flags'. Also a new asm
header named 'papr-scm.h' is added that describes the interface
between PHYP and guest kernel.
Following flags are reported via 'papr_flags' sysfs attribute contents
of which are space separated string flags indicating various nvdimm
states:
* "not_armed" : Indicating that nvdimm contents wont survive a power
cycle.
s/wont/will not/
* "save_fail" : Indicating that nvdimm contents couldn't be flushed
during last shutdown event.
In the nfit definition this description is "flush_fail". The
"save_fail" flag was specific to hybrid devices that don't have
persistent media and instead scuttle away data from DRAM to flash on
power-failure.
* "restore_fail": Indicating that nvdimm contents couldn't be restored
during dimm initialization.
* "encrypted" : Dimm contents are encrypted.
This does not seem like a health flag to me, have you considered the
libnvdimm security interface for this indicator?
* "smart_notify": There is health event for the nvdimm.
Are you also going to signal the sysfs attribute when this event happens?
* "scrubbed" : Indicating that contents of the nvdimm have been
scrubbed.
This one seems odd to me what does it mean if it is not set? What does
it mean if a new scrub has been launched. Basically, is there value in
exposing this state?
* "locked" : Indicating that nvdimm contents cant be modified
until next power cycle.
There is the generic NDD_LOCKED flag, can you use that? ...and in
general I wonder if we should try to unify all the common papr_scm and
nfit health flags in a generic location. It will already be the case
the ndctl needs to look somewhere papr specific for this data maybe it
all should have been generic from the beginning.
In any event, can you also add this content to a new
Documentation/ABI/testing/sysfs-bus-papr? See sysfs-bus-nfit for
comparison.
quoted hunk
[1]: commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5 : None
v3..v4 : None
v2..v3 : Removed PAPR_SCM_DIMM_HEALTH_NON_CRITICAL as a condition for
NVDIMM unarmed [Aneesh]
v1..v2 : New patch in the series.
---
arch/powerpc/include/asm/papr_scm.h | 48 ++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 105 +++++++++++++++++++++-
2 files changed, 151 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/papr_scm.h
@@ -0,0 +1,48 @@+/* SPDX-License-Identifier: GPL-2.0-or-later */+/*+*Structuresanddefinesneededtomanagenvdimmsforspaprguests.+*/+#ifndef _ASM_POWERPC_PAPR_SCM_H_+#define _ASM_POWERPC_PAPR_SCM_H_++#include<linux/types.h>+#include<asm/bitsperlong.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)++/* 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)++#endif
@@ -39,6 +40,13 @@ struct papr_scm_priv {structresourceres;structnd_region*region;structnd_interleave_setnd_set;++/* Protect dimm data from concurrent access */+structmutexdimm_mutex;++/* Health information for the dimm */+__be64health_bitmap;+__be64health_bitmap_valid;};staticintdrc_pmem_bind(structpapr_scm_priv*p)
@@ -144,6 +152,35 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)returndrc_pmem_bind(p);}+staticintdrc_pmem_query_health(structpapr_scm_priv*p)+{+unsignedlongret[PLPAR_HCALL_BUFSIZE];+int64_trc;++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)+returnrc;++/* 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);+return0;+}staticintpapr_scm_meta_get(structpapr_scm_priv*p,structnd_cmd_get_config_data_hdr*hdr)
@@ -304,6 +341,67 @@ static inline int papr_scm_node(int node)returnmin_node;}+staticssize_tpapr_flags_show(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+structnvdimm*dimm=to_nvdimm(dev);+structpapr_scm_priv*p=nvdimm_provider_data(dimm);+__be64health;+intrc;++rc=drc_pmem_query_health(p);+if(rc)+returnrc;
This attribute is user readable which means userspace can trigger and
ongoing stream of device requests. Is that desired? The nfit driver
caches this flag data, and limits attributes with device side-effects
to root-only. The expectation is that ndctl submits commands to
retrieve the up to date state especially because that payload has
other interesting data like temperature that can't be cached.
+
+ /* Protect against modifications to papr_scm_priv with the mutex */
What papr_scm_priv modifications are you worried about because none of
the below looks like it needs to be locked, and papr_scm_priv had
better be stable otherwise the above usages would have crashed.
+ 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 ");
See above about whether we event need this here...
From: Michael Ellerman <hidden> Date: 2020-04-02 11:20:10
Vaibhav Jain [off-list ref] writes:
quoted hunk
Implement support for fetching nvdimm 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 partially exposed to user-space via
newly introduced dimm specific attribute 'papr_flags'. Also a new asm
header named 'papr-scm.h' is added that describes the interface
between PHYP and guest kernel.
Following flags are reported via 'papr_flags' sysfs attribute contents
of which are space separated string flags indicating various nvdimm
states:
* "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 <redacted>
---
Changelog:
v4..v5 : None
v3..v4 : None
v2..v3 : Removed PAPR_SCM_DIMM_HEALTH_NON_CRITICAL as a condition for
NVDIMM unarmed [Aneesh]
v1..v2 : New patch in the series.
---
arch/powerpc/include/asm/papr_scm.h | 48 ++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 105 +++++++++++++++++++++-
2 files changed, 151 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/papr_scm.h
@@ -0,0 +1,48 @@+/* SPDX-License-Identifier: GPL-2.0-or-later */+/*+*Structuresanddefinesneededtomanagenvdimmsforspaprguests.+*/+#ifndef _ASM_POWERPC_PAPR_SCM_H_+#define _ASM_POWERPC_PAPR_SCM_H_++#include<linux/types.h>+#include<asm/bitsperlong.h>++/* DIMM health bitmap bitmap indicators */+/* SCM device is unable to persist memory contents */+#define PAPR_SCM_DIMM_UNARMED PPC_BIT(0)
Please don't use PPC_BIT, it's just unncessary obfuscation for folks
who are reading the code without access to the docs (ie. more or less
everyone other than you :)
@@ -39,6 +40,13 @@ struct papr_scm_priv {structresourceres;structnd_region*region;structnd_interleave_setnd_set;++/* Protect dimm data from concurrent access */+structmutexdimm_mutex;++/* Health information for the dimm */+__be64health_bitmap;+__be64health_bitmap_valid;
It's much less error prone to store the data in CPU endian and do the
endian conversion only at the point where the data either comes from or
goes to firmware.
That would also mean you can define flags above without needing PPC_BIT
because they'll be in CPU endian too.
quoted hunk
@@ -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;
Use kernel types please, ie. s64, or just long.
quoted hunk
+ 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)
No need for __be64 here if health_bitmap was stored in CPU endian.
+ 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;
This is all you ever do with the health_bitmap? In which case why not
just do the masking before storing it into priv and save yourself 8
bytes?
+ /* Check for various masks in bitmap and set the buffer */
+ if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+ rc += sprintf(buf, "not_armed ");
I know buf is "big enough" but using sprintf() in 2020 is a bit ... :)
seq_buf is a pretty thin wrapper over a buffer you can use to make this
cleaner and also handles overflow for you.
See eg. show_user_instructions() for an example.
From: Dan Williams <hidden> Date: 2020-04-03 01:03:01
On Wed, Apr 1, 2020 at 8:08 PM Dan Williams [off-list ref] wrote:
[..]
quoted
* "locked" : Indicating that nvdimm contents cant be modified
until next power cycle.
There is the generic NDD_LOCKED flag, can you use that? ...and in
general I wonder if we should try to unify all the common papr_scm and
nfit health flags in a generic location. It will already be the case
the ndctl needs to look somewhere papr specific for this data maybe it
all should have been generic from the beginning.
The more I think about this more I think this would be a good time to
introduce a common "health/" attribute group under the generic nmemX
sysfs, and then have one flag per-file / attribute. Not only does that
match the recommended sysfs ABI better, but it allows ndctl to
enumerate which flags are supported in addition to their state.
In any event, can you also add this content to a new
Documentation/ABI/testing/sysfs-bus-papr? See sysfs-bus-nfit for
comparison.
From: Dan Williams <hidden> Date: 2020-04-03 17:42:11
On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain [off-list ref] wrote:
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.
They aren't ACPI Device Specific Methods in the papr_scm case, right?
I'd call them what the papr_scm specification calls them and replace
"DSM" throughout.
The layout of the DSM commands as we expect from libnvdimm/libndctl is
described in newly introduced uapi header 'papr_scm_dsm.h' which
defines a new 'struct nd_papr_scm_cmd_pkg' 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().
You don't necessarily need to have command number separation like
that. The function number spaces are unique per family.
quoted hunk
A new function cmd_to_func() is
implemented that reads the args to papr_scm_ndctl() and performs
sanity tests on them. In case of a DSM command being sent via
ND_CMD_CALL a newly introduced function papr_scm_service_dsm() is
called to handle the request.
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5: Fixed a bug in new implementation of papr_scm_ndctl().
v3..v4: Updated papr_scm_ndctl() to delegate DSM command handling to a
different function papr_scm_service_dsm(). [Aneesh]
v2..v3: Updated the nd_papr_scm_cmd_pkg to use __xx types as its
exported to the userspace [Aneesh]
v1..v2: New patch in the series.
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 161 +++++++++++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 97 ++++++++++-
2 files changed, 252 insertions(+), 6 deletions(-)
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h
@@ -0,0 +1,161 @@+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */+/*+*PAPRSCMDevicespecificmethodsandstructforlibndctlandndctl+*+*(C)CopyrightIBM2020+*+*Author:VaibhavJain<vaibhavatlinux.ibm.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2,or(atyouroption)+*anylaterversion.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.
These 2 paragraphs of redundant license text can be dropped. The SPDX
line is sufficient.
+ */
+
+#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
+
+/*
+ * DSM Envelope:
+ *
+ * The ioctl ND_CMD_CALL transfers data between user-space and kernel via
+ * 'envelopes' which consists of a header and user-defined payload sections.
+ * The header is described by 'struct nd_papr_scm_cmd_pkg' which expects a
+ * payload following it and offset of which relative to the struct is provided
+ * by 'nd_papr_scm_cmd_pkg.payload_offset'. *
+ *
+ * +-------------+---------------------+---------------------------+
+ * | 64-Bytes | 8-Bytes | Max 184-Bytes |
+ * +-------------+---------------------+---------------------------+
+ * | nd_papr_scm_cmd_pkg | |
+ * |-------------+ | |
+ * | nd_cmd_pkg | | |
+ * +-------------+---------------------+---------------------------+
+ * | nd_family | | |
+ * | nd_size_out | cmd_status | |
+ * | nd_size_in | payload_version | PAYLOAD |
+ * | nd_command | payload_offset -----> |
+ * | nd_fw_size | | |
+ * +-------------+---------------------+---------------------------+
+ *
+ * DSM Header:
+ *
+ * The header is defined as 'struct nd_papr_scm_cmd_pkg' which embeds a
+ * 'struct nd_cmd_pkg' instance. The DSM command is assigned to member
+ * 'nd_cmd_pkg.nd_command'. Apart from size information of the envelop which is
s/envelop/envelope/
There's a difference between commands and functions in libnvdimm.
Commands are the top-level ioctl methods where ND_CMD_CALL is the
generic payload command and functions are communicated inside of that
wrapper. Otherwise if the command does not wrap other commands like
ND_CMD_{GET,SET}_CONFIG_DATA then the individual bus providers
translate them to the bus-local functionality. For example
ND_CMD_{GET,SET}_CONFIG_DATA correspond to either the _LSx ACPI
methods, or the Intel DSM label methods. The details of those
implementations are hidden from the caller, the caller just sees the
kernel's ND_CMD_{GET,SET}_CONFIG_DATA interface.
+ * contained in 'struct nd_cmd_pkg', the header also has members following
+ * members:
+ *
+ * 'cmd_status' : (Out) Errors if any encountered while servicing DSM.
+ * 'payload_version' : (In/Out) Version number associated with the payload.
+ * 'payload_offset' : (In)Relative offset of payload from start of envelope.
+ *
+ * DSM Payload:
+ *
+ * The layout of the DSM Payload is defined by various structs shared between
+ * papr_scm and libndctl so that contents of payload can be interpreted. 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.
+ *
+ * '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).
+ *
+ * Payload Version:
+ *
+ * A 'payload_version' field is present in DSM header that indicates a specific
+ * version of the structure present in DSM Payload for a given DSM command. This
+ * provides backward compatibility in case the DSM Payload structure evolves
+ * and different structures are supported by 'papr_scm' and 'libndctl'.
+ *
+ * When sending a DSM Payload to 'papr_scm', 'libndctl' should send the version
+ * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
+ * module when servicing the DSM envelop checks the 'payload_version' and then
s/envelop/envelope/
+ * uses 'payload struct version' == MIN('payload_version field',
+ * 'max payload-struct-version supported by papr_scm') to service the DSM. After
+ * servicing the DSM, 'papr_scm' put the negotiated version of payload struct in
+ * returned 'payload_version' field.
+ *
+ * Libndctl on receiving the envelop back from papr_scm again checks the
s/envelop/envelope/
+ * 'payload_version' field and based on it use the appropriate version dsm
+ * struct to parse the results.
+ *
+ * Backward Compatibility:
+ *
+ * Above scheme of exchanging different versioned DSM struct between libndctl
+ * and papr_scm should provide backward compatibility until following two
+ * assumptions/conditions when defining new DSM structs hold:
+ *
+ * Let T(X) = { set of attributes in 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 from previous version
+ *
+ * 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.
+ */
+
+/* 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 */
+ __s32 cmd_status; /* Out: Sub-cmd status returned back */
+ __u16 payload_offset; /* In: offset from start of struct */
+ __u16 payload_version; /* In/Out: version of the payload */
+ __u8 payload[]; /* In/Out: Sub-cmd data buffer */
Does this structure pack the same on all compilers?
+};
+
+/*
+ * 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(). These commands are sent to the kernel via
+ * 'nd_papr_scm_cmd_pkg.hdr.nd_command'
+ */
With nd_family set to NVDIMM_FAMILY_PAPR_SCM the function number space
can overlap the function numbers from other families.
quoted hunk
+enum dsm_papr_scm {
+ DSM_PAPR_SCM_MIN = 0x10000,
+ DSM_PAPR_SCM_MAX,
+};
+
+/* 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 envelope-content size (papr_scm-header + payload) */
+#define ND_PAPR_SCM_ENVELOPE_CONTENT_SIZE(_type_) \
+ (sizeof(_type_) + ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE)
+
+/* 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_ */
@@ -283,15 +285,92 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,return0;}+/*+*Validatetheinputsargstodimm-controlfunctionandreturn'0'ifvalid.+*ThisalsodoesinitialsanityvalidationtoND_CMD_CALLsub-commandpackages.+*/+staticintis_cmd_valid(structnvdimm*nvdimm,unsignedintcmd,void*buf,+unsignedintbuf_len)+{+unsignedlongcmd_mask=PAPR_SCM_DIMM_CMD_MASK;+structnd_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);
Please convert all the pr_debug() statements in this file to
dev_dbg(). Also you don't need to put the __func__ in the print
statement because you can enable that via dynamic_debug_control see
the "f" flag in addition to the "p" flag for dynamic debug enabling.
+ return -EINVAL;
+ } else if (cmd == ND_CMD_CALL) {
+
+ /* Verify the envelop package */
s/envelop/envelope/
quoted hunk
+ if (!buf || buf_len < sizeof(struct nd_papr_scm_cmd_pkg)) {
+ pr_debug("%s: Invalid pkg size=%u\n", __func__,
+ buf_len);
+ return -EINVAL;
+ }
+
+ /* Verify that the DSM command family is valid */
+ 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;
+
+ }
+
+ /* We except a payload with all DSM commands */
+ if (papr_scm_pcmd_to_payload(pkg) == NULL) {
+ pr_debug("%s: Empty payload for sub-command=0x%llx\n",
+ __func__, pkg->hdr.nd_command);
+ return -EINVAL;
+ }
+ }
+
+ /* Command looks valid */
+ return 0;
+}
+
+static int papr_scm_service_dsm(struct papr_scm_priv *p,
+ struct nd_papr_scm_cmd_pkg *call_pkg)
+{
+ /* unknown subcommands return error in packages */
+ if (call_pkg->hdr.nd_command <= DSM_PAPR_SCM_MIN ||
+ call_pkg->hdr.nd_command >= DSM_PAPR_SCM_MAX) {
+ pr_debug("Invalid DSM command 0x%llx\n",
+ call_pkg->hdr.nd_command);
+ call_pkg->cmd_status = -EINVAL;
+ return 0;
+ }
+
+ /* Depending on the DSM command call appropriate service routine */
+ switch (call_pkg->hdr.nd_command) {
+ default:
+ pr_debug("Unsupported DSM command 0x%llx\n",
+ call_pkg->hdr.nd_command);
+ call_pkg->cmd_status = -ENOENT;
+ 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)
{
struct nd_cmd_get_config_size *get_size_hdr;
struct papr_scm_priv *p;
+ struct nd_papr_scm_cmd_pkg *call_pkg = NULL;
+ int 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_rc = is_cmd_valid(nvdimm, cmd, buf, buf_len);
+ if (*cmd_rc) {
+ pr_debug("%s: Invalid cmd=0x%x. Err=%d\n", __func__,
+ cmd, *cmd_rc);
+ return *cmd_rc;
+ }
p = nvdimm_provider_data(nvdimm);
From: Dan Williams <hidden> Date: 2020-04-03 18:44:14
On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain [off-list ref] wrote:
This patch implements support for papr_scm command
'DSM_PAPR_SCM_HEALTH' that returns a newly introduced 'struct
nd_papr_scm_dimm_health_stat' instance containing dimm health
information back to user space in response to ND_CMD_CALL. This
functionality is implemented in newly introduced papr_scm_get_health()
that queries the scm-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 introduces a new member a new member 'struct
papr_scm_priv.health' thats an instance of 'struct
nd_papr_scm_dimm_health_stat' to cache the health information of a
scm-dimm. As a result functions drc_pmem_query_health() and
papr_flags_show() are updated to populate and use this new struct
instead of two be64 integers that we earlier used.
Link to HCALL specification?
quoted hunk
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5: None
v3..v4: Call the DSM_PAPR_SCM_HEALTH service function from
papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
v2..v3: Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx'
types as its exported to the userspace [Aneesh]
Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm
health from enum to #defines [Aneesh]
v1..v2: New patch in the series
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 40 +++++++
arch/powerpc/platforms/pseries/papr_scm.c | 109 ++++++++++++++++---
2 files changed, 132 insertions(+), 17 deletions(-)
Does the structure pack the same across different compilers and configurations?
quoted hunk
+
+/*
+ * Typedef the current struct for dimm_health so that any application
+ * or kernel recompiled after introducing a new version automatically
+ * 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
+
#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
@@ -47,8 +47,7 @@ struct papr_scm_priv {structmutexdimm_mutex;/* Health information for the dimm */-__be64health_bitmap;-__be64health_bitmap_valid;+structnd_papr_scm_dimm_health_stathealth;};staticintdrc_pmem_bind(structpapr_scm_priv*p)
@@ -158,6 +157,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p){unsignedlongret[PLPAR_HCALL_BUFSIZE];int64_trc;+__be64health;rc=plpar_hcall(H_SCM_HEALTH,ret,p->drc_index);if(rc!=H_SUCCESS){
@@ -172,13 +172,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)returnrc;/* 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;+}
I don't think bool is suitable for ioctl ABI. For example the true
value may be positive, or negative depending on the arch. This should
be using explicit integer values.
I assume the reason you are translating this rather than transmitting
that raw 64-bit health value is for future compatibility if the HCALL
format changes / is extended?
+
+ 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;
quoted hunk
mutex_unlock(&p->dimm_mutex);
return 0;
@@ -331,6 +359,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf, return 0; }+/* Fetch the DIMM health info and populate it in provided package. */+static int papr_scm_get_health(struct papr_scm_priv *p,+ struct nd_papr_scm_cmd_pkg *pkg)+{+ int rc;+ size_t copysize = sizeof(p->health);++ rc = drc_pmem_query_health(p);+ if (rc)+ goto out;+ /*+ * If the requested payload version is greater than one we know+ * about, return the payload version we know about and let+ * caller/userspace handle.+ */+ if (pkg->payload_version > ND_PAPR_SCM_DIMM_HEALTH_VERSION)+ pkg->payload_version = ND_PAPR_SCM_DIMM_HEALTH_VERSION;++ 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;+}+ static int papr_scm_service_dsm(struct papr_scm_priv *p, struct nd_papr_scm_cmd_pkg *call_pkg) {
@@ -345,6 +418,9 @@ static int papr_scm_service_dsm(struct papr_scm_priv *p, /* Depending on the DSM command call appropriate service routine */ switch (call_pkg->hdr.nd_command) {+ case DSM_PAPR_SCM_HEALTH:+ return papr_scm_get_health(p, call_pkg);+ default: pr_debug("Unsupported DSM command 0x%llx\n", call_pkg->hdr.nd_command);
@@ -443,26 +518,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 ");
Per my patch1 comment is this "save_fail" or "flush_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.25.1
Hi Dan / Mpe,
I have sent out a v6 of this patch set that addresses your review
comments so far. Also I have added a new doc patch in the patchset that
adds documentation for PAPR_SCM_HEALTH hcall specification.
Requesting you to please review the new patchset at
https://lore.kernel.org/linux-nvdimm/20200420070711.223545-1-vaibhav@linux.ibm.com
Thanks,
~ Vaibhav Jain
Dan Williams [off-list ref] writes:
On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain [off-list ref] wrote:
quoted
This patch implements support for papr_scm command
'DSM_PAPR_SCM_HEALTH' that returns a newly introduced 'struct
nd_papr_scm_dimm_health_stat' instance containing dimm health
information back to user space in response to ND_CMD_CALL. This
functionality is implemented in newly introduced papr_scm_get_health()
that queries the scm-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 introduces a new member a new member 'struct
papr_scm_priv.health' thats an instance of 'struct
nd_papr_scm_dimm_health_stat' to cache the health information of a
scm-dimm. As a result functions drc_pmem_query_health() and
papr_flags_show() are updated to populate and use this new struct
instead of two be64 integers that we earlier used.
Link to HCALL specification?
quoted
Signed-off-by: Vaibhav Jain <redacted>
---
Changelog:
v4..v5: None
v3..v4: Call the DSM_PAPR_SCM_HEALTH service function from
papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
v2..v3: Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx'
types as its exported to the userspace [Aneesh]
Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm
health from enum to #defines [Aneesh]
v1..v2: New patch in the series
---
arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 40 +++++++
arch/powerpc/platforms/pseries/papr_scm.c | 109 ++++++++++++++++---
2 files changed, 132 insertions(+), 17 deletions(-)
Does the structure pack the same across different compilers and configurations?
quoted
+
+/*
+ * Typedef the current struct for dimm_health so that any application
+ * or kernel recompiled after introducing a new version automatically
+ * 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
+
#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
@@ -47,8 +47,7 @@ struct papr_scm_priv {structmutexdimm_mutex;/* Health information for the dimm */-__be64health_bitmap;-__be64health_bitmap_valid;+structnd_papr_scm_dimm_health_stathealth;};staticintdrc_pmem_bind(structpapr_scm_priv*p)
@@ -158,6 +157,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p){unsignedlongret[PLPAR_HCALL_BUFSIZE];int64_trc;+__be64health;rc=plpar_hcall(H_SCM_HEALTH,ret,p->drc_index);if(rc!=H_SUCCESS){
@@ -172,13 +172,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)returnrc;/* 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;+}
I don't think bool is suitable for ioctl ABI. For example the true
value may be positive, or negative depending on the arch. This should
be using explicit integer values.
I assume the reason you are translating this rather than transmitting
that raw 64-bit health value is for future compatibility if the HCALL
format changes / is extended?
quoted
+
+ 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;
quoted
mutex_unlock(&p->dimm_mutex);
return 0;
@@ -331,6 +359,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf, return 0; }+/* Fetch the DIMM health info and populate it in provided package. */+static int papr_scm_get_health(struct papr_scm_priv *p,+ struct nd_papr_scm_cmd_pkg *pkg)+{+ int rc;+ size_t copysize = sizeof(p->health);++ rc = drc_pmem_query_health(p);+ if (rc)+ goto out;+ /*+ * If the requested payload version is greater than one we know+ * about, return the payload version we know about and let+ * caller/userspace handle.+ */+ if (pkg->payload_version > ND_PAPR_SCM_DIMM_HEALTH_VERSION)+ pkg->payload_version = ND_PAPR_SCM_DIMM_HEALTH_VERSION;++ 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;+}+ static int papr_scm_service_dsm(struct papr_scm_priv *p, struct nd_papr_scm_cmd_pkg *call_pkg) {
@@ -345,6 +418,9 @@ static int papr_scm_service_dsm(struct papr_scm_priv *p, /* Depending on the DSM command call appropriate service routine */ switch (call_pkg->hdr.nd_command) {+ case DSM_PAPR_SCM_HEALTH:+ return papr_scm_get_health(p, call_pkg);+ default: pr_debug("Unsupported DSM command 0x%llx\n", call_pkg->hdr.nd_command);
@@ -443,26 +518,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 ");
Per my patch1 comment is this "save_fail" or "flush_fail"?
quoted
- 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.25.1