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