diff mbox series

[qemu,v2,2/2] m68k: align bootinfo strings and data to 4 bytes

Message ID 20220926113900.1256630-2-Jason@zx2c4.com
State New
Headers show
Series [qemu,v2,1/2] m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED | expand

Commit Message

Jason A. Donenfeld Sept. 26, 2022, 11:39 a.m. UTC
Various tools, such as kexec-tools and m68k-bootinfo, expect each
bootinfo entry to be aligned to 4 bytes, not 2 bytes. So adjust the
padding to fill this out as such.

Also, break apart the padding additions from the other field length
additions, so that it's more clear why these magic numbers are being
added, and comment them too.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/m68k/bootinfo.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Laurent Vivier Sept. 26, 2022, 1 p.m. UTC | #1
Le 26/09/2022 à 13:39, Jason A. Donenfeld a écrit :
> Various tools, such as kexec-tools and m68k-bootinfo, expect each
> bootinfo entry to be aligned to 4 bytes, not 2 bytes. So adjust the
> padding to fill this out as such.

Agree, I found the same problem using petitboot as a ROM for the virt machine [1].
(I didn't update BOOTINFOSTR() but I think I should).

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[1] https://github.com/vivier/qemu-m68k/commits/m68k-virt
> 
> Also, break apart the padding additions from the other field length
> additions, so that it's more clear why these magic numbers are being
> added, and comment them too.
> 
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   hw/m68k/bootinfo.h | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
> index bd8b212fd3..897162b818 100644
> --- a/hw/m68k/bootinfo.h
> +++ b/hw/m68k/bootinfo.h
> @@ -48,13 +48,14 @@
>           stw_phys(as, base, id); \
>           base += 2; \
>           stw_phys(as, base, \
> -                 (sizeof(struct bi_record) + strlen(string) + 2) & ~1); \
> +                 (sizeof(struct bi_record) + strlen(string) + \
> +                  1 /* null termination */ + 3 /* padding */) & ~3); \
>           base += 2; \
>           for (i = 0; string[i]; i++) { \
>               stb_phys(as, base++, string[i]); \
>           } \
>           stb_phys(as, base++, 0); \
> -        base = (base + 1) & ~1; \
> +        base = (base + 3) & ~3; \
>       } while (0)
>   
>   #define BOOTINFODATA(as, base, id, data, len) \
> @@ -63,13 +64,14 @@
>           stw_phys(as, base, id); \
>           base += 2; \
>           stw_phys(as, base, \
> -                 (sizeof(struct bi_record) + len + 3) & ~1); \
> +                 (sizeof(struct bi_record) + len + \
> +                  2 /* length field */ + 3 /* padding */) & ~3); \
>           base += 2; \
>           stw_phys(as, base, len); \
>           base += 2; \
>           for (i = 0; i < len; ++i) { \
>               stb_phys(as, base++, data[i]); \
>           } \
> -        base = (base + 1) & ~1; \
> +        base = (base + 3) & ~3; \
>       } while (0)
>   #endif
Laurent Vivier Sept. 26, 2022, 9:37 p.m. UTC | #2
Le 26/09/2022 à 13:39, Jason A. Donenfeld a écrit :
> Various tools, such as kexec-tools and m68k-bootinfo, expect each
> bootinfo entry to be aligned to 4 bytes, not 2 bytes. So adjust the
> padding to fill this out as such.
> 
> Also, break apart the padding additions from the other field length
> additions, so that it's more clear why these magic numbers are being
> added, and comment them too.
> 
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   hw/m68k/bootinfo.h | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 

Applied to my m68k-for-7.2 branch

Thanks,
Laurent
Jason A. Donenfeld Sept. 26, 2022, 9:40 p.m. UTC | #3
On Mon, Sep 26, 2022 at 11:37 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 26/09/2022 à 13:39, Jason A. Donenfeld a écrit :
> > Various tools, such as kexec-tools and m68k-bootinfo, expect each
> > bootinfo entry to be aligned to 4 bytes, not 2 bytes. So adjust the
> > padding to fill this out as such.
> >
> > Also, break apart the padding additions from the other field length
> > additions, so that it's more clear why these magic numbers are being
> > added, and comment them too.
> >
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Cc: Laurent Vivier <laurent@vivier.eu>
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> >   hw/m68k/bootinfo.h | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
>
> Applied to my m68k-for-7.2 branch

What about 1/2?

Jason
Laurent Vivier Sept. 26, 2022, 9:42 p.m. UTC | #4
Le 26/09/2022 à 23:40, Jason A. Donenfeld a écrit :
> On Mon, Sep 26, 2022 at 11:37 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 26/09/2022 à 13:39, Jason A. Donenfeld a écrit :
>>> Various tools, such as kexec-tools and m68k-bootinfo, expect each
>>> bootinfo entry to be aligned to 4 bytes, not 2 bytes. So adjust the
>>> padding to fill this out as such.
>>>
>>> Also, break apart the padding additions from the other field length
>>> additions, so that it's more clear why these magic numbers are being
>>> added, and comment them too.
>>>
>>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> Cc: Laurent Vivier <laurent@vivier.eu>
>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>>> ---
>>>    hw/m68k/bootinfo.h | 10 ++++++----
>>>    1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>
>> Applied to my m68k-for-7.2 branch
> 
> What about 1/2?
> 

