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 09:33:50AM +0100, Ryan Roberts wrote:
> On 02/04/2026 21:43, Catalin Marinas wrote:
> > On Mon, Mar 30, 2026 at 05:17:02PM +0100, Ryan Roberts wrote:
> >>  int split_kernel_leaf_mapping(unsigned long start, unsigned long end)
> >>  {
> >>  	int ret;
> >>  
> >> -	/*
> >> -	 * !BBML2_NOABORT systems should not be trying to change permissions on
> >> -	 * anything that is not pte-mapped in the first place. Just return early
> >> -	 * and let the permission change code raise a warning if not already
> >> -	 * pte-mapped.
> >> -	 */
> >> -	if (!system_supports_bbml2_noabort())
> >> -		return 0;
> >> -
> >>  	/*
> >>  	 * If the region is within a pte-mapped area, there is no need to try to
> >>  	 * split. Additionally, CONFIG_DEBUG_PAGEALLOC and CONFIG_KFENCE may
> >>  	 * change permissions from atomic context so for those cases (which are
> >>  	 * always pte-mapped), we must not go any further because taking the
> >> -	 * mutex below may sleep.
> >> +	 * mutex below may sleep. Do not call force_pte_mapping() here because
> >> +	 * it could return a confusing result if called from a secondary cpu
> >> +	 * prior to finalizing caps. Instead, linear_map_requires_bbml2 gives us
> >> +	 * what we need.
> >>  	 */
> >> -	if (force_pte_mapping() || is_kfence_address((void *)start))
> >> +	if (!linear_map_requires_bbml2 || is_kfence_address((void *)start))
> >>  		return 0;
> >>  
> >> +	if (!system_supports_bbml2_noabort()) {
> >> +		/*
> >> +		 * !BBML2_NOABORT systems should not be trying to change
> >> +		 * permissions on anything that is not pte-mapped in the first
> >> +		 * place. Just return early and let the permission change code
> >> +		 * raise a warning if not already pte-mapped.
> >> +		 */
> >> +		if (system_capabilities_finalized())
> >> +			return 0;
> >> +
> >> +		/*
> >> +		 * Boot-time: split_kernel_leaf_mapping_locked() allocates from
> >> +		 * page allocator. Can't split until it's available.
> >> +		 */
> >> +		if (WARN_ON(!page_alloc_available))
> >> +			return -EBUSY;
> >> +
> >> +		/*
> >> +		 * Boot-time: Started secondary cpus but don't know if they
> >> +		 * support BBML2_NOABORT yet. Can't allow splitting in this
> >> +		 * window in case they don't.
> >> +		 */
> >> +		if (WARN_ON(num_online_cpus() > 1))
> >> +			return -EBUSY;
> >> +	}
> > 
> > I think sashiko is over cautions here
> > (https://sashiko.dev/#/patchset/20260330161705.3349825-1-ryan.roberts@arm.com)
> > but it has a somewhat valid point from the perspective of
> > num_online_cpus() semantics. We have have num_online_cpus() == 1 while
> > having a secondary CPU just booted and with its MMU enabled. I don't
> > think we can have any asynchronous tasks running at that point to
> > trigger a spit though. Even async_init() is called after smp_init().
> 
> Yes I saw the Sashiko report, but we had previously had a (private) discussion
> where I thought we had already concluded that this approach is safe in practice
> due to the way that the boot cpu brings the secondaries online.

Yes, I thought I'd mention it. I don't see how this could go wrong in
practice as we don't expect any page splitting on the primary CPU while
it waits for the secondaries to come up.

> > An option may be to attempt cpus_read_trylock() as this lock is taken by
> > _cpu_up(). If it fails, return -EBUSY, otherwise check num_online_cpus()
> > and unlock (and return -EBUSY if secondaries already started).
> 
> That sounds neat; I could dig deeper and have a go at something like this if you
> want?

I don't think it's worth it. We'll probably need to keep the lock for
the duration of the splitting (though that's only before the cpu
features are fully initialised). It's something we can look into if we
ever see an issue here. For now, the num_online_cpus() test will do.

> > 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).
> 
> My understanding is that this is a pre-existing (and known) bug. It's not
> related to the "map linear map by large leaves and split dynamically" feature so
> wasn't attempting to fix it.

Yes, it's an existing bug I noticed while trying to understand the code
paths. It doesn't need any action on this series but we should try to
fix it separately.

-- 
Catalin