All of lore.kernel.org
 help / color / mirror / Atom feed
From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>,
	Will Deacon <will@kernel.org>,
	"David Hildenbrand (Arm)" <david@kernel.org>,
	Dev Jain <dev.jain@arm.com>,
	Yang Shi <yang@os.amperecomputing.com>,
	Jinjiang Tu <tujinjiang@huawei.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH v2 1/3] arm64: mm: Fix rodata=full block mapping support for realm guests
Date: Thu, 9 Apr 2026 10:38:03 +0100	[thread overview]
Message-ID: <d1ecba64-898f-433b-93d4-7a33b9c3f378@arm.com> (raw)
In-Reply-To: <adU9KxLC7yKgmyJy@arm.com>

On 07/04/2026 18:21, Catalin Marinas wrote:
> On Tue, Apr 07, 2026 at 10:57:35AM +0100, Suzuki K Poulose 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().
>>>
>>> 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).
>>>
>>> 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).
>>
>> That is correct. map_mem() comes from paginig_init(), which gets called
>> before arm64_rsi_init(). Realm check was delayed until psci_xx_init().
>> We had a version which parsed the DT for PSCI conduit early enough
>> to be able to make the SMC calls to detect the Realm. But there
>> were concerns around it.
> 
> Ah, yes, I remember.
> 
> Does it mean that commit 42be24a4178f ("arm64: Enable memory encrypt for
> Realms") was broken without rodata=full w.r.t. the linear map? Commit

Apparently, it looks like we missed this when we demoted the RSI
detection later.

> a166563e7ec3 ("arm64: mm: support large block mapping when rodata=full")
> introduced force_pte_mapping() but it just copied the logic in the
> existing can_set_direct_map(). Looking at the linear_map_requires_bbml2
> assignment, we get (!is_realm_world() && is_realm_world()) and it
> cancels out, no effect on it but we don't get pte mappings either (even
> if we don't have BBML2).

Yep, that's right.
> 
> I think we need at least some safety checks:
> 
> 1. BBML2_NOABORT support on the boot CPU - continue with the existing
>     logic (as per Ryan's series)
> 
> 2. !system_supports_bbml2_noabort() - split in
>     linear_map_maybe_split_to_ptes(). This does not currently happen
>     because linear_map_requires_bbml2 may be false in the absence of
>     rodata=full. Not sure how to fix this without some variable telling
>     us how the linear map was mapped. The requires_bbml2 flag doesn't
> 
> 3. Panic in arm64_rsi_init() if !BBML2_NOABORT on the boot CPU _and_ we
>     have block mappings already. People can avoid it with rodata=full

It looks like this will be a common case :-(

> 
> 4. If (3) is a common case, a better alternative is to rewrite the
>     linear map sometime after arm64_rsi_init() but before we call
>     split_kernel_leaf_mapping().

We will explore this route.

The other option is to move the RSI detection (and the PSCI probe)
earlier to be able to make better decisions early on. I will play with
that a bit too.

Suzuki


> 



  reply	other threads:[~2026-04-09  9:38 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-30 16:17 [PATCH v2 0/3] Fix bugs for realm guest plus BBML2_NOABORT Ryan Roberts
2026-03-30 16:17 ` [PATCH v2 1/3] arm64: mm: Fix rodata=full block mapping support for realm guests Ryan Roberts
2026-03-31 14:35   ` Suzuki K Poulose
2026-04-02 20:43   ` Catalin Marinas
2026-04-03 10:31     ` Catalin Marinas
2026-04-07  8:43       ` Ryan Roberts
2026-04-07  9:32         ` Catalin Marinas
2026-04-07 10:13           ` Ryan Roberts
2026-04-07 10:52             ` Catalin Marinas
2026-04-07 13:06               ` Ryan Roberts
2026-04-07 17:37                 ` Catalin Marinas
2026-04-09  9:53               ` Kevin Brodsky
2026-04-09 15:20                 ` Catalin Marinas
2026-04-09 16:48                   ` Yang Shi
2026-04-09 18:33                     ` Catalin Marinas
2026-04-09 23:08                       ` Yang Shi
2026-04-13 14:57                         ` Kevin Brodsky
2026-04-16 23:41                           ` Yang Shi
2026-04-07  8:33     ` Ryan Roberts
2026-04-07  9:19       ` Catalin Marinas
2026-04-07  9:57     ` Suzuki K Poulose
2026-04-07 17:21       ` Catalin Marinas
2026-04-09  9:38         ` Suzuki K Poulose [this message]
2026-04-09 14:09           ` Catalin Marinas
2026-04-09 14:18           ` Suzuki K Poulose
2026-04-13 11:47   ` Kevin Brodsky
2026-03-30 16:17 ` [PATCH v2 2/3] arm64: mm: Handle invalid large leaf mappings correctly Ryan Roberts
2026-03-30 16:17 ` [PATCH v2 3/3] arm64: mm: Remove pmd_sect() and pud_sect() Ryan Roberts
2026-04-02 21:11 ` [PATCH v2 0/3] Fix bugs for realm guest plus BBML2_NOABORT Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d1ecba64-898f-433b-93d4-7a33b9c3f378@arm.com \
    --to=suzuki.poulose@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=kevin.brodsky@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=stable@vger.kernel.org \
    --cc=tujinjiang@huawei.com \
    --cc=will@kernel.org \
    --cc=yang@os.amperecomputing.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.