I'd like to wait a little to see what happens on the linux side.

Thanks,
Laurent
Jason A. Donenfeld Sept. 26, 2022, 9:42 p.m. UTC | #5
On Mon, Sep 26, 2022 at 11:42 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 26/09/2022 à 23:40, Jason A. Donenfeld a écrit :
> > On Mon, Sep 26, 2022 at 11:37 PM Laurent Vivier <laurent@vivier.eu> wrote:
> >>
> >> Le 26/09/2022 à 13:39, Jason A. Donenfeld a écrit :
> >>> Various tools, such as kexec-tools and m68k-bootinfo, expect each
> >>> bootinfo entry to be aligned to 4 bytes, not 2 bytes. So adjust the
> >>> padding to fill this out as such.
> >>>
> >>> Also, break apart the padding additions from the other field length
> >>> additions, so that it's more clear why these magic numbers are being
> >>> added, and comment them too.
> >>>
> >>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >>> Cc: Laurent Vivier <laurent@vivier.eu>
> >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> >>> ---
> >>>    hw/m68k/bootinfo.h | 10 ++++++----
> >>>    1 file changed, 6 insertions(+), 4 deletions(-)
> >>>
> >>
> >> Applied to my m68k-for-7.2 branch
> >
> > What about 1/2?
> >
>
> I'd like to wait a little to see what happens on the linux side.

Alright, makes sense. Just please don't forget about it.

Jason
Jason A. Donenfeld Sept. 28, 2022, 11:13 p.m. UTC | #6
On Mon, Sep 26, 2022 at 11:42:37PM +0200, Jason A. Donenfeld wrote:
> On Mon, Sep 26, 2022 at 11:42 PM Laurent Vivier <laurent@vivier.eu> wrote:
> >
> > Le 26/09/2022 à 23:40, Jason A. Donenfeld a écrit :
> > > On Mon, Sep 26, 2022 at 11:37 PM Laurent Vivier <laurent@vivier.eu> wrote:
> > >>
> > >> Le 26/09/2022 à 13:39, Jason A. Donenfeld a écrit :
> > >>> Various tools, such as kexec-tools and m68k-bootinfo, expect each
> > >>> bootinfo entry to be aligned to 4 bytes, not 2 bytes. So adjust the
> > >>> padding to fill this out as such.
> > >>>
> > >>> Also, break apart the padding additions from the other field length
> > >>> additions, so that it's more clear why these magic numbers are being
> > >>> added, and comment them too.
> > >>>
> > >>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > >>> Cc: Laurent Vivier <laurent@vivier.eu>
> > >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > >>> ---
> > >>>    hw/m68k/bootinfo.h | 10 ++++++----
> > >>>    1 file changed, 6 insertions(+), 4 deletions(-)
> > >>>
> > >>
> > >> Applied to my m68k-for-7.2 branch
> > >
> > > What about 1/2?
> > >
> >
> > I'd like to wait a little to see what happens on the linux side.
> 
> Alright, makes sense. Just please don't forget about it.

Okay, all set. Uneventful.
https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git/commit/?id=f1bb20c8be1929743fdb313b4770601afc39c1b7

Jason
diff mbox series

Patch

diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
index bd8b212fd3..897162b818 100644
--- a/hw/m68k/bootinfo.h
+++ b/hw/m68k/bootinfo.h
@@ -48,13 +48,14 @@ 
         stw_phys(as, base, id); \
         base += 2; \
         stw_phys(as, base, \
-                 (sizeof(struct bi_record) + strlen(string) + 2) & ~1); \
+                 (sizeof(struct bi_record) + strlen(string) + \
+                  1 /* null termination */ + 3 /* padding */) & ~3); \
         base += 2; \
         for (i = 0; string[i]; i++) { \
             stb_phys(as, base++, string[i]); \
         } \
         stb_phys(as, base++, 0); \
-        base = (base + 1) & ~1; \
+        base = (base + 3) & ~3; \
     } while (0)
 
 #define BOOTINFODATA(as, base, id, data, len) \
@@ -63,13 +64,14 @@ 
         stw_phys(as, base, id); \
         base += 2; \
         stw_phys(as, base, \
-                 (sizeof(struct bi_record) + len + 3) & ~1); \
+                 (sizeof(struct bi_record) + len + \
+                  2 /* length field */ + 3 /* padding */) & ~3); \
         base += 2; \
         stw_phys(as, base, len); \
         base += 2; \
         for (i = 0; i < len; ++i) { \
             stb_phys(as, base++, data[i]); \
         } \
-        base = (base + 1) & ~1; \
+        base = (base + 3) & ~3; \
     } while (0)
 #endif