On Mon, Feb 03, 2025, Paolo Bonzini wrote: > On 2/3/25 19:45, Sean Christopherson wrote: > > Unless there's a very, very good reason to support a use case that generates > > ExtInts during boot, but _only_ during boot, and otherwise doesn't have any APICv > > ihibits, I'm leaning towards making SVM's IRQ window inhibit sticky, i.e. never > > clear it. > > BIOS tends to use PIT, so that may be too much. With respect to Naveen's report > of contention on apicv_update_lock, I would go with the sticky-bit idea but apply > it to APICV_INHIBIT_REASON_PIT_REINJ. That won't work, at least not with yet more changes, because KVM creates the in-kernel PIT with reinjection enabled by default. The stick-bit idea is that if a bit is set and can never be cleared, then there's no need to track new updates. Since userspace needs to explicitly disable reinjection, the inhibit can't be sticky. I assume We could fudge around that easily enough by deferring the inhibit until a vCPU is created (or run?), but piggybacking PIT_REINJ won't help the userspace I/O APIC case. > I don't love adding another inhibit reason but, together, these two should > remove the contention on apicv_update_lock. Another idea could be to move > IRQWIN to per-vCPU reason but Maxim tells me that it's not so easy. Oh, yeah, that reminds me of the other reason I would vote for a sticky flag: if inhibition really is toggling rapidly, performance is going to be quite bad because inhibiting APICv requires (a) zapping APIC SPTEs and (b) serializing writers if multiple vCPUs trigger the 0=>1 transition. And there's some amount of serialization even if there's only a single writer, as KVM kicks all vCPUs to toggle APICv (and again to flush TLBs, if necessary). Hmm, something doesn't add up. Naveen's changelog says: KVM additionally inhibits AVIC for requesting a IRQ window every time it has to inject external interrupts resulting in a barrage of inhibits being set and cleared. This shows significant performance degradation compared to AVIC being disabled, due to high contention on apicv_update_lock. But if this is a "real world" use case where the only source of ExtInt is the PIT, and kernels typically only wire up the PIT to the BSP, why is there contention on apicv_update_lock? APICv isn't actually being toggled, so readers blocking writers to handle KVM_REQ_APICV_UPDATE shouldn't be a problem. Naveen, do you know why there's a contention on apicv_update_lock? Are multiple vCPUs actually trying to inject ExtInt?
