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

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

Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:

> 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.
>

I guess you are suggesting to leave it to firmware to set up the device
tree "reg-offset" with shared bit set?

-aneesh