[09/10] commands/file: use definitions from arm/linux.h

Message ID 20180201181858.1472-10-leif.lindholm@linaro.org
State Superseded
Headers show
Series
  • various cleanup
Related show

Commit Message

Leif Lindholm Feb. 1, 2018, 6:18 p.m.
Clean up code for matching IS_ARM slightly by making use of struct
linux_arm_kernel_header and GRUB_LINUX_ARM_MAGIC_SIGNATURE.

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

---
 grub-core/commands/file.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

-- 
2.11.0


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Comments

Daniel Kiper Feb. 15, 2018, 10:30 a.m. | #1
On Thu, Feb 01, 2018 at 06:18:57PM +0000, Leif Lindholm wrote:
> Clean up code for matching IS_ARM slightly by making use of struct

> linux_arm_kernel_header and GRUB_LINUX_ARM_MAGIC_SIGNATURE.

>

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---

>  grub-core/commands/file.c | 14 ++++++--------

>  1 file changed, 6 insertions(+), 8 deletions(-)

>

> diff --git a/grub-core/commands/file.c b/grub-core/commands/file.c

> index 63c84499b..fad191202 100644

> --- a/grub-core/commands/file.c

> +++ b/grub-core/commands/file.c

> @@ -27,6 +27,7 @@

>  #include <grub/elf.h>

>  #include <grub/xen_file.h>

>  #include <grub/efi/pe32.h>

> +#include <grub/arm/linux.h>

>  #include <grub/i386/linux.h>

>  #include <grub/xnu.h>

>  #include <grub/machoload.h>

> @@ -383,21 +384,18 @@ grub_cmd_file (grub_extcmd_context_t ctxt, int argc, char **args)

>        }

>      case IS_ARM_LINUX:

>        {

> -	grub_uint32_t sig, sig_pi;

> -	if (grub_file_read (file, &sig_pi, 4) != 4)

> +	struct linux_arm_kernel_header lh;

> +	if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh))

>  	  break;

>  	/* Raspberry pi.  */

> -	if (sig_pi == grub_cpu_to_le32_compile_time (0xea000006))

> +	if (lh.code0 == grub_cpu_to_le32_compile_time (0xea000006))


Could you define 0xea000006 as a constant? If you add the comment
what does it mean that will be perfect.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Leif Lindholm Feb. 27, 2018, 12:36 p.m. | #2
On Thu, Feb 15, 2018 at 11:30:32AM +0100, Daniel Kiper wrote:
> On Thu, Feb 01, 2018 at 06:18:57PM +0000, Leif Lindholm wrote:

> > Clean up code for matching IS_ARM slightly by making use of struct

> > linux_arm_kernel_header and GRUB_LINUX_ARM_MAGIC_SIGNATURE.

> >

> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> > ---

> >  grub-core/commands/file.c | 14 ++++++--------

> >  1 file changed, 6 insertions(+), 8 deletions(-)

> >

> > diff --git a/grub-core/commands/file.c b/grub-core/commands/file.c

> > index 63c84499b..fad191202 100644

> > --- a/grub-core/commands/file.c

> > +++ b/grub-core/commands/file.c

> > @@ -27,6 +27,7 @@

> >  #include <grub/elf.h>

> >  #include <grub/xen_file.h>

> >  #include <grub/efi/pe32.h>

> > +#include <grub/arm/linux.h>

> >  #include <grub/i386/linux.h>

> >  #include <grub/xnu.h>

> >  #include <grub/machoload.h>

> > @@ -383,21 +384,18 @@ grub_cmd_file (grub_extcmd_context_t ctxt, int argc, char **args)

> >        }

> >      case IS_ARM_LINUX:

> >        {

> > -	grub_uint32_t sig, sig_pi;

> > -	if (grub_file_read (file, &sig_pi, 4) != 4)

> > +	struct linux_arm_kernel_header lh;

> > +	if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh))

> >  	  break;

> >  	/* Raspberry pi.  */

> > -	if (sig_pi == grub_cpu_to_le32_compile_time (0xea000006))

> > +	if (lh.code0 == grub_cpu_to_le32_compile_time (0xea000006))

> 

> Could you define 0xea000006 as a constant? If you add the comment

> what does it mean that will be perfect.


I'll be completely honest here and say "I don't have a clue".

