fix endianess in IEEE-1275 integer properties

PATCH In review Robert Millan 3 participants 10 messages Jan 13, 2008 – Jan 15, 2008

Share & tools

Robert Millan

author

2008-01-13 20:32 UTC #0 lore raw

PATCH 1/1 fix endianess in IEEE-1275 integer properties

Integer properties are always in big endian. This (incomplete) patch is my suggested approach to deal with this. I'd like to receive input about it before I complete it (checking all calls to grub_ieee1275_get_property through GRUB and replacing with grub_ieee1275_get_integer_property when appropiate).

Again, some testing on Apple/IBM/Genesi hardware would be nice.

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

include/grub/ieee1275/ieee1275.h +4 -0

diff -urp endianess.old/include/grub/ieee1275/ieee1275.h endianess/include/grub/ieee1275/ieee1275.h
@@ -97,6 +97,10 @@ int EXPORT_FUNC(grub_ieee1275_get_proper
 					     const char *property, void *buf,
 					     grub_size_t size,
 					     grub_ssize_t *actual);
+int EXPORT_FUNC(grub_ieee1275_get_integer_property) (grub_ieee1275_phandle_t phandle,
+						     const char *property, grub_uint32_t *buf,
+						     grub_size_t size,
+						     grub_ssize_t *actual);
 int EXPORT_FUNC(grub_ieee1275_next_property) (grub_ieee1275_phandle_t phandle,
 					      char *prev_prop, char *prop);
 int EXPORT_FUNC(grub_ieee1275_get_property_length)

kern/ieee1275/ieee1275.c +20 -0

diff -urp endianess.old/kern/ieee1275/ieee1275.c endianess/kern/ieee1275/ieee1275.c
@@ -18,6 +18,7 @@
  */
 
 #include <grub/ieee1275/ieee1275.h>
+#include <grub/types.h>
 
 #define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_phandle_t) -1)
 #define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_ihandle_t) 0)

@@ -89,6 +90,25 @@ grub_ieee1275_get_property (grub_ieee127
 }
 
 int
+grub_ieee1275_get_integer_property (grub_ieee1275_phandle_t phandle,
+				    const char *property, grub_uint32_t *buf,
+				    grub_size_t size, grub_ssize_t *actual)
+{
+  int ret;
+  ret = grub_ieee1275_get_property (phandle, property, (void *) buf, size, actual);
+#ifndef GRUB_CPU_WORDS_BIGENDIAN
+  /* Integer properties are always in big endian.  */
+  {
+    int i;
+    size /= sizeof (grub_uint32_t);
+    for (i = 0; i < size; i++)
+      buf[i] = grub_be_to_cpu32 (buf[i]);
+  }
+#endif
+  return ret;
+}
+
+int
 grub_ieee1275_next_property (grub_ieee1275_phandle_t phandle, char *prev_prop,
 			     char *prop)
 {

kern/powerpc/ieee1275/openfw.c +2 -2

diff -urp endianess.old/kern/powerpc/ieee1275/openfw.c endianess/kern/powerpc/ieee1275/openfw.c
@@ -157,8 +157,8 @@ grub_err_t grub_available_iterate (int (
   if (grub_ieee1275_finddevice ("/memory", &memory))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
 		       "Couldn't find /memory node");
-  if (grub_ieee1275_get_property (memory, "available", available,
-				  sizeof available, &available_size))
+  if (grub_ieee1275_get_integer_property (memory, "available", available,
+					  sizeof available, &available_size))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
 		       "Couldn't examine /memory/available property");

Pavel Roskin

2008-01-14 14:20 UTC #1 lore raw

Quoted 7 lines · Robert Millan

Integer properties are always in big endian. This (incomplete) patch is my suggested approach to deal with this. I'd like to receive input about it before I complete it (checking all calls to grub_ieee1275_get_property through GRUB and replacing with grub_ieee1275_get_integer_property when appropiate).
Again, some testing on Apple/IBM/Genesi hardware would be nice.

It doesn't apply cleanly, and I don't see how the result of grub_ieee1275_get_integer_property() is used. I suggest that you go ahead with the changes as long as you get them right for your platform.

Signature · 2 lines

Regards,
Pavel Roskin

Robert Millan

author

2008-01-14 14:29 UTC #2 lore raw

Quoted 10 lines · Pavel Roskin

Quoted 7 lines · Robert Millan
Integer properties are always in big endian. This (incomplete) patch is my suggested approach to deal with this. I'd like to receive input about it before I complete it (checking all calls to grub_ieee1275_get_property through GRUB and replacing with grub_ieee1275_get_integer_property when appropiate).
Again, some testing on Apple/IBM/Genesi hardware would be nice.
It doesn't apply cleanly,

It depends on my previous patch, "restrict parsing of `available'".

Quoted 2 lines

and I don't see how the result of
grub_ieee1275_get_integer_property() is used.

If something went wrong, you'd most likely see those claim-related errors you're familiar with ;-)

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Pavel Roskin

