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

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, Jul 31, 2024 at 09:26:31PM +0530, Aneesh Kumar K.V wrote:
> 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:
> >>> > 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?

As you know, we've been through these options internally and we
concluded not to encode this information in the DT for various reasons.

Personally I don't like this IPA split but that's too late to change it
now in the RMM spec.

-- 
Catalin