Messages in this thread | | | From | Paolo Bonzini <> | Subject | Re: [PATCH v3 09/12] KVM: x86: hyper-v: Deactivate APICv only when AutoEOI feature is in use | Date | Tue, 3 Aug 2021 11:11:13 +0200 |
| |
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
| |