2023-01-31 00:49:36

by Dionna Amalie Glaze

[permalink] [raw]
Subject: [PATCH v2, RESEND] x86/efi: Safely enable unaccepted memory in UEFI

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. This is only a problem for AMD
SEV-SNP, since Linux has had support for it since 5.19. The other
technology that can make use of unaccepted memory, Intel TDX, does not
yet have Linux support, so it can strictly require unaccepted memory
support as a dependency of CONFIG_TDX and not require communication with
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

This protocol will be removed after the end of life of the first LTS
that includes it, in order to give firmware implementations an
expiration date for it. When the protocol is removed, firmware will
strictly infer that a SEV-SNP VM is running an OS that supports the
unaccepted memory type. At the earliest convenience, when unaccepted
memory support is added to Linux, SEV-SNP may take strict dependence in
it. After the firmware removes support for the protocol, this patch
should be reverted.

Change since v1:
* protocol name, as it is in OVMF
https://github.com/tianocore/edk2/commit/26847fb6be7fff83a834a3154224588afede0073
* protocol typedef moved before struct definition.

Cc: Ard Biescheuvel <[email protected]>
Cc: "Min M. Xu" <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: James Bottomley <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Jiewen Yao <[email protected]>
Cc: Erdem Aktas <[email protected]>
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Borislav Petkov <[email protected]>

Signed-off-by: Dionna Glaze <[email protected]>
---
drivers/firmware/efi/libstub/x86-stub.c | 37 +++++++++++++++++++++++++
include/linux/efi.h | 4 +++
2 files changed, 41 insertions(+)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index a0bfd31358ba..e4c04444edab 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 sev_memory_acceptance_protocol sev_memory_acceptance_protocol_t;
+union sev_memory_acceptance_protocol {
+ struct {
+ efi_status_t (__efiapi *allow_unaccepted_memory)(
+ sev_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 = OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID;
+ sev_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..ac812978a03a 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -434,6 +434,10 @@ void efi_native_runtime_setup(void);
#define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
#define AMD_SEV_MEM_ENCRYPT_GUID EFI_GUID(0x0cf29b71, 0x9e51, 0x433a, 0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75)

+/* OVMF protocol GUIDs */
+#define OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID EFI_GUID(0xc5a010fe, 0x38a7, 0x4531, 0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49)
+
+
typedef struct {
efi_guid_t guid;
u64 table;
--
2.39.1.456.gfc5497dd1b-goog



2023-01-31 08:34:54

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2, RESEND] x86/efi: Safely enable unaccepted memory in UEFI

On Tue, 31 Jan 2023 at 01:49, Dionna Glaze <[email protected]> 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. This is only a problem for AMD
> SEV-SNP, since Linux has had support for it since 5.19. The other
> technology that can make use of unaccepted memory, Intel TDX, does not
> yet have Linux support, so it can strictly require unaccepted memory
> support as a dependency of CONFIG_TDX and not require communication with
> 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
>
> This protocol will be removed after the end of life of the first LTS
> that includes it, in order to give firmware implementations an
> expiration date for it. When the protocol is removed, firmware will
> strictly infer that a SEV-SNP VM is running an OS that supports the
> unaccepted memory type. At the earliest convenience, when unaccepted
> memory support is added to Linux, SEV-SNP may take strict dependence in
> it. After the firmware removes support for the protocol, this patch
> should be reverted.
>
> Change since v1:
> * protocol name, as it is in OVMF
> https://github.com/tianocore/edk2/commit/26847fb6be7fff83a834a3154224588afede0073
> * protocol typedef moved before struct definition.
>
> Cc: Ard Biescheuvel <[email protected]>
> Cc: "Min M. Xu" <[email protected]>
> Cc: Gerd Hoffmann <[email protected]>
> Cc: James Bottomley <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: Jiewen Yao <[email protected]>
> Cc: Erdem Aktas <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Borislav Petkov <[email protected]>
>
> Signed-off-by: Dionna Glaze <[email protected]>
> ---
> drivers/firmware/efi/libstub/x86-stub.c | 37 +++++++++++++++++++++++++
> include/linux/efi.h | 4 +++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index a0bfd31358ba..e4c04444edab 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 sev_memory_acceptance_protocol sev_memory_acceptance_protocol_t;
> +union sev_memory_acceptance_protocol {
> + struct {
> + efi_status_t (__efiapi *allow_unaccepted_memory)(
> + sev_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 = OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID;
> + sev_memory_acceptance_protocol_t *proto;
> + efi_status_t status;
> +
> + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))

Do we need to check for IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT) here as well?

> + 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..ac812978a03a 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -434,6 +434,10 @@ void efi_native_runtime_setup(void);
> #define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
> #define AMD_SEV_MEM_ENCRYPT_GUID EFI_GUID(0x0cf29b71, 0x9e51, 0x433a, 0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75)
>
> +/* OVMF protocol GUIDs */
> +#define OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID EFI_GUID(0xc5a010fe, 0x38a7, 0x4531, 0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49)
> +
> +
> typedef struct {
> efi_guid_t guid;
> u64 table;
> --
> 2.39.1.456.gfc5497dd1b-goog
>

2023-01-31 16:08:23

by Dionna Amalie Glaze

[permalink] [raw]
Subject: Re: [PATCH v2, RESEND] x86/efi: Safely enable unaccepted memory in UEFI

> > + efi_status_t status;
> > +
> > + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
>
> Do we need to check for IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT) here as well?
>

Arguably no, since the firmware should only make the protocol
available when it determines that the protocol should be used. In our
case, that's just SEV-SNP. The firmware's TDX logic will not expose
this protocol.

This maintains flexibility for the rare case that the TDX go-to-market
schedule doesn't align with upstream's acceptance of unaccepted memory
support, but does accept the generic TDX support. Best not paint
ourselves into a corner.

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

2023-01-31 16:34:58

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2, RESEND] x86/efi: Safely enable unaccepted memory in UEFI

On 1/31/23 08:08, Dionna Amalie Glaze wrote:
>>> + efi_status_t status;
>>> +
>>> + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
>> Do we need to check for IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT) here as well?
>>
> Arguably no, since the firmware should only make the protocol
> available when it determines that the protocol should be used. In our
> case, that's just SEV-SNP. The firmware's TDX logic will not expose
> this protocol.
>
> This maintains flexibility for the rare case that the TDX go-to-market
> schedule doesn't align with upstream's acceptance of unaccepted memory
> support, but does accept the generic TDX support. Best not paint
> ourselves into a corner.

Yes, please. Maintaining this functionality for TDX would provide some
more flexibility in how things get accepted upstream.