Messages in this thread | | | Date | Tue, 1 Mar 2022 16:45:57 +0700 | Subject | Re: [RFC PATCH 03/13] KVM: SVM: Detect X2APIC virtualization (x2AVIC) support | From | Suravee Suthikulpanit <> |
| |
Hi Maxim,
On 2/24/22 11:52 PM, Maxim Levitsky wrote: > On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote: >> .... >> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c >> index 472445aaaf42..abde08ca23ab 100644 >> --- a/arch/x86/kvm/svm/avic.c >> +++ b/arch/x86/kvm/svm/avic.c >> @@ -40,6 +40,15 @@ >> #define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK) >> #define AVIC_GATAG_TO_VCPUID(x) (x & AVIC_VCPU_ID_MASK) >> >> +#define IS_AVIC_MODE_X1(x) (avic_get_vcpu_apic_mode(x) == AVIC_MODE_X1) >> +#define IS_AVIC_MODE_X2(x) (avic_get_vcpu_apic_mode(x) == AVIC_MODE_X2) >> + >> +enum avic_modes { >> + AVIC_MODE_NONE = 0, >> + AVIC_MODE_X1, >> + AVIC_MODE_X2, >> +}; >> + >> /* Note: >> * This hash table is used to map VM_ID to a struct kvm_svm, >> * when handling AMD IOMMU GALOG notification to schedule in >> @@ -50,6 +59,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 DEFINE_SPINLOCK(svm_vm_data_hash_lock); >> +static enum avic_modes avic_mode; >> >> /* >> * This is a wrapper of struct amd_iommu_ir_data. >> @@ -59,6 +69,15 @@ struct amd_svm_iommu_ir { >> void *data; /* Storing pointer to struct amd_ir_data */ >> }; >> >> +static inline enum avic_modes avic_get_vcpu_apic_mode(struct vcpu_svm *svm) >> +{ >> + if (svm->vmcb->control.int_ctl & X2APIC_MODE_MASK) >> + return AVIC_MODE_X2; >> + else if (svm->vmcb->control.int_ctl & AVIC_ENABLE_MASK) >> + return AVIC_MODE_X1; >> + else >> + return AVIC_MODE_NONE; >> +} > I a bit don't like it. > > By definition if a vCPU has x2apic, it will use x2avic and if it is in > xapic mode it will use plain avic, unless avic is inhibited, > which will also be the case when vCPU is in x2apic mode but hardware > doesn't support x2avic. > > But I might have beeing mistaken here - anyway this function should > be added when it is used so it will be clear how and why it is needed.
I will remove this part.
>> >> /* Note: >> * This function is called from IOMMU driver to notify >> @@ -1016,3 +1035,28 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu) >> >> put_cpu(); >> } >> + >> +/* >> + * Note: >> + * - The module param avic enable both xAPIC and x2APIC mode. >> + * - Hypervisor can support both xAVIC and x2AVIC in the same guest. >> + * - The mode can be switched at run-time. >> + */ >> +bool avic_hardware_setup(struct kvm_x86_ops *x86_ops) >> +{ >> + if (!npt_enabled) >> + return false; >> + >> + if (boot_cpu_has(X86_FEATURE_AVIC)) { >> + avic_mode = AVIC_MODE_X1; >> + pr_info("AVIC enabled\n"); >> + } >> + >> + if (boot_cpu_has(X86_FEATURE_X2AVIC)) { >> + avic_mode = AVIC_MODE_X2; >> + pr_info("x2AVIC enabled\n"); >> + } >> + >> + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > If AVIC is not enabled, I guess no need to register GA log notifier?
GA log is only used when AVIC is enabled. I'll restore the AVIC-enabled check.
Regards, Suravee
| |