Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Mon, 26 Sep 2022 12:27:13 +0200 | Subject | Re: [PATCH v3 3/3] m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED |
| |
Hi Jason,
On Fri, Sep 23, 2022 at 7:03 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > This is useful on !virt platforms for kexec, so change things from > BI_VIRT_RNG_SEED to be BI_RNG_SEED, and simply remove BI_VIRT_RNG_SEED > because it only ever lasted one release, and nothing is broken by not > having it. At the same time, keep a comment noting that it's been > removed, so that ID isn't reused. > > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: Laurent Vivier <laurent@vivier.eu> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
The approach LGTM, but given my doubts on [PATCH v3 2/3], I think this patch should be moved up, to not depend on [2/3].
> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h > +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h > @@ -13,13 +13,8 @@ > #define BI_VIRT_VIRTIO_BASE 0x8004 > #define BI_VIRT_CTRL_BASE 0x8005 > > -/* > - * A random seed used to initialize the RNG. Record format: > - * > - * - length [ 2 bytes, 16-bit big endian ] > - * - seed data [ `length` bytes, padded to preserve 2-byte alignment ] > - */ > -#define BI_VIRT_RNG_SEED 0x8006 > +/* No longer used -- replaced with BI_RNG_SEED -- but don't reuse this index: > + * #define BI_VIRT_RNG_SEED 0x8006 */
Please drop the space before TAB.
> --- a/arch/m68k/include/uapi/asm/bootinfo.h > +++ b/arch/m68k/include/uapi/asm/bootinfo.h > @@ -64,7 +64,13 @@ struct mem_info { > /* (struct mem_info) */ > #define BI_COMMAND_LINE 0x0007 /* kernel command line parameters */ > /* (string) */ > - > +/* > + * A random seed used to initialize the RNG. Record format: > + * > + * - length [ 2 bytes, 16-bit big endian ] > + * - seed data [ `length` bytes, padded to preserve 2-byte alignment ]
While experimenting, I noticed this must actually be a 4-byte alignment, as data[] is an integral multiple of __be32 words:
struct bi_record { __be16 tag; /* tag ID */ __be16 size; /* size of record (in bytes) */ __be32 data[]; /* data */ };
m68kboot[1] and kexec-boot[2] do implement the correct rounding, but qemu's BOOTINFOSTR()[3] and BOOTINFODATA()[4] do not. Sorry for missing that before, I guess I was misled by the existing bug in BOOTINFOSTR().
Consequently, both kexec-tools[5] and the m68k-bootinfo tool[6] cannot parse /proc/bootinfo if the size is not a multiple of 4, which can easily be triggered by changing the command line of the m68k virt machine.
> + */ > +#define BI_RNG_SEED 0x0008 > > /* > * Linux/m68k Architectures (BI_MACHTYPE)
[1] https://github.com/geertu/m68kboot/blob/master/common/bootinf.c#L171 [2] https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/m68k/bootinfo.c#n60 [3] https://git.qemu.org/?p=qemu.git;a=blob;f=hw/m68k/bootinfo.h;h=bd8b212fd35ce216917e07eb67783b5b69f1e11b;hb=HEAD#l57 [4] https://git.qemu.org/?p=qemu.git;a=blob;f=hw/m68k/bootinfo.h;h=bd8b212fd35ce216917e07eb67783b5b69f1e11b;hb=HEAD#l73 [5] https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/m68k/bootinfo.c#n106 [6] https://github.com/geertu/m68k-bootinfo/blob/master/m68k-bootinfo.c#L466
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |