RE: [PATCH v1 1/1] libnvdimm: Don't use GUID APIs against raw buffer

[Kaneda, Erik]

+Bob and Rafael

> -----Original Message-----
> From: Dan Williams <dan.j.williams@xxxxxxxxx>
> Sent: Friday, April 16, 2021 3:09 PM
> To: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: linux-nvdimm <linux-nvdimm@xxxxxxxxxxxx>; Linux Kernel Mailing List
> <linux-kernel@xxxxxxxxxxxxxxx>; Verma, Vishal L
> <vishal.l.verma@xxxxxxxxx>; Jiang, Dave <dave.jiang@xxxxxxxxx>; Weiny, Ira
> <ira.weiny@xxxxxxxxx>; Kaneda, Erik <erik.kaneda@xxxxxxxxx>
> Subject: Re: [PATCH v1 1/1] libnvdimm: Don't use GUID APIs against raw
> buffer
> 
> On Fri, Apr 16, 2021 at 1:42 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > On Fri, Apr 16, 2021 at 01:08:06PM -0700, Dan Williams wrote:
> > > [ add Erik ]
> > >
> > > On Fri, Apr 16, 2021 at 10:36 AM Andy Shevchenko
> > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > >
> > > > On Thu, Apr 15, 2021 at 05:37:54PM +0300, Andy Shevchenko wrote:
> > > > > Strictly speaking the comparison between guid_t and raw buffer
> > > > > is not correct. Return to plain memcmp() since the data structures
> > > > > haven't changed to use uuid_t / guid_t the current state of affairs
> > > > > is inconsistent. Either it should be changed altogether or left
> > > > > as is.
> > > >
> > > > Dan, please review this one as well. I think here you may agree with me.
> > >
> > > You know, this is all a problem because ACPICA is using a raw buffer.
> >
> > And this is fine. It might be any other representation as well.
> >
> > > Erik, would it be possible to use the guid_t type in ACPICA? That
> > > would allow NFIT to drop some ugly casts.
> >
> > guid_t is internal kernel type. If we ever decide to deviate from the current
> > representation it wouldn't be possible in case a 3rd party is using it 1:1
> > (via typedef or so).
> 
Hi Dan,

> I'm thinking something like ACPICA defining that space as a union with
> the correct typing just for Linux.

I'm assuming that you mean using something like guid_t type for ACPI data table fields like NFIT rather than objects returned by ACPI namespace object such as _DSD.

For ACPI data tables, we could to use macros so that different operating systems can provide their own definitions for a guid. For ACPICA, it will expands to a 16-byte array. Linux can have it's own definition that contains a union or their own guid type (guid_t). As long as the OS-supplied definition is 16bytes, I don't think there would be an issue.

Bob, do you have any thoughts on this?
Erik