[PATCH v4 00/66] KVM: arm64: ARMv8.3/8.4 Nested Virtualization support

[Marc Zyngier]

On Thu, 03 Jun 2021 09:39:09 +0100,
Marc Zyngier <maz at kernel.org> wrote:
> 
> Hi Jamie,
> 
> Funny, your email has a "Mail-Followup-To:" field that contains
> everyone but you... Not ideal! ;-)
> 
> On Thu, 03 Jun 2021 08:07:22 +0100,
> Jamie Iles <jamie at nuviainc.com> wrote:
> > 
> > Hi Marc,
> > 
> > On Mon, May 10, 2021 at 05:58:14PM +0100, Marc Zyngier wrote:
> > > Here the bi-annual drop of the KVM/arm64 NV support code.
> > > 
> > > Not a lot has changed since [1], except for a discovery mechanism for
> > > the EL2 support, some tidying up in the idreg emulation, dropping RMR
> > > support, and a rebase on top of 5.13-rc1.
> > > 
> > > As usual, blame me for any bug, and nobody else.
> > > 
> > > It is still massively painful to run on the FVP, but if you have a
> > > Neoverse V1 or N2 system that is collecting dust, I have the right
> > > stuff to keep it busy!
> > 
> > I've been testing this series on FVP and get a crash when returning from 
> > __kvm_vcpu_run_vhe because the autiasp is failing.
> 
> Ah, the joy of testing with older guests. I guess i should upgrade by
> test rig and play with some newer guests at L1.
> 
> > 
> > The problem is when the L1 boots and during EL2 setup sets hcr_el2 to 
> > HCR_HOST_NVHE_FLAGS and so enables HCR_APK|HCR_API.  Then the guest 
> > enter+exit logic in L0 starts performing the key save restore, but as we 
> > didn't go through __hyp_handle_ptrauth, we haven't saved the host keys 
> > and invoked vcpu_ptrauth_enable() so restore the host keys back to 0.
> > 
> > I wonder if the pointer auth keys should be saved+restored 
> > unconditionally for a guest when running nested rather than the lazy 
> > faulting that we have today?
> 
> I'd like to try and avoid that in order to keep the basic logic as
> simple as possible for the time being, and as close to the tried and
> trusted flow we have today.
> 
> > Alternatively we would need to duplicate
> > the lazy logic for hcr_el2 writes.  A quick hack of saving the host keys 
> > in __kvm_vcpu_run_vhe before sysreg_save_host_state_vhe is enough to 
> > allow me to boot an L1 with --nested and then an L2.
> >
> > Do we also need to filter out HCR_APK|HCR_API for hcr_el2 writes when 
> > pointer authentication hasn't been exposed to the guest?  I haven't yet 
> > tried making ptrauth visible to the L1.
> 
> I think this is the real thing. We should never propagate trap bits
> for features we don't want to support in guests. The L1 kernel sets
> these bits unconditionally, despite PtrAuth never being advertised,
> which trips the host code.
> 
> Could you try the untested hack below?
> 
> Thanks,
> 
> 	M.
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index ce682bcce56f..54301d5ce58c 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -82,6 +82,7 @@
>  			 HCR_BSU_IS | HCR_FB | HCR_TAC | \
>  			 HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
>  			 HCR_FMO | HCR_IMO | HCR_PTW )
> +#define HCR_GUEST_NV_FILTER_FLAGS (HCR_ATA | HCR_API | HCR_APK | HCR_RW)
>  #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
>  #define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA)
>  #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC)
> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index 67f8b7d89db6..bf39bf4ef63c 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -64,6 +64,8 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
>  			 */
>  			u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2);
>  
> +			vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS;
> +
>  			/*
>  			 * We already set TVM to handle set/way cache maint
>  			 * ops traps, this somewhat collides with the nested
> @@ -91,7 +93,10 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
>  				       SYS_VNCR_EL2);
>  		}
>  	} else if (nested_virt_in_use(vcpu)) {
> -		hcr |= __vcpu_sys_reg(vcpu, HCR_EL2);
> +		u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2);
> +
> +		vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS;
> +		hcr |= vhcr_el2;
>  	}
>  
>  	___activate_traps(vcpu, hcr);
> 

FWIW, I can boot a 5.13-rc4 kernel as a L1 guest without any visible
issue with this patch (and another guest as L2). I've folded this into
the series and pushed the result out (with a rebase on -rc4 for a good
measure).

Thanks again,

	M.

-- 
Without deviation from the norm, progress is not possible.