These seven patches build on the lazy-accept patch series

"Introduce Lazy-accept for Tdx guest"

by adding SEV-SNP support for the MemoryAccept protocol, and
importantly making eager memory acceptance the default behavior.

We implement a standardized event group from UEFI v2.9,
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides exactly
the right invocation point for eagerly accepting memory if eager
acceptance has not been disabled.

To make use of this event group, we add a new driver that is meant to
carry behavior that is needed for all confidential compute technologies,
not just specific platforms, CocoDxe. In CocoDxe we implement the
default safe behavior to accept all unaccepted memory and invalidate
the MemoryMap on ExitBootServices.

To allow the OS loader to prevent the eager acceptance, we add a new
protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
This protocol has one interface, Disable(). The OS loader can inform the
UEFI that it supports the unaccepted memory type and accepts the
responsibility to accept it.

All images that support unaccepted memory must now locate and call this
new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable
function.

Changes since v6:
 - Added implementation of EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
 - Changed callback protocol of v5 to instead use the standardized event
   group for before_exit_boot_services.

Changes since v5:
 - Generic callback protocol moved to MdeModulePkg
 - Removed use of EFI_WARN_STALE_DATA and added comment that the callback
   should only return EFI_SUCCESS or EFI_INVALID_PARAMETER.
 - Removed errant log statement and fixed formatting.

Changes since v4:
 - Commit message wording
 - Replaced direct change to DxeMain with a more generic callback
   protocol.
 - Implemented the direct change as an instance of the callback protocol
   from a new CocoDxe driver.
 - Replaced "enable" protocol with a "disable" protocol, since the name
   was confusing. The AcceptAllUnacceptedMemory protocol directly names
   the behavior that is disabling.

Changes since v3:
 - "DxeMain accepts all memory" patch split into 3 to make each patch
   affect only one package at a time.

Changes since v2:
 - Removed the redundant memory accept interface and added the accept
   behavior to the DXE implementation of
   MemEncryptSevSnpPreValidateSystemRam.
 - Fixed missing #include in >=4GB patch.

Changes since v1:
 - Added a patch to classify SEV-SNP memory above 4GB unaccepted.
 - Fixed style problems in EfiMemoryAcceptProtocol implementation.

Cc: Ard Biescheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Dionna Glaze (7):
  OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
  MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
  MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
  OvmfPkg: Introduce CocoDxe driver
  MdePkg: Introduce the AcceptAllUnacceptedMemory protocol
  OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
  OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

 MdeModulePkg/Core/Dxe/DxeMain.inf                                  |   1 +
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c                            |   6 +
 MdePkg/Include/Guid/EventGroup.h                                   |   5 +
 MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h          |  40 +++++
 MdePkg/MdePkg.dec                                                  |   8 +-
 OvmfPkg/AmdSev/AmdSevX64.dsc                                       |   1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf                                       |   1 +
 OvmfPkg/AmdSevDxe/AmdSevDxe.c                                      |  55 ++++++-
 OvmfPkg/AmdSevDxe/AmdSevDxe.inf                                    |   3 +
 OvmfPkg/CocoDxe/CocoDxe.c                                          | 165 ++++++++++++++++++++
 OvmfPkg/CocoDxe/CocoDxe.inf                                        |  46 ++++++
 OvmfPkg/IntelTdx/IntelTdxX64.dsc                                   |   1 +
 OvmfPkg/IntelTdx/IntelTdxX64.fdf                                   |   1 +
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c |  24 ++-
 OvmfPkg/OvmfPkgIa32X64.dsc                                         |   1 +
 OvmfPkg/OvmfPkgIa32X64.fdf                                         |   1 +
 OvmfPkg/OvmfPkgX64.dsc                                             |   1 +
 OvmfPkg/OvmfPkgX64.fdf                                             |   1 +
 OvmfPkg/PlatformPei/AmdSev.c                                       |   5 +
 19 files changed, 357 insertions(+), 9 deletions(-)
 create mode 100644 MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf

