Re: [PATCH v6] KVM: SVM: Allow AVIC support on system w/ physical APIC ID > 255

[Paolo Bonzini]

On 2/16/22 23:59, Suthikulpanit, Suravee wrote:
> Paolo,
>
> Do you have any other concerns regarding this patch?

Queued it now, thanks.  I had left it aside for a second because it 
didn't apply cleanly.  I'll include it in 5.18 for now; we'll then have 
to repost this version as a <=5.17 backport for Greg, after the merge 
window.

Paolo

> Regards,
> Suravee
>
> On 2/11/2022 7:08 AM, Suravee Suthikulpanit wrote:
> > Expand KVM's mask for the AVIC host physical ID to the full 12 bits 
> > defined
> > by the architecture.  The number of bits consumed by hardware is model
> > specific, e.g. early CPUs ignored bits 11:8, but there is no way for KVM
> > to enumerate the "true" size.  So, KVM must allow using all bits, else it
> > risks rejecting completely legal x2APIC IDs on newer CPUs.
> >
> > This means KVM relies on hardware to not assign x2APIC IDs that exceed 
> > the
> > "true" width of the field, but presumably hardware is smart enough to tie
> > the width to the max x2APIC ID.  KVM also relies on hardware to 
> > support at
> > least 8 bits, as the legacy xAPIC ID is writable by software.  But, those
> > assumptions are unavoidable due to the lack of any way to enumerate the
> > "true" width.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Cc: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> > Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Reviewed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Fixes: 44a95dae1d22 ("KVM: x86: Detect and Initialize AVIC support")
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> > ---
> >   arch/x86/kvm/svm/avic.c | 7 +------
> >   arch/x86/kvm/svm/svm.h  | 2 +-
> >   2 files changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> > index 90364d02f22a..e4cfd8bf4f24 100644
> > --- a/arch/x86/kvm/svm/avic.c
> > +++ b/arch/x86/kvm/svm/avic.c
> > @@ -974,17 +974,12 @@ avic_update_iommu_vcpu_affinity(struct kvm_vcpu 
> > *vcpu, int cpu, bool r)
> >   void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> >   {
> >       u64 entry;
> > -    /* ID = 0xff (broadcast), ID > 0xff (reserved) */
> >       int h_physical_id = kvm_cpu_get_apicid(cpu);
> >       struct vcpu_svm *svm = to_svm(vcpu);
> >       lockdep_assert_preemption_disabled();
> > -    /*
> > -     * Since the host physical APIC id is 8 bits,
> > -     * we can support host APIC ID upto 255.
> > -     */
> > -    if (WARN_ON(h_physical_id > 
> > AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK))
> > +    if (WARN_ON(h_physical_id & 
> > ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK))
> >           return;
> >       /*
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index 47ef8f4a9358..cede59cd8999 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -565,7 +565,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops;
> >   #define AVIC_LOGICAL_ID_ENTRY_VALID_BIT            31
> >   #define AVIC_LOGICAL_ID_ENTRY_VALID_MASK        (1 << 31)
> > -#define AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK    (0xFFULL)
> > +#define AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK    
> > GENMASK_ULL(11, 0)
> >   #define AVIC_PHYSICAL_ID_ENTRY_BACKING_PAGE_MASK    (0xFFFFFFFFFFULL 
> > << 12)
> >   #define AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK        (1ULL << 62)
> >   #define AVIC_PHYSICAL_ID_ENTRY_VALID_MASK        (1ULL << 63)