On Wed, Jun 11, 2025 at 03:45:15PM -0700, Sean Christopherson wrote:
> 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 <mlevitsk@xxxxxxxxxx>
> Tested-by: Sairaj Kodilkar <sarunkod@xxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> 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
