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

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Will Deacon <will@kernel.org> writes:

> On Wed, Jul 31, 2024 at 08:11:16PM +0530, Aneesh Kumar K.V wrote:
>> Will Deacon <will@kernel.org> writes:
>> 
>> > From: Marc Zyngier <maz@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@kernel.org>
>> > Signed-off-by: Will Deacon <will@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?
>
> I think it's horribly fragile to try detecting all of this stuff before
> we're allowed to touch the console. We don't even bother with pKVM --
> it's the guest firmware's responsibility to MMIO_GUARD the UART if it
> detects a debuggable payload.
>

To mark something shared, we need to know the mask value which is
returned via rsi_get_realm_config() call.

-aneesh