Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jeff Moyer]

Jerry Hoemann  writes:

> The ndn_pkg.buf struct uses a flexible array definition.  This is in C99.
> An explicit zero length array is a gcc extension that has been around much
> longer.  They behave in a similar fashion, but aren't identical.   In my
> limited use they behave the same.

I could swear I've been bitten by this before, but you are correct.

Thanks,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jeff Moyer]

Jerry Hoemann  writes:

> The ndn_pkg.buf struct uses a flexible array definition.  This is in C99.
> An explicit zero length array is a gcc extension that has been around much
> longer.  They behave in a similar fashion, but aren't identical.   In my
> limited use they behave the same.

I could swear I've been bitten by this before, but you are correct.

Thanks,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 4:44 PM, Jerry Hoemann  wrote:
> On Tue, Nov 10, 2015 at 03:26:38PM -0500, Jeff Moyer wrote:
[..]
>> I see.  I misread that, because you didn't actually make buf a zero
>> length array (see the structure definition quoted above).  I guess you
>> meant to write this:
>>
>> unsigned char buf[0];
>>
>
> The ndn_pkg.buf struct uses a flexible array definition.  This is in C99.
> An explicit zero length array is a gcc extension that has been around much
> longer.  They behave in a similar fashion, but aren't identical.   In my
> limited use they behave the same.

"buf[0]" is more idiomatic for Linux.  I know I expressed concern
about compiler compatibility for ACPICA, but this path does not have
ACPICA interactions.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

On Tue, Nov 10, 2015 at 03:26:38PM -0500, Jeff Moyer wrote:
> >
> > The definition for the IOCTLs are in a user space application.
> > These aren't required in the kernel as the kernel is only a
> > pass thru.
> 
> OK, I don't see the harm in including it in the kernel headers, but I'm
> not going to insist on it.
> 

The IOCTL are defined in terms of the data structures representing
the dsm functions.  Since I'm not supposed to share the definitions
of the DSM at this point, I can't share the IOCTL definitions.

When this restriction is lifted, I would be interested in pushing
these definitions to the appropriate header file.



> > As the DSM I'm working with isn't yet finalized, I've been told that
> > i can't share the user space portion yet.
> 
> That's OK, I don't think providing the userspace code is necessary for
> this patch set to make progress.  (I didn't actually ask for it, to be
> clear.)


Understood.  But it is sometimes nice to have a concrete example(s)
of an interfaces usage.


...

