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>
---
Changes since v5:
 * Added comments and a WARN() in realm_set_memory_{en,de}crypted() to
   explain that memory is leaked if the transition fails. This means the
   callers no longer need to provide their own WARN.
Changed since v4:
 * Reworked to use the new dispatcher for the mem_encrypt API
Changes since v3:
 * Provide pgprot_{de,en}crypted() macros
 * Rename __set_memory_encrypted() to __set_memory_enc_dec() since it
   both encrypts and decrypts.
Changes since v2:
 * Fix location of set_memory_{en,de}crypted() and export them.
 * Break-before-make when changing the top bit of the IPA for
   transitioning to/from shared.
---
 arch/arm64/Kconfig                   |  3 +
 arch/arm64/include/asm/mem_encrypt.h |  9 +++
 arch/arm64/include/asm/pgtable.h     |  5 ++
 arch/arm64/include/asm/set_memory.h  |  3 +
 arch/arm64/kernel/rsi.c              | 16 +++++
 arch/arm64/mm/pageattr.c             | 90 +++++++++++++++++++++++++++-
 6 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3e29b44d2d7b..ccea9c22d6df 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -21,6 +21,7 @@ config ARM64
 	select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
 	select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
 	select ARCH_HAS_CACHE_LINE_SIZE
+	select ARCH_HAS_CC_PLATFORM
 	select ARCH_HAS_CURRENT_STACK_POINTER
 	select ARCH_HAS_DEBUG_VIRTUAL
 	select ARCH_HAS_DEBUG_VM_PGTABLE
@@ -44,6 +45,8 @@ config ARM64
 	select ARCH_HAS_SETUP_DMA_OPS
 	select ARCH_HAS_SET_DIRECT_MAP
 	select ARCH_HAS_SET_MEMORY
+	select ARCH_HAS_MEM_ENCRYPT
+	select ARCH_HAS_FORCE_DMA_UNENCRYPTED
 	select ARCH_STACKWALK
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_STRICT_MODULE_RWX
diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
index b0c9a86b13a4..f8f78f622dd2 100644
--- a/arch/arm64/include/asm/mem_encrypt.h
+++ b/arch/arm64/include/asm/mem_encrypt.h
@@ -2,6 +2,8 @@
 #ifndef __ASM_MEM_ENCRYPT_H
 #define __ASM_MEM_ENCRYPT_H
 
+#include <asm/rsi.h>
+
 struct arm64_mem_crypt_ops {
 	int (*encrypt)(unsigned long addr, int numpages);
 	int (*decrypt)(unsigned long addr, int numpages);
@@ -12,4 +14,11 @@ int arm64_mem_crypt_ops_register(const struct arm64_mem_crypt_ops *ops);
 int set_memory_encrypted(unsigned long addr, int numpages);
 int set_memory_decrypted(unsigned long addr, int numpages);
 
+int realm_register_memory_enc_ops(void);
+
+static inline bool force_dma_unencrypted(struct device *dev)
+{
+	return is_realm_world();
+}
+
 #endif	/* __ASM_MEM_ENCRYPT_H */
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index c329ea061dc9..7e4bdc8259a2 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -684,6 +684,11 @@ static inline void set_pud_at(struct mm_struct *mm, unsigned long addr,
 #define pgprot_nx(prot) \
 	__pgprot_modify(prot, PTE_MAYBE_GP, PTE_PXN)
 
+#define pgprot_decrypted(prot) \
+	__pgprot_modify(prot, PROT_NS_SHARED, PROT_NS_SHARED)
+#define pgprot_encrypted(prot) \
+	__pgprot_modify(prot, PROT_NS_SHARED, 0)
+
 /*
  * Mark the prot value as uncacheable and unbufferable.
  */
diff --git a/arch/arm64/include/asm/set_memory.h b/arch/arm64/include/asm/set_memory.h
index 917761feeffd..37774c793006 100644
--- a/arch/arm64/include/asm/set_memory.h
+++ b/arch/arm64/include/asm/set_memory.h
@@ -15,4 +15,7 @@ int set_direct_map_invalid_noflush(struct page *page);
 int set_direct_map_default_noflush(struct page *page);
 bool kernel_page_present(struct page *page);
 
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
 #endif /* _ASM_ARM64_SET_MEMORY_H */
diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
index a23c0a7154d2..3031f25c32ef 100644
--- a/arch/arm64/kernel/rsi.c
+++ b/arch/arm64/kernel/rsi.c
@@ -7,8 +7,10 @@
 #include <linux/memblock.h>
 #include <linux/psci.h>
 #include <linux/swiotlb.h>
+#include <linux/cc_platform.h>
 
 #include <asm/io.h>
+#include <asm/mem_encrypt.h>
 #include <asm/rsi.h>
 
 static struct realm_config config;
@@ -19,6 +21,17 @@ EXPORT_SYMBOL(prot_ns_shared);
 DEFINE_STATIC_KEY_FALSE_RO(rsi_present);
 EXPORT_SYMBOL(rsi_present);
 
+bool cc_platform_has(enum cc_attr attr)
+{
+	switch (attr) {
+	case CC_ATTR_MEM_ENCRYPT:
+		return is_realm_world();
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL_GPL(cc_platform_has);
+
 static bool rsi_version_matches(void)
 {
 	unsigned long ver_lower, ver_higher;
@@ -119,6 +132,9 @@ void __init arm64_rsi_init(void)
 	if (arm64_ioremap_prot_hook_register(realm_ioremap_hook))
 		return;
 
+	if (realm_register_memory_enc_ops())
+		return;
+
 	arm64_rsi_setup_memory();
 
 	static_branch_enable(&rsi_present);
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();
 }
 
 static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
@@ -198,6 +202,86 @@ int set_direct_map_default_noflush(struct page *page)
 				   PAGE_SIZE, change_page_range, &data);
 }
 
+static int __set_memory_enc_dec(unsigned long addr,
+				int numpages,
+				bool encrypt)
+{
+	unsigned long set_prot = 0, clear_prot = 0;
+	phys_addr_t start, end;
+	int ret;
+
+	if (!is_realm_world())
+		return 0;
+
+	if (!__is_lm_address(addr))
+		return -EINVAL;
+
+	start = __virt_to_phys(addr);
+	end = start + numpages * PAGE_SIZE;
+
+	if (encrypt)
+		clear_prot = PROT_NS_SHARED;
+	else
+		set_prot = PROT_NS_SHARED;
+
+	/*
+	 * Break the mapping before we make any changes to avoid stale TLB
+	 * entries or Synchronous External Aborts caused by RIPAS_EMPTY
+	 */
+	ret = __change_memory_common(addr, PAGE_SIZE * numpages,
+				     __pgprot(set_prot),
+				     __pgprot(clear_prot | PTE_VALID));
+
+	if (ret)
+		return ret;
+
+	if (encrypt)
+		ret = rsi_set_memory_range_protected(start, end);
+	else
+		ret = rsi_set_memory_range_shared(start, end);
+
+	if (ret)
+		return ret;
+
+	return __change_memory_common(addr, PAGE_SIZE * numpages,
+				      __pgprot(PTE_VALID),
+				      __pgprot(0));
+}
+
+static int realm_set_memory_encrypted(unsigned long addr, int numpages)
+{
+	int ret = __set_memory_enc_dec(addr, numpages, true);
+
+	/*
+	 * If the request to change state fails, then the only sensible cause
+	 * of action for the caller is to leak the memory
+	 */
+	WARN(ret, "Failed to encrypt memory, %d pages will be leaked",
+	     numpages);
+
+	return ret;
+}
+
+static int realm_set_memory_decrypted(unsigned long addr, int numpages)
+{
+	int ret = __set_memory_enc_dec(addr, numpages, false);
+
+	WARN(ret, "Failed to decrypt memory, %d pages will be leaked",
+	     numpages);
+
+	return ret;
+}
+
+static const struct arm64_mem_crypt_ops realm_crypt_ops = {
+	.encrypt = realm_set_memory_encrypted,
+	.decrypt = realm_set_memory_decrypted,
+};
+
+int realm_register_memory_enc_ops(void)
+{
+	return arm64_mem_crypt_ops_register(&realm_crypt_ops);
+}
+
 #ifdef CONFIG_DEBUG_PAGEALLOC
 void __kernel_map_pages(struct page *page, int numpages, int enable)
 {
-- 
2.34.1

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)

On Wed, Feb 19, 2025 at 02:30:28PM +0000, Steven Price wrote:
> 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.

I'd go with (1) since the end result is the same even if we implemented
(2) - i.e. we still avoid block mappings in realms.

> 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");

It's a bit strange to complain about rodata since, in principle, it
doesn't have anything to do with realms. Its only side-effect is that we
avoid block kernel mappings. Maybe "cannot set the kernel direct map,
consider rodata=full" or something like that.

-- 
Catalin

On Wed, Feb 26, 2025 at 07:03:01PM +0000, Catalin Marinas wrote:
> On Wed, Feb 19, 2025 at 02:30:28PM +0000, Steven Price wrote:
> > > @@ -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.
> 
> I'd go with (1) since the end result is the same even if we implemented
> (2) - i.e. we still avoid block mappings in realms.

Is it, though? The config option is about the default behaviour but there's
still an "rodata=" option on the command-line.

Will

On Thu, Feb 27, 2025 at 12:23:31AM +0000, Will Deacon wrote:
> On Wed, Feb 26, 2025 at 07:03:01PM +0000, Catalin Marinas wrote:
> > On Wed, Feb 19, 2025 at 02:30:28PM +0000, Steven Price wrote:
> > > > @@ -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.
> > 
> > I'd go with (1) since the end result is the same even if we implemented
> > (2) - i.e. we still avoid block mappings in realms.
> 
> Is it, though? The config option is about the default behaviour but there's
> still an "rodata=" option on the command-line.

Yeah, that's why I suggested the pr_err() to only state that it cannot
set the direct map and consider rodata=full rather than a config option.
We already force CONFIG_STRICT_KERNEL_RWX.

But we can also revisit the decision not to probe the RSI early.

-- 
Catalin

On Thu, Feb 27, 2025 at 10:55:00AM +0000, Catalin Marinas wrote:
> On Thu, Feb 27, 2025 at 12:23:31AM +0000, Will Deacon wrote:
> > On Wed, Feb 26, 2025 at 07:03:01PM +0000, Catalin Marinas wrote:
> > > On Wed, Feb 19, 2025 at 02:30:28PM +0000, Steven Price wrote:
> > > > > @@ -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.
> > > 
> > > I'd go with (1) since the end result is the same even if we implemented
> > > (2) - i.e. we still avoid block mappings in realms.
> > 
> > Is it, though? The config option is about the default behaviour but there's
> > still an "rodata=" option on the command-line.
> 
> Yeah, that's why I suggested the pr_err() to only state that it cannot
> set the direct map and consider rodata=full rather than a config option.
> We already force CONFIG_STRICT_KERNEL_RWX.

rodata=full has absolutely nothing to do with realms, though. It just
happens to result in the linear map being created at page granularity
and I don't think we should expose that implementation detail like this.

> But we can also revisit the decision not to probe the RSI early.

Alternatively, could we predicate realm support on BBM level-3 w/o TLB
conflicts? Then we could crack the blocks in the linear map.

Will

On Thu, Feb 27, 2025 at 05:22:55PM +0000, Will Deacon wrote:
> On Thu, Feb 27, 2025 at 10:55:00AM +0000, Catalin Marinas wrote:
> > On Thu, Feb 27, 2025 at 12:23:31AM +0000, Will Deacon wrote:
> > > On Wed, Feb 26, 2025 at 07:03:01PM +0000, Catalin Marinas wrote:
> > > > On Wed, Feb 19, 2025 at 02:30:28PM +0000, Steven Price wrote:
> > > > > > @@ -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.
> > > > 
> > > > I'd go with (1) since the end result is the same even if we implemented
> > > > (2) - i.e. we still avoid block mappings in realms.
> > > 
> > > Is it, though? The config option is about the default behaviour but there's
> > > still an "rodata=" option on the command-line.
> > 
> > Yeah, that's why I suggested the pr_err() to only state that it cannot
> > set the direct map and consider rodata=full rather than a config option.
> > We already force CONFIG_STRICT_KERNEL_RWX.
> 
> rodata=full has absolutely nothing to do with realms, though.

I fully agree, that's what I said a couple of emails earlier (towards
the end, not quoted above).

> It just
> happens to result in the linear map being created at page granularity
> and I don't think we should expose that implementation detail like this.

I wasn't keen on adding a new realms=on or whatever command line option,
so I suggested the lazy but confusing rodata=full.

> > But we can also revisit the decision not to probe the RSI early.
> 
> Alternatively, could we predicate realm support on BBM level-3 w/o TLB
> conflicts? Then we could crack the blocks in the linear map.

Long term, I agree that's a better option. It needs wiring up though,
with some care to handle page table allocation failures at run-time. I
think most callers already handle the return code from set_memory_*().

-- 
Catalin

On 27/02/2025 00:23, Will Deacon wrote:
> On Wed, Feb 26, 2025 at 07:03:01PM +0000, Catalin Marinas wrote:
>> On Wed, Feb 19, 2025 at 02:30:28PM +0000, Steven Price wrote:
>>>> @@ -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.
>>
>> I'd go with (1) since the end result is the same even if we implemented
>> (2) - i.e. we still avoid block mappings in realms.
> 
> Is it, though? The config option is about the default behaviour but there's
> still an "rodata=" option on the command-line.

I think the question comes down to is there any value in having page
mappings and not setting the read-only permissions? I.e.
rodata_full=false but we're still avoiding block mappings.

(1) as I've currently proposed doesn't allow that combination - if you
disable rodata_full you also break realms (assuming
DEBUG_PAGEALLOC/kfence don't otherwise force can_set_direct_map().

(2) forces page mappings if there's an RMM present, but does allow
disabling the read-only permissions with "rodata=".

So I guess there's also another option:

(3) Provide another compile/command line flag which forces page mapping
which is different from rodata_full. That would then allow realms
without affecting the permissions.

or indeed:

(4) Change can_set_direct_map() to always return true! ;)

Thanks,
Steve