-- 
2.38.0.rc1.362.ged0d419d3c-goog



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94763): https://edk2.groups.io/g/devel/message/94763
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> 
> To allow the OS loader to prevent the eager acceptance, we add a new
> protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
> This protocol has one interface, Disable(). The OS loader can inform the
> UEFI that it supports the unaccepted memory type and accepts the
> responsibility to accept it.
> 
> All images that support unaccepted memory must now locate and call this
> new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable
> function.
> 

Can OsIndicationsSupported UEFI variable provide the similar functionality?



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95196): https://edk2.groups.io/g/devel/message/95196
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

>
> Can OsIndicationsSupported UEFI variable provide the similar functionality?
>

Similar, but not the same. If we use a UEFI variable, its value will
persist across boots. This can be fine if you only boot one OS, but if
you have two where one supports unaccepted memory and the other
doesn't then you have a problem.
The protocol here is specific to the code that will be calling
ExitBootServices, and thus won't mess up another OS once we reboot and
select it.

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95251): https://edk2.groups.io/g/devel/message/95251
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Fri, 14 Oct 2022 at 23:30, Dionna Glaze via groups.io
<dionnaglaze=google.com@groups.io> wrote:
>
> >
> > Can OsIndicationsSupported UEFI variable provide the similar functionality?
> >
>
> Similar, but not the same. If we use a UEFI variable, its value will
> persist across boots.

That assumes the variable is non-volatile, but I suppose we could use
a volatile variable [other than OsIndications] as well.

> This can be fine if you only boot one OS, but if
> you have two where one supports unaccepted memory and the other
> doesn't then you have a problem.
> The protocol here is specific to the code that will be calling
> ExitBootServices, and thus won't mess up another OS once we reboot and
> select it.
>
> --
> -Dionna Glaze, PhD (she/her)
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95366): https://edk2.groups.io/g/devel/message/95366
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> That assumes the variable is non-volatile, but I suppose we could use
> a volatile variable [other than OsIndications] as well.
>

I suppose that's true. Would you prefer volatile versions of
OsIndications/OsIndicationsSupported added to the spec, or this
proposed one-off toggle protocol? No specified global variables seem
overloadable with this duty.

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95452): https://edk2.groups.io/g/devel/message/95452
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Fri, 21 Oct 2022 at 00:37, Dionna Amalie Glaze
<dionnaglaze@google.com> wrote:
>
> > That assumes the variable is non-volatile, but I suppose we could use
> > a volatile variable [other than OsIndications] as well.
> >
>
> I suppose that's true. Would you prefer volatile versions of
> OsIndications/OsIndicationsSupported added to the spec, or this
> proposed one-off toggle protocol? No specified global variables seem
> overloadable with this duty.
>

No it would have to be a completely separate variable, I don't think
we can use OsIndications for this.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95471): https://edk2.groups.io/g/devel/message/95471
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

> > I suppose that's true. Would you prefer volatile versions of
> > OsIndications/OsIndicationsSupported added to the spec, or this
> > proposed one-off toggle protocol? No specified global variables seem
> > overloadable with this duty.
> >
>
> No it would have to be a completely separate variable, I don't think
> we can use OsIndications for this.

Certainly, I meant new variables that are volatile and also play a
similar role as OsIndications[Supported].

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95472): https://edk2.groups.io/g/devel/message/95472
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hey!

Sorry if this was asked. I was wondering if this patchset is in some git repo which I pull from as I struggle to get a buildable tree by applying this patchset to whatever I can find.

I tried https://github.com/deeglaze/edk2.git ( https://github.com/deeglaze/edk2.git ) enable_umv7 but it does not build (missing ExitBootServicesCallback.h and PrePiHob.h).
I tried rebasing it on top of https://github.com/mxu9/edk2.git lazyaccept.v1/2/3/4 but it seems only v1 kinda works but see the above and other versions have some reworks (EFI_RESOURCE_MEMORY_UNACCEPTED vs. BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED.

Thanks!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95536): https://edk2.groups.io/g/devel/message/95536
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