> >> > +struct ndn_pkg {
> >> > +struct {
> >> > +__u8dsm_uuid[16];
> >> > +__u32   dsm_in; /* size of _DSM input   
> >> >  */
> >> > +__u32   dsm_out;/* size of user buffer  
> >> >  */
> >> > +__u32   dsm_rev;/* revision of dsm call 
> >> >  */
> >> > +__u32   res[8]; /* reserved must be 
> >> > zero */
> >> > +__u32   dsm_size;   /* size _DSM would 
> >> > write */
> >> > +} h;
> >> > +unsigned char buf[];
> >> 
> >> Please change that to:
> >>__u8 *buf;
> >> since acpi_object.buffer.pointer is a u8 *.
> >
> > buf isn't being passed to acpi_evaluate_dsm.  its just being used for 
> > pointer offset
> > in acpi_nfit_ctl_passthru.  The "payload" that will be passed to 
> > acpi_evaluate_dsm
> > follows.
> 
> + in_buf.buffer.pointer = (void *) >buf;
> 
> I see.  I misread that, because you didn't actually make buf a zero
> length array (see the structure definition quoted above).  I guess you
> meant to write this:
> 
> unsigned char buf[0];
> 

The ndn_pkg.buf struct uses a flexible array definition.  This is in C99.
An explicit zero length array is a gcc extension that has been around much
longer.  They behave in a similar fashion, but aren't identical.   In my
limited use they behave the same.

-- 

-
Jerry HoemannSoftware Engineer  Hewlett-Packard Enterprise
-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 12:53 PM, Linda Knippers  wrote:
> On 11/10/2015 3:27 PM, Dan Williams wrote:
>>
>> On Tue, Nov 10, 2015 at 11:49 AM, Jerry Hoemann 
>> wrote:
>>>
>>> On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:

 Jerry Hoemann  writes:

> Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.


 Can't you just make passthrough a separate command?  If you actually add
>>>
>>>
>>> There are multiple conflicting NVDIMM _DSM running around, they
>>> are "device specific".  So, we should plan in general and not just
>>> for the example DSM that Intel added support for.  These DSM have
>>> over lapping and incompatible function ids.
>>>
>>> The Intel example is an example,  not standard. They are free to
>>> change it at will. So, we can't be certain there won't be a
>>> conflict some time in the future if we try to use their number space.
>>>
>>> I'm trying to create a generic pass thru that any vendors can use.
>>> Putting
>>> this in the Intel function number space doesn't make a lot of sense to
>>> me.
>>
>>
>> It isn't the "Intel" function number space.  The fact that they
>> currently align is just a happy accident.
>
>
> It's not really a happy accident. Your commit message says it
> was derived from the Intel spec 'for convenience', which I think is
> convenient
> for anything that implements that spec.

Right, and now its no longer convenient to keep things one to one.

> We've discussed ways of supporting different command sets with you
> and determined that this pass-through mechanism was a good approach
> because it allows multiple different command sets to be support in
> a generic way.  Blending the two flavors (generic pass through and explicit
> function definitions) is confusing to me.
>
>> The kernel is free to break
>> the 1:1 ioctl number to DSM function number relationship, and I think
>> it would make the implementation cleaner in this case.
>
>
> To me it's less clean and even for your own example spec, less
> convenient if Intel ever updates that spec.

If that spec is ever updated any new commands will be implemented with
this new generic envelope as the marshaling mechanism.  I'd also look
to convert the existing commands into this new envelope and deprecate
the existing per-DSM-function number approach.  Finally I don't look
at this as purely "passthru" as the kernel will want to crack open the
input payload for commands that it cares about with kernel relevant
side effects, like namespace label updates.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

On Tue, Nov 10, 2015 at 12:04:22PM -0700, Elliott, Robert (Persistent Memory) 
wrote:
> > -Original Message-
> > From: Linux-nvdimm [mailto:linux-nvdimm-boun...@lists.01.org] On Behalf Of
> > Hoemann, Jerry
> > Sent: Friday, November 6, 2015 4:27 PM
> > Subject: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.
> ...
> > diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> ...
> > +struct ndn_pkg {
> > +   struct {
> > +   __u8dsm_uuid[16];
> > +   __u32   dsm_in; /* size of _DSM input*/
> > +   __u32   dsm_out;/* size of user buffer   */
> > +   __u32   dsm_rev;/* revision of dsm call  */
> > +   __u32   res[8]; /* reserved must be zero */
> > +   __u32   dsm_size;   /* size _DSM would write */
> > +   } h;
> > +   unsigned char buf[];
> > +} __packed;
> 
> Given that the _DSM arguments are defined as:
> * Arg0 UUID: Buffer of 16 bytes
> * Arg1 Revision ID: Integer (8 bytes)
> * Arg2 Function Index: Integer (8 bytes)
> * Arg3 Package: function-specific
> 
> 1. The __u32 for dsm_rev is not big enough to express all
> possible 8 byte Revision IDs.
> 
> 2. The unsigned int cmd (carried outside this structure)
> is not big enough on all platforms (e.g., 32-bit) to 
> express all possible Function Indexes.
> 
> 3. The Revision ID and Function Index values passed to 
> the _DSM are defined as little-endian.  Are they
> intended to use native endianness or be little-endian
> in this structure?
> 

  Thanks, Robert.  I will look at these for version 2.

  Jerry
-- 

-
Jerry HoemannSoftware Engineer  Hewlett-Packard Enterprise

3404 E Harmony Rd. MS 36phone:  (970) 898-1022
Ft. Collins, CO 80528   FAX:(970) 898-0707
email:  jerry.hoem...@hpe.com
-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Linda Knippers]

On 11/10/2015 3:27 PM, Dan Williams wrote:

On Tue, Nov 10, 2015 at 11:49 AM, Jerry Hoemann  wrote:

On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:

Jerry Hoemann  writes:


Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.


Can't you just make passthrough a separate command?  If you actually add


There are multiple conflicting NVDIMM _DSM running around, they
are "device specific".  So, we should plan in general and not just
for the example DSM that Intel added support for.  These DSM have
over lapping and incompatible function ids.

The Intel example is an example,  not standard. They are free to
change it at will. So, we can't be certain there won't be a
conflict some time in the future if we try to use their number space.

I'm trying to create a generic pass thru that any vendors can use.  Putting
this in the Intel function number space doesn't make a lot of sense to me.


It isn't the "Intel" function number space.  The fact that they
currently align is just a happy accident.


It's not really a happy accident. Your commit message says it
was derived from the Intel spec 'for convenience', which I think is convenient
for anything that implements that spec.

We've discussed ways of supporting different command sets with you
and determined that this pass-through mechanism was a good approach
because it allows multiple different command sets to be support in
a generic way.  Blending the two flavors (generic pass through and explicit
function definitions) is confusing to me.


The kernel is free to break
the 1:1 ioctl number to DSM function number relationship, and I think
it would make the implementation cleaner in this case.


To me it's less clean and even for your own example spec, less
convenient if Intel ever updates that spec.

-- ljk

___
Linux-nvdimm mailing list
linux-nvd...@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 11:49 AM, Jerry Hoemann  wrote:
> On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:
>> Jerry Hoemann  writes:
>>
>> > Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
>>
>> Can't you just make passthrough a separate command?  If you actually add
>
> There are multiple conflicting NVDIMM _DSM running around, they
> are "device specific".  So, we should plan in general and not just
> for the example DSM that Intel added support for.  These DSM have
> over lapping and incompatible function ids.
>
> The Intel example is an example,  not standard. They are free to
> change it at will. So, we can't be certain there won't be a
> conflict some time in the future if we try to use their number space.
>
> I'm trying to create a generic pass thru that any vendors can use.  Putting
> this in the Intel function number space doesn't make a lot of sense to me.

