Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220625153841.143928-1-Jason@zx2c4.com> <512bdf97-5468-e2d2-75bd-24107aaf8a34@vivier.eu>
In-Reply-To: <512bdf97-5468-e2d2-75bd-24107aaf8a34@vivier.eu>
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
Date:   Sat, 25 Jun 2022 18:19:56 +0200
Message-ID: <CAHmME9qKkL8r7QSwq+2DA54mpMHzqpnq=pi5f0DhKOcB-qezoQ@mail.gmail.com>
Subject: Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block
To:     Laurent Vivier <laurent@vivier.eu>
Cc:     Geert Uytterhoeven <geert@linux-m68k.org>,
        linux-m68k <linux-m68k@lists.linux-m68k.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sat, Jun 25, 2022 at 6:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 25/06/2022 =C3=A0 17:38, Jason A. Donenfeld a =C3=A9crit :
> > Other virt VMs can pass RNG seeds via the "rng-seed" device tree
> > property or via UEFI, but m68k doesn't have either. Instead it has its
> > own bootinfo protocol. So this commit adds support for receiving a RNG
> > seed from it, which will be used at the earliest possible time in boot,
> > just like device tree.
> >
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> >   arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
> >   arch/m68k/virt/config.c                    | 4 ++++
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/inc=
lude/uapi/asm/bootinfo-virt.h
> > index e4db7e2213ab..7c3044acdf4a 100644
> > --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > @@ -12,6 +12,7 @@
> >   #define BI_VIRT_GF_TTY_BASE 0x8003
> >   #define BI_VIRT_VIRTIO_BASE 0x8004
> >   #define BI_VIRT_CTRL_BASE   0x8005
> > +#define BI_VIRT_RNG_SEED     0x8006
> >
> >   #define VIRT_BOOTI_VERSION  MK_BI_VERSION(2, 0)
> >
> > diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
> > index 632ba200ad42..ad71af8273ec 100644
> > --- a/arch/m68k/virt/config.c
> > +++ b/arch/m68k/virt/config.c
> > @@ -2,6 +2,7 @@
> >
> >   #include <linux/reboot.h>
> >   #include <linux/serial_core.h>
> > +#include <linux/random.h>
> >   #include <clocksource/timer-goldfish.h>
> >
> >   #include <asm/bootinfo.h>
> > @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record=
 *record)
> >               data +=3D 4;
> >               virt_bi_data.virtio.irq =3D be32_to_cpup(data);
> >               break;
> > +     case BI_VIRT_RNG_SEED:
> > +             add_bootloader_randomness(data + 4, be32_to_cpup(data));
>
> In fact, why don't you use the record->size to get the size of the buffer=
?
>
> It seems useless to encode twice the length of the buffer, the second tim=
e on a 32bit while the
> length cannot exceed a 16bit value.

Doesn't that make the length ambiguous because of required alignment?
Would rather keep this general. As is, it's also much more like the
others and more uniform to keep it that way. You were able to review
it and see that it was right after glancing for a second. That seems
superior to any imaginary gains we'd get by overloading the record
size.

Jason