Re: [PATCH 2/3] i386: factor out x86_firmware_configure()

[Xiaoyao Li]

On 4/1/2022 6:36 PM, Philippe Mathieu-Daudé wrote:
> On 1/4/22 07:28, Xiaoyao Li wrote:
> > On 4/1/2022 1:08 PM, Gerd Hoffmann wrote:
> > > > >                if (sev_enabled()) {
> > > >
> > > >                      ^^^
> > >
> > > > Can we remove the SEV check ...
> > >
> > > > > +    pc_system_parse_ovmf_flash(ptr, size);
> > > > > +
> > > > > +    if (sev_enabled()) {
> > > >
> > > > ... because we are still checking SEV here.
> > >
> > > Well, the two checks have slightly different purposes.  The first check
> > > will probably become "if (sev || tdx)" soon, 
> >
> > Not soon for TDX since the hacky pflash interface to load TDVF is 
> > rejected.
>
> You can still convince us you need a pflash for TDX, and particularly
> "a pflash that doesn't behave like pflash". 

I'm fine with "-bios" option to load TDVF. :)

> Also, see the comment in
> the next patch of this series:
>
> +         * [...] there is no need to register
> +         * the firmware as rom to properly re-initialize on reset.
> +         * Just go for a straight file load instead.
> +         */

Yes, Gerd's this series make it easier for TDX to load TDVF via -bios.

> > > whereas the second will
> > > become "if (sev) { ... } if (tdx) { ... }".
> > >
> > > We could remove the first.  pc_system_parse_ovmf_flash() would run
> > > unconditionally then.  Not needed, but should not have any bad side
> > > effects.
>
> OK, then:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>