Re: [PATCH v4 00/31] Add AMD Secure Nested Paging (SEV-SNP) support

[Paolo Bonzini]

On Mon, Jun 3, 2024 at 4:16 PM Michael Roth <michael.roth@amd.com> wrote:
> Paolo mentioned he dropped the this hunk from:
>
>   hw/i386: Add support for loading BIOS using guest_memfd
>
>   diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>   index de606369b0..d076b30ccb 100644
>   --- a/hw/i386/x86.c
>   +++ b/hw/i386/x86.c
>   @@ -1147,10 +1147,18 @@ void x86_bios_rom_init(MachineState *ms, const char 
> *default_firmware,
>        }
>        if (bios_size <= 0 ||
>            (bios_size % 65536) != 0) {
>   -        goto bios_error;
>   +        if (!machine_require_guest_memfd(ms)) {
>   +            g_warning("%s: Unaligned BIOS size %d", __func__, bios_size);
>   +            goto bios_error;
>   +        }
>
> without that, OVMF with split CODE/VARS won't work because the CODE
> portion is not 64KB aligned.
>
> If I add that back the split builds work for qemu-coco-queue as well.
>
> We need to understand why the 64KB alignment exists in the first place, why
> it's not necessary for SNP, and then resubmit the above change with proper
> explanation.

I think it was only needed to make sure that people weren't using
"unpadded" BIOS (not OVMF) binaries. I think we can delete it
altogether, and it can be submitted separately from this series.

> However, if based on Daniel's comments we decide not to support split
> CODE/VARS for SNP, then the above change won't be needed. But if we do,
> then it goes make sense that the above change is grouped with (or
> submitted as a fix-up for):

Yes, I think we want to go for a variable store that is not "right
below BIOS". The advantage of something that isn't pflash-based is
that it can be used by any code-only firmware binary.

Paolo