Well, that's not true, I'm broken enough to see that that is a short
forward branch in the A32 ("ARM") instruction set. What I don't
understand is what significance that has for Raspberry Pi.
I just kept the logic for compatibility.

Vladimir?

/
    Leif

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Daniel Kiper March 5, 2018, 3:13 p.m. | #3
On Tue, Feb 27, 2018 at 12:36:37PM +0000, Leif Lindholm wrote:
> On Thu, Feb 15, 2018 at 11:30:32AM +0100, Daniel Kiper wrote:

> > On Thu, Feb 01, 2018 at 06:18:57PM +0000, Leif Lindholm wrote:

> > > Clean up code for matching IS_ARM slightly by making use of struct

> > > linux_arm_kernel_header and GRUB_LINUX_ARM_MAGIC_SIGNATURE.

> > >

> > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> > > ---

> > >  grub-core/commands/file.c | 14 ++++++--------

> > >  1 file changed, 6 insertions(+), 8 deletions(-)

> > >

> > > diff --git a/grub-core/commands/file.c b/grub-core/commands/file.c

> > > index 63c84499b..fad191202 100644

> > > --- a/grub-core/commands/file.c

> > > +++ b/grub-core/commands/file.c

> > > @@ -27,6 +27,7 @@

> > >  #include <grub/elf.h>

> > >  #include <grub/xen_file.h>

> > >  #include <grub/efi/pe32.h>

> > > +#include <grub/arm/linux.h>

> > >  #include <grub/i386/linux.h>

> > >  #include <grub/xnu.h>

> > >  #include <grub/machoload.h>

> > > @@ -383,21 +384,18 @@ grub_cmd_file (grub_extcmd_context_t ctxt, int argc, char **args)

> > >        }

> > >      case IS_ARM_LINUX:

> > >        {

> > > -	grub_uint32_t sig, sig_pi;

> > > -	if (grub_file_read (file, &sig_pi, 4) != 4)

> > > +	struct linux_arm_kernel_header lh;

> > > +	if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh))

> > >  	  break;

> > >  	/* Raspberry pi.  */

> > > -	if (sig_pi == grub_cpu_to_le32_compile_time (0xea000006))

> > > +	if (lh.code0 == grub_cpu_to_le32_compile_time (0xea000006))

> >

> > Could you define 0xea000006 as a constant? If you add the comment

> > what does it mean that will be perfect.

>

> I'll be completely honest here and say "I don't have a clue".

>

> Well, that's not true, I'm broken enough to see that that is a short

> forward branch in the A32 ("ARM") instruction set. What I don't

> understand is what significance that has for Raspberry Pi.

> I just kept the logic for compatibility.

>

> Vladimir?


If Vladimir or anybody else is not able to say then I would not like to
hold on you any longer. I am happy with your guess in the comment if you
say that it is a guess.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Patch

diff --git a/grub-core/commands/file.c b/grub-core/commands/file.c
index 63c84499b..fad191202 100644
--- a/grub-core/commands/file.c
+++ b/grub-core/commands/file.c
@@ -27,6 +27,7 @@ 
 #include <grub/elf.h>
 #include <grub/xen_file.h>
 #include <grub/efi/pe32.h>
+#include <grub/arm/linux.h>
 #include <grub/i386/linux.h>
 #include <grub/xnu.h>
 #include <grub/machoload.h>
@@ -383,21 +384,18 @@  grub_cmd_file (grub_extcmd_context_t ctxt, int argc, char **args)
       }
     case IS_ARM_LINUX:
       {
-	grub_uint32_t sig, sig_pi;
-	if (grub_file_read (file, &sig_pi, 4) != 4)
+	struct linux_arm_kernel_header lh;
+	if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh))
 	  break;
 	/* Raspberry pi.  */
-	if (sig_pi == grub_cpu_to_le32_compile_time (0xea000006))
+	if (lh.code0 == grub_cpu_to_le32_compile_time (0xea000006))
 	  {
 	    ret = 1;
 	    break;
 	  }
 
-	if (grub_file_seek (file, 0x24) == (grub_size_t) -1)
-	  break;
-	if (grub_file_read (file, &sig, 4) != 4)
-	  break;
-	if (sig == grub_cpu_to_le32_compile_time (0x016f2818))
+	if (lh.magic ==
+	    grub_cpu_to_le32_compile_time (GRUB_LINUX_ARM_MAGIC_SIGNATURE))
 	  {
 	    ret = 1;
 	    break;