Message-Id: <20210623113002.111448-1-mlevitsk@redhat.com>
Date:   Wed, 23 Jun 2021 14:29:52 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     kvm@...r.kernel.org
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Sean Christopherson <seanjc@...gle.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Joerg Roedel <joro@...tes.org>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        linux-kernel@...r.kernel.org (open list:X86 ARCHITECTURE (32-BIT AND
        64-BIT)),
        x86@...nel.org (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)),
        Jim Mattson <jmattson@...gle.com>,
        Maxim Levitsky <mlevitsk@...hat.com>
Subject: [PATCH 00/10] My AVIC patch queue

Hi!

This is a series of bugfixes to the AVIC dynamic inhibition, which was
made while trying to fix bugs as much as possible, in this area and trying
to make the AVIC+SYNIC conditional enablement work.

First 4 patches in this series fix a race condition which can cause
a lost write from a guest to APIC when the APIC write races
the AVIC un-inhibition.

Next four patches hopefully correctly address an issue of possible
mismatch between the AVIC inhibit state and AVIC enable state on all vCPUs.

Since AVICs state is changed via a request there is a window during which
the states differ which can lead to various warnings and errors.

There was an earlier attempt to fix this by changing the AVIC enable state
on the current vCPU immediately when the AVIC inhibit request is created,
however while this fixes the common case, it actually hides the issue deeper,
because on all other vCPUs but current one, the two states can still
mismatch till the KVM_REQ_APICV_UPDATE is processed on each of them.

My take on this is to fix the places where the mismatch causes the
issues instead and then drop the special case of toggling the AVIC right
away in kvm_request_apicv_update.

Patch 9 is an attempt to fix yet another issue I found in AVIC inhibit
code:
Currently avic_vcpu_load/avic_vcpu_put are called on userspace entry/exit
from KVM (aka kvm_vcpu_get/kvm_vcpu_put), and these functions update the
"is running" bit in the AVIC physical ID remap table and update the
target vCPU in iommu code.

However both of these functions don't do anything when AVIC is inhibited
thus the "is running" bit will be kept enabled during exit to userspace.
This shouldn't be a big issue as the caller
doesn't use the AVIC when inhibited but still inconsistent and can trigger
a warning about this in avic_vcpu_load.

To be on the safe side I think it makes sense to call
avic_vcpu_put/avic_vcpu_load when inhibiting/uninhibiting the AVIC.
This will ensure that the work these functions do is matched.

Patch 10 is the patch from Vitaly about allowing AVIC with SYNC
as long as the guest doesn’t use the AutoEOI feature. I only slightly
changed it to drop the SRCU lock around call to kvm_request_apicv_update
and also expose the AutoEOI cpuid bit regardless of AVIC enablement.

Despite the fact that this is the last patch in this series, this patch
doesn't depend on the other fixes.

Lastly I should note that I spent quite some time last week trying
to avoid dropping the SRCU lock around call to kvm_request_apicv_update,
which is needed due to the fact that this function changes memslots
and needs to do SRCU synchronization.

I tried to make this function such that it would only raise
the KVM_REQ_APICV_UPDATE, and let all vCPUs try to toggle the memory slot,
while processing this request,
but that approach was doomed to fail due to various races.

Using a delayed work for this as was suggested doesn't work either as it can't
update the VM's memory slots (this has to be done from the VM's process).

It is possible to brute force this by raising a new request,
say KVM_REQUEST_AVIC_INHIBITION on say VCPU0, let the new request
processing code drop the srcu lock and then do the things that
kvm_request_apicv_update does. I don't know if this would be better
than the current state of the things.

Best regards,
	Maxim Levitsky

Maxim Levitsky (9):
  KVM: x86: extract block/allow guest enteries
  KVM: x86: APICv: fix race in kvm_request_apicv_update on SVM
  KVM: x86: rename apic_access_page_done to apic_access_memslot_enabled
  KVM: SVM: add warning for mistmatch between AVIC state and AVIC access
    page state
  KVM: SVM: svm_set_vintr don't warn if AVIC is active but is about to
    be deactivated
  KVM: SVM: tweak warning about enabled AVIC on nested entry
  KVM: SVM: use vmcb01 in svm_refresh_apicv_exec_ctrl
  KVM: x86: APICv: drop immediate APICv disablement on current vCPU
  KVM: SVM: call avic_vcpu_load/avic_vcpu_put when enabling/disabling
    AVIC

Vitaly Kuznetsov (1):
  KVM: x86: hyper-v: Deactivate APICv only when AutoEOI feature is in
    use

 arch/x86/include/asm/kvm_host.h | 10 +++++--
 arch/x86/kvm/hyperv.c           | 34 +++++++++++++++++++----
 arch/x86/kvm/svm/avic.c         | 49 +++++++++++++++++----------------
 arch/x86/kvm/svm/nested.c       |  2 +-
 arch/x86/kvm/svm/svm.c          | 18 +++++++++---
 arch/x86/kvm/vmx/vmx.c          |  4 +--
 arch/x86/kvm/x86.c              | 49 ++++++++++++++++++---------------
 7 files changed, 105 insertions(+), 61 deletions(-)

-- 
2.26.3

Powered by blists - more mailing lists