linux-kernel - Re: [PATCH v6] KVM: SVM: Allow AVIC support on system w/ physical APIC ID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5dd76348-f89c-58d9-1f6b-a6031b984330@amd.com>
Date:   Thu, 17 Feb 2022 05:59:37 +0700
From:   "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org, x86@...nel.org
Cc:     seanjc@...gle.com, mlevitsk@...hat.com, pbonzini@...hat.com,
        joro@...tes.org, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, peterz@...radead.org, hpa@...or.com,
        jon.grimm@....com, wei.huang2@....com, terry.bowman@....com,
        stable@...r.kernel.org
Subject: Re: [PATCH v6] KVM: SVM: Allow AVIC support on system w/ physical
 APIC ID > 255

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@...r.kernel.org
> Cc: Maxim Levitsky <mlevitsk@...hat.com>
> Suggested-by: Sean Christopherson <seanjc@...gle.com>
> Reviewed-by: Sean Christopherson <seanjc@...gle.com>
> Fixes: 44a95dae1d22 ("KVM: x86: Detect and Initialize AVIC support")
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@....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)