qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 0/4] Refactor x86_load_linux and pass RNG seed via setup_data


From: Michael S. Tsirkin
Subject: Re: [PATCH 0/4] Refactor x86_load_linux and pass RNG seed via setup_data entry
Date: Thu, 21 Jul 2022 10:52:32 -0400

On Thu, Jul 21, 2022 at 02:29:33PM +0200, Paolo Bonzini wrote:
> As mentioned in the reviews of Jason's patches, the fw_cfg data, or at
> least its structure including the size, is part of the guest ABI and
> must match across two sides of migration.
> 
> It would be possible to handle this with some duplicated code between
> the rng seed and DTB handling, but the conditionals to handle the linked
> list would be ugly.  Unfortunately the code of x86_load_linux has no
> data structures available, it's all of a jumble of local variables.
> Hence the first two and largest patches in this series, which remove all
> non-Linux code from the function and move the local variables to a struct
> as necessary.  The function was long overdue for some cleanup anyway.
> 
> With this in place, adding the seed setup_data entry is just a
> couple lines of code, plus the scaffolding for a new machine property
> "linuxboot-seed".  The property supports on/off/auto values, where "auto"
> disables/enables depending on the kernel support for setup data (which was
> added in 2.6.26); "on" currently fails when starting with an old kernel,
> and probably it should also fail when starting a PVH or multiboot kernel.
> 
> Paolo

I like the refactoring

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

To avoid creating extra work for Jason and confusing
attribution, maybe apply Jason's patch then your refactoring
on top?

> Jason A. Donenfeld (1):
>   hw/i386: pass RNG seed via setup_data entry
> 
> Paolo Bonzini (3):
>   hw/i386: extract PVH load to a separate function
>   hw/i386: define a struct for Linux boot protocol data
>   hw/i386: extract handling of setup data linked list
> 
>  hw/i386/pc.c                                 |   1 +
>  hw/i386/x86.c                                | 303 +++++++++++--------
>  include/hw/i386/x86.h                        |   2 +
>  include/standard-headers/asm-x86/bootparam.h |   1 +
>  4 files changed, 185 insertions(+), 122 deletions(-)
> 
> -- 
> 2.36.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]