diff mbox series

[v2,1/3] KVM: SVM: Fix a benign off-by-one bug in AVIC physical table mask

Message ID 20230207002156.521736-2-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: SVM: Fix GATag bug for >256 vCPUs | expand

Commit Message

Sean Christopherson Feb. 7, 2023, 12:21 a.m. UTC
Define the "physical table max index mask" as bits 8:0, not 9:0.  x2AVIC
currently supports a max of 512 entries, i.e. the max index is 511, and
the inputs to GENMASK_ULL() are inclusive.  The bug is benign as bit 9 is
reserved and never set by KVM, i.e. KVM is just clearing bits that are
guaranteed to be zero.

Note, as of this writing, APM "Rev. 3.39-October 2022" incorrectly states
that bits 11:8 are reserved in Table B-1. VMCB Layout, Control Area.  I.e.
that table wasn't updated when x2AVIC support was added.

Opportunistically fix the comment for the max AVIC ID to align with the
code, and clean up comment formatting too.

Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode")
Cc: stable@vger.kernel.org
Cc: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/svm.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Suthikulpanit, Suravee Feb. 15, 2023, 3:38 p.m. UTC | #1
On 2/7/2023 7:21 AM, Sean Christopherson wrote:
> Define the "physical table max index mask" as bits 8:0, not 9:0.  x2AVIC
> currently supports a max of 512 entries, i.e. the max index is 511, and
> the inputs to GENMASK_ULL() are inclusive.  The bug is benign as bit 9 is
> reserved and never set by KVM, i.e. KVM is just clearing bits that are
> guaranteed to be zero.
> 
> Note, as of this writing, APM "Rev. 3.39-October 2022" incorrectly states
> that bits 11:8 are reserved in Table B-1. VMCB Layout, Control Area.  I.e.
> that table wasn't updated when x2AVIC support was added.
> 
> Opportunistically fix the comment for the max AVIC ID to align with the
> code, and clean up comment formatting too.
> 
> Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode")
> Cc: stable@vger.kernel.org
> Cc: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/include/asm/svm.h | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index cb1ee53ad3b1..770dcf75eaa9 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -261,20 +261,22 @@ enum avic_ipi_failure_cause {
>   	AVIC_IPI_FAILURE_INVALID_BACKING_PAGE,
>   };
>   
> -#define AVIC_PHYSICAL_MAX_INDEX_MASK	GENMASK_ULL(9, 0)
> +#define AVIC_PHYSICAL_MAX_INDEX_MASK	GENMASK_ULL(8, 0)
>   
>   /*
> - * For AVIC, the max index allowed for physical APIC ID
> - * table is 0xff (255).
> + * For AVIC, the max index allowed for physical APIC ID table is 0xfe (254), as
> + * 0xff is a broadcast to all CPUs, i.e. can't be targeted individually.
>    */
>   #define AVIC_MAX_PHYSICAL_ID		0XFEULL
>   
>   /*
> - * For x2AVIC, the max index allowed for physical APIC ID
> - * table is 0x1ff (511).
> + * For x2AVIC, the max index allowed for physical APIC ID table is 0x1ff (511).
>    */
>   #define X2AVIC_MAX_PHYSICAL_ID		0x1FFUL
>   
> +static_assert((AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == AVIC_MAX_PHYSICAL_ID);
> +static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_MAX_PHYSICAL_ID);
> +
>   #define AVIC_HPA_MASK	~((0xFFFULL << 52) | 0xFFF)
>   #define VMCB_AVIC_APIC_BAR_MASK		0xFFFFFFFFFF000ULL
>   

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Thanks,
Suravee
diff mbox series

Patch

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index cb1ee53ad3b1..770dcf75eaa9 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -261,20 +261,22 @@  enum avic_ipi_failure_cause {
 	AVIC_IPI_FAILURE_INVALID_BACKING_PAGE,
 };
 
-#define AVIC_PHYSICAL_MAX_INDEX_MASK	GENMASK_ULL(9, 0)
+#define AVIC_PHYSICAL_MAX_INDEX_MASK	GENMASK_ULL(8, 0)
 
 /*
- * For AVIC, the max index allowed for physical APIC ID
- * table is 0xff (255).
+ * For AVIC, the max index allowed for physical APIC ID table is 0xfe (254), as
+ * 0xff is a broadcast to all CPUs, i.e. can't be targeted individually.
  */
 #define AVIC_MAX_PHYSICAL_ID		0XFEULL
 
 /*
- * For x2AVIC, the max index allowed for physical APIC ID
- * table is 0x1ff (511).
+ * For x2AVIC, the max index allowed for physical APIC ID table is 0x1ff (511).
  */
 #define X2AVIC_MAX_PHYSICAL_ID		0x1FFUL
 
+static_assert((AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == AVIC_MAX_PHYSICAL_ID);
+static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_MAX_PHYSICAL_ID);
+
 #define AVIC_HPA_MASK	~((0xFFFULL << 52) | 0xFFF)
 #define VMCB_AVIC_APIC_BAR_MASK		0xFFFFFFFFFF000ULL