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

[Paolo Bonzini <pbonzini@redhat.com>]

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)