>
> Hey!
>
> Sorry if this was asked. I was wondering if this patchset is in some git repo which I pull from as I struggle to get a buildable tree by applying this patchset to whatever I can find.
>
> I tried https://github.com/deeglaze/edk2.git   enable_umv7 but it does not build (missing ExitBootServicesCallback.h and PrePiHob.h).
> I tried rebasing it on top of https://github.com/mxu9/edk2.git   lazyaccept.v1/2/3/4 but it seems only v1 kinda works but see the above and other versions have some reworks (EFI_RESOURCE_MEMORY_UNACCEPTED vs. BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED.
>
> Thanks!

Oh that would be because I forgot to git add those files (it built on
my machine TM). The callback.h file is from a previous version of this
series that can be removed. PrePiHob.h is from a lazy accept patch,
"MdePkg: Add UEFI Unaccepted memory definition". I can send out a v8
that removes the unnecessary #include and changes the names to those
suggested in the 3987 bug. I'll fix my github branch to include the
missing file.

Thanks for trying this patch series :)

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95505): https://edk2.groups.io/g/devel/message/95505
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Dionna,

Thanks for updating the tree, builds nicely now! However the VM's kernel 
does not boot - the guest kernel reports

EFI stub: ERROR: exit_boot() failed!

and hangs. I am not quite sure how it is supposed to work (still 
learning) but "Accepting all memory" happens twice (should it?) and the 
actual reason for the CoreExitBootService() failure is that MapKey != 
mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9 
or  0x7AE1 vs 0x7AE3 (the diff is always 2).

How do you test it exactly, is there any command line change needed in 
addition to enabling SNP?

My guest kernel uses 
https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/ 
with the TDX prerequisite. Thanks,


On 25/10/2022 02:24, Dionna Amalie Glaze wrote:
>>
>> Hey!
>>
>> Sorry if this was asked. I was wondering if this patchset is in some git repo which I pull from as I struggle to get a buildable tree by applying this patchset to whatever I can find.
>>
>> I tried https://github.com/deeglaze/edk2.git   enable_umv7 but it does not build (missing ExitBootServicesCallback.h and PrePiHob.h).
>> I tried rebasing it on top of https://github.com/mxu9/edk2.git   lazyaccept.v1/2/3/4 but it seems only v1 kinda works but see the above and other versions have some reworks (EFI_RESOURCE_MEMORY_UNACCEPTED vs. BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED.
>>
>> Thanks!
> 
> Oh that would be because I forgot to git add those files (it built on
> my machine TM). The callback.h file is from a previous version of this
> series that can be removed. PrePiHob.h is from a lazy accept patch,
> "MdePkg: Add UEFI Unaccepted memory definition". I can send out a v8
> that removes the unnecessary #include and changes the names to those
> suggested in the 3987 bug. I'll fix my github branch to include the
> missing file.
> 
> Thanks for trying this patch series :)
> 

-- 
Alexey


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95576): https://edk2.groups.io/g/devel/message/95576
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> Hi Dionna,
>
> Thanks for updating the tree, builds nicely now! However the VM's kernel
> does not boot - the guest kernel reports
>
> EFI stub: ERROR: exit_boot() failed!
>
> and hangs. I am not quite sure how it is supposed to work (still
> learning) but "Accepting all memory" happens twice (should it?) and the
> actual reason for the CoreExitBootService() failure is that MapKey !=
> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
> or  0x7AE1 vs 0x7AE3 (the diff is always 2).
>

"Accepting all memory" may happen twice, but it's idempotent. The
debug_info log happening twice might be confusing, so I can change
that if you'd like.

The first accept will remove all unaccepted memory regions from the
address space map.
CoreExitBootService should fail the first time since the first accept
will change the memory map.
That failure means that the caller should GetMemoryMap again and try
CoreExitBootService again.

> How do you test it exactly, is there any command line change needed in
> addition to enabling SNP?
>
> My guest kernel uses
> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
> with the TDX prerequisite. Thanks,
>

It's a few name changes behind, but this branch of Linux is what I've
been using:

https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum

Specific enablement patch here
https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61