2008-01-15 04:49 UTC #3 lore raw

Quoting Robert Millan <[email protected]>:

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
Again, some testing on Apple/IBM/Genesi hardware would be nice.
It doesn't apply cleanly,
It depends on my previous patch, "restrict parsing of `available'".

Applied both, everything is fine on PowerMac G3.

Signature · 2 lines

Regards,
Pavel Roskin

Robert Millan

author

2008-01-15 12:14 UTC #4 lore raw

Quoted 9 lines · Pavel Roskin

Quoting Robert Millan <[email protected]>:
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Again, some testing on Apple/IBM/Genesi hardware would be nice.
It doesn't apply cleanly,
It depends on my previous patch, "restrict parsing of `available'".
Applied both, everything is fine on PowerMac G3.

Ok. Checked in the first one. I'll complete the second now.

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Robert Millan

author

2008-01-15 12:24 UTC #5 lore raw

Quoted 2 lines · Robert Millan

Ok. Checked in the first one. I'll complete the second now.

Here. Please can you test?

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

* include/grub/ieee1275/ieee1275.h (grub_ieee1275_get_integer_property): New function prototype.

* kern/ieee1275/ieee1275.c: Include `<grub/types.h>'. (grub_ieee1275_get_integer_property): New function. Wraps around grub_ieee1275_get_property() to handle endianess.

* kern/powerpc/ieee1275/cmain.c (grub_ieee1275_find_options): Replace grub_ieee1275_get_property() with grub_ieee1275_get_integer_property() where appropiate.
* kern/powerpc/ieee1275/openfw.c (grub_available_iterate): Likewise. (grub_map): Likewise.
* kern/sparc64/ieee1275/openfw.c (grub_map): Likewise.

include/grub/ieee1275/ieee1275.h +4 -0

diff -urp grub2/include/grub/ieee1275/ieee1275.h integer/include/grub/ieee1275/ieee1275.h
@@ -97,6 +97,10 @@ int EXPORT_FUNC(grub_ieee1275_get_proper
 					     const char *property, void *buf,
 					     grub_size_t size,
 					     grub_ssize_t *actual);
+int EXPORT_FUNC(grub_ieee1275_get_integer_property) (grub_ieee1275_phandle_t phandle,
+						     const char *property, grub_uint32_t *buf,
+						     grub_size_t size,
+						     grub_ssize_t *actual);
 int EXPORT_FUNC(grub_ieee1275_next_property) (grub_ieee1275_phandle_t phandle,
 					      char *prev_prop, char *prop);
 int EXPORT_FUNC(grub_ieee1275_get_property_length)

kern/ieee1275/ieee1275.c +21 -0

diff -urp grub2/kern/ieee1275/ieee1275.c integer/kern/ieee1275/ieee1275.c
@@ -18,6 +18,7 @@
  */
 
 #include <grub/ieee1275/ieee1275.h>
+#include <grub/types.h>
 
 #define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_phandle_t) -1)
 #define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_ihandle_t) 0)

@@ -89,6 +90,26 @@ grub_ieee1275_get_property (grub_ieee127
 }
 
 int
+grub_ieee1275_get_integer_property (grub_ieee1275_phandle_t phandle,
+				    const char *property, grub_uint32_t *buf,
+				    grub_size_t size, grub_ssize_t *actual)
+{
+  int ret;
+  ret = grub_ieee1275_get_property (phandle, property, (void *) buf, size, actual);
+#ifndef GRUB_CPU_WORDS_BIGENDIAN
+  /* Integer properties are always in big endian.  */
+  if (ret == 0)
+    {
+      int i;
+      size /= sizeof (grub_uint32_t);
+      for (i = 0; i < size; i++)
+	buf[i] = grub_be_to_cpu32 (buf[i]);
+    }
+#endif
+  return ret;
+}
+
+int
 grub_ieee1275_next_property (grub_ieee1275_phandle_t phandle, char *prev_prop,
 			     char *prop)
 {

kern/powerpc/ieee1275/cmain.c +2 -2

diff -urp grub2/kern/powerpc/ieee1275/cmain.c integer/kern/powerpc/ieee1275/cmain.c
@@ -56,8 +56,8 @@ grub_ieee1275_find_options (void)
   int is_smartfirmware = 0;
 
   grub_ieee1275_finddevice ("/options", &options);
-  rc = grub_ieee1275_get_property (options, "real-mode?", &realmode,
-				   sizeof realmode, 0);
+  rc = grub_ieee1275_get_integer_property (options, "real-mode?", &realmode,
+					   sizeof realmode, 0);
   if ((rc >= 0) && realmode)
     grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_REAL_MODE);

kern/powerpc/ieee1275/openfw.c +8 -8

diff -urp grub2/kern/powerpc/ieee1275/openfw.c integer/kern/powerpc/ieee1275/openfw.c
@@ -148,17 +148,17 @@ grub_err_t grub_available_iterate (int (
 
   /* Determine the format of each entry in `available'.  */
   grub_ieee1275_finddevice ("/", &root);