It isn't the "Intel" function number space.  The fact that they
currently align is just a happy accident.  The kernel is free to break
the 1:1 ioctl number to DSM function number relationship, and I think
it would make the implementation cleaner in this case.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jeff Moyer]

Jerry Hoemann  writes:

> On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:
>> Jerry Hoemann  writes:
>> 
>> > Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
>> 
>> Can't you just make passthrough a separate command?  If you actually add
>
> There are multiple conflicting NVDIMM _DSM running around, they
> are "device specific".  So, we should plan in general and not just
> for the example DSM that Intel added support for.  These DSM have
> over lapping and incompatible function ids.
>
> The Intel example is an example,  not standard. They are free to
> change it at will. So, we can't be certain there won't be a
> conflict some time in the future if we try to use their number space.
>
> I'm trying to create a generic pass thru that any vendors can use.  Putting
> this in the Intel function number space doesn't make a lot of sense to me.

OK, I see your point.

>> the ioctl definition for passthrough (which you didn't do for some
>> reason?), it looks odd:
>
> The definition for the IOCTLs are in a user space application.
> These aren't required in the kernel as the kernel is only a
> pass thru.

OK, I don't see the harm in including it in the kernel headers, but I'm
not going to insist on it.

> As the DSM I'm working with isn't yet finalized, I've been told that
> i can't share the user space portion yet.

That's OK, I don't think providing the userspace code is necessary for
this patch set to make progress.  (I didn't actually ask for it, to be
clear.)

>> #define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, 
>> ND_CMD_PASSTHRU, \
>>  struct ndn_package)
>> 
>> Care to comment on why you chose a different type instead of specifying
>> a new command?
>> 
>> > +struct ndn_pkg {
>> > +  struct {
>> > +  __u8dsm_uuid[16];
>> > +  __u32   dsm_in; /* size of _DSM input*/
>> > +  __u32   dsm_out;/* size of user buffer   */
>> > +  __u32   dsm_rev;/* revision of dsm call  */
>> > +  __u32   res[8]; /* reserved must be zero */
>> > +  __u32   dsm_size;   /* size _DSM would write */
>> > +  } h;
>> > +  unsigned char buf[];
>> 
>> Please change that to:
>>  __u8 *buf;
>> since acpi_object.buffer.pointer is a u8 *.
>
> buf isn't being passed to acpi_evaluate_dsm.  its just being used for pointer 
> offset
> in acpi_nfit_ctl_passthru.  The "payload" that will be passed to 
> acpi_evaluate_dsm
> follows.

+   in_buf.buffer.pointer = (void *) >buf;

I see.  I misread that, because you didn't actually make buf a zero
length array (see the structure definition quoted above).  I guess you
meant to write this:

unsigned char buf[0];

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:
> Jerry Hoemann  writes:
> 
> > Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
> 
> Can't you just make passthrough a separate command?  If you actually add

There are multiple conflicting NVDIMM _DSM running around, they
are "device specific".  So, we should plan in general and not just
for the example DSM that Intel added support for.  These DSM have
over lapping and incompatible function ids.

The Intel example is an example,  not standard. They are free to
change it at will. So, we can't be certain there won't be a
conflict some time in the future if we try to use their number space.

I'm trying to create a generic pass thru that any vendors can use.  Putting
this in the Intel function number space doesn't make a lot of sense to me.

> the ioctl definition for passthrough (which you didn't do for some
> reason?), it looks odd:

The definition for the IOCTLs are in a user space application.
These aren't required in the kernel as the kernel is only a
pass thru.

As the DSM I'm working with isn't yet finalized, I've been told that
i can't share the user space portion yet.


> 
> #define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, 
> ND_CMD_PASSTHRU, \
>  struct ndn_package)
> 
> Care to comment on why you chose a different type instead of specifying
> a new command?
> 
> > +struct ndn_pkg {
> > +   struct {
> > +   __u8dsm_uuid[16];
> > +   __u32   dsm_in; /* size of _DSM input*/
> > +   __u32   dsm_out;/* size of user buffer   */
> > +   __u32   dsm_rev;/* revision of dsm call  */
> > +   __u32   res[8]; /* reserved must be zero */
> > +   __u32   dsm_size;   /* size _DSM would write */
> > +   } h;
> > +   unsigned char buf[];
> 
> Please change that to:
>   __u8 *buf;
> since acpi_object.buffer.pointer is a u8 *.

buf isn't being passed to acpi_evaluate_dsm.  its just being used for pointer 
offset
in acpi_nfit_ctl_passthru.  The "payload" that will be passed to 
acpi_evaluate_dsm
follows.

> 
> Note that the size of this structure will be different for 32 vs. 64
> bit, but I don't think it matters since offsets won't change (the
> pointer is at the end of the structure).

  I assume you mean size of struct changes if I use the pointer as
  substitute for the zero sized array?  or are you saying that the
  packed attribute doesn't affect the layout of the anonymous struct?

-- 

-
Jerry HoemannSoftware Engineer  Hewlett-Packard Enterprise

3404 E Harmony Rd. MS 36phone:  (970) 898-1022
Ft. Collins, CO 80528   FAX:(970) 898-0707
email:  jerry.hoem...@hpe.com
-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Elliott, Robert (Persistent Memory)]

> -Original Message-
> From: Linux-nvdimm [mailto:linux-nvdimm-boun...@lists.01.org] On Behalf Of
> Hoemann, Jerry
> Sent: Friday, November 6, 2015 4:27 PM
> Subject: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.
...
> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
...
> +struct ndn_pkg {
> + struct {
> + __u8dsm_uuid[16];
> + __u32   dsm_in; /* size of _DSM input*/
> + __u32   dsm_out;/* size of user buffer   */
> + __u32   dsm_rev;/* revision of dsm call  */
> + __u32   res[8]; /* reserved must be zero */
> + __u32   dsm_size;   /* size _DSM would write */
> + } h;
> + unsigned char buf[];
> +} __packed;

Given that the _DSM arguments are defined as:
* Arg0 UUID: Buffer of 16 bytes
* Arg1 Revision ID: Integer (8 bytes)
* Arg2 Function Index: Integer (8 bytes)
* Arg3 Package: function-specific

1. The __u32 for dsm_rev is not big enough to express all
possible 8 byte Revision IDs.

2. The unsigned int cmd (carried outside this structure)
is not big enough on all platforms (e.g., 32-bit) to 
express all possible Function Indexes.

3. The Revision ID and Function Index values passed to 
the _DSM are defined as little-endian.  Are they
intended to use native endianness or be little-endian
in this structure?

---
Robert Elliott, HPE Persistent Memory



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 9:51 AM, Jeff Moyer  wrote:
> Jerry Hoemann  writes:
>
>> Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
>
> Can't you just make passthrough a separate command?  If you actually add
> the ioctl definition for passthrough (which you didn't do for some
> reason?), it looks odd:
>
> #define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, 
> ND_CMD_PASSTHRU, \
>  struct ndn_package)
>
> Care to comment on why you chose a different type instead of specifying
> a new command?

+1 for making this just a new command number without a new top-level
number space.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jeff Moyer]

Jerry Hoemann  writes:

> Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.

Can't you just make passthrough a separate command?  If you actually add
the ioctl definition for passthrough (which you didn't do for some
reason?), it looks odd:

#define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, ND_CMD_PASSTHRU, \
 struct ndn_package)

Care to comment on why you chose a different type instead of specifying
a new command?

> +struct ndn_pkg {
> + struct {
> + __u8dsm_uuid[16];
> + __u32   dsm_in; /* size of _DSM input*/
> + __u32   dsm_out;/* size of user buffer   */
> + __u32   dsm_rev;/* revision of dsm call  */
> + __u32   res[8]; /* reserved must be zero */
> + __u32   dsm_size;   /* size _DSM would write */
> + } h;
> + unsigned char buf[];

Please change that to:
__u8 *buf;
since acpi_object.buffer.pointer is a u8 *.

Note that the size of this structure will be different for 32 vs. 64
bit, but I don't think it matters since offsets won't change (the
pointer is at the end of the structure).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jeff Moyer]

Jerry Hoemann  writes:

> Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.

Can't you just make passthrough a separate command?  If you actually add
the ioctl definition for passthrough (which you didn't do for some
reason?), it looks odd:

#define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, ND_CMD_PASSTHRU, \
 struct ndn_package)

Care to comment on why you chose a different type instead of specifying
a new command?

> +struct ndn_pkg {
> + struct {
> + __u8dsm_uuid[16];
> + __u32   dsm_in; /* size of _DSM input*/
> + __u32   dsm_out;/* size of user buffer   */
> + __u32   dsm_rev;/* revision of dsm call  */
> + __u32   res[8]; /* reserved must be zero */
> + __u32   dsm_size;   /* size _DSM would write */
> + } h;
> + unsigned char buf[];

Please change that to:
__u8 *buf;
since acpi_object.buffer.pointer is a u8 *.

Note that the size of this structure will be different for 32 vs. 64
bit, but I don't think it matters since offsets won't change (the
pointer is at the end of the structure).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 9:51 AM, Jeff Moyer  wrote:
> Jerry Hoemann  writes:
>
>> Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
>
> Can't you just make passthrough a separate command?  If you actually add
> the ioctl definition for passthrough (which you didn't do for some
> reason?), it looks odd:
>
> #define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, 
> ND_CMD_PASSTHRU, \
>  struct ndn_package)
>
> Care to comment on why you chose a different type instead of specifying
> a new command?

+1 for making this just a new command number without a new top-level
number space.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Elliott, Robert (Persistent Memory)]

