On Thu, 2021-12-02 at 17:58 -0600, Suravee Suthikulpanit wrote:
> The AVIC physical APIC ID table entry contains the host physical
> APIC ID field, which the hardware uses to keep track of where each
> vCPU is running. Originally, the field is an 8-bit value, which can
> only support physical APIC ID up to 255.
>
> To support system with larger APIC ID, the AVIC hardware extends
> this field to support up to the largest possible physical APIC ID
> available on the system.
>
> Therefore, replace the hard-coded mask value with the value
> calculated from the maximum possible physical APIC ID in the system.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> ---
> arch/x86/kvm/svm/avic.c | 28 ++++++++++++++++++++--------
> arch/x86/kvm/svm/svm.h | 1 -
> 2 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 6aca1682f4b7..2a0f58e6edf5 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -19,6 +19,7 @@
> #include <linux/amd-iommu.h>
> #include <linux/kvm_host.h>
>
> +#include <asm/apic.h>
> #include <asm/irq_remapping.h>
>
> #include "trace.h"
> @@ -63,6 +64,7 @@
> static DEFINE_HASHTABLE(svm_vm_data_hash, SVM_VM_DATA_HASH_BITS);
> static u32 next_vm_id = 0;
> static bool next_vm_id_wrapped = 0;
> +static u64 avic_host_physical_id_mask;
> static DEFINE_SPINLOCK(svm_vm_data_hash_lock);
>
> /*
> @@ -133,6 +135,20 @@ void avic_vm_destroy(struct kvm *kvm)
> spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
> }
>
> +static void avic_init_host_physical_apicid_mask(void)
> +{
> + if (!x2apic_mode) {
Wonder why this is a exported global variable and not function.
Not the patch fault though.
> + /* If host is in xAPIC mode, default to only 8-bit mask. */
> + avic_host_physical_id_mask = 0xffULL;
> + } else {
> + u32 count = get_count_order(apic_get_max_phys_apicid());
> +
> + avic_host_physical_id_mask = BIT(count) - 1;
I think that there were some complains about using this macro and instead encouraged
to use 1 << x directly, but I see it used already in other places in avic.c so I don't know.
> + }
> + pr_debug("Using AVIC host physical APIC ID mask %#0llx\n",
> + avic_host_physical_id_mask);
> +}
> +
> int avic_vm_init(struct kvm *kvm)
> {
> unsigned long flags;
> @@ -943,22 +959,17 @@ 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);
>
> - /*
> - * 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_host_physical_id_mask))
> return;
>
> entry = READ_ONCE(*(svm->avic_physical_id_cache));
> WARN_ON(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK);
>
> - entry &= ~AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK;
> - entry |= (h_physical_id & AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK);
> + entry &= ~avic_host_physical_id_mask;
> + entry |= h_physical_id;
>
> entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
> if (svm->avic_is_running)
> @@ -1018,6 +1029,7 @@ bool avic_hardware_setup(bool avic, bool npt)
> return false;
>
> pr_info("AVIC enabled\n");
> + avic_init_host_physical_apicid_mask();
> amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier);
> return true;
> }
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 1d2d72e56dd1..b4cb71c538b3 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -497,7 +497,6 @@ 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_BACKING_PAGE_MASK (0xFFFFFFFFFFULL << 12)
> #define AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK (1ULL << 62)
> #define AVIC_PHYSICAL_ID_ENTRY_VALID_MASK (1ULL << 63)
Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Best regards,
Maxim Levitsky