-  grub_ieee1275_get_property (root, "#address-cells", &address_cells,
-	sizeof address_cells, 0);
-  grub_ieee1275_get_property (root, "#size-cells", &size_cells,
-	sizeof size_cells, 0);
+  grub_ieee1275_get_integer_property (root, "#address-cells", &address_cells,
+				      sizeof address_cells, 0);
+  grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
+				      sizeof size_cells, 0);
 
   /* Load `/memory/available'.  */
   if (grub_ieee1275_finddevice ("/memory", &memory))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
 		       "Couldn't find /memory node");
-  if (grub_ieee1275_get_property (memory, "available", available,
-				  sizeof available, &available_size))
+  if (grub_ieee1275_get_integer_property (memory, "available", available,
+					  sizeof available, &available_size))
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
 		       "Couldn't examine /memory/available property");

@@ -203,8 +203,8 @@ grub_map (grub_addr_t phys, grub_addr_t 
   grub_ieee1275_ihandle_t mmu;
   int len;
 
-  grub_ieee1275_get_property (grub_ieee1275_chosen, "mmu", &mmu, sizeof mmu,
-			      &len);
+  grub_ieee1275_get_integer_property (grub_ieee1275_chosen, "mmu", &mmu, sizeof mmu,
+				      &len);
   if (len != sizeof mmu)
     return -1;

kern/sparc64/ieee1275/openfw.c +2 -2

diff -urp grub2/kern/sparc64/ieee1275/openfw.c integer/kern/sparc64/ieee1275/openfw.c
@@ -183,8 +183,8 @@ grub_map (grub_addr_t phys, grub_addr_t 
   grub_ieee1275_ihandle_t mmu;
   grub_ssize_t len;
 
-  grub_ieee1275_get_property (grub_ieee1275_chosen, "mmu", &mmu, sizeof mmu,
-			      &len);
+  grub_ieee1275_get_integer_property (grub_ieee1275_chosen, "mmu", &mmu, sizeof mmu,
+				      &len);
   if (len != sizeof mmu)
     return -1;

Pavel Roskin

2008-01-15 14:41 UTC #6 lore raw

Quoting Robert Millan <[email protected]>:

Quoted 5 lines · Robert Millan

Quoted 2 lines
Ok. Checked in the first one. I'll complete the second now.
Here. Please can you test?

It's working fine for me. I don't see any difference.

Signature · 2 lines

Regards,
Pavel Roskin

Robert Millan

author

2008-01-15 16:15 UTC #7 lore raw

Quoted 9 lines · Pavel Roskin

Quoting Robert Millan <[email protected]>:
Quoted 5 lines · Robert Millan
Quoted 2 lines
Ok. Checked in the first one. I'll complete the second now.
Here. Please can you test?
It's working fine for me. I don't see any difference.

Thank you. Committed now.

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Marco Gerards

2008-01-15 12:29 UTC #8 lore raw

In reply to Robert Millan

Quoted 5 lines · Robert Millan

Integer properties are always in big endian. This (incomplete) patch is my suggested approach to deal with this. I'd like to receive input about it before I complete it (checking all calls to grub_ieee1275_get_property through GRUB and replacing with grub_ieee1275_get_integer_property when appropiate).

It looks fine to me. The Changelog entry is still missing.

Quoted 1 line

Again, some testing on Apple/IBM/Genesi hardware would be nice.

All my PPC boxes do not like themselves. One died and the other also has some problems... :-/

--
Marco

Robert Millan

author

2008-01-15 12:34 UTC #9 lore raw

Quoted 9 lines · Marco Gerards

Quoted 5 lines · Robert Millan
Integer properties are always in big endian. This (incomplete) patch is my suggested approach to deal with this. I'd like to receive input about it before I complete it (checking all calls to grub_ieee1275_get_property through GRUB and replacing with grub_ieee1275_get_integer_property when appropiate).
It looks fine to me. The Changelog entry is still missing.

Check my last mail!

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)