2022-02-02 10:07:57

by Dov Murik

Subject: [PATCH v7 2/5] efi/libstub: Reserve confidential computing secret area

Some older firmware declare the confidential computing secret area as
EFI_BOOT_SERVICES_DATA region. Fix this up by treating this memory
region as EFI_RESERVED_TYPE, as it should be.

If that memory region is already EFI_RESERVED_TYPE then this has no
effect on the E820 map.

Signed-off-by: Dov Murik <[email protected]>
---
drivers/firmware/efi/libstub/x86-stub.c | 28 +++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 01ddd4502e28..4f1218cb87ca 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -24,6 +24,7 @@
const efi_system_table_t *efi_system_table;
extern u32 image_offset;
static efi_loaded_image_t *image = NULL;
+static u64 efi_coco_secret_phys_addr = U64_MAX;

static efi_status_t
preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
@@ -443,6 +444,21 @@ static void add_e820ext(struct boot_params *params,
params->hdr.setup_data = (unsigned long)e820ext;
}

+#ifdef CONFIG_EFI_COCO_SECRET
+static void efi_set_coco_secret_phys_addr(void)
+{
+ struct linux_efi_coco_secret_area *secret_area =
+ get_efi_config_table(LINUX_EFI_COCO_SECRET_AREA_GUID);
+
+ if (!secret_area || secret_area->size == 0 || secret_area->size >= SZ_4G)
+ return;
+
+ efi_coco_secret_phys_addr = secret_area->base_pa;
+}
+#else
+static void efi_set_coco_secret_phys_addr(void) {}
+#endif
+
static efi_status_t
setup_e820(struct boot_params *params, struct setup_data *e820ext, u32 e820ext_size)
{
@@ -494,6 +510,16 @@ setup_e820(struct boot_params *params, struct setup_data *e820ext, u32 e820ext_s
e820_type = E820_TYPE_SOFT_RESERVED;
else
e820_type = E820_TYPE_RAM;
+#ifdef CONFIG_EFI_COCO_SECRET
+ if (d->phys_addr == efi_coco_secret_phys_addr)
+ /*
+ * Fix a quirk in firmwares which don't mark
+ * the EFI confidential computing area as
+ * EFI_RESERVED_TYPE, but instead as
+ * EFI_BOOT_SERVICES_DATA.
+ */
+ e820_type = E820_TYPE_RESERVED;
+#endif
break;

case EFI_ACPI_MEMORY_NVS:
@@ -787,6 +813,8 @@ unsigned long efi_main(efi_handle_t handle,

efi_retrieve_tpm2_eventlog();

+ efi_set_coco_secret_phys_addr();
+
setup_graphics(boot_params);

setup_efi_pci(boot_params);
--
2.25.1

2022-02-02 12:55:31

by Dov Murik

Subject: Re: [PATCH v7 2/5] efi/libstub: Reserve confidential computing secret area

On 02/02/2022 10:41, Gerd Hoffmann wrote:
> On Tue, Feb 01, 2022 at 12:44:10PM +0000, Dov Murik wrote:
>> Some older firmware declare the confidential computing secret area as
>> EFI_BOOT_SERVICES_DATA region. Fix this up by treating this memory
>> region as EFI_RESERVED_TYPE, as it should be.
>>
>> If that memory region is already EFI_RESERVED_TYPE then this has no
>> effect on the E820 map.
>
> Hmm, sure we actually want merge this? I suspect by the time this
> landed in an upstream kernel "older firmware" isn't much of a problem
> any more.
>

When we originally wrote this patch the OVMF fix was not yet upstream
(and currently it is still not part of an official edk2 stable tag/release).

But I agree that as time goes by, the need for this fix is diminishing.

I'll consider dropping this patch entirely in the next round.

Thanks,
-Dov

2022-02-03 07:57:37

by Gerd Hoffmann

Subject: Re: [PATCH v7 2/5] efi/libstub: Reserve confidential computing secret area

On Tue, Feb 01, 2022 at 12:44:10PM +0000, Dov Murik wrote:
> Some older firmware declare the confidential computing secret area as
> EFI_BOOT_SERVICES_DATA region. Fix this up by treating this memory
> region as EFI_RESERVED_TYPE, as it should be.
>
> If that memory region is already EFI_RESERVED_TYPE then this has no
> effect on the E820 map.

Hmm, sure we actually want merge this? I suspect by the time this
landed in an upstream kernel "older firmware" isn't much of a problem
any more.

take care,
Gerd