Re: [PATCH v6 4/6] x86/e820: Tag e820_entry with crypto capabilities

Kees Cook
2022-02-07T21:56:18Z

On Thu, Feb 03, 2022 at 01:43:26PM -0300, Martin Fernandez wrote:
> Add a new enum for crypto capabilities.
>
> Add a new member in e820_entry to hold whether an entry is able to do
> hardware memory encryption or not.
>
> Add a new function e820__range_set_crypto_capable to mark all the
> entries in a range of addresses as encryptable. This will be called
> when initializing EFI.
>
> Change e820__update_table to handle merging and overlap problems
> taking into account crypto_capable.
>
> Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com>
> ---
> arch/x86/include/asm/e820/api.h | 1 +
> arch/x86/include/asm/e820/types.h | 12 +++-
> arch/x86/kernel/e820.c | 114 ++++++++++++++++++++++++++++--
> 3 files changed, 119 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h
> index e8f58ddd06d9..4b3b01fafdd1 100644
> --- a/arch/x86/include/asm/e820/api.h
> +++ b/arch/x86/include/asm/e820/api.h
> @@ -17,6 +17,7 @@ extern bool e820__mapped_all(u64 start, u64 end, enum e820_type type);
> extern void e820__range_add (u64 start, u64 size, enum e820_type type);
> extern u64 e820__range_update(u64 start, u64 size, enum e820_type old_type, enum e820_type new_type);
> extern u64 e820__range_remove(u64 start, u64 size, enum e820_type old_type, bool check_type);
> +extern u64 e820__range_set_crypto_capable(u64 start, u64 size);
>
> extern void e820__print_table(char *who);
> extern int e820__update_table(struct e820_table *table);
> diff --git a/arch/x86/include/asm/e820/types.h b/arch/x86/include/asm/e820/types.h
> index 314f75d886d0..aef03c665f5e 100644
> --- a/arch/x86/include/asm/e820/types.h
> +++ b/arch/x86/include/asm/e820/types.h
> @@ -46,6 +46,11 @@ enum e820_type {
> E820_TYPE_RESERVED_KERN = 128,
> };
>
> +enum e820_crypto_capabilities {
> + E820_NOT_CRYPTO_CAPABLE = 0,
> + E820_CRYPTO_CAPABLE = 1,
> +};

Is this expected to grow beyond a bool?

> +
> /*
> * A single E820 map entry, describing a memory range of [addr...addr+size-1],
> * of 'type' memory type:
> @@ -53,9 +58,10 @@ enum e820_type {
> * (We pack it because there can be thousands of them on large systems.)
> */
> struct e820_entry {
> - u64 addr;
> - u64 size;
> - enum e820_type type;
> + u64 addr;
> + u64 size;
> + enum e820_type type;
> + enum e820_crypto_capabilities crypto_capable;
> } __attribute__((packed));

Is there any concern about growing this structure? The "thousands" note
in the comment is likely rare. FWIW, this seems fine to me, but I
thought I'd mention it.

--
Kees Cook