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, 16 Dec 2022 19:40:29 +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 11/32] KVM: x86: Inhibit APIC memslot if x2APIC and
 AVIC are enabled
Message-ID: <Y5zJraa0ddooauXB@google.com>
References: <20221001005915.2041642-1-seanjc@google.com>
 <20221001005915.2041642-12-seanjc@google.com>
 <90d4a2a1733cdb21e7c00843ddafee78ce52bbdc.camel@redhat.com>
 <Y5zBH+2VuPJi4yYV@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y5zBH+2VuPJi4yYV@google.com>
Precedence: bulk

On Fri, Dec 16, 2022, Sean Christopherson wrote:
> On Thu, Dec 08, 2022, Maxim Levitsky wrote:
> > I prefer to just have a boolean 'is_avic' or,
> > '.needs_x2apic_memslot_inhibition' in the vendor ops, and check it in
> > 'kvm_vcpu_update_apicv' with the above comment on top of it.
> > 
> > need_x2apic_memslot_inhibition can even be set to false when x2avic is
> > supported at the initalization time, because then AVIC behaves just like
> > APICv (when x2avic bit is enabled, AVIC mmio is no longer decoded).
> 
> Oh, so SVM does effectively have independent controls, it's only the "hybrid" mode
> that's affected?  In that case, how about this?
> 
> 	/*
> 	 * Due to sharing page tables across vCPUs, the xAPIC memslot must be
> 	 * deleted if any vCPU has x2APIC enabled and hardware doesn't support
> 	 * x2APIC virtualization.  E.g. some AMD CPUs support AVIC but not
> 	 * x2AVIC.  KVM still allows enabling AVIC in this case so that KVM can
> 	 * the AVIC doorbell to inject interrupts to running vCPUs, but KVM
> 	 * mustn't create SPTEs for the APIC base as the vCPU would incorrectly
> 	 * be able to access the vAPIC page via MMIO despite being in x2APIC
> 	 * mode.  For simplicity, inhibiting the APIC access page is sticky.
> 	 */
> 	if (apic_x2apic_mode(vcpu->arch.apic) &&
> 	    !kvm_x86_ops.has_hardware_x2apic_virtualization)

Hrm, that's not quite right either since it's obviously possible to have an Intel
CPU that supports APICv but not x2APIC virtualization.  And in that case KVM
doesn't need to inhibit the memslot, e.g. if not all vCPUs are in x2APIC.

I was hoping to have a name that communicate _why_ the memslot needs to be
inhibited, but it's turning out to be really hard to come up with a name that's
descriptive without being ridiculously verbose.  The best I've come up with is:

	allow_apicv_in_x2apic_without_x2apic_virtualization

It's heinous, but I'm inclined to go with it unless someone has a better idea.