Re: [PATCH v7 09/11] arm64: Enable memory encrypt for Realms

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

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