> -Original Message-
> From: Linux-nvdimm [mailto:linux-nvdimm-boun...@lists.01.org] On Behalf Of
> Hoemann, Jerry
> Sent: Friday, November 6, 2015 4:27 PM
> Subject: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.
...
> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
...
> +struct ndn_pkg {
> + struct {
> + __u8dsm_uuid[16];
> + __u32   dsm_in; /* size of _DSM input*/
> + __u32   dsm_out;/* size of user buffer   */
> + __u32   dsm_rev;/* revision of dsm call  */
> + __u32   res[8]; /* reserved must be zero */
> + __u32   dsm_size;   /* size _DSM would write */
> + } h;
> + unsigned char buf[];
> +} __packed;

Given that the _DSM arguments are defined as:
* Arg0 UUID: Buffer of 16 bytes
* Arg1 Revision ID: Integer (8 bytes)
* Arg2 Function Index: Integer (8 bytes)
* Arg3 Package: function-specific

1. The __u32 for dsm_rev is not big enough to express all
possible 8 byte Revision IDs.

2. The unsigned int cmd (carried outside this structure)
is not big enough on all platforms (e.g., 32-bit) to 
express all possible Function Indexes.

3. The Revision ID and Function Index values passed to 
the _DSM are defined as little-endian.  Are they
intended to use native endianness or be little-endian
in this structure?

---
Robert Elliott, HPE Persistent Memory



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jeff Moyer]

Jerry Hoemann  writes:

> On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:
>> Jerry Hoemann  writes:
>> 
>> > Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
>> 
>> Can't you just make passthrough a separate command?  If you actually add
>
> There are multiple conflicting NVDIMM _DSM running around, they
> are "device specific".  So, we should plan in general and not just
> for the example DSM that Intel added support for.  These DSM have
> over lapping and incompatible function ids.
>
> The Intel example is an example,  not standard. They are free to
> change it at will. So, we can't be certain there won't be a
> conflict some time in the future if we try to use their number space.
>
> I'm trying to create a generic pass thru that any vendors can use.  Putting
> this in the Intel function number space doesn't make a lot of sense to me.

OK, I see your point.

>> the ioctl definition for passthrough (which you didn't do for some
>> reason?), it looks odd:
>
> The definition for the IOCTLs are in a user space application.
> These aren't required in the kernel as the kernel is only a
> pass thru.

OK, I don't see the harm in including it in the kernel headers, but I'm
not going to insist on it.

> As the DSM I'm working with isn't yet finalized, I've been told that
> i can't share the user space portion yet.

That's OK, I don't think providing the userspace code is necessary for
this patch set to make progress.  (I didn't actually ask for it, to be
clear.)

