Hi Mpe,

Thanks for looking into this patch.

Michael Ellerman <m...@ellerman.id.au> writes:

> Shivaprasad G Bhat <sb...@linux.ibm.com> writes:
>> papr_scm and ndtest share common PDSM payload structs like
>> nd_papr_pdsm_health. Presently these structs are duplicated across 
>> papr_pdsm.h
>> and ndtest.h header files. Since 'ndtest' is essentially arch independent 
>> and can
>> run on platforms other than PPC64, a way needs to be deviced to avoid 
>> redundancy
>> and duplication of PDSM structs in future.
>> So the patch proposes moving the PDSM header from arch/powerpc/include/uapi/ 
>> to
>> the generic include/uapi/linux directory. Also, there are some #defines 
>> common
>> between papr_scm and ndtest which are not exported to the user space. So, 
>> move
>> them to a header file which can be shared across ndtest and papr_scm via 
>> newly
>> introduced include/linux/papr_scm.h.
>> Signed-off-by: Shivaprasad G Bhat <sb...@linux.ibm.com>
>> Signed-off-by: Vaibhav Jain <vaib...@linux.ibm.com>
>> Suggested-by: "Aneesh Kumar K.V" <aneesh.ku...@linux.ibm.com>
>> ---
>> Changelog:
>> Since v1:
>> Link: 
>> https://patchwork.kernel.org/project/linux-nvdimm/patch/162505488483.72147.12741153746322191381.stgit@56e104a48989/
>> * Removed dependency on this patch for the other patches
>>  MAINTAINERS                               |    2 
>>  arch/powerpc/include/uapi/asm/papr_pdsm.h |  165 
>> -----------------------------
>>  arch/powerpc/platforms/pseries/papr_scm.c |   43 --------
>>  include/linux/papr_scm.h                  |   48 ++++++++
>>  include/uapi/linux/papr_pdsm.h            |  165 
>> +++++++++++++++++++++++++++++
> This doesn't make sense to me.
> Anything with papr (or PAPR) in the name is fundamentally powerpc
> specific, it doesn't belong in a generic header, or in a generic
> location.
> What's the actual problem you're trying to solve?
The ndtest module (tools/testing/nvdimm/test/ndtest.c) is implemented in
an arch independed way to enable testing of PAPR PDSMs on non ppc64
platforms like x86_64. It uses the same PDSM structs as used by papr_scm
to communicate with libndctl userspace. 

Since papr_scm is ppc64 arch specific we were so far duplicating the
PDSM structures between ndtest and papr_scm. The patch tries to solve
this duplication by moving the shared structs to arch independent common
include dirs.

Secondly, PDSMs describes how userspace can use NVDIMM_FAMILY_PAPR to
interact with NVDIMMs. So potentially a new NVDIMM beyond powerpc arch
can add its support and would need access to the same structs used by
papr_scm and ndtest. In that context it would make sense to move PDSM
headers to generic include dirs.

> If it's just including papr_scm bits into ndtest.c then that should be
> as simple as:
> #ifdef __powerpc__
> #include <asm/papr_scm.h>
> #endif
> Shouldn't it?
No, as ndtest implements support for NVDIMM_FAMILY_PAPR and would need
access to PDSM related structs which presently are only available for

> cheers

~ Vaibhav

Reply via email to