I incorporate Kirill's patch set v7 for basic unaccepted memory
support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
memory, and I have a single patch that calls the protocol.
This branch doesn't have Kirill's TDX patches.
I've run it with a regular SEV-SNP enabled guest kernel too. At this
point all the tests have used kernel injection rather than having the
kernels all baked into the image.


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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95578): https://edk2.groups.io/g/devel/message/95578
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 26/10/2022 12:07, Dionna Amalie Glaze wrote:
> On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> Hi Dionna,
>>
>> Thanks for updating the tree, builds nicely now! However the VM's kernel
>> does not boot - the guest kernel reports
>>
>> EFI stub: ERROR: exit_boot() failed!
>>
>> and hangs. I am not quite sure how it is supposed to work (still
>> learning) but "Accepting all memory" happens twice (should it?) and the
>> actual reason for the CoreExitBootService() failure is that MapKey !=
>> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
>> or  0x7AE1 vs 0x7AE3 (the diff is always 2).
>>
> 
> "Accepting all memory" may happen twice, but it's idempotent. The
> debug_info log happening twice might be confusing, so I can change
> that if you'd like.

Nah, it is fine as long as the thing boots.


> The first accept will remove all unaccepted memory regions from the
> address space map.
> CoreExitBootService should fail the first time since the first accept
> will change the memory map.
> That failure means that the caller should GetMemoryMap again and try
> CoreExitBootService again.

Ah, ok.

>> How do you test it exactly, is there any command line change needed in
>> addition to enabling SNP?
>>
>> My guest kernel uses
>> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
>> with the TDX prerequisite. Thanks,
>>
> 
> It's a few name changes behind, but this branch of Linux is what I've
> been using:
> 
> https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum

This does not build though due to unresolved rebase conflict, the fix is 
kinda trivial but I do not get those "Accepting all memory" anymore so I 
wonder what else is missing.

> 
> Specific enablement patch here
> https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61
> 
> I incorporate Kirill's patch set v7 for basic unaccepted memory
> support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
> memory, and I have a single patch that calls the protocol.
> This branch doesn't have Kirill's TDX patches.
> I've run it with a regular SEV-SNP enabled guest kernel too. At this
> point all the tests have used kernel injection rather than having the
> kernels all baked into the image.

I am trying with "-m 2G" and "-m 8G" and see no difference - "Accepting 
all memory" is not appearing with this kernel. What do I miss? Thanks,


-- 
Alexey


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95587): https://edk2.groups.io/g/devel/message/95587
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 26/10/2022 12:35, Alexey Kardashevskiy wrote:
> 
> 
> On 26/10/2022 12:07, Dionna Amalie Glaze wrote:
>> On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> 
>> wrote:
>>>
>>> Hi Dionna,
>>>
>>> Thanks for updating the tree, builds nicely now! However the VM's kernel
>>> does not boot - the guest kernel reports
>>>
>>> EFI stub: ERROR: exit_boot() failed!
>>>
>>> and hangs. I am not quite sure how it is supposed to work (still
>>> learning) but "Accepting all memory" happens twice (should it?) and the
>>> actual reason for the CoreExitBootService() failure is that MapKey !=
>>> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
>>> or  0x7AE1 vs 0x7AE3 (the diff is always 2).
>>>
>>
>> "Accepting all memory" may happen twice, but it's idempotent. The
>> debug_info log happening twice might be confusing, so I can change
>> that if you'd like.
> 
> Nah, it is fine as long as the thing boots.
> 
> 
>> The first accept will remove all unaccepted memory regions from the
>> address space map.
>> CoreExitBootService should fail the first time since the first accept
>> will change the memory map.
>> That failure means that the caller should GetMemoryMap again and try
>> CoreExitBootService again.
> 
> Ah, ok.
> 
>>> How do you test it exactly, is there any command line change needed in
>>> addition to enabling SNP?
>>>
>>> My guest kernel uses
>>> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
>>> with the TDX prerequisite. Thanks,
>>>
>>
>> It's a few name changes behind, but this branch of Linux is what I've
>> been using:
>>
>> https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum
> 
> This does not build though due to unresolved rebase conflict, the fix is 
> kinda trivial but I do not get those "Accepting all memory" anymore so I 
> wonder what else is missing.

