lkml.org 
[lkml]   [2022]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC PATCH 03/13] KVM: SVM: Detect X2APIC virtualization (x2AVIC) support
From
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

\
 
 \ /
  Last update: 2022-03-01 10:46    [W:0.088 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site