On Mon, Feb 10, 2025 at 05:21:59PM +0000, Mark Rutland wrote:
> On Mon, Feb 10, 2025 at 04:53:27PM +0000, Will Deacon wrote:
> > On Thu, Feb 06, 2025 at 02:11:02PM +0000, Mark Rutland wrote:
> > > +static inline void fpsimd_lazy_switch_to_host(struct kvm_vcpu *vcpu)
> > > +{
> > > + u64 zcr_el1, zcr_el2;
> > > +
> > > + if (!guest_owns_fp_regs())
> > > + return;
> > > +
> > > + if (vcpu_has_sve(vcpu)) {
> > > + zcr_el1 = read_sysreg_el1(SYS_ZCR);
> > > + __vcpu_sys_reg(vcpu, vcpu_sve_zcr_elx(vcpu)) = zcr_el1;
> > > +
> > > + /*
> > > + * The guest's state is always saved using the guest's max VL.
> > > + * Ensure that the host has the guest's max VL active such that
> > > + * the host can save the guest's state lazily, but don't
> > > + * artificially restrict the host to the guest's max VL.
> > > + */
> > > + if (has_vhe()) {
> > > + zcr_el2 = vcpu_sve_max_vq(vcpu) - 1;
> > > + write_sysreg_el2(zcr_el2, SYS_ZCR);
> > > + } else {
> > > + zcr_el2 = sve_vq_from_vl(kvm_host_sve_max_vl) - 1;
> > > + write_sysreg_el2(zcr_el2, SYS_ZCR);
> > > +
> > > + zcr_el1 = vcpu_sve_max_vq(vcpu) - 1;
> > > + write_sysreg_el1(zcr_el1, SYS_ZCR);
> >
> > Do we need an ISB before this to make sure that the CPTR traps have been
> > deactivated properly?
>
> Sorry, I had meant to add a comment here that this relies upon a
> subtlety that avoids the need for the ISB.
Ah yes, it really all hinges on guest_owns_fp_regs() and so I think a
comment would be helpful, thanks.
Just on this, though:
> When the guest owns the FP regs here, we know:
>
> * If the guest doesn't have SVE, then we're not poking anything, and so
> no ISB is necessary.
>
> * If the guest has SVE, then either:
>
> - The guest owned the FP regs when it was entered.
>
> - The guest *didn't* own the FP regs when it was entered, but acquired
> ownership via a trap which executed kvm_hyp_handle_fpsimd().
>
> ... and in either case, *after* disabling the traps there's been an
> ERET to the guest and an exception back to hyp, either of which
> provides the necessary context synchronization such that the traps are
> disabled here.
What about the case where we find that there's an interrupt pending on
return to the guest? In that case, I think we elide the ERET and go back
to the host (see the check of isr_el1 in hyp/entry.S).
Will
