Re: [PATCH v2 1/1] powerpc/papr_scm: Properly handle UUID types and API
On Wed, 16 Jun 2021 16:43:03 +0300, Andy Shevchenko wrote: > Parse to and export from UUID own type, before dereferencing. > This also fixes wrong comment (Little Endian UUID is something else) > and should eliminate the direct strict types assignments. Applied to powerpc/next. [1/1] powerpc/papr_scm: Properly handle UUID types and API https://git.kernel.org/powerpc/c/0e8554b5d7801b0aebc6c348a0a9f7706aa17b3b cheers
Re: [PATCH v2 1/1] powerpc/papr_scm: Properly handle UUID types and API
Andy Shevchenko writes: > On Wed, Jun 16, 2021 at 04:43:03PM +0300, Andy Shevchenko wrote: >> Parse to and export from UUID own type, before dereferencing. >> This also fixes wrong comment (Little Endian UUID is something else) >> and should eliminate the direct strict types assignments. > > Any comments on this version? Can it be applied? Yeah I'll grab it. cheers
Re: [PATCH v2 1/1] powerpc/papr_scm: Properly handle UUID types and API
On Tue, Jun 22, 2021 at 03:44:56PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 16, 2021 at 04:43:03PM +0300, Andy Shevchenko wrote:
> > Parse to and export from UUID own type, before dereferencing.
> > This also fixes wrong comment (Little Endian UUID is something else)
> > and should eliminate the direct strict types assignments.
>
> Any comments on this version? Can it be applied?
"Any _other_ comments..."
> > Fixes: 43001c52b603 ("powerpc/papr_scm: Use ibm,unit-guid as the iset
> > cookie")
> > Fixes: 259a948c4ba1 ("powerpc/pseries/scm: Use a specific endian format for
> > storing uuid from the device tree")
AFAIU it's fine to have Fixes tags, but if anybody insist I will remove them
and send v3.
> > ---
> > v2: added missed header (Vaibhav), updated comment (Aneesh),
> > rewrite part of the commit message to avoid mentioning the Sparse
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/1] powerpc/papr_scm: Properly handle UUID types and API
On Wed, Jun 16, 2021 at 04:43:03PM +0300, Andy Shevchenko wrote:
> Parse to and export from UUID own type, before dereferencing.
> This also fixes wrong comment (Little Endian UUID is something else)
> and should eliminate the direct strict types assignments.
Any comments on this version? Can it be applied?
> Fixes: 43001c52b603 ("powerpc/papr_scm: Use ibm,unit-guid as the iset cookie")
> Fixes: 259a948c4ba1 ("powerpc/pseries/scm: Use a specific endian format for
> storing uuid from the device tree")
> Cc: Oliver O'Halloran
> Cc: Aneesh Kumar K.V
> Signed-off-by: Andy Shevchenko
> ---
> v2: added missed header (Vaibhav), updated comment (Aneesh),
> rewrite part of the commit message to avoid mentioning the Sparse
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/1] powerpc/papr_scm: Properly handle UUID types and API
On Wed, Jun 16, 2021 at 07:17:03PM +0530, Aneesh Kumar K.V wrote:
> On 6/16/21 7:13 PM, Andy Shevchenko wrote:
> > Parse to and export from UUID own type, before dereferencing.
> > This also fixes wrong comment (Little Endian UUID is something else)
> > and should eliminate the direct strict types assignments.
> >
> > Fixes: 43001c52b603 ("powerpc/papr_scm: Use ibm,unit-guid as the iset
> > cookie")
> > Fixes: 259a948c4ba1 ("powerpc/pseries/scm: Use a specific endian format for
> > storing uuid from the device tree")
>
> Do we need the Fixes: there? It didn't change any functionality right? The
> format with which we stored cookie1 remains the same with older and newer
> code. The newer one is better?
Depends if you are okay with Sparse warnings.
> Reviewed-by: Aneesh Kumar K.V
Thanks for review!
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/1] powerpc/papr_scm: Properly handle UUID types and API
On 6/16/21 7:13 PM, Andy Shevchenko wrote:
Parse to and export from UUID own type, before dereferencing.
This also fixes wrong comment (Little Endian UUID is something else)
and should eliminate the direct strict types assignments.
Fixes: 43001c52b603 ("powerpc/papr_scm: Use ibm,unit-guid as the iset cookie")
Fixes: 259a948c4ba1 ("powerpc/pseries/scm: Use a specific endian format for storing
uuid from the device tree")
Do we need the Fixes: there? It didn't change any functionality right?
The format with which we stored cookie1 remains the same with older and
newer code. The newer one is better?
Reviewed-by: Aneesh Kumar K.V
Cc: Oliver O'Halloran
Cc: Aneesh Kumar K.V
Signed-off-by: Andy Shevchenko
---
v2: added missed header (Vaibhav), updated comment (Aneesh),
rewrite part of the commit message to avoid mentioning the Sparse
arch/powerpc/platforms/pseries/papr_scm.c | 27 +++
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
b/arch/powerpc/platforms/pseries/papr_scm.c
index e2b69cc3beaf..b43be41e8ff7 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#define BIND_ANY_ADDR (~0ul)
@@ -1101,8 +1102,9 @@ static int papr_scm_probe(struct platform_device *pdev)
u32 drc_index, metadata_size;
u64 blocks, block_size;
struct papr_scm_priv *p;
+ u8 uuid_raw[UUID_SIZE];
const char *uuid_str;
- u64 uuid[2];
+ uuid_t uuid;
int rc;
/* check we have all the required DT properties */
@@ -1145,16 +1147,23 @@ static int papr_scm_probe(struct platform_device *pdev)
p->hcall_flush_required = of_property_read_bool(dn,
"ibm,hcall-flush-required");
/* We just need to ensure that set cookies are unique across */
- uuid_parse(uuid_str, (uuid_t *) uuid);
+ uuid_parse(uuid_str, &uuid);
+
/*
-* cookie1 and cookie2 are not really little endian
-* we store a little endian representation of the
-* uuid str so that we can compare this with the label
-* area cookie irrespective of the endian config with which
-* the kernel is built.
+* The cookie1 and cookie2 are not really little endian.
+* We store a raw buffer representation of the
+* uuid string so that we can compare this with the label
+* area cookie irrespective of the endian configuration
+* with which the kernel is built.
+*
+* Historically we stored the cookie in the below format.
+* for a uuid string 72511b67-0b3b-42fd-8d1d-5be3cae8bcaa
+* cookie1 was 0xfd423b0b671b5172
+* cookie2 was 0xaabce8cae35b1d8d
*/
- p->nd_set.cookie1 = cpu_to_le64(uuid[0]);
- p->nd_set.cookie2 = cpu_to_le64(uuid[1]);
+ export_uuid(uuid_raw, &uuid);
+ p->nd_set.cookie1 = get_unaligned_le64(&uuid_raw[0]);
+ p->nd_set.cookie2 = get_unaligned_le64(&uuid_raw[8]);
/* might be zero */
p->metadata_size = metadata_size;
[PATCH v2 1/1] powerpc/papr_scm: Properly handle UUID types and API
Parse to and export from UUID own type, before dereferencing.
This also fixes wrong comment (Little Endian UUID is something else)
and should eliminate the direct strict types assignments.
Fixes: 43001c52b603 ("powerpc/papr_scm: Use ibm,unit-guid as the iset cookie")
Fixes: 259a948c4ba1 ("powerpc/pseries/scm: Use a specific endian format for
storing uuid from the device tree")
Cc: Oliver O'Halloran
Cc: Aneesh Kumar K.V
Signed-off-by: Andy Shevchenko
---
v2: added missed header (Vaibhav), updated comment (Aneesh),
rewrite part of the commit message to avoid mentioning the Sparse
arch/powerpc/platforms/pseries/papr_scm.c | 27 +++
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
b/arch/powerpc/platforms/pseries/papr_scm.c
index e2b69cc3beaf..b43be41e8ff7 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#define BIND_ANY_ADDR (~0ul)
@@ -1101,8 +1102,9 @@ static int papr_scm_probe(struct platform_device *pdev)
u32 drc_index, metadata_size;
u64 blocks, block_size;
struct papr_scm_priv *p;
+ u8 uuid_raw[UUID_SIZE];
const char *uuid_str;
- u64 uuid[2];
+ uuid_t uuid;
int rc;
/* check we have all the required DT properties */
@@ -1145,16 +1147,23 @@ static int papr_scm_probe(struct platform_device *pdev)
p->hcall_flush_required = of_property_read_bool(dn,
"ibm,hcall-flush-required");
/* We just need to ensure that set cookies are unique across */
- uuid_parse(uuid_str, (uuid_t *) uuid);
+ uuid_parse(uuid_str, &uuid);
+
/*
-* cookie1 and cookie2 are not really little endian
-* we store a little endian representation of the
-* uuid str so that we can compare this with the label
-* area cookie irrespective of the endian config with which
-* the kernel is built.
+* The cookie1 and cookie2 are not really little endian.
+* We store a raw buffer representation of the
+* uuid string so that we can compare this with the label
+* area cookie irrespective of the endian configuration
+* with which the kernel is built.
+*
+* Historically we stored the cookie in the below format.
+* for a uuid string 72511b67-0b3b-42fd-8d1d-5be3cae8bcaa
+* cookie1 was 0xfd423b0b671b5172
+* cookie2 was 0xaabce8cae35b1d8d
*/
- p->nd_set.cookie1 = cpu_to_le64(uuid[0]);
- p->nd_set.cookie2 = cpu_to_le64(uuid[1]);
+ export_uuid(uuid_raw, &uuid);
+ p->nd_set.cookie1 = get_unaligned_le64(&uuid_raw[0]);
+ p->nd_set.cookie2 = get_unaligned_le64(&uuid_raw[8]);
/* might be zero */
p->metadata_size = metadata_size;
--
2.30.2
