linux-kernel - Re: [PATCH 07/14] KVM: Don't block+unblock when halt-polling is successful

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <878rzlass2.wl-maz@kernel.org>
Date:   Sat, 25 Sep 2021 10:50:05 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Huacai Chen <chenhuacai@...nel.org>,
        Aleksandar Markovic <aleksandar.qemu.devel@...il.com>,
        Paul Mackerras <paulus@...abs.org>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Janosch Frank <frankja@...ux.ibm.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        James Morse <james.morse@....com>,
        Alexandru Elisei <alexandru.elisei@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        David Hildenbrand <david@...hat.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-mips@...r.kernel.org, kvm@...r.kernel.org,
        kvm-ppc@...r.kernel.org, linux-kernel@...r.kernel.org,
        David Matlack <dmatlack@...gle.com>,
        Jing Zhang <jingzhangos@...gle.com>
Subject: Re: [PATCH 07/14] KVM: Don't block+unblock when halt-polling is successful

On Sat, 25 Sep 2021 01:55:21 +0100,
Sean Christopherson <seanjc@...gle.com> 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.