qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v4 1/2] x86: return modified setup_data only if read as memor


From: Jason A. Donenfeld
Subject: Re: [PATCH v4 1/2] x86: return modified setup_data only if read as memory, not as file
Date: Wed, 21 Sep 2022 11:15:54 +0200

On Wed, Sep 21, 2022 at 11:15 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Sep 14, 2022 at 12:41:34AM +0100, Jason A. Donenfeld wrote:
> > If setup_data is being read into a specific memory location, then
> > generally the setup_data address parameter is read first, so that the
> > caller knows where to read it into. In that case, we should return
> > setup_data containing the absolute addresses that are hard coded and
> > determined a priori. This is the case when kernels are loaded by BIOS,
> > for example. In contrast, when setup_data is read as a file, then we
> > shouldn't modify setup_data, since the absolute address will be wrong by
> > definition. This is the case when OVMF loads the image.
> >
> > This allows setup_data to be used like normal, without crashing when EFI
> > tries to use it.
> >
> > (As a small development note, strangely, fw_cfg_add_file_callback() was
> > exported but fw_cfg_add_bytes_callback() wasn't, so this makes that
> > consistent.)
> >
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Laurent Vivier <laurent@vivier.eu>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Cc: Richard Henderson <richard.henderson@linaro.org>
> > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> >  hw/i386/x86.c             | 37 +++++++++++++++++++++++++++----------
> >  hw/nvram/fw_cfg.c         | 12 ++++++------
> >  include/hw/nvram/fw_cfg.h | 22 ++++++++++++++++++++++
> >  3 files changed, 55 insertions(+), 16 deletions(-)
> >
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index 050eedc0c8..933bbdd836 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -764,6 +764,18 @@ static bool load_elfboot(const char *kernel_filename,
> >      return true;
> >  }
> >
> > +struct setup_data_fixup {
> > +    void *pos;
> > +    hwaddr val;
> > +    uint32_t addr;
> > +};
> > +
>
> btw
>
>         typedef struct SetupDataFixup {
>             void *pos;
>             hwaddr val;
>             uint32_t addr;
>         } SetupDataFixup;
>
>
> and use typedef everywhere.

Okay no problem. Will do for v5.

Jason



reply via email to

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