Re: [PATCH v2 8/8] KVM: arm64: Eagerly switch ZCR_EL{1,2}

[Will Deacon <will@kernel.org>]

On Mon, Feb 10, 2025 at 06:56:51PM +0000, Mark Rutland wrote:
> On Mon, Feb 10, 2025 at 06:20:09PM +0000, Will Deacon wrote:
> > 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:
> > > 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).
> 
> Ah; I had missed that, and evidently I had not looked at the entry code.
> 
> Given that, I think the options are:
> 
> (a) Add an ISB after disabling the traps, before returning to the guest.
> 
> (b) Add an ISB in fpsimd_lazy_switch_to_host() above.
> 
> (c) Add an ISB in that sequence in hyp/entry.S, just before the ret, to
>     ensure that __guest_enter() always provides a context
>     synchronization event even when it doesn't enter the guest,
>     regardless of ARM64_HAS_RAS_EXTN.
> 
> I think (c) is probably the nicest, since that avoids the need for
> redundant barriers in the common case, and those short-circuited exits
> are hopefully rare.

(c) sounds like the most robust thing to do and, even though the ISB
might be expensive, we're still avoiding an ERET+IRQ.

> Obviously that would mean adding comments in both __guest_enter() and
> fpsimd_lazy_switch_to_host().

Yup.

Cheers,

Will