Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 9 Dec 2022 00:56:17 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Maxim Levitsky <mlevitsk@redhat.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Alejandro Jimenez <alejandro.j.jimenez@oracle.com>,
        Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
        Li RongQing <lirongqing@baidu.com>
Subject: Re: [PATCH v4 24/32] KVM: x86: Inhibit APICv/AVIC if the optimized
 physical map is disabled
Message-ID: <Y5KHsdsA/T/gaswC@google.com>
References: <20221001005915.2041642-1-seanjc@google.com>
 <20221001005915.2041642-25-seanjc@google.com>
 <6de0c4fd228055840648452bd5a3b4955f70d881.camel@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6de0c4fd228055840648452bd5a3b4955f70d881.camel@redhat.com>
Precedence: bulk

On Thu, Dec 08, 2022, Maxim Levitsky wrote:
> On Sat, 2022-10-01 at 00:59 +0000, Sean Christopherson wrote:
> > Inhibit APICv/AVIC if the optimized physical map is disabled so that KVM
> > KVM provides consistent APIC behavior if xAPIC IDs are aliased due to
> > vcpu_id being truncated and the x2APIC hotplug hack isn't enabled.  If
> > the hotplug hack is disabled, events that are emulated by KVM will follow
> > architectural behavior (all matching vCPUs receive events, even if the
> > "match" is due to truncation), whereas APICv and AVIC will deliver events
> > only to the first matching vCPU, i.e. the vCPU that matches without
> > truncation.
> > 
> > Note, the "extra" inhibit is needed because  KVM deliberately ignores
> > mismatches due to truncation when applying the APIC_ID_MODIFIED inhibit
> > so that large VMs (>255 vCPUs) can run with APICv/AVIC.
> > 
> > Fixes: TDB
> Do you mean Trade & Development Bank of Mongolia ? ;-)

Heh, this fixes a patch/commit earlier in the series, hence the placeholder.

> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h |  6 ++++++
> >  arch/x86/kvm/lapic.c            | 13 ++++++++++++-
> >  arch/x86/kvm/svm/avic.c         |  1 +
> >  arch/x86/kvm/vmx/vmx.c          |  1 +
> >  4 files changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index ac28bbfbf0e3..171e38b94c89 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1104,6 +1104,12 @@ enum kvm_apicv_inhibit {
> >  	 */
> >  	APICV_INHIBIT_REASON_BLOCKIRQ,
> >  
> > +	/*
> > +	 * APICv is disabled because not all vCPUs have a 1:1 mapping between
> > +	 * APIC ID and vCPU, _and_ KVM is not applying its x2APIC hotplug hack.
> > +	 */
> > +	APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED,
> > +
> >  	/*
> >  	 * For simplicity, the APIC acceleration is inhibited
> >  	 * first time either APIC ID or APIC base are changed by the guest
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 340c2d3e781b..f6f328d36ae2 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -381,6 +381,16 @@ void kvm_recalculate_apic_map(struct kvm *kvm)
> >  		cluster[ldr] = apic;
> >  	}
> >  out:
> > +	/*
> > +	 * The optimized map is effectively KVM's internal version of APICv,
> Nitpick: APICv/AVIC?

Hmm.  I think my preference for common code that doesn't need to differentiate
between Intel and AMD (or AVIC vs. x2AVIC) is to use just APICv.  "APICv" is a
mostly a KVM term, e.g. Intel uses "APIC-virtualization" as an umbrella term for
a massive pile of features, so I think bending the term to cover Intel+AMD is ok?

Typing APICv/AVIC everywhere will get tedious and creates its own flavor of
confusion, e.g. the inhibits all use APICV.