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

[Ryan Roberts]

On 07/04/2026 11:52, Catalin Marinas wrote:
> 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.

OK, I read it as "might" as in a hint that we might want to change the direct
map permissions.

> 
> 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).

ACK.

> 
>>> 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:

I'll admit it is ugly but it's not a bug; the system capabilitites are finalized
by the time we call linear_map_maybe_split_to_ptes().

The "if (!linear_map_requires_bbml2 || is_kfence_address((void *)start))" check
in split_kernel_leaf_mapping() would ideally be "if (!force_pte_mapping() ||
is_kfence_address((void *)start))", but it is not safe to call
force_pte_mapping() from a secondary cpu prior to finalizing the system caps.
I'm reusing the flag that I already had available to work around that.

> 
> 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.

I'm not sure I see the distinction between "might" and "can" with your
definition. But regardless, I think we are talking about the pre-existing
is_real_world() bug, so I'm not personally planning to do anything further here
unless you shout.

Thanks,
Ryan