| Message ID | 20230113212926.2904735-1-dionnaglaze@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] x86/efi: Safely enable unaccepted memory in UEFI | expand |
On Fri, Jan 13, 2023 at 09:29:26PM +0000, Dionna Glaze wrote: > This patch depends on Kirill A. Shutemov's series > > [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory > > The UEFI v2.9 specification includes a new memory type to be used in > environments where the OS must accept memory that is provided from its > host. Before the introduction of this memory type, all memory was > accepted eagerly in the firmware. In order for the firmware to safely > stop accepting memory on the OS's behalf, the OS must affirmatively > indicate support to the firmware. I think it is a bad idea. This approach breaks use case with a bootloader between BIOS and OS. As the bootloader does ExitBootServices() it has to make the call on behalf of OS when it has no idea if the OS supports unaccepted. Note that kexec is such use-case: original kernel has to make a decision on whether it is okay to leave some memory unaccepted for the new kernel. And we add this protocol to address very temporary problem: once unaccepted memory support get upstream it is just a dead weight. Let's not do this.
On Sat, Jan 14, 2023 at 01:20:24AM +0300, Kirill A. Shutemov wrote: > On Fri, Jan 13, 2023 at 09:29:26PM +0000, Dionna Glaze wrote: > > This patch depends on Kirill A. Shutemov's series > > > > [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory > > > > The UEFI v2.9 specification includes a new memory type to be used in > > environments where the OS must accept memory that is provided from its > > host. Before the introduction of this memory type, all memory was > > accepted eagerly in the firmware. In order for the firmware to safely > > stop accepting memory on the OS's behalf, the OS must affirmatively > > indicate support to the firmware. > > I think it is a bad idea. > > This approach breaks use case with a bootloader between BIOS and OS. > As the bootloader does ExitBootServices() it has to make the call on > behalf of OS when it has no idea if the OS supports unaccepted. Nothing breaks, it'll error on the safe side. If the protocol callback is not called the firmware will simply accept all memory. The guest OS will only see unaccepted memory if it explicitly asked for it (assuming the firmware wants know to support both cases, of course the firmware could also enforce the one or the other and just not offer the protocol). > Note that kexec is such use-case: original kernel has to make a > decision on whether it is okay to leave some memory unaccepted for the > new kernel. Not sure what you are trying to tell. The kexec case doesn't go through the efi stub anyway. > And we add this protocol to address very temporary problem: once > unaccepted memory support get upstream it is just a dead weight. Maybe, maybe not. unaccepted memory support has a Kconfig switch after all. If we figure in 3-5 years that all distros have enabled it anyway we can drop it again. For the transition period it will surely be useful. take care, Gerd
On Mon, Jan 16, 2023 at 11:56:48AM +0100, Gerd Hoffmann wrote: > On Sat, Jan 14, 2023 at 01:20:24AM +0300, Kirill A. Shutemov wrote: > > On Fri, Jan 13, 2023 at 09:29:26PM +0000, Dionna Glaze wrote: > > > This patch depends on Kirill A. Shutemov's series > > > > > > [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory > > > > > > The UEFI v2.9 specification includes a new memory type to be used in > > > environments where the OS must accept memory that is provided from its > > > host. Before the introduction of this memory type, all memory was > > > accepted eagerly in the firmware. In order for the firmware to safely > > > stop accepting memory on the OS's behalf, the OS must affirmatively > > > indicate support to the firmware. > > > > I think it is a bad idea. > > > > This approach breaks use case with a bootloader between BIOS and OS. > > As the bootloader does ExitBootServices() it has to make the call on > > behalf of OS when it has no idea if the OS supports unaccepted. > > Nothing breaks, it'll error on the safe side. If the protocol callback > is not called the firmware will simply accept all memory. The guest OS > will only see unaccepted memory if it explicitly asked for it (assuming > the firmware wants know to support both cases, of course the firmware > could also enforce the one or the other and just not offer the > protocol). How bootloader suppose to know if OS will ask for unaccepted memory? It can't. It means the use-case with bootloader cannot ever use unaccepted memory. That's broken design.
On Mon, 16 Jan 2023 at 13:31, Kirill A. Shutemov <kirill@shutemov.name> wrote: > > On Mon, Jan 16, 2023 at 11:56:48AM +0100, Gerd Hoffmann wrote: > > On Sat, Jan 14, 2023 at 01:20:24AM +0300, Kirill A. Shutemov wrote: > > > On Fri, Jan 13, 2023 at 09:29:26PM +0000, Dionna Glaze wrote: > > > > This patch depends on Kirill A. Shutemov's series > > > > > > > > [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory > > > > > > > > The UEFI v2.9 specification includes a new memory type to be used in > > > > environments where the OS must accept memory that is provided from its > > > > host. Before the introduction of this memory type, all memory was > > > > accepted eagerly in the firmware. In order for the firmware to safely > > > > stop accepting memory on the OS's behalf, the OS must affirmatively > > > > indicate support to the firmware. > > > > > > I think it is a bad idea. > > > > > > This approach breaks use case with a bootloader between BIOS and OS. > > > As the bootloader does ExitBootServices() it has to make the call on > > > behalf of OS when it has no idea if the OS supports unaccepted. > > > > Nothing breaks, it'll error on the safe side. If the protocol callback > > is not called the firmware will simply accept all memory. The guest OS > > will only see unaccepted memory if it explicitly asked for it (assuming > > the firmware wants know to support both cases, of course the firmware > > could also enforce the one or the other and just not offer the > > protocol). > > How bootloader suppose to know if OS will ask for unaccepted memory? > It can't. It means the use-case with bootloader cannot ever use > unaccepted memory. That's broken design. > I still don't understand why we need to support every imaginable combination of firmware, bootloader and OS. Unaccepted memory only exists on a special kind of virtual machine, which provides very little added value unless you opt into the security and attestation features, which are all heavily based on firmware protocols. So why should care about a EFI-aware bootloader calling ExitBootServices() and subsequently doing a legacy boot of Linux on such systems?
On Mon, Jan 16, 2023 at 02:11:26PM +0100, Ard Biesheuvel wrote: > On Mon, 16 Jan 2023 at 13:31, Kirill A. Shutemov <kirill@shutemov.name> wrote: > > > > On Mon, Jan 16, 2023 at 11:56:48AM +0100, Gerd Hoffmann wrote: > > > On Sat, Jan 14, 2023 at 01:20:24AM +0300, Kirill A. Shutemov wrote: > > > > On Fri, Jan 13, 2023 at 09:29:26PM +0000, Dionna Glaze wrote: > > > > > This patch depends on Kirill A. Shutemov's series > > > > > > > > > > [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory > > > > > > > > > > The UEFI v2.9 specification includes a new memory type to be used in > > > > > environments where the OS must accept memory that is provided from its > > > > > host. Before the introduction of this memory type, all memory was > > > > > accepted eagerly in the firmware. In order for the firmware to safely > > > > > stop accepting memory on the OS's behalf, the OS must affirmatively > > > > > indicate support to the firmware. > > > > > > > > I think it is a bad idea. > > > > > > > > This approach breaks use case with a bootloader between BIOS and OS. > > > > As the bootloader does ExitBootServices() it has to make the call on > > > > behalf of OS when it has no idea if the OS supports unaccepted. > > > > > > Nothing breaks, it'll error on the safe side. If the protocol callback > > > is not called the firmware will simply accept all memory. The guest OS > > > will only see unaccepted memory if it explicitly asked for it (assuming > > > the firmware wants know to support both cases, of course the firmware > > > could also enforce the one or the other and just not offer the > > > protocol). > > > > How bootloader suppose to know if OS will ask for unaccepted memory? > > It can't. It means the use-case with bootloader cannot ever use > > unaccepted memory. That's broken design. > > > > I still don't understand why we need to support every imaginable > combination of firmware, bootloader and OS. Unaccepted memory only > exists on a special kind of virtual machine, which provides very > little added value unless you opt into the security and attestation > features, which are all heavily based on firmware protocols. So why > should care about a EFI-aware bootloader calling ExitBootServices() > and subsequently doing a legacy boot of Linux on such systems? Why break what works? Some users want it. This patch adds complexity, breaks what works and the only upside will turn into a dead weight soon. There's alternative to add option to instruct firmware to accept all memory from VMM side. It will serve legacy OS that doesn't know about unaccepted memory and it is also can be use by latency-sensitive users later on (analog of qemu -mem-prealloc).
> > I still don't understand why we need to support every imaginable > > combination of firmware, bootloader and OS. Unaccepted memory only > > exists on a special kind of virtual machine, which provides very > > little added value unless you opt into the security and attestation > > features, which are all heavily based on firmware protocols. So why > > should care about a EFI-aware bootloader calling ExitBootServices() > > and subsequently doing a legacy boot of Linux on such systems? > > Why break what works? Some users want it. > The users that want legacy boot features will not be broken, they'll only get a safe view of the memory map. I don't think it's right to choose unsafe behavior for a legacy setup. > This patch adds complexity, breaks what works and the only upside will > turn into a dead weight soon. > > There's alternative to add option to instruct firmware to accept all > memory from VMM side. It will serve legacy OS that doesn't know about > unaccepted memory and it is also can be use by latency-sensitive users > later on (analog of qemu -mem-prealloc). > This means that users of a distro that has not enabled unaccepted memory support cannot simply start a VM with the usual command, but instead have to know a baroque extra flag to get access to all the memory that they configured the machine (and for a CSP customer, paid for). That's not a good experience. With GCE at least, you can't (shouldn't) associate the boot feature flag with a disk image because disks are mutable. If a customer upgrades their kernel after initially starting their VM, they can't remove the flag due to the way image annotations work. All of this headache goes away by adopting a small patch to the kernel that calls a 0-ary protocol interface and keeping safe acceptance behavior in the firmware. I think Gerd is right here that we should treat it as a transition feature that we can remove later. > -- > Kiryl Shutsemau / Kirill A. Shutemov
On 1/16/23 02:56, Gerd Hoffmann wrote: >> And we add this protocol to address very temporary problem: once >> unaccepted memory support get upstream it is just a dead weight. > Maybe, maybe not. unaccepted memory support has a Kconfig switch after > all. If we figure in 3-5 years that all distros have enabled it anyway > we can drop it again. For the transition period it will surely be > useful. I agree with Kirill here. Having unaccepted memory *AND* this firmware-driven feature really is just implementing the same thing twice. I'd much rather have the Kconfig option forced on for all guests that *might* need unaccepted memory support than carry redundant implementations. Also, _if_ we allow folks to turn the Kconfig off and get access to all their memory, they might get used to that. Removing this firmware interface from the kernel in a few years could be viewed as a regression. Then, we'll be stuck with this forever. In any case, the firmware side of things didn't seem like _that_ much code. So, I'm not protesting *that* strongly. But, I also don't believe for a second that this is going to be removed in 3-5 years.
On 1/16/23 15:22, Dave Hansen wrote: > On 1/16/23 02:56, Gerd Hoffmann wrote: >>> And we add this protocol to address very temporary problem: once >>> unaccepted memory support get upstream it is just a dead weight. >> Maybe, maybe not. unaccepted memory support has a Kconfig switch after >> all. If we figure in 3-5 years that all distros have enabled it anyway >> we can drop it again. For the transition period it will surely be >> useful. > > I agree with Kirill here. > > Having unaccepted memory *AND* this firmware-driven feature really is > just implementing the same thing twice. I'm not sure I follow you. This feature merely tells the firmware whether or not the OS supports unaccepted memory, which then tells the firmware whether it needs to accept the memory or whether the kernel can. We have had SNP guest support since 5.19 without support for unaccepted memory. Imagine now using a newer OVMF that can support unaccepted memory. How does the firmware know whether it must accept all the memory or whether it can advertise unaccepted memory. By having the kernel call this boot service protocol once support for unaccepted memory is in place, the firmware now knows that it need not accept all the memory. Thanks, Tom > > I'd much rather have the Kconfig option forced on for all guests that > *might* need unaccepted memory support than carry redundant implementations. > > Also, _if_ we allow folks to turn the Kconfig off and get access to all > their memory, they might get used to that. Removing this firmware > interface from the kernel in a few years could be viewed as a > regression. Then, we'll be stuck with this forever. > > In any case, the firmware side of things didn't seem like _that_ much > code. So, I'm not protesting *that* strongly. But, I also don't > believe for a second that this is going to be removed in 3-5 years.
On Mon, Jan 16, 2023 at 11:43:15AM -0800, Dionna Amalie Glaze wrote: > > > I still don't understand why we need to support every imaginable > > > combination of firmware, bootloader and OS. Unaccepted memory only > > > exists on a special kind of virtual machine, which provides very > > > little added value unless you opt into the security and attestation > > > features, which are all heavily based on firmware protocols. So why > > > should care about a EFI-aware bootloader calling ExitBootServices() > > > and subsequently doing a legacy boot of Linux on such systems? > > > > Why break what works? Some users want it. > > > > The users that want legacy boot features will not be broken, Why do you call boot with a bootloader a legacy feature? > they'll only get a safe view of the memory map. I don't think it's right > to choose unsafe behavior for a legacy setup. Present memory map with unaccepted memory to OS that doesn't about it is perfectly safe. This portion of the memory will be ignored. It is "feature not [yet] implemented" case. > > This patch adds complexity, breaks what works and the only upside will > > turn into a dead weight soon. > > > > There's alternative to add option to instruct firmware to accept all > > memory from VMM side. It will serve legacy OS that doesn't know about > > unaccepted memory and it is also can be use by latency-sensitive users > > later on (analog of qemu -mem-prealloc). > > > > This means that users of a distro that has not enabled unaccepted > memory support cannot simply start a VM with the usual command, but > instead have to know a baroque extra flag to get access to all the > memory that they configured the machine (and for a CSP customer, paid > for). That's not a good experience. New features require enabling. It is not something new. > With GCE at least, you can't (shouldn't) associate the boot feature > flag with a disk image because disks are mutable. If a customer > upgrades their kernel after initially starting their VM, they can't > remove the flag due to the way image annotations work. I guess a new VM has to be created, right? Doesn't sound like a big deal to me. The old will not break with upgraded kernel. Just not get benefit of the feature. > All of this headache goes away by adopting a small patch to the kernel > that calls a 0-ary protocol interface and keeping safe acceptance > behavior in the firmware. I think Gerd is right here that we should > treat it as a transition feature that we can remove later. Removing a feature is harder than adding one. How do you define that "later" has come? Anyway, I think we walk in a circle. I consider it a misfeature. If you want still go this path, please add my Nacked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
On Tue, Jan 17, 2023 at 02:17:11AM +0300, Kirill A. Shutemov wrote: > On Mon, Jan 16, 2023 at 11:43:15AM -0800, Dionna Amalie Glaze wrote: > > > > I still don't understand why we need to support every imaginable > > > > combination of firmware, bootloader and OS. Unaccepted memory only > > > > exists on a special kind of virtual machine, which provides very > > > > little added value unless you opt into the security and attestation > > > > features, which are all heavily based on firmware protocols. So why > > > > should care about a EFI-aware bootloader calling ExitBootServices() > > > > and subsequently doing a legacy boot of Linux on such systems? > > > > > > Why break what works? Some users want it. > > > > > > > The users that want legacy boot features will not be broken, > > Why do you call boot with a bootloader a legacy feature? Linux efi kernels can be booted in two ways: (1) old/legacy: boot loader calls ExitBootServices and jumps to the kernel entry point. (2) new/efi stub: boot loader does *not* call ExitBootServices, but loads the linux kernel as efi binary instead. The linux kernel efi stub calls ExitBootServices then. All kernel version relevant here (new enough to support SEV-SNP / TDX) have efi stub support, so (1) does not really matter in practice. the efi stub was added *exactly* to handle cases like this one: the kernel can do efi calls needed on its own without depending on the boot loader doing it on behalf of the kernel. > > This means that users of a distro that has not enabled unaccepted > > memory support cannot simply start a VM with the usual command, but > > instead have to know a baroque extra flag to get access to all the > > memory that they configured the machine (and for a CSP customer, paid > > for). That's not a good experience. > > New features require enabling. It is not something new. Asking user to manually configure something which can be handled automatically just fine is a bad design. take care, Gerd
Hi, > In any case, the firmware side of things didn't seem like _that_ much > code. So, I'm not protesting *that* strongly. But, I also don't > believe for a second that this is going to be removed in 3-5 years. If things are going roughly as I expect them to go (both tdx support and unaccepted memory support land upstream this year; distros enable it by default) we should be able to drop this when the 6.1-lts kernel goes EOL. First in edk2, later in linux too. take care, Gerd
> > Why do you call boot with a bootloader a legacy feature? > Gerd answered this about EBS called from the bootloader. > > they'll only get a safe view of the memory map. I don't think it's right > > to choose unsafe behavior for a legacy setup. > > Present memory map with unaccepted memory to OS that doesn't about it is > perfectly safe. This portion of the memory will be ignored. It is "feature > not [yet] implemented" case. > SNP guest support is already in Linux, and it gets a full view of the memory given to the VM. If the firmware ever introduces unaccepted memory, then the kernel's behavior is retroactively broken without the "accept all if AllowUnacceptedMemory() not called" behavior of the UEFI. The memory that existed before becomes ignored. This is not the right approach IMO. > > > This patch adds complexity, breaks what works and the only upside will > > > turn into a dead weight soon. > > > > > > There's alternative to add option to instruct firmware to accept all > > > memory from VMM side. It will serve legacy OS that doesn't know about > > > unaccepted memory and it is also can be use by latency-sensitive users > > > later on (analog of qemu -mem-prealloc). > > > > > > > This means that users of a distro that has not enabled unaccepted > > memory support cannot simply start a VM with the usual command, but > > instead have to know a baroque extra flag to get access to all the > > memory that they configured the machine (and for a CSP customer, paid > > for). That's not a good experience. > > New features require enabling. It is not something new. > What I'm saying is that you're suggesting a feature _dis_abling requirement, which is an antipattern. Any SNP user right now would need to add a "don't use an unimplemented feature" flag to get access to all its memory again. > > With GCE at least, you can't (shouldn't) associate the boot feature > > flag with a disk image because disks are mutable. If a customer > > upgrades their kernel after initially starting their VM, they can't > > remove the flag due to the way image annotations work. > > I guess a new VM has to be created, right? Doesn't sound like a big deal > to me. > Usually it's not, but the retroactive need to create a new VM once the firmware adds UEFI v2.9 support with unaccepted memory is a big deal. > The old will not break with upgraded kernel. Just not get benefit of the > feature. > A user buys access to a high memory VM: 768GiB. They then shut down and bring it back up on a new firmware that uses unaccepted memory. That VM goes from 785GiB free memory to 3GiB free memory at boot. This is because all memory above 4GiB (and nothing there for the 3-4GiB MMIO hole) would be the unknown unaccepted memory type. We need the accept-all-if-support-not-acked semantics with the protocol. > > All of this headache goes away by adopting a small patch to the kernel > > that calls a 0-ary protocol interface and keeping safe acceptance > > behavior in the firmware. I think Gerd is right here that we should > > treat it as a transition feature that we can remove later. > > Removing a feature is harder than adding one. How do you define that > "later" has come? > Gerd's response of after 6.1-lts EOL is reasonable to me. At the same time, both SEV-SNP and TDX's Kconfig would need to strictly require unaccepted memory. The semantics of the UEFI under the proposed protocol is allowed to change the default behavior when the protocol is not exposed to the OS. The default would then be to always introduce unaccepted memory for TDX and SEV-SNP guests. To Gerd's point, removing "first in edk2, later in linux too" I think is backwards. We need all users of the protocol to agree that SEV-SNP and TDX strictly imply unaccepted memory support. Only then can we remove the protocol from EDK2. > Anyway, I think we walk in a circle. I consider it a misfeature. If you > want still go this path, please add my > > Nacked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Thanks for your time discussing.
Hi, > To Gerd's point, removing "first in edk2, later in linux too" I think > is backwards. We need all users of the protocol to agree that SEV-SNP > and TDX strictly imply unaccepted memory support. Only then can we > remove the protocol from EDK2. Its not backwards. edk2 dropping support first would result in break kernels without support for unaccepted memory. Which is why we wait until such kernels are EOL. Linux dropping support first would result in firmware accepting all memory again. So that isn't a good plan. Thats why linux should support the protocol a bit longer, while firmware versions which expect negotiation happening are still in use. take care, Gerd
(cc'ing some folks whom I've discussed this with off-list today) Full discussion here: https://lore.kernel.org/linux-efi/20230113212926.2904735-1-dionnaglaze@google.com/ On Mon, 16 Jan 2023 at 23:46, Tom Lendacky <thomas.lendacky@amd.com> wrote: > > On 1/16/23 15:22, Dave Hansen wrote: > > On 1/16/23 02:56, Gerd Hoffmann wrote: > >>> And we add this protocol to address very temporary problem: once > >>> unaccepted memory support get upstream it is just a dead weight. > >> Maybe, maybe not. unaccepted memory support has a Kconfig switch after > >> all. If we figure in 3-5 years that all distros have enabled it anyway > >> we can drop it again. For the transition period it will surely be > >> useful. > > > > I agree with Kirill here. > > > > Having unaccepted memory *AND* this firmware-driven feature really is > > just implementing the same thing twice. > > I'm not sure I follow you. This feature merely tells the firmware whether > or not the OS supports unaccepted memory, which then tells the firmware > whether it needs to accept the memory or whether the kernel can. > > We have had SNP guest support since 5.19 without support for unaccepted > memory. Imagine now using a newer OVMF that can support unaccepted memory. > How does the firmware know whether it must accept all the memory or > whether it can advertise unaccepted memory. By having the kernel call this > boot service protocol once support for unaccepted memory is in place, the > firmware now knows that it need not accept all the memory. > So if people deploying SEV agree that this is a useful feature to have, and people working on TDX saying this protocol must never exist, I think the obvious conclusion is that OVMF should only expose it when running on SEV. However, I am still failing to grasp why there is disagreement here. Linux already implements SEV support but unaccepted memory protocol is not supported yet, and so it is crystal clear that we need something to bridge the compatibility gap. Without this protocol, firmware must never accept memory, and the OS must always take charge of this, even if it prefers to accept memory eagerly. With this protocol in place, acceptance becomes a policy decision, where the default policy is 'accept' for OS implementations that have no understanding of unaccepted memory or the protocol. 'Enlightened' OSes can still decide not to call the protocol and therefore not having to bother with acceptance at all, given that the firmware will take care of it. As for the 'legacy' boot method: this bootloader can decide for itself whether or not it needs to invoke the protocol, and can invent its own methods for communicating/inspecting the OS image to obtain the information to base this decision on. This is outside of the scope of EFI. However, I also disagree with the binary 'no solution shall exist' vs 'a solution must cover every imaginable combination of bootloader and OS image': it makes sense to be pragmatic here, and limit ourselves to what people are actually deploying. And given the default behavior fo 'accept everything', the only penalty for ignoring the legacy bootloader case is a slower boot. I have been on the sidelines for most of the OVMF and Linux development regarding confidential compute, but where I did get involved, it was to try and reach consensus between the different technologies (including the ARM one), to avoid ending up with radical different approaches for doing the same thing. However, I guess we're at a point where SEV and TDX really want different solutions, so I think divergence might be the way to proceed.
On 1/18/23 07:09, Ard Biesheuvel wrote: > However, I guess we're at a point where SEV and TDX really want > different solutions, so I think divergence might be the way to > proceed. I don't think they want different things really. TDX doesn't need this protocol. It sounds like SEV does need it, though. That doesn't mean they really diverge. They're *both* going to have to poke at this protocol knob to get the firmware to not accept the memory. This does slightly change the motivation for doing explicit unaccepted memory support in the kernel. I also don't know _quite_ how this will look to a guest. For instance, will they see different memory maps based on which protocol they are using? I assume so, but didn't see any of that explicitly mentioned in this patch.
On Wed, 18 Jan 2023 at 16:41, Dave Hansen <dave.hansen@intel.com> wrote: > > On 1/18/23 07:09, Ard Biesheuvel wrote: > > However, I guess we're at a point where SEV and TDX really want > > different solutions, so I think divergence might be the way to > > proceed. > > I don't think they want different things really. > > TDX doesn't need this protocol. It sounds like SEV does need it, > though. That doesn't mean they really diverge. They're *both* going to > have to poke at this protocol knob to get the firmware to not accept the > memory. > No, on TDX, the firmware would never accept all memory. On SEV, it would only do so if the protocol has not been called prior to the call to ExitBootServices(). > This does slightly change the motivation for doing explicit unaccepted > memory support in the kernel. > Not on TDX. > I also don't know _quite_ how this will look to a guest. For instance, > will they see different memory maps based on which protocol they are > using? I assume so, but didn't see any of that explicitly mentioned in > this patch. The EFI memory map will not contain ranges of type EFI_UNACCEPTED_MEMORY if the memory was accepted on behalf of the OS by the firmware. That is the point, really, as non-enlightened OSes will ignore those.
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index a0bfd31358ba..abf31e5ade55 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -26,6 +26,17 @@ const efi_dxe_services_table_t *efi_dxe_table; u32 image_offset __section(".data"); static efi_loaded_image_t *image = NULL; +typedef union memory_acceptance_protocol memory_acceptance_protocol_t; +union memory_acceptance_protocol { + struct { + efi_status_t (__efiapi *allow_unaccepted_memory)( + memory_acceptance_protocol_t *); + }; + struct { + u32 allow_unaccepted_memory; + } mixed_mode; +}; + static efi_status_t preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom) { @@ -310,6 +321,30 @@ setup_memory_protection(unsigned long image_base, unsigned long image_size) #endif } + +static void setup_unaccepted_memory(void) +{ + efi_guid_t mem_acceptance_proto = EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUID; + memory_acceptance_protocol_t *proto; + efi_status_t status; + + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY)) + return; + + /* + * Enable unaccepted memory before calling exit boot services in order + * for the UEFI to not accept all memory on EBS. + */ + status = efi_bs_call(locate_protocol, &mem_acceptance_proto, NULL, + (void **)&proto); + if (status != EFI_SUCCESS) + return; + + status = efi_call_proto(proto, allow_unaccepted_memory); + if (status != EFI_SUCCESS) + efi_err("Memory acceptance protocol failed\n"); +} + static const efi_char16_t apple[] = L"Apple"; static void setup_quirks(struct boot_params *boot_params, @@ -899,6 +934,8 @@ asmlinkage unsigned long efi_main(efi_handle_t handle, setup_quirks(boot_params, bzimage_addr, buffer_end - buffer_start); + setup_unaccepted_memory(); + status = exit_boot(boot_params, handle); if (status != EFI_SUCCESS) { efi_err("exit_boot() failed!\n"); diff --git a/include/linux/efi.h b/include/linux/efi.h index 4b27519143f5..bfc0e4f2aba5 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -391,6 +391,7 @@ void efi_native_runtime_setup(void); #define EFI_RT_PROPERTIES_TABLE_GUID EFI_GUID(0xeb66918a, 0x7eef, 0x402a, 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9) #define EFI_DXE_SERVICES_TABLE_GUID EFI_GUID(0x05ad34ba, 0x6f02, 0x4214, 0x95, 0x2e, 0x4d, 0xa0, 0x39, 0x8e, 0x2b, 0xb9) #define EFI_SMBIOS_PROTOCOL_GUID EFI_GUID(0x03583ff6, 0xcb36, 0x4940, 0x94, 0x7e, 0xb9, 0xb3, 0x9f, 0x4a, 0xfa, 0xf7) +#define EFI_MEMORY_ACCEPTANCE_PROTOCOL_GUID EFI_GUID(0xc5a010fe, 0x38a7, 0x4531, 0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49) #define EFI_IMAGE_SECURITY_DATABASE_GUID EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596, 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f) #define EFI_SHIM_LOCK_GUID EFI_GUID(0x605dab50, 0xe046, 0x4300, 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
This patch depends on Kirill A. Shutemov's series [PATCHv8 00/14] mm, x86/cc: Implement support for unaccepted memory The UEFI v2.9 specification includes a new memory type to be used in environments where the OS must accept memory that is provided from its host. Before the introduction of this memory type, all memory was accepted eagerly in the firmware. In order for the firmware to safely stop accepting memory on the OS's behalf, the OS must affirmatively indicate support to the firmware. Enabling unaccepted memory requires calling a 0-argument enablement protocol before ExitBootServices. This call is only made if the kernel is compiled with UNACCEPTED_MEMORY=y The naming of the protocol guid is dependent on the standardization of the protocol, which is being discussed. Acceptance is contingent on the kernel community's approval. Cc: Ard Biescheuvel <ardb@kernel.org> Cc: "Min M. Xu" <min.m.xu@intel.org> 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: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Cc: Dave Hansen <dave.hansen@linux.intel.com> Signed-off-by: Dionna Glaze <dionnaglaze@google.com> --- drivers/firmware/efi/libstub/x86-stub.c | 37 +++++++++++++++++++++++++ include/linux/efi.h | 1 + 2 files changed, 38 insertions(+)