Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

["Dionna Glaze" <dionnaglaze@google.com>]

On Fri, Feb 10, 2023 at 5:56 AM Gupta, Pankaj <pankaj.gupta@amd.com> wrote:
>
> On 2/9/2023 10:27 PM, Dionna Amalie Glaze wrote:
> > On Thu, Feb 9, 2023 at 8:52 AM Dionna Amalie Glaze
> > <dionnaglaze@google.com> wrote:
> >>
> >>> With this patch I observe an issue where my Linux (6.2.0-rc7) guest
> >>> recur to Bootloader menu again. I am testing this with SEV SNP (w/o
> >>> UPM). Also, guest don't have lazy memory acceptance support.
> >>>
> >>
> >> Thanks for the report. I'll try to reproduce it on our UEFI and if I'm
> >> unable, then we'll discuss next steps.
> >>
> >
> > I don't see this in our test Ubuntu 22.04 image from Canonical. Do you
>
> Ubuntu 22.04 guest by default run 5.15 kernel? But SEV SNP got
> merged in 5.19. I don't know currently how we are handling accepting
> the memory on "ExitBootServices" with or w/o guest supporting SNP.
>

It does, but I used the Qemu kernel injection pathway in Ovmf to run a
build of 6.2.0-rc7. Our testing setup doesn't give the user a boot
menu to select a kernel, so I wasn't aware that this return to
bootmenu could happen.


>This looks to me like it is entering the 'accept' path twice, and so
> ExitBootServices() is failing twice, resulting in a failed boot.

The double log is expected behavior because I didn't add a check for
whether accepting all memory would be a no-op.

The "Accepting all memory" message occurs twice if the guest does not
have support for unaccepted memory following this control flow:

1. EBS
2. [...] Log "Accepting all memory"
3. Loop through all memory spaces
3a. If the memory space is unaccepted, accept it.
3b. Remove the unaccepted memory space.
3c. Add a conventional memory space back with the same range and capabilities.
4. EBS returns an error since the map key is different.
5. OS calls GetMemoryMap to get the updated key.
6. OS calls EBS with the updated key.
7. [...] Log "Accepting all memory"
8. Loop through all memory spaces
8a. There are no unaccepted memory spaces left, so nothing happens.
9. Return successfully (one would hope)

>Accepting all memory^M
>Accepting all memory^M
>EFI stub: ERROR: exit_boot() failed!^M
>EFI stub: ERROR: efi_main() failed!^M

This now does suggest that EBS is failing twice, since after the
supposed no-op of the second log, the EFI stub's exit_boot claims
failure. I can't reproduce this part. Would you try adding a log
within the acceptance loop inside the if that checks for unaccepted
memory? I'd be curious if the loop is indeed changing the map again,
despite my claims at idempotency.

-- 
-Dionna Glaze, PhD (she/her)