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

["Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>]

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)