linux-kernel - Re: [PATCH v3 09/12] KVM: x86: hyper-v: Deactivate APICv only when AutoEOI feature is in use

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: <eb012e5e-e116-98e9-ef77-8690be66b06c@redhat.com>
Date:   Tue, 3 Aug 2021 11:11:13 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>, kvm@...r.kernel.org
Cc:     Wanpeng Li <wanpengli@...cent.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Joerg Roedel <joro@...tes.org>, Borislav Petkov <bp@...en8.de>,
        Sean Christopherson <seanjc@...gle.com>,
        Jim Mattson <jmattson@...gle.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@...r.kernel.org>,
        Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v3 09/12] KVM: x86: hyper-v: Deactivate APICv only when
 AutoEOI feature is in use

On 02/08/21 20:33, Maxim Levitsky wrote:
> +	if (!auto_eoi_old && auto_eoi_new) {
> +		if (atomic_inc_return(&hv->synic_auto_eoi_used) == 1)
> +			kvm_request_apicv_update(vcpu->kvm, false,
> +						APICV_INHIBIT_REASON_HYPERV);
> +	} else if (!auto_eoi_new && auto_eoi_old) {
> +		if (atomic_dec_return(&hv->synic_auto_eoi_used) == 0)
> +			kvm_request_apicv_update(vcpu->kvm, true,
> +						APICV_INHIBIT_REASON_HYPERV);

Hmm no, Reviewed-by rescinded.  This is racy, you cannot use atomics. 
The check for zero needs to happen within the lock.

The easiest way is to have a __kvm_request_apicv_update function that 
leaves it to the caller to take the lock.  Then synic_update_vector can do

	if (!!auto_eoi_old == !!auto_eoi_new)
		return;

	mutex_lock(&kvm->apicv_update_lock);
	bool was_active = hv->synic_auto_eoi_used;
	if (auto_eoi_new)
		hv->synic_auto_eoi_used++;
	else
		hv->synic_auto_eoi_used--;

	if (!!hv->synic_auto_eoi_used != !!was_active)
		__kvm_request_apicv_update(vcpu->kvm,
					   !!hv->synic_auto_eoi_used,
					   APICV_INHIBIT_REASON_HYPERV);
	mutex_unlock(&kvm->apicv_update_lock);

Please add a note to synic_auto_eoi_used that it is protected by 
apicv_update_lock.

Thanks,

Paolo