On Sat, 25 Sep 2021 01:55:21 +0100,
Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> Invoke the arch hooks for block+unblock if and only if KVM actually
> attempts to block the vCPU. The only non-nop implementation is on arm64,
> and if halt-polling is successful, there is no need for arm64 to put/load
> the vGIC as KVM hasn't relinquished control of the vCPU in any way.
This doesn't mean that there is no requirement for any state
change. The put/load on GICv4 is crucial for performance, and the VMCR
resync is a correctness requirement.
>
> The primary motivation is to allow future cleanup to split out "block"
> from "halt", but this is also likely a small performance boost on arm64
> when halt-polling is successful.
>
> Adjust the post-block path to update "cur" after unblocking, i.e. include
> vGIC load time in halt_wait_ns and halt_wait_hist, so that the behavior
> is consistent. Moving just the pre-block arch hook would result in only
> the vGIC put latency being included in the halt_wait stats. There is no
> obvious evidence that one way or the other is correct, so just ensure KVM
> is consistent.
This effectively reverts 07ab0f8d9a12 ("KVM: Call
kvm_arch_vcpu_blocking early into the blocking sequence"), which was a
huge gain on arm64, not to mention a correctness fix.
Without this, a GICv4 machine will always pay for the full poll
penalty, going into schedule(), and only then get a doorbell interrupt
signalling telling the kernel that there was an interrupt.
On a non-GICv4 machine, it means that interrupts injected by another
thread during the pooling will be evaluated with an outdated priority
mask, which can result in either a spurious wake-up or a missed
wake-up.
If it means introducing a new set of {pre,post}-poll arch-specific
hooks, so be it. But I don't think this change is acceptable as is.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
