[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z79lZdYqWINaHfrp@arm.com>
Date: Wed, 26 Feb 2025 19:03:01 +0000
From: Catalin Marinas <catalin.marinas@....com>
To: Steven Price <steven.price@....com>
Cc: "Aneesh Kumar K . V" <aneesh.kumar@...nel.org>,
Will Deacon <will@...nel.org>,
Suzuki K Poulose <suzuki.poulose@....com>,
Marc Zyngier <maz@...nel.org>, James Morse <james.morse@....com>,
Oliver Upton <oliver.upton@...ux.dev>,
Zenghui Yu <yuzenghui@...wei.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Joey Gouly <joey.gouly@....com>,
Alexandru Elisei <alexandru.elisei@....com>,
Christoffer Dall <christoffer.dall@....com>,
Fuad Tabba <tabba@...gle.com>, linux-coco@...ts.linux.dev,
Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>,
Gavin Shan <gshan@...hat.com>,
Shanker Donthineni <sdonthineni@...dia.com>,
Alper Gun <alpergun@...gle.com>, kvmarm@...ts.linux.dev,
kvm@...r.kernel.org
Subject: Re: [PATCH v7 09/11] arm64: Enable memory encrypt for Realms
On Wed, Feb 19, 2025 at 02:30:28PM +0000, Steven Price wrote:
> On 17/10/2024 14:14, Steven Price wrote:
> > From: Suzuki K Poulose <suzuki.poulose@....com>
> >
> > Use the memory encryption APIs to trigger a RSI call to request a
> > transition between protected memory and shared memory (or vice versa)
> > and updating the kernel's linear map of modified pages to flip the top
> > bit of the IPA. This requires that block mappings are not used in the
> > direct map for realm guests.
> >
> > Reviewed-by: Catalin Marinas <catalin.marinas@....com>
> > Reviewed-by: Gavin Shan <gshan@...hat.com>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
> > Co-developed-by: Steven Price <steven.price@....com>
> > Signed-off-by: Steven Price <steven.price@....com>
> > ---
> [...]
> > diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> > index 547a9e0b46c2..6ae6ae806454 100644
> > --- a/arch/arm64/mm/pageattr.c
> > +++ b/arch/arm64/mm/pageattr.c
> > @@ -5,10 +5,12 @@
> > #include <linux/kernel.h>
> > #include <linux/mm.h>
> > #include <linux/module.h>
> > +#include <linux/mem_encrypt.h>
> > #include <linux/sched.h>
> > #include <linux/vmalloc.h>
> >
> > #include <asm/cacheflush.h>
> > +#include <asm/pgtable-prot.h>
> > #include <asm/set_memory.h>
> > #include <asm/tlbflush.h>
> > #include <asm/kfence.h>
> > @@ -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.
> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
> index ce4778141ec7..48a6ef0f401c 100644
> --- a/arch/arm64/kernel/rsi.c
> +++ b/arch/arm64/kernel/rsi.c
> @@ -126,6 +126,10 @@ void __init arm64_rsi_init(void)
> return;
> if (!rsi_version_matches())
> return;
> + if (!can_set_direct_map()) {
> + pr_err("rodata_full disabled, unable to run as a realm guest. Please enable CONFIG_RODATA_FULL_DEFAULT_ENABLED\n");
It's a bit strange to complain about rodata since, in principle, it
doesn't have anything to do with realms. Its only side-effect is that we
avoid block kernel mappings. Maybe "cannot set the kernel direct map,
consider rodata=full" or something like that.
--
Catalin
Powered by blists - more mailing lists