[PATCH v6 06/64] KVM: arm64: nv: Add nested virt VCPU primitives for vEL2 VCPU state

[Marc Zyngier]

On 2022-02-14 12:39, Miguel Luis wrote:
> Hi Marc,
> 
>> On 28 Jan 2022, at 11:18, Marc Zyngier <maz at kernel.org> wrote:
>> 
>> From: Christoffer Dall <christoffer.dall at arm.com>
>> 
>> When running a nested hypervisor we commonly have to figure out if
>> the VCPU mode is running in the context of a guest hypervisor or guest
>> guest, or just a normal guest.
>> 
>> Add convenient primitives for this.
>> 
>> Reviewed-by: Russell King (Oracle) <rmk+kernel at armlinux.org.uk>
>> Signed-off-by: Christoffer Dall <christoffer.dall at arm.com>
>> Signed-off-by: Marc Zyngier <maz at kernel.org>
>> ---
>> arch/arm64/include/asm/kvm_emulate.h | 53 ++++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>> 
>> diff --git a/arch/arm64/include/asm/kvm_emulate.h 
>> b/arch/arm64/include/asm/kvm_emulate.h
>> index d62405ce3e6d..ea9a130c4b6a 100644
>> --- a/arch/arm64/include/asm/kvm_emulate.h
>> +++ b/arch/arm64/include/asm/kvm_emulate.h
>> @@ -178,6 +178,59 @@ static __always_inline void vcpu_set_reg(struct 
>> kvm_vcpu *vcpu, u8 reg_num,
>> 		vcpu_gp_regs(vcpu)->regs[reg_num] = val;
>> }
>> 
>> +static inline bool vcpu_is_el2_ctxt(const struct kvm_cpu_context 
>> *ctxt)
>> +{
>> +	switch (ctxt->regs.pstate & (PSR_MODE32_BIT | PSR_MODE_MASK)) {
>> +	case PSR_MODE_EL2h:
>> +	case PSR_MODE_EL2t:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
> 
> PSR_MODE_EL2{h,t} values the least significant nibble, so why the
> PSR_MODE32_BIT in the condition?

Because that's part of the M bits in SPSR, and this has to be
valid on any code path, no matter what execution state the
guest is in. You can't evaluate the M[3:0] on their own *unless*
you have already checked that M[4] is 0 (an AArch32 guest would
have M[4]==1).

Yes, we are so far lucky that AArch32 and AArch64 don't overlap
in their values of SPSR_EL2.M[3:0]. We may run out of luck at
some point.

> 
> For the scope of this function as is, may I suggest:
> 
> 	switch (ctxt->regs.pstate & PSR_MODE_MASK) {
> 
> which should be sufficient to check if vcpu_is_el2_ctx.

I don't think this is wise. It makes the code more fragile,
and harder to reason about.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...