[PATCH 1/6] firmware/smccc: Call arch-specific hook on discovering KVM services

[Will Deacon]

On Fri, Aug 02, 2024 at 04:30:30PM +0100, Suzuki K Poulose wrote:
> On 31/07/2024 16:50, Will Deacon wrote:
> > On Wed, Jul 31, 2024 at 08:11:16PM +0530, Aneesh Kumar K.V wrote:
> > > Will Deacon <will at kernel.org> writes:
> > > 
> > > > From: Marc Zyngier <maz at kernel.org>
> > > > 
> > > > arm64 will soon require its own callback to initialise services
> > > > that are only available on this architecture. Introduce a hook
> > > > that can be overloaded by the architecture.
> > > > 
> > > > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > > > Signed-off-by: Will Deacon <will at kernel.org>
> > > > ---
> > > >   arch/arm/include/asm/hypervisor.h   | 2 ++
> > > >   arch/arm64/include/asm/hypervisor.h | 4 ++++
> > > >   drivers/firmware/smccc/kvm_guest.c  | 2 ++
> > > >   3 files changed, 8 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/include/asm/hypervisor.h b/arch/arm/include/asm/hypervisor.h
> > > > index bd61502b9715..8a648e506540 100644
> > > > --- a/arch/arm/include/asm/hypervisor.h
> > > > +++ b/arch/arm/include/asm/hypervisor.h
> > > > @@ -7,4 +7,6 @@
> > > >   void kvm_init_hyp_services(void);
> > > >   bool kvm_arm_hyp_service_available(u32 func_id);
> > > > +static inline void kvm_arch_init_hyp_services(void) { };
> > > > +
> > > >   #endif
> > > > diff --git a/arch/arm64/include/asm/hypervisor.h b/arch/arm64/include/asm/hypervisor.h
> > > > index 0ae427f352c8..8cab2ab535b7 100644
> > > > --- a/arch/arm64/include/asm/hypervisor.h
> > > > +++ b/arch/arm64/include/asm/hypervisor.h
> > > > @@ -7,4 +7,8 @@
> > > >   void kvm_init_hyp_services(void);
> > > >   bool kvm_arm_hyp_service_available(u32 func_id);
> > > > +static inline void kvm_arch_init_hyp_services(void)
> > > > +{
> > > > +};
> > > > +
> > > >   #endif
> > > > diff --git a/drivers/firmware/smccc/kvm_guest.c b/drivers/firmware/smccc/kvm_guest.c
> > > > index 89a68e7eeaa6..f3319be20b36 100644
> > > > --- a/drivers/firmware/smccc/kvm_guest.c
> > > > +++ b/drivers/firmware/smccc/kvm_guest.c
> > > > @@ -39,6 +39,8 @@ void __init kvm_init_hyp_services(void)
> > > >   	pr_info("hypervisor services detected (0x%08lx 0x%08lx 0x%08lx 0x%08lx)\n",
> > > >   		 res.a3, res.a2, res.a1, res.a0);
> > > > +
> > > > +	kvm_arch_init_hyp_services();
> > > >   }
> > > > 
> > > 
> > > That is a bit late to detect RMM? One of the requirements is to
> > > figure out the pgprot_t flags for early_ioremap so that "earlycon" will
> > > work (by mapping the address as shared alias). To do that we need to
> > > make an RSI call to detect PROT_NS_SHARED mask as below.
> > > 
> > > 	if (rsi_get_realm_config(&config))
> > > 		return;
> > > 	prot_ns_shared = BIT(config.ipa_bits - 1);
> > 
> > Why can't the earlycon MMIO address just have that high bit set?
> 
> Do you mean the "earlycon=<MMIO-address-to-use>" ? That could work,
> except that :
> 1. It breaks the Realm's view of the {I}"PA" space

Breaks in what sense? It won't work?

> 2. If the address is missed, it is fatal for the Realm.

If earlycon= has broken/missing addresses, it's going to be fatal for
whoever is consuming it anyway, no?

> 3. All higher level tools that specify the command line parameter now
>    need to fixup the "MMIO" address, based on the "ipa_bits" chosen by
>    the VMM (which could vary with the VMMs and Hyp/System)

Should be fine for earlycon, though?

If not, then perhaps the RSI should include calls for putchar() so that
earlycon can be implemented using a service rather than trying to use
MMIO with a bunch of awkward caveats.

Will