linux-kernel - Re: [RFC PATCH 04/13] KVM: SVM: Only call vcpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <214b5d5e-f936-f50c-b26c-334ecbbdface@amd.com>
Date:   Tue, 1 Mar 2022 16:59:23 +0700
From:   Suravee Suthikulpanit <suravee.suthikulpanit@....com>
To:     Maxim Levitsky <mlevitsk@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, seanjc@...gle.com
Cc:     pbonzini@...hat.com, joro@...tes.org, jon.grimm@....com,
        wei.huang2@....com, terry.bowman@....com
Subject: Re: [RFC PATCH 04/13] KVM: SVM: Only call vcpu_(un)blocking when AVIC
 is enabled.

Hi Maxim,

On 2/24/22 11:54 PM, Maxim Levitsky wrote:
> On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote:
>> The kvm_x86_ops.vcpu_(un)blocking are needed by AVIC only.
>> Therefore, set the ops only when AVIC is enabled.
>>
>> Suggested-by: Sean Christopherson<seanjc@...gle.com>
>> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@....com>
>> ---
>>   arch/x86/kvm/svm/avic.c | 12 ++++++++++--
>>   arch/x86/kvm/svm/svm.c  |  7 -------
>>   arch/x86/kvm/svm/svm.h  |  2 --
>>   3 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
>> index abde08ca23ab..0040824e4376 100644
>> --- a/arch/x86/kvm/svm/avic.c
>> +++ b/arch/x86/kvm/svm/avic.c
>> @@ -996,7 +996,7 @@ void avic_vcpu_put(struct kvm_vcpu *vcpu)
>>   	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
>>   }
>>   
>> -void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
>> +static void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
>>   {
>>   	if (!kvm_vcpu_apicv_active(vcpu))
>>   		return;
>> @@ -1021,7 +1021,7 @@ void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
>>   	preempt_enable();
>>   }
>>   
>> -void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>> +static void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>>   {
>>   	int cpu;
>>   
>> @@ -1057,6 +1057,14 @@ bool avic_hardware_setup(struct kvm_x86_ops *x86_ops)
>>   		pr_info("x2AVIC enabled\n");
>>   	}
>>   
>> +	if (avic_mode) {
>> +		x86_ops->vcpu_blocking = avic_vcpu_blocking;
>> +		x86_ops->vcpu_unblocking = avic_vcpu_unblocking;
>> +	} else {
>> +		x86_ops->vcpu_blocking = NULL;
>> +		x86_ops->vcpu_unblocking = NULL;
>> +	}
>> +
>>   	amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier);
>>   	return !!avic_mode;
>>   }
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 3048f4b758d6..3687026f2859 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -4531,8 +4531,6 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
>>   	.prepare_guest_switch = svm_prepare_guest_switch,
>>   	.vcpu_load = svm_vcpu_load,
>>   	.vcpu_put = svm_vcpu_put,
>> -	.vcpu_blocking = avic_vcpu_blocking,
>> -	.vcpu_unblocking = avic_vcpu_unblocking,
>>   
>>   	.update_exception_bitmap = svm_update_exception_bitmap,
>>   	.get_msr_feature = svm_get_msr_feature,
>> @@ -4819,11 +4817,6 @@ static __init int svm_hardware_setup(void)
>>   
>>   	enable_apicv = avic = avic && avic_hardware_setup(&svm_x86_ops);
>>   
>> -	if (!enable_apicv) {
>> -		svm_x86_ops.vcpu_blocking = NULL;
>> -		svm_x86_ops.vcpu_unblocking = NULL;
>> -	}
> Isn't this code already zeros these callbacks when avic is not enabled?

Ah, right. I'll remove the setting to NULL.

> I am not sure why this patch is needed to be honest.

It's not related to x2AVIC. It was recommended by Sean earlier
in another patch series:

   https://lore.kernel.org/lkml/Yc3r1U6WFVDtJCZn@google.com/

Since this series introduces the helper function avic_hardware_setup(),
and re-factor the AVIC setup code into the function. So, I am including
his recommendation in this series instead..

Regards,
Suravee