Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.
From: Jerry Hoemann
Date: Tue Nov 10 2015 - 19:44:37 EST
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 {
> >> > + __u8 dsm_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 *) &pkg->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 Hoemann Software Engineer Hewlett-Packard Enterprise
-----------------------------------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/