[PATCH] OvmfPkg: Remove gbs FreePool in AcceptAllMemory()

[PATCH] OvmfPkg: Remove gbs FreePool in AcceptAllMemory()

[Gupta, Pankaj]

System Memory map is changed when a memory range is Accepted.
While returning from AcceptAllMemory(), "gBS->FreePool" is wrongly
used which results in changing memory map and hence return an error.
Fix this by removing the "gBs->FreePool" call altogether.

Before this patch, KVM guest throws an error and control goes to the
boat loader menu every time we select an OS:

EFI stub: ERROR: exit_boot() failed!
EFI stub: ERROR: efi_main() failed!
StartImage failed: Invalid Parameter

Fixes: a00e2e5513 ("OvmfPkg: Add memory acceptance event in AmdSevDxe")
Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>
---
 OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..f52dbfe597 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -123,7 +123,6 @@ AcceptAllMemory (
     }
   }
 
-  gBS->FreePool (AllDescMap);
   return Status;
 }
 
-- 
2.34.1

Re: [PATCH] OvmfPkg: Remove gbs FreePool in AcceptAllMemory()

[Ard Biesheuvel]

On Tue, 14 Feb 2023 at 23:15, Pankaj Gupta <pankaj.gupta@amd.com> wrote:
>
> System Memory map is changed when a memory range is Accepted.
> While returning from AcceptAllMemory(), "gBS->FreePool" is wrongly
> used which results in changing memory map and hence return an error.
> Fix this by removing the "gBs->FreePool" call altogether.
>
> Before this patch, KVM guest throws an error and control goes to the
> boat loader menu every time we select an OS:
>
> EFI stub: ERROR: exit_boot() failed!
> EFI stub: ERROR: efi_main() failed!
> StartImage failed: Invalid Parameter
>
> Fixes: a00e2e5513 ("OvmfPkg: Add memory acceptance event in AmdSevDxe")
> Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>

Queued as #4040 - thanks!

> ---
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 6391d1f775..f52dbfe597 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -123,7 +123,6 @@ AcceptAllMemory (
>      }
>    }
>
> -  gBS->FreePool (AllDescMap);
>    return Status;
>  }
>
> --
> 2.34.1
>

Re: [PATCH] OvmfPkg: Remove gbs FreePool in AcceptAllMemory()

[Ard Biesheuvel]

On Tue, 14 Feb 2023 at 23:35, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 14 Feb 2023 at 23:15, Pankaj Gupta <pankaj.gupta@amd.com> wrote:
> >
> > System Memory map is changed when a memory range is Accepted.
> > While returning from AcceptAllMemory(), "gBS->FreePool" is wrongly
> > used which results in changing memory map and hence return an error.
> > Fix this by removing the "gBs->FreePool" call altogether.
> >
> > Before this patch, KVM guest throws an error and control goes to the
> > boat loader menu every time we select an OS:
> >
> > EFI stub: ERROR: exit_boot() failed!
> > EFI stub: ERROR: efi_main() failed!
> > StartImage failed: Invalid Parameter
> >
> > Fixes: a00e2e5513 ("OvmfPkg: Add memory acceptance event in AmdSevDxe")
> > Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>
>
> Queued as #4040 - thanks!
>

I've dropped it again - it seems AcceptAllMemory() should never be
called a second time (and it is debatable whether the
gEfiEventBeforeExitBootServicesGuid event should be signaled the
second time), so it would be better to address that instead.


> > ---
> >  OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > index 6391d1f775..f52dbfe597 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -123,7 +123,6 @@ AcceptAllMemory (
> >      }
> >    }
> >
> > -  gBS->FreePool (AllDescMap);
> >    return Status;
> >  }
> >
> > --
> > 2.34.1
> >