Re: [PATCH v3 3/3] m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED

Geert Uytterhoeven
2022-09-26T10:52:40Z

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