>> #define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, 
>> ND_CMD_PASSTHRU, \
>>  struct ndn_package)
>> 
>> Care to comment on why you chose a different type instead of specifying
>> a new command?
>> 
>> > +struct ndn_pkg {
>> > +  struct {
>> > +  __u8dsm_uuid[16];
>> > +  __u32   dsm_in; /* size of _DSM input*/
>> > +  __u32   dsm_out;/* size of user buffer   */
>> > +  __u32   dsm_rev;/* revision of dsm call  */
>> > +  __u32   res[8]; /* reserved must be zero */
>> > +  __u32   dsm_size;   /* size _DSM would write */
>> > +  } h;
>> > +  unsigned char buf[];
>> 
>> Please change that to:
>>  __u8 *buf;
>> since acpi_object.buffer.pointer is a u8 *.
>
> buf isn't being passed to acpi_evaluate_dsm.  its just being used for pointer 
> offset
> in acpi_nfit_ctl_passthru.  The "payload" that will be passed to 
> acpi_evaluate_dsm
> follows.

+   in_buf.buffer.pointer = (void *) >buf;

I see.  I misread that, because you didn't actually make buf a zero
length array (see the structure definition quoted above).  I guess you
meant to write this:

unsigned char buf[0];

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:
> Jerry Hoemann  writes:
> 
> > Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
> 
> Can't you just make passthrough a separate command?  If you actually add

There are multiple conflicting NVDIMM _DSM running around, they
are "device specific".  So, we should plan in general and not just
for the example DSM that Intel added support for.  These DSM have
over lapping and incompatible function ids.

The Intel example is an example,  not standard. They are free to
change it at will. So, we can't be certain there won't be a
conflict some time in the future if we try to use their number space.

I'm trying to create a generic pass thru that any vendors can use.  Putting
this in the Intel function number space doesn't make a lot of sense to me.

> the ioctl definition for passthrough (which you didn't do for some
> reason?), it looks odd:

The definition for the IOCTLs are in a user space application.
These aren't required in the kernel as the kernel is only a
pass thru.

As the DSM I'm working with isn't yet finalized, I've been told that
i can't share the user space portion yet.


> 
> #define ND_IOCTL_PASSTHRU_IOWR(NVDIMM_TYPE_PASSTHRU,, 
> ND_CMD_PASSTHRU, \
>  struct ndn_package)
> 
> Care to comment on why you chose a different type instead of specifying
> a new command?
> 
> > +struct ndn_pkg {
> > +   struct {
> > +   __u8dsm_uuid[16];
> > +   __u32   dsm_in; /* size of _DSM input*/
> > +   __u32   dsm_out;/* size of user buffer   */
> > +   __u32   dsm_rev;/* revision of dsm call  */
> > +   __u32   res[8]; /* reserved must be zero */
> > +   __u32   dsm_size;   /* size _DSM would write */
> > +   } h;
> > +   unsigned char buf[];
> 
> Please change that to:
>   __u8 *buf;
> since acpi_object.buffer.pointer is a u8 *.

buf isn't being passed to acpi_evaluate_dsm.  its just being used for pointer 
offset
in acpi_nfit_ctl_passthru.  The "payload" that will be passed to 
acpi_evaluate_dsm
follows.

> 
> Note that the size of this structure will be different for 32 vs. 64
> bit, but I don't think it matters since offsets won't change (the
> pointer is at the end of the structure).

  I assume you mean size of struct changes if I use the pointer as
  substitute for the zero sized array?  or are you saying that the
  packed attribute doesn't affect the layout of the anonymous struct?

-- 

-
Jerry HoemannSoftware Engineer  Hewlett-Packard Enterprise

3404 E Harmony Rd. MS 36phone:  (970) 898-1022
Ft. Collins, CO 80528   FAX:(970) 898-0707
email:  jerry.hoem...@hpe.com
-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

On Tue, Nov 10, 2015 at 12:04:22PM -0700, Elliott, Robert (Persistent Memory) 
wrote:
> > -Original Message-
> > From: Linux-nvdimm [mailto:linux-nvdimm-boun...@lists.01.org] On Behalf Of
> > Hoemann, Jerry
> > Sent: Friday, November 6, 2015 4:27 PM
> > Subject: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.
> ...
> > diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> ...
> > +struct ndn_pkg {
> > +   struct {
> > +   __u8dsm_uuid[16];
> > +   __u32   dsm_in; /* size of _DSM input*/
> > +   __u32   dsm_out;/* size of user buffer   */
> > +   __u32   dsm_rev;/* revision of dsm call  */
> > +   __u32   res[8]; /* reserved must be zero */
> > +   __u32   dsm_size;   /* size _DSM would write */
> > +   } h;
> > +   unsigned char buf[];
> > +} __packed;
> 
> Given that the _DSM arguments are defined as:
> * Arg0 UUID: Buffer of 16 bytes
> * Arg1 Revision ID: Integer (8 bytes)
> * Arg2 Function Index: Integer (8 bytes)
> * Arg3 Package: function-specific
> 
> 1. The __u32 for dsm_rev is not big enough to express all
> possible 8 byte Revision IDs.
> 
> 2. The unsigned int cmd (carried outside this structure)
> is not big enough on all platforms (e.g., 32-bit) to 
> express all possible Function Indexes.
> 
> 3. The Revision ID and Function Index values passed to 
> the _DSM are defined as little-endian.  Are they
> intended to use native endianness or be little-endian
> in this structure?
> 

  Thanks, Robert.  I will look at these for version 2.

  Jerry
-- 

-
Jerry HoemannSoftware Engineer  Hewlett-Packard Enterprise

3404 E Harmony Rd. MS 36phone:  (970) 898-1022
Ft. Collins, CO 80528   FAX:(970) 898-0707
email:  jerry.hoem...@hpe.com
-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 11:49 AM, Jerry Hoemann  wrote:
> On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:
>> Jerry Hoemann  writes:
>>
>> > Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.
>>
>> Can't you just make passthrough a separate command?  If you actually add
>
> There are multiple conflicting NVDIMM _DSM running around, they
> are "device specific".  So, we should plan in general and not just
> for the example DSM that Intel added support for.  These DSM have
> over lapping and incompatible function ids.
>
> The Intel example is an example,  not standard. They are free to
> change it at will. So, we can't be certain there won't be a
> conflict some time in the future if we try to use their number space.
>
> I'm trying to create a generic pass thru that any vendors can use.  Putting
> this in the Intel function number space doesn't make a lot of sense to me.

It isn't the "Intel" function number space.  The fact that they
currently align is just a happy accident.  The kernel is free to break
the 1:1 ioctl number to DSM function number relationship, and I think
it would make the implementation cleaner in this case.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Linda Knippers]

On 11/10/2015 3:27 PM, Dan Williams wrote:

On Tue, Nov 10, 2015 at 11:49 AM, Jerry Hoemann  wrote:

On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:

Jerry Hoemann  writes:


Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.


Can't you just make passthrough a separate command?  If you actually add


There are multiple conflicting NVDIMM _DSM running around, they
are "device specific".  So, we should plan in general and not just
for the example DSM that Intel added support for.  These DSM have
over lapping and incompatible function ids.

The Intel example is an example,  not standard. They are free to
change it at will. So, we can't be certain there won't be a
conflict some time in the future if we try to use their number space.

I'm trying to create a generic pass thru that any vendors can use.  Putting
this in the Intel function number space doesn't make a lot of sense to me.


It isn't the "Intel" function number space.  The fact that they
currently align is just a happy accident.


It's not really a happy accident. Your commit message says it
was derived from the Intel spec 'for convenience', which I think is convenient
for anything that implements that spec.

We've discussed ways of supporting different command sets with you
and determined that this pass-through mechanism was a good approach
because it allows multiple different command sets to be support in
a generic way.  Blending the two flavors (generic pass through and explicit
function definitions) is confusing to me.


The kernel is free to break
the 1:1 ioctl number to DSM function number relationship, and I think
it would make the implementation cleaner in this case.


To me it's less clean and even for your own example spec, less
convenient if Intel ever updates that spec.

-- ljk

___
Linux-nvdimm mailing list
linux-nvd...@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 12:53 PM, Linda Knippers  wrote:
> On 11/10/2015 3:27 PM, Dan Williams wrote:
>>
>> On Tue, Nov 10, 2015 at 11:49 AM, Jerry Hoemann 
>> wrote:
>>>
>>> On Tue, Nov 10, 2015 at 12:51:59PM -0500, Jeff Moyer wrote:

 Jerry Hoemann  writes:

> Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.


 Can't you just make passthrough a separate command?  If you actually add
>>>
>>>
>>> There are multiple conflicting NVDIMM _DSM running around, they
>>> are "device specific".  So, we should plan in general and not just
>>> for the example DSM that Intel added support for.  These DSM have
>>> over lapping and incompatible function ids.
>>>
>>> The Intel example is an example,  not standard. They are free to
>>> change it at will. So, we can't be certain there won't be a
>>> conflict some time in the future if we try to use their number space.
>>>
>>> I'm trying to create a generic pass thru that any vendors can use.
>>> Putting
>>> this in the Intel function number space doesn't make a lot of sense to
>>> me.
>>
>>
>> It isn't the "Intel" function number space.  The fact that they
>> currently align is just a happy accident.
>
>
> It's not really a happy accident. Your commit message says it
> was derived from the Intel spec 'for convenience', which I think is
> convenient
> for anything that implements that spec.

Right, and now its no longer convenient to keep things one to one.

> We've discussed ways of supporting different command sets with you
> and determined that this pass-through mechanism was a good approach
> because it allows multiple different command sets to be support in
> a generic way.  Blending the two flavors (generic pass through and explicit
> function definitions) is confusing to me.
>
>> The kernel is free to break
>> the 1:1 ioctl number to DSM function number relationship, and I think
>> it would make the implementation cleaner in this case.
>
>
> To me it's less clean and even for your own example spec, less
> convenient if Intel ever updates that spec.

If that spec is ever updated any new commands will be implemented with
this new generic envelope as the marshaling mechanism.  I'd also look
to convert the existing commands into this new envelope and deprecate
the existing per-DSM-function number approach.  Finally I don't look
at this as purely "passthru" as the kernel will want to crack open the
input payload for commands that it cares about with kernel relevant
side effects, like namespace label updates.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

On Tue, Nov 10, 2015 at 03:26:38PM -0500, Jeff Moyer wrote:
> >
> > The definition for the IOCTLs are in a user space application.
> > These aren't required in the kernel as the kernel is only a
> > pass thru.
> 
> OK, I don't see the harm in including it in the kernel headers, but I'm
> not going to insist on it.
> 

The IOCTL are defined in terms of the data structures representing
the dsm functions.  Since I'm not supposed to share the definitions
of the DSM at this point, I can't share the IOCTL definitions.

When this restriction is lifted, I would be interested in pushing
these definitions to the appropriate header file.



> > As the DSM I'm working with isn't yet finalized, I've been told that
> > i can't share the user space portion yet.
> 
> That's OK, I don't think providing the userspace code is necessary for
> this patch set to make progress.  (I didn't actually ask for it, to be
> clear.)


Understood.  But it is sometimes nice to have a concrete example(s)
of an interfaces usage.


...

> >> > +struct ndn_pkg {
> >> > +struct {
> >> > +__u8dsm_uuid[16];
> >> > +__u32   dsm_in; /* size of _DSM input   
> >> >  */
> >> > +__u32   dsm_out;/* size of user buffer  
> >> >  */
> >> > +__u32   dsm_rev;/* revision of dsm call 
> >> >  */
> >> > +__u32   res[8]; /* reserved must be 
> >> > zero */
> >> > +__u32   dsm_size;   /* size _DSM would 
> >> > write */
> >> > +} h;
> >> > +unsigned char buf[];
> >> 
> >> Please change that to:
> >>__u8 *buf;
> >> since acpi_object.buffer.pointer is a u8 *.
> >
> > buf isn't being passed to acpi_evaluate_dsm.  its just being used for 
> > pointer offset
> > in acpi_nfit_ctl_passthru.  The "payload" that will be passed to 
> > acpi_evaluate_dsm
> > follows.
> 
> + in_buf.buffer.pointer = (void *) >buf;
> 
> I see.  I misread that, because you didn't actually make buf a zero
> length array (see the structure definition quoted above).  I guess you
> meant to write this:
> 
> unsigned char buf[0];
> 

The ndn_pkg.buf struct uses a flexible array definition.  This is in C99.
An explicit zero length array is a gcc extension that has been around much
longer.  They behave in a similar fashion, but aren't identical.   In my
limited use they behave the same.

-- 

-
Jerry HoemannSoftware Engineer  Hewlett-Packard Enterprise
-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Dan Williams]

