Re: [PATCH v2 07/13] KVM: arm64: nv: Honor HFGITR_EL2.ERET being set

[Marc Zyngier <maz@kernel.org>]

Hi Joey,

On Fri, 01 Mar 2024 18:07:34 +0000,
Joey Gouly <joey.gouly@arm.com> wrote:
> 
> Got a question about this one,
> 
> On Mon, Feb 26, 2024 at 10:05:55AM +0000, Marc Zyngier wrote:
> > If the L1 hypervisor decides to trap ERETs while running L2,
> > make sure we don't try to emulate it, just like we wouldn't
> > if it had its NV bit set.
> > 
> > The exception will be reinjected from the core handler.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/hyp/vhe/switch.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> > index eaf242b8e0cf..3ea9bdf6b555 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -220,7 +220,8 @@ static bool kvm_hyp_handle_eret(struct kvm_vcpu *vcpu, u64 *exit_code)
> >  	 * Unless the trap has to be forwarded further down the line,
> >  	 * of course...
> >  	 */
> > -	if (__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_NV)
> > +	if ((__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_NV) ||
> > +	    (__vcpu_sys_reg(vcpu, HFGITR_EL2) & HFGITR_EL2_ERET))
> >  		return false;
> >  
> >  	spsr = read_sysreg_el1(SYS_SPSR);
> 
> Are we missing a forward_traps() call in kvm_emulated_nested_eret() for the
> HFGITR case?
> 
> Trying to follow the code path here, and I'm unsure of where else the
> HFIGTR_EL2_ERET trap would be forwarded:
> 
> kvm_arm_vcpu_enter_exit ->
> 	ERET executes in guest
> 	fixup_guest_exit ->
> 		kvm_hyp_handle_eret (returns false)
> 
> handle_exit ->
> 	kvm_handle_eret ->
> 		kvm_emulated_nested_eret
> 			if HCR_NV
> 				forward traps
> 			else
> 				emulate ERET

There's a bit more happening in kvm_handle_eret().

> 
> 
> And if the answer is that it is being reinjected somewhere, putting that
> function name in the commit instead of 'core handler' would help with
> understanding!

Let's look at the code:

	static int kvm_handle_eret(struct kvm_vcpu *vcpu)
	{
		[...]
	
		if (is_hyp_ctxt(vcpu))
			kvm_emulate_nested_eret(vcpu);

If we're doing an ERET from guest EL2, then we just emulate it,
because there is nothing else to do. Crucially, HFGITR_EL2.ERET
doesn't apply to EL2.

		else
			kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu));

In any other case, we simply reinject the trap into the guest EL2,
because that's the only possible outcome. And that's what you were
missing.

		return 1;
	}
	

> I need to find the time to get an NV setup set-up, so I can do some experiments
> myself.

The FVP should be a good enough environment if you can bare the
glacial speed. Other than that, I hear that QEMU has grown some NV
support lately, but I haven't tried it yet. HW-wise, M2 is the only
machine that can be bought by a human being (everything else is
vapourware, or they would have already taken my money).

Thanks,

	M.

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