Re: [PATCH v3 12/62] KVM: SVM: Inhibit AVIC if ID is too big instead of rejecting vCPU creation
From: Naveen N Rao <naveen@kernel.org>
Date: 2025-06-17 14:30:18
Also in:
kvm, kvmarm, linux-iommu, lkml
On Wed, Jun 11, 2025 at 03:45:15PM -0700, Sean Christopherson wrote:
quoted hunk ↗ jump to hunk
Inhibit AVIC with a new "ID too big" flag if userspace creates a vCPU with an ID that is too big, but otherwise allow vCPU creation to succeed. Rejecting KVM_CREATE_VCPU with EINVAL violates KVM's ABI as KVM advertises that the max vCPU ID is 4095, but disallows creating vCPUs with IDs bigger than 254 (AVIC) or 511 (x2AVIC). Alternatively, KVM could advertise an accurate value depending on which AVIC mode is in use, but that wouldn't really solve the underlying problem, e.g. would be a breaking change if KVM were to ever try and enable AVIC or x2AVIC by default. Cc: Maxim Levitsky <redacted> Tested-by: Sairaj Kodilkar <redacted> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/include/asm/kvm_host.h | 9 ++++++++- arch/x86/kvm/svm/avic.c | 14 ++++++++++++-- arch/x86/kvm/svm/svm.h | 3 ++- 3 files changed, 22 insertions(+), 4 deletions(-)diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 2a6ef1398da7..a9b709db7c59 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h@@ -1314,6 +1314,12 @@ enum kvm_apicv_inhibit { */ APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED, + /* + * AVIC is disabled because the vCPU's APIC ID is beyond the max + * supported by AVIC/x2AVIC, i.e. the vCPU is unaddressable. + */ + APICV_INHIBIT_REASON_PHYSICAL_ID_TOO_BIG, + NR_APICV_INHIBIT_REASONS, };@@ -1332,7 +1338,8 @@ enum kvm_apicv_inhibit { __APICV_INHIBIT_REASON(IRQWIN), \ __APICV_INHIBIT_REASON(PIT_REINJ), \ __APICV_INHIBIT_REASON(SEV), \ - __APICV_INHIBIT_REASON(LOGICAL_ID_ALIASED) + __APICV_INHIBIT_REASON(LOGICAL_ID_ALIASED), \ + __APICV_INHIBIT_REASON(PHYSICAL_ID_TOO_BIG) struct kvm_arch { unsigned long n_used_mmu_pages;diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index ab228872a19b..f0a74b102c57 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c@@ -277,9 +277,19 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu) int id = vcpu->vcpu_id; struct vcpu_svm *svm = to_svm(vcpu); + /* + * Inhibit AVIC if the vCPU ID is bigger than what is supported by AVIC + * hardware. Immediately clear apicv_active, i.e. don't wait until the + * KVM_REQ_APICV_UPDATE request is processed on the first KVM_RUN, as + * avic_vcpu_load() expects to be called if and only if the vCPU has + * fully initialized AVIC. + */ if ((!x2avic_enabled && id > AVIC_MAX_PHYSICAL_ID) || - (id > X2AVIC_MAX_PHYSICAL_ID)) - return -EINVAL; + (id > X2AVIC_MAX_PHYSICAL_ID)) { + kvm_set_apicv_inhibit(vcpu->kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_TOO_BIG); + vcpu->arch.apic->apicv_active = false;
This bothers me a bit. kvm_create_lapic() does this:
if (enable_apicv) {
apic->apicv_active = true;
kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
}
But, setting apic->apicv_active to false here means KVM_REQ_APICV_UPDATE
is going to be a no-op.
This does not look to be a big deal given that kvm_create_lapic() itself
is called just a bit before svm_vcpu_create() (which calls the above
function through avic_init_vcpu()) in kvm_arch_vcpu_create(), so there
isn't that much done before apicv_active is toggled.
But, this made me wonder if introducing a kvm_x86_op to check and
enable/disable apic->apicv_active in kvm_create_lapic() might be cleaner
overall. Maybe even have it be the initialization point for APICv:
apicv_init(), so we can invoke avic_init_vcpu() right away?
- Naveen