AH right, stupid me, getting rid of accepting all memory is the purpose 
of these patches. Never mind :) Thanks,

> 
>>
>> Specific enablement patch here
>> https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61
>>
>> I incorporate Kirill's patch set v7 for basic unaccepted memory
>> support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
>> memory, and I have a single patch that calls the protocol.
>> This branch doesn't have Kirill's TDX patches.
>> I've run it with a regular SEV-SNP enabled guest kernel too. At this
>> point all the tests have used kernel injection rather than having the
>> kernels all baked into the image.
> 
> I am trying with "-m 2G" and "-m 8G" and see no difference - "Accepting 
> all memory" is not appearing with this kernel. What do I miss? Thanks,
> 
> 

-- 
Alexey


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95590): https://edk2.groups.io/g/devel/message/95590
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 26/10/2022 13:49, Alexey Kardashevskiy wrote:
> 
> 
> On 26/10/2022 12:35, Alexey Kardashevskiy wrote:
>>
>>
>> On 26/10/2022 12:07, Dionna Amalie Glaze wrote:
>>> On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> 
>>> wrote:
>>>>
>>>> Hi Dionna,
>>>>
>>>> Thanks for updating the tree, builds nicely now! However the VM's 
>>>> kernel
>>>> does not boot - the guest kernel reports
>>>>
>>>> EFI stub: ERROR: exit_boot() failed!
>>>>
>>>> and hangs. I am not quite sure how it is supposed to work (still
>>>> learning) but "Accepting all memory" happens twice (should it?) and the
>>>> actual reason for the CoreExitBootService() failure is that MapKey !=
>>>> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
>>>> or  0x7AE1 vs 0x7AE3 (the diff is always 2).


btw it still fails in CoreTerminateMemoryMap() with the current upstream 
kernel which is not aware of the lazy memory acceptance, is this 
something known? Thanks,


>>>>
>>>
>>> "Accepting all memory" may happen twice, but it's idempotent. The
>>> debug_info log happening twice might be confusing, so I can change
>>> that if you'd like.
>>
>> Nah, it is fine as long as the thing boots.
>>
>>
>>> The first accept will remove all unaccepted memory regions from the
>>> address space map.
>>> CoreExitBootService should fail the first time since the first accept
>>> will change the memory map.
>>> That failure means that the caller should GetMemoryMap again and try
>>> CoreExitBootService again.
>>
>> Ah, ok.
>>
>>>> How do you test it exactly, is there any command line change needed in
>>>> addition to enabling SNP?
>>>>
>>>> My guest kernel uses
>>>> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
>>>> with the TDX prerequisite. Thanks,
>>>>
>>>
>>> It's a few name changes behind, but this branch of Linux is what I've
>>> been using:
>>>
>>> https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum
>>
>> This does not build though due to unresolved rebase conflict, the fix 
>> is kinda trivial but I do not get those "Accepting all memory" anymore 
>> so I wonder what else is missing.
> 
> AH right, stupid me, getting rid of accepting all memory is the purpose 
> of these patches. Never mind :) Thanks,
> 
>>
>>>
>>> Specific enablement patch here
>>> https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61
>>>
>>> I incorporate Kirill's patch set v7 for basic unaccepted memory
>>> support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
>>> memory, and I have a single patch that calls the protocol.
>>> This branch doesn't have Kirill's TDX patches.
>>> I've run it with a regular SEV-SNP enabled guest kernel too. At this
>>> point all the tests have used kernel injection rather than having the
>>> kernels all baked into the image.
>>
>> I am trying with "-m 2G" and "-m 8G" and see no difference - 
>> "Accepting all memory" is not appearing with this kernel. What do I 
>> miss? Thanks,
>>
>>
> 

-- 
Alexey


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95631): https://edk2.groups.io/g/devel/message/95631
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

>
> btw it still fails in CoreTerminateMemoryMap() with the current upstream
> kernel which is not aware of the lazy memory acceptance, is this
> something known? Thanks,
>

It wasn't known. I'll take a look. Thanks for the report.

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95648): https://edk2.groups.io/g/devel/message/95648
Mute This Topic: https://groups.io/mt/94144524/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-