Re: [PATCH v7 09/11] arm64: Enable memory encrypt for Realms

[Steven Price <steven.price@arm.com>]

On 17/10/2024 14:14, Steven Price wrote:
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> Use the memory encryption APIs to trigger a RSI call to request a
> transition between protected memory and shared memory (or vice versa)
> and updating the kernel's linear map of modified pages to flip the top
> bit of the IPA. This requires that block mappings are not used in the
> direct map for realm guests.
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Co-developed-by: Steven Price <steven.price@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
[...]
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 547a9e0b46c2..6ae6ae806454 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -5,10 +5,12 @@
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
>  #include <linux/module.h>
> +#include <linux/mem_encrypt.h>
>  #include <linux/sched.h>
>  #include <linux/vmalloc.h>
>  
>  #include <asm/cacheflush.h>
> +#include <asm/pgtable-prot.h>
>  #include <asm/set_memory.h>
>  #include <asm/tlbflush.h>
>  #include <asm/kfence.h>
> @@ -23,14 +25,16 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
>  bool can_set_direct_map(void)
>  {
>  	/*
> -	 * rodata_full and DEBUG_PAGEALLOC require linear map to be
> -	 * mapped at page granularity, so that it is possible to
> +	 * rodata_full, DEBUG_PAGEALLOC and a Realm guest all require linear
> +	 * map to be mapped at page granularity, so that it is possible to
>  	 * protect/unprotect single pages.
>  	 *
>  	 * KFENCE pool requires page-granular mapping if initialized late.
> +	 *
> +	 * Realms need to make pages shared/protected at page granularity.
>  	 */
>  	return rodata_full || debug_pagealloc_enabled() ||
> -	       arm64_kfence_can_set_direct_map();
> +		arm64_kfence_can_set_direct_map() || is_realm_world();
>  }

Aneesh pointed out that this call to is_realm_world() is now too early 
since the decision to delay the RSI detection. The upshot is that a 
realm guest which doesn't have page granularity forced for other reasons 
will fail to share pages with the host.

At the moment I can think of a couple of options:

(1) Make rodata_full a requirement for realm guests. 
    CONFIG_RODATA_FULL_DEFAULT_ENABLED is already "default y" so this 
    isn't a big ask.

(2) Revisit the idea of detecting when running as a realm guest early. 
    This has the advantage of also "fixing" earlycon (no need to 
    manually specify the shared-alias of an unprotected UART).

I'm currently leaning towards (1) because it's the default anyway. But 
if we're going to need to fix earlycon (or indeed find other similar 
issues) then (2) would obviously make sense.

Any thoughts on the best option here.

Untested patch for (1) below. Although updating the docs would be
probably be a good idea too ;)

Thanks,
Steve

----8<---
diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
index ce4778141ec7..48a6ef0f401c 100644
--- a/arch/arm64/kernel/rsi.c
+++ b/arch/arm64/kernel/rsi.c
@@ -126,6 +126,10 @@ void __init arm64_rsi_init(void)
 		return;
 	if (!rsi_version_matches())
 		return;
+	if (!can_set_direct_map()) {
+		pr_err("rodata_full disabled, unable to run as a realm guest. Please enable CONFIG_RODATA_FULL_DEFAULT_ENABLED\n");
+		return;
+	}
 	if (WARN_ON(rsi_get_realm_config(&config)))
 		return;
 	prot_ns_shared = BIT(config.ipa_bits - 1);
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 39fd1f7ff02a..f8fd8a3816fb 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -25,16 +25,14 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
 bool can_set_direct_map(void)
 {
 	/*
-	 * rodata_full, DEBUG_PAGEALLOC and a Realm guest all require linear
-	 * map to be mapped at page granularity, so that it is possible to
+	 * rodata_full, DEBUG_PAGEALLOC require linear map to be
+	 * mapped at page granularity, so that it is possible to
 	 * protect/unprotect single pages.
 	 *
 	 * KFENCE pool requires page-granular mapping if initialized late.
-	 *
-	 * Realms need to make pages shared/protected at page granularity.
 	 */
 	return rodata_full || debug_pagealloc_enabled() ||
-		arm64_kfence_can_set_direct_map() || is_realm_world();
+		arm64_kfence_can_set_direct_map();
 }
 
 static int change_page_range(pte_t *ptep, unsigned long addr, void *data)