On Tue, Nov 10, 2015 at 4:44 PM, Jerry Hoemann  wrote:
> On Tue, Nov 10, 2015 at 03:26:38PM -0500, Jeff Moyer wrote:
[..]
>> I see.  I misread that, because you didn't actually make buf a zero
>> length array (see the structure definition quoted above).  I guess you
>> meant to write this:
>>
>> unsigned char buf[0];
>>
>
> The ndn_pkg.buf struct uses a flexible array definition.  This is in C99.
> An explicit zero length array is a gcc extension that has been around much
> longer.  They behave in a similar fashion, but aren't identical.   In my
> limited use they behave the same.

"buf[0]" is more idiomatic for Linux.  I know I expressed concern
about compiler compatibility for ACPICA, but this path does not have
ACPICA interactions.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.

Add struct ndn_pkg which the pass thru IOCTL interfaces uses.
ndn_pkg serves as a wrapper for the data being passed to the
underlying DSM and specifies siz data in a uniform manner allowing
the kernel to call the DSM without knowing specifics of the DSM.

Signed-off-by: Jerry Hoemann 
---
 include/uapi/linux/ndctl.h | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index 5b4a4be..1c81a99 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -15,6 +15,9 @@
 
 #include 
 
+#define NVDIMM_TYPE_INTEL  'N'
+#define NVDIMM_TYPE_PASSTHRU   'P'
+
 struct nd_cmd_smart {
__u32 status;
__u8 data[128];
@@ -148,7 +151,8 @@ static inline const char *nvdimm_cmd_name(unsigned cmd)
return "unknown";
 }
 
-#define ND_IOCTL 'N'
+#define ND_IOCTL   NVDIMM_TYPE_INTEL
+
 
 #define ND_IOCTL_SMART _IOWR(ND_IOCTL, ND_CMD_SMART,\
struct nd_cmd_smart)
@@ -204,4 +208,18 @@ enum ars_masks {
ARS_STATUS_MASK = 0x,
ARS_EXT_STATUS_SHIFT = 16,
 };
+
+
+struct ndn_pkg {
+   struct {
+   __u8dsm_uuid[16];
+   __u32   dsm_in; /* size of _DSM input*/
+   __u32   dsm_out;/* size of user buffer   */
+   __u32   dsm_rev;/* revision of dsm call  */
+   __u32   res[8]; /* reserved must be zero */
+   __u32   dsm_size;   /* size _DSM would write */
+   } h;
+   unsigned char buf[];
+} __packed;
+
 #endif /* __NDCTL_H__ */
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.

Add struct ndn_pkg which the pass thru IOCTL interfaces uses.
ndn_pkg serves as a wrapper for the data being passed to the
underlying DSM and specifies siz data in a uniform manner allowing
the kernel to call the DSM without knowing specifics of the DSM.

Signed-off-by: Jerry Hoemann 
---
 include/uapi/linux/ndctl.h | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index 5b4a4be..1c81a99 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -15,6 +15,9 @@
 
 #include 
 
+#define NVDIMM_TYPE_INTEL  'N'
+#define NVDIMM_TYPE_PASSTHRU   'P'
+
 struct nd_cmd_smart {
__u32 status;
__u8 data[128];
@@ -148,7 +151,8 @@ static inline const char *nvdimm_cmd_name(unsigned cmd)
return "unknown";
 }
 
-#define ND_IOCTL 'N'
+#define ND_IOCTL   NVDIMM_TYPE_INTEL
+
 
 #define ND_IOCTL_SMART _IOWR(ND_IOCTL, ND_CMD_SMART,\
struct nd_cmd_smart)
@@ -204,4 +208,18 @@ enum ars_masks {
ARS_STATUS_MASK = 0x,
ARS_EXT_STATUS_SHIFT = 16,
 };
+
+
+struct ndn_pkg {
+   struct {
+   __u8dsm_uuid[16];
+   __u32   dsm_in; /* size of _DSM input*/
+   __u32   dsm_out;/* size of user buffer   */
+   __u32   dsm_rev;/* revision of dsm call  */
+   __u32   res[8]; /* reserved must be zero */
+   __u32   dsm_size;   /* size _DSM would write */
+   } h;
+   unsigned char buf[];
+} __packed;
+
 #endif /* __NDCTL_H__ */
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/