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

[Suzuki K Poulose]

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
2. If the address is missed, it is fatal for the Realm.
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)

Also, we are making some changes to the guest support to make it future
proof for running the same guest in a less privileged context within
R_EL1 (read unprivileged plane with RMM v1.1), where the console
could be "private". And we are modifying the code to "apply" the prot_ns
dynamically (instead of hard coding it for all I/O).

Suzuki

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