Re: [PATCH] KVM: SVM: Modify AVIC GATag to support max number of 512 vCPUs

Suthikulpanit, Suravee
2023-02-06T03:52:41Z

Hi Sean / Paolo,

Do you have any concerns or questions regarding this patch?

Thanks,
Suravee

On 1/17/2023 5:08 PM, Suravee Suthikulpanit wrote:
> AVIC GATag is managed by the SVM driver, and is used by the IOMMU driver
> to program the AMD IOMMU IRTE to provide reference back to SVM in case
> the IOMMU cannot inject interrupt to the non-running vCPU. In such case,
> the IOMMU hardware notify the IOMMU driver by creating GALog entry with
> the corresponded GATag. The IOMMU driver processes the GALog entry and
> notifies SVM to schedule in the target vCPU.
>
> Currently, SVM uses 8-bit vCPU ID and 24-bit VM ID to encode 32-bit GATag.
> Since x2AVIC supports upto 512 vCPUs, it requires to use at least 9-bit
> vCPU ID.
>
> Therefore, modify the GATag enconding to use the number of bits required
> to support the maximum vCPUs.
>
> Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> arch/x86/include/asm/svm.h | 3 ++-
> arch/x86/kvm/svm/avic.c | 9 ++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 0361626841bc..6738faf155e4 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -256,7 +256,8 @@ 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_BITS 9
> +#define AVIC_PHYSICAL_MAX_INDEX_MASK GENMASK_ULL(AVIC_PHYSICAL_MAX_INDEX_BITS, 0)
>
> /*
> * For AVIC, the max index allowed for physical APIC ID
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index dd0e41d454a7..b1f8f51bbbd9 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -28,16 +28,15 @@
> #include "svm.h"
>
> /* AVIC GATAG is encoded using VM and VCPU IDs */
> -#define AVIC_VCPU_ID_BITS 8
> -#define AVIC_VCPU_ID_MASK ((1 << AVIC_VCPU_ID_BITS) - 1)
> +#define AVIC_VCPU_ID_MASK ((1 << AVIC_PHYSICAL_MAX_INDEX_BITS) - 1)
>
> -#define AVIC_VM_ID_BITS 24
> +#define AVIC_VM_ID_BITS (32 - AVIC_PHYSICAL_MAX_INDEX_BITS)
> #define AVIC_VM_ID_NR (1 << AVIC_VM_ID_BITS)
> #define AVIC_VM_ID_MASK ((1 << AVIC_VM_ID_BITS) - 1)
>
> -#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_VCPU_ID_BITS) | \
> +#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_PHYSICAL_MAX_INDEX_BITS) | \
> (y & AVIC_VCPU_ID_MASK))
> -#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK)
> +#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_PHYSICAL_MAX_INDEX_BITS) & AVIC_VM_ID_MASK)
> #define AVIC_GATAG_TO_VCPUID(x) (x & AVIC_VCPU_ID_MASK)
>
> static bool force_avic;