Message ID | 20220211000851.185799-1-suravee.suthikulpanit@amd.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v6] KVM: SVM: Allow AVIC support on system w/ physical APIC ID > 255 | expand |
Paolo, Do you have any other concerns regarding this patch? 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@vger.kernel.org > Cc: Maxim Levitsky <mlevitsk@redhat.com> > Suggested-by: Sean Christopherson <seanjc@google.com> > Reviewed-by: Sean Christopherson <seanjc@google.com> > Fixes: 44a95dae1d22 ("KVM: x86: Detect and Initialize AVIC support") > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > 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)
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@vger.kernel.org >> Cc: Maxim Levitsky <mlevitsk@redhat.com> >> Suggested-by: Sean Christopherson <seanjc@google.com> >> Reviewed-by: Sean Christopherson <seanjc@google.com> >> Fixes: 44a95dae1d22 ("KVM: x86: Detect and Initialize AVIC support") >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> --- >> 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)
On 2/22/2022 1:32 AM, Paolo Bonzini wrote: > 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 Thank you. I'll repost this for <= 5.17 backport after the merge window. Suravee
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)