Re: [PATCH v2 1/3] arm64: mm: Fix rodata=full block mapping support for realm guests

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Apr 07, 2026 at 11:13:07AM +0100, Ryan Roberts wrote:
> On 07/04/2026 10:32, Catalin Marinas wrote:
> > On Tue, Apr 07, 2026 at 09:43:42AM +0100, Ryan Roberts wrote:
> >> On 03/04/2026 11:31, Catalin Marinas wrote:
> >>> On Thu, Apr 02, 2026 at 09:43:59PM +0100, Catalin Marinas wrote:
> >>>> Another thing I couldn't get my head around - IIUC is_realm_world()
> >>>> won't return true for map_mem() yet (if in a realm). Can we have realms
> >>>> on hardware that does not support BBML2_NOABORT? We may not have
> >>>> configuration with rodata_full set (it should be complementary to realm
> >>>> support).
> >>>
> >>> With rodata_full==false, can_set_direct_map() returns false initially
> >>> but after arm64_rsi_init() it starts returning true if is_realm_world().
> >>> The side-effect is that map_mem() goes for block mappings and
> >>> linear_map_requires_bbml2 set to false. Later on,
> >>> linear_map_maybe_split_to_ptes() will skip the splitting.
> >>>
> >>> Unless I'm missing something, is_realm_world() calls in
> >>> force_pte_mapping() and can_set_direct_map() are useless. I'd remove
> >>> them and either require BBML2_NOABORT with CCA or get the user to force
> >>> rodata_full when running in realms. Or move arm64_rsi_init() even
> >>> earlier?
> >>
> >> I'd need Suzuki to comment on this. As I said in the other mail, I was treating
> >> this like a pre-existing bug. But I guess linear_map_requires_bbml2 ending up
> >> wrong is a problem here. I'm not sure it's quite as simple as requiring
> >> BBML2_NOABORT with CCA as we still need can_set_direct_map() to return true if
> >> we are in a realm.
> > 
> > can_set_direct_map() == true is not a property of the realm but rather a
> > requirement. 
> 
> Yes indeed. It would be better to call it might_set_direct_map() or something
> like that...

The way it is used means "is allowed to set the direct map". I guess
"may set..." works as well. My reading of "might" is more like in
might_sleep(), more of hint than a permission check.

If you only look at the linear_map_requires_bbml2 setting in map_mem(),
yes, something like might_set_direct_map() makes sense but that's not
how this function is used in the rest of the kernel (to reject the
direct map change if not supported).

> > In the absence of BBML2_NOABORT, I guess the test was added
> > under the assumption that force_pte_mapping() also returns true if
> > is_realm_world(). We might as well add a variable or static label to
> > track whether can_set_direct_map() is possible and avoid tests that
> > duplicate force_pte_mapping().
> 
> I'm not sure I follow. We have linear_map_requires_bbml2 which is inteded to
> track this shape of thing;

As the name implies, linear_map_requires_bbml2 tracks only this -
BBML2_NOABORT is required because the linear map uses large blocks.
Prior to your patches, that's only used as far as
linear_map_maybe_split_to_ptes() and if splitting took place, this
variable is no longer relevant (should be turned to false but since it's
not used, it doesn't matter).

With your patches, its use was extended to runtime and I think it
remains true even if linear_map_maybe_split_to_ptes() changed the block
mappings. Do we need this:

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index dcee56bb622a..595d35fdd8c3 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -988,6 +988,7 @@ void __init linear_map_maybe_split_to_ptes(void)
 	if (linear_map_requires_bbml2 && !system_supports_bbml2_noabort()) {
 		init_idmap_kpti_bbml2_flag();
 		stop_machine(linear_map_split_to_ptes, NULL, cpu_online_mask);
+		linear_map_requires_bbml2 = false;
 	}
 }
 

> if we have forced pte mapping then the value of
> can_set_direct_map() is irrelevant - we will never need to split because we are
> already pte-mapped.

can_set_direct_map() is used in other places, so its value is relevant,
e.g. sys_memfd_secret() is rejected if this function returns false.

> But if can_set_direct_map() initially returns false because
> is_realm_world() incorrectly returns false in the early boot environment, then
> linear_map_requires_bbml2 will be set to false, and we will incorrectly
> short-circuit splitting any block mappings in split_kernel_leaf_mapping().
> 
> I think we are agreed on the problem. But I don't understand how tracking
> can_set_direct_map() in a cached variable helps with that.

It's not about the map_mem() decision and linear_map_requires_bbml2
setting but rather its other uses like sys_memfd_secret().

> > This won't solve the is_realm_world() changing polarity during boot but
> > at least we know it won't suddenly make can_set_direct_map() return
> > true when it shouldn't.
> 
> But is_real_world() _should_ make can_set_direct_map() return true, shouldn't
> it?

Yes but not directly. If is_realm_world() is true, we either have
(linear_map_requires_bbml2 && system_supports_bbml2_noabort()) or
linear_map_requires_bbml2 is false and we have pte mappings. Adding
is_realm_world() to can_set_direct_map() does not imply any of these.
It's just a hope that something before actually ensured the conditions
are true.

It might be better if we rename the current function to
might_set_direct_map() and introduce a new can_set_direct_map() that
actually tells the truth if all the conditions are met. I suggested a
variable or static label but checking some conditions related to the
actual linear map work as well, just not is_realm_world() directly.

-- 
Catalin