From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Marc Zyngier <maz@kernel.org>,
Huacai Chen <chenhuacai@kernel.org>,
Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>,
Paul Mackerras <paulus@ozlabs.org>,
Anup Patel <anup.patel@wdc.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Janosch Frank <frankja@linux.ibm.com>,
James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Atish Patra <atish.patra@wdc.com>,
David Hildenbrand <david@redhat.com>,
Cornelia Huck <cohuck@redhat.com>,
Claudio Imbrenda <imbrenda@linux.ibm.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org,
kvm@vger.kernel.org, kvm-ppc@vger.kernel.org,
kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org, David Matlack <dmatlack@google.com>,
Oliver Upton <oupton@google.com>,
Jing Zhang <jingzhangos@google.com>
Subject: Re: [PATCH v2 39/43] KVM: VMX: Don't do full kick when triggering posted interrupt "fails"
Date: Wed, 27 Oct 2021 16:04:40 +0000 [thread overview]
Message-ID: <YXl4mK7CyUBnPaQV@google.com> (raw)
In-Reply-To: <335822ac-b98b-1eec-4911-34e4d0e99907@redhat.com>
On Mon, Oct 25, 2021, Paolo Bonzini wrote:
> On 09/10/21 04:12, Sean Christopherson wrote:
> > + /*
> > + * The smp_wmb() in kvm_make_request() pairs with the smp_mb_*()
> > + * after setting vcpu->mode in vcpu_enter_guest(), thus the vCPU
> > + * is guaranteed to see the event request if triggering a posted
> > + * interrupt "fails" because vcpu->mode != IN_GUEST_MODE.
>
> This explanation doesn't make much sense to me. This is just the usual
> request/kick pattern explained in Documentation/virt/kvm/vcpu-requests.rst;
> except that we don't bother with a "kick" out of guest mode because the
> entry always goes through kvm_check_request (in the nVMX case) or
> sync_pir_to_irr (if non-nested) and completes the delivery itself.
>
> In other word, it is a similar idea as patch 43/43.
>
> What this smp_wmb() pair with, is the smp_mb__after_atomic in
> kvm_check_request(KVM_REQ_EVENT, vcpu).
I don't think that's correct. There is no kvm_check_request() in the relevant path.
kvm_vcpu_exit_request() uses kvm_request_pending(), which is just a READ_ONCE()
without a barrier. The smp_mb__after_atomic ensures that any assets that were
modified prior to making the request are seen by the vCPU handling the request.
It does not provide any guarantees for a different vCPU/task making a request
and checking vcpu->mode versus the target vCPU setting vcpu->mode and checking
for a pending request.
> Setting the interrupt in the PIR orders before kvm_make_request in this
> thread, and orders after kvm_make_request in the vCPU thread.
>
> Here, instead:
>
> > + /*
> > + * The implied barrier in pi_test_and_set_on() pairs with the smp_mb_*()
> > + * after setting vcpu->mode in vcpu_enter_guest(), thus the vCPU is
> > + * guaranteed to see PID.ON=1 and sync the PIR to IRR if triggering a
> > + * posted interrupt "fails" because vcpu->mode != IN_GUEST_MODE.
> > + */
> > if (vcpu != kvm_get_running_vcpu() &&
> > !kvm_vcpu_trigger_posted_interrupt(vcpu, false))
> > - kvm_vcpu_kick(vcpu);
> > + kvm_vcpu_wake_up(vcpu);
>
> it pairs with the smp_mb__after_atomic in vmx_sync_pir_to_irr(). As
> explained again in vcpu-requests.rst, the ON bit has the same function as
> vcpu->request in the previous case.
Same as above, I don't think that's correct. The smp_mb__after_atomic() ensures
that there's no race between the IOMMU writing vIRR and setting ON, and KVM
clearing ON and processing the vIRR.
pi_test_on() is not an atomic operation, and there's no memory barrier if ON=0.
It's the same behavior as kvm_check_request(), but again the ordering with respect
to vcpu->mode isn't being handled by PID.ON/kvm_check_request().
AIUI, this is the barrier that's paired with the PI barriers. This is even called
out in (2).
vcpu->mode = IN_GUEST_MODE;
srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
/*
* 1) We should set ->mode before checking ->requests. Please see
* the comment in kvm_vcpu_exiting_guest_mode().
*
* 2) For APICv, we should set ->mode before checking PID.ON. This
* pairs with the memory barrier implicit in pi_test_and_set_on
* (see vmx_deliver_posted_interrupt).
*
* 3) This also orders the write to mode from any reads to the page
* tables done while the VCPU is running. Please see the comment
* in kvm_flush_remote_tlbs.
*/
smp_mb__after_srcu_read_unlock();
next prev parent reply other threads:[~2021-10-27 16:04 UTC|newest]
Thread overview: 141+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-09 2:11 [PATCH v2 00/43] KVM: Halt-polling and x86 APICv overhaul Sean Christopherson
2021-10-09 2:11 ` [PATCH v2 01/43] KVM: VMX: Don't unblock vCPU w/ Posted IRQ if IRQs are disabled in guest Sean Christopherson
2021-10-09 2:11 ` [PATCH v2 02/43] KVM: SVM: Ensure target pCPU is read once when signalling AVIC doorbell Sean Christopherson
2021-10-25 13:21 ` Paolo Bonzini
2021-10-27 9:50 ` Maxim Levitsky
2021-10-09 2:11 ` [PATCH v2 03/43] KVM: s390: Ensure kvm_arch_no_poll() is read once when blocking vCPU Sean Christopherson
2021-10-09 2:11 ` [PATCH v2 04/43] KVM: Force PPC to define its own rcuwait object Sean Christopherson
2021-10-09 2:11 ` [PATCH v2 05/43] KVM: Update halt-polling stats if and only if halt-polling was attempted Sean Christopherson
2021-10-27 10:18 ` Maxim Levitsky
2021-10-09 2:11 ` [PATCH v2 06/43] KVM: Refactor and document halt-polling stats update helper Sean Christopherson
2021-10-27 10:56 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 07/43] KVM: Reconcile discrepancies in halt-polling stats Sean Christopherson
2021-10-27 11:29 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 08/43] KVM: s390: Clear valid_wakeup in kvm_s390_handle_wait(), not in arch hook Sean Christopherson
2021-10-09 2:12 ` [PATCH v2 09/43] KVM: Drop obsolete kvm_arch_vcpu_block_finish() Sean Christopherson
2021-10-22 15:02 ` Anup Patel
2021-10-09 2:12 ` [PATCH v2 10/43] KVM: arm64: Move vGIC v4 handling for WFI out arch callback hook Sean Christopherson
2021-10-25 13:31 ` Paolo Bonzini
2021-10-26 15:41 ` Marc Zyngier
2021-10-26 16:12 ` Paolo Bonzini
2021-11-30 11:39 ` Paolo Bonzini
2021-11-30 12:04 ` Marc Zyngier
2021-11-30 16:07 ` Paolo Bonzini
2021-10-09 2:12 ` [PATCH v2 11/43] KVM: Don't block+unblock when halt-polling is successful Sean Christopherson
2021-10-27 13:40 ` Maxim Levitsky
2021-11-28 22:16 ` Maxim Levitsky
2021-11-29 17:25 ` Sean Christopherson
2021-11-29 17:53 ` Paolo Bonzini
2021-11-29 18:55 ` Sean Christopherson
2021-11-29 19:18 ` Paolo Bonzini
2021-11-29 22:53 ` Maxim Levitsky
2021-12-02 0:20 ` Maxim Levitsky
2021-12-02 2:00 ` Sean Christopherson
2021-12-02 10:31 ` Paolo Bonzini
2021-11-29 17:55 ` Paolo Bonzini
2021-11-29 22:55 ` Maxim Levitsky
2021-12-02 10:20 ` Maxim Levitsky
2021-12-02 10:47 ` Maxim Levitsky
2021-12-02 12:02 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 12/43] KVM: x86: Tweak halt emulation helper names to free up kvm_vcpu_halt() Sean Christopherson
2021-10-27 14:10 ` Maxim Levitsky
2021-10-27 14:18 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 13/43] KVM: Rename kvm_vcpu_block() => kvm_vcpu_halt() Sean Christopherson
2021-10-22 15:04 ` Anup Patel
2021-10-09 2:12 ` [PATCH v2 14/43] KVM: Split out a kvm_vcpu_block() helper from kvm_vcpu_halt() Sean Christopherson
2021-10-09 2:12 ` [PATCH v2 15/43] KVM: stats: Add stat to detect if vcpu is currently blocking Sean Christopherson
2021-10-09 2:12 ` [PATCH v2 16/43] KVM: Don't redo ktime_get() when calculating halt-polling stop/deadline Sean Christopherson
2021-10-25 14:26 ` Paolo Bonzini
2021-10-27 14:35 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 17/43] KVM: x86: Directly block (instead of "halting") UNINITIALIZED vCPUs Sean Christopherson
2021-10-27 14:46 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 18/43] KVM: x86: Invoke kvm_vcpu_block() directly for non-HALTED wait states Sean Christopherson
2021-10-27 15:18 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 19/43] KVM: Add helpers to wake/query blocking vCPU Sean Christopherson
2021-10-25 14:06 ` Paolo Bonzini
2021-10-27 19:27 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 20/43] KVM: VMX: Skip Posted Interrupt updates if APICv is hard disabled Sean Christopherson
2021-10-25 13:48 ` Paolo Bonzini
2021-10-28 9:12 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 21/43] KVM: VMX: Clean up PI pre/post-block WARNs Sean Christopherson
2021-10-28 10:20 ` Maxim Levitsky
2021-10-28 15:34 ` Sean Christopherson
2021-10-09 2:12 ` [PATCH v2 22/43] KVM: VMX: Drop unnecessary PI logic to handle impossible conditions Sean Christopherson
2021-10-25 13:53 ` Paolo Bonzini
2021-10-28 14:36 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 23/43] KVM: VMX: Use boolean returns for Posted Interrupt "test" helpers Sean Christopherson
2021-10-28 6:05 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 24/43] KVM: VMX: Drop pointless PI.NDST update when blocking Sean Christopherson
2021-10-25 14:01 ` Paolo Bonzini
2021-10-27 14:26 ` Sean Christopherson
2021-10-28 10:53 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 25/43] KVM: VMX: Save/restore IRQs (instead of CLI/STI) during PI pre/post block Sean Christopherson
2021-10-28 10:54 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 26/43] KVM: VMX: Read Posted Interrupt "control" exactly once per loop iteration Sean Christopherson
2021-10-28 10:58 ` Maxim Levitsky
2021-10-28 15:55 ` Sean Christopherson
2021-10-31 22:48 ` Maxim Levitsky
2021-11-01 17:41 ` Sean Christopherson
2021-10-09 2:12 ` [PATCH v2 27/43] KVM: VMX: Move Posted Interrupt ndst computation out of write loop Sean Christopherson
2021-10-28 11:28 ` Maxim Levitsky
2021-10-28 16:09 ` Maxim Levitsky
2021-10-28 16:12 ` Sean Christopherson
2021-10-31 22:51 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 28/43] KVM: VMX: Remove vCPU from PI wakeup list before updating PID.NV Sean Christopherson
2021-10-28 12:53 ` Maxim Levitsky
2021-10-28 17:19 ` Sean Christopherson
2021-10-31 22:52 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 29/43] KVM: VMX: Handle PI wakeup shenanigans during vcpu_put/load Sean Christopherson
2021-10-28 15:14 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 30/43] KVM: Drop unused kvm_vcpu.pre_pcpu field Sean Christopherson
2021-10-28 15:16 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 31/43] KVM: Move x86 VMX's posted interrupt list_head to vcpu_vmx Sean Christopherson
2021-10-28 15:33 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 32/43] KVM: VMX: Move preemption timer <=> hrtimer dance to common x86 Sean Christopherson
2021-10-28 15:45 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 33/43] KVM: x86: Unexport LAPIC's switch_to_{hv,sw}_timer() helpers Sean Christopherson
2021-10-28 15:45 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 34/43] KVM: x86: Remove defunct pre_block/post_block kvm_x86_ops hooks Sean Christopherson
2021-10-28 15:46 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 35/43] KVM: SVM: Signal AVIC doorbell iff vCPU is in guest mode Sean Christopherson
2021-10-25 14:26 ` Paolo Bonzini
2021-10-27 15:06 ` Sean Christopherson
2021-10-27 15:36 ` Paolo Bonzini
2021-10-27 16:08 ` Sean Christopherson
2021-10-27 16:14 ` Paolo Bonzini
2021-10-28 16:12 ` Maxim Levitsky
2021-10-28 17:06 ` Sean Christopherson
2021-10-09 2:12 ` [PATCH v2 36/43] KVM: SVM: Don't bother checking for "running" AVIC when kicking for IPIs Sean Christopherson
2021-10-31 14:50 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 37/43] KVM: SVM: Unconditionally mark AVIC as running on vCPU load (with APICv) Sean Christopherson
2021-10-25 14:22 ` Paolo Bonzini
2021-10-25 15:48 ` Sean Christopherson
2021-10-25 15:57 ` Paolo Bonzini
2021-10-25 16:00 ` Sean Christopherson
2021-10-31 16:34 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 38/43] KVM: Drop defunct kvm_arch_vcpu_(un)blocking() hooks Sean Christopherson
2021-10-31 16:35 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 39/43] KVM: VMX: Don't do full kick when triggering posted interrupt "fails" Sean Christopherson
2021-10-25 14:34 ` Paolo Bonzini
2021-10-27 16:04 ` Sean Christopherson [this message]
2021-10-27 22:09 ` Paolo Bonzini
2021-10-31 22:15 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 40/43] KVM: VMX: Wake vCPU when delivering posted IRQ even if vCPU == this vCPU Sean Christopherson
2021-10-25 15:05 ` Paolo Bonzini
2021-10-27 15:30 ` Sean Christopherson
2021-10-31 22:19 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 41/43] KVM: VMX: Pass desired vector instead of bool for triggering posted IRQ Sean Christopherson
2021-10-31 22:25 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 42/43] KVM: VMX: Fold fallback path into triggering posted IRQ helper Sean Christopherson
2021-10-31 22:30 ` Maxim Levitsky
2021-10-09 2:12 ` [PATCH v2 43/43] KVM: VMX: Don't do full kick when handling posted interrupt wakeup Sean Christopherson
2021-10-25 14:16 ` Paolo Bonzini
2021-10-31 22:33 ` Maxim Levitsky
2021-10-25 14:13 ` [PATCH v2 00/43] KVM: Halt-polling and x86 APICv overhaul Paolo Bonzini
2021-10-27 14:41 ` Sean Christopherson
2021-10-27 14:57 ` Paolo Bonzini
2021-10-27 15:28 ` Sean Christopherson
2021-10-27 15:37 ` Paolo Bonzini
2021-10-26 7:20 ` Christian Borntraeger
2021-10-26 14:48 ` Sean Christopherson
2021-10-26 18:29 ` Christian Borntraeger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YXl4mK7CyUBnPaQV@google.com \
--to=seanjc@google.com \
--cc=aleksandar.qemu.devel@gmail.com \
--cc=alexandru.elisei@arm.com \
--cc=anup.patel@wdc.com \
--cc=aou@eecs.berkeley.edu \
--cc=atish.patra@wdc.com \
--cc=borntraeger@de.ibm.com \
--cc=chenhuacai@kernel.org \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=dmatlack@google.com \
--cc=frankja@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=james.morse@arm.com \
--cc=jingzhangos@google.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm-riscv@lists.infradead.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=maz@kernel.org \
--cc=oupton@google.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=paulus@ozlabs.org \
--cc=pbonzini@redhat.com \
--cc=suzuki.poulose@arm.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).