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

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

[Andy Shevchenko]

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.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/nvdimm/btt_devs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvdimm/btt_devs.c b/drivers/nvdimm/btt_devs.c
index 05feb97e11ce..82bcd2e86a18 100644
--- a/drivers/nvdimm/btt_devs.c
+++ b/drivers/nvdimm/btt_devs.c
@@ -244,13 +244,14 @@ struct device *nd_btt_create(struct nd_region *nd_region)
  */
 bool nd_btt_arena_is_valid(struct nd_btt *nd_btt, struct btt_sb *super)
 {
+	static const u8 null_uuid[16];
 	const u8 *parent_uuid = nd_dev_to_uuid(&nd_btt->ndns->dev);
 	u64 checksum;
 
 	if (memcmp(super->signature, BTT_SIG, BTT_SIG_LEN) != 0)
 		return false;
 
-	if (!guid_is_null((guid_t *)&super->parent_uuid))
+	if (memcmp(super->parent_uuid, null_uuid, 16) != 0)
 		if (memcmp(super->parent_uuid, parent_uuid, 16) != 0)
 			return false;
 
-- 
2.30.2
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

[Dan Williams]

[ add Erik ]

On Fri, Apr 16, 2021 at 10:36 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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.

Erik, would it be possible to use the guid_t type in ACPICA? That
would allow NFIT to drop some ugly casts.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

[Andy Shevchenko]

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@linux.intel.com> 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).

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

[Dan Williams]

On Fri, Apr 16, 2021 at 1:42 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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@linux.intel.com> 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).

I'm thinking something like ACPICA defining that space as a union with
the correct typing just for Linux.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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@intel.com>
> Sent: Friday, April 16, 2021 3:09 PM
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: linux-nvdimm <linux-nvdimm@lists.01.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; Verma, Vishal L
> <vishal.l.verma@intel.com>; Jiang, Dave <dave.jiang@intel.com>; Weiny, Ira
> <ira.weiny@intel.com>; Kaneda, Erik <erik.kaneda@intel.com>
> 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@linux.intel.com> 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@linux.intel.com> 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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

[Moore, Robert]

-----Original Message-----
From: Kaneda, Erik <erik.kaneda@intel.com> 
Sent: Monday, April 19, 2021 3:56 PM
To: Williams, Dan J <dan.j.williams@intel.com>; Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Moore, Robert <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>
Cc: linux-nvdimm <linux-nvdimm@lists.01.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Verma, Vishal L <vishal.l.verma@intel.com>; Jiang, Dave <dave.jiang@intel.com>; Weiny, Ira <ira.weiny@intel.com>
Subject: RE: [PATCH v1 1/1] libnvdimm: Don't use GUID APIs against raw buffer

+Bob and Rafael

> -----Original Message-----
> From: Dan Williams <dan.j.williams@intel.com>
> Sent: Friday, April 16, 2021 3:09 PM
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: linux-nvdimm <linux-nvdimm@lists.01.org>; Linux Kernel Mailing 
> List <linux-kernel@vger.kernel.org>; Verma, Vishal L 
> <vishal.l.verma@intel.com>; Jiang, Dave <dave.jiang@intel.com>; Weiny, 
> Ira <ira.weiny@intel.com>; Kaneda, Erik <erik.kaneda@intel.com>
> 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@linux.intel.com> 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@linux.intel.com> 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.

Erik,
I don't like to add these kinds of macros, since it is simply another item that needs to be added to port ACPICA to different hosts. (And must be known by the porter.)

Other than that, I suppose we can add such a macro.

Bob, do you have any thoughts on this?
Erik
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org