Messages in this thread | | | From | Paolo Bonzini <> | Subject | Re: [PATCH v3 06/12] KVM: x86: don't disable APICv memslot when inhibited | Date | Tue, 3 Aug 2021 10:44:09 +0200 |
| |
Reviewing this patch and the next one together.
On 02/08/21 20:33, Maxim Levitsky wrote: > +static int avic_alloc_access_page(struct kvm *kvm) > { > void __user *ret; > int r = 0; > > mutex_lock(&kvm->slots_lock); > + > + if (kvm->arch.apic_access_memslot_enabled) > goto out;
This variable is overloaded between "is access enabled" and "is the memslot allocated". I think you should check kvm->arch.apicv_inhibit_reasons instead in kvm_faultin_pfn.
> + if (!activate) > + kvm_zap_gfn_range(kvm, gpa_to_gfn(APIC_DEFAULT_PHYS_BASE), > + gpa_to_gfn(APIC_DEFAULT_PHYS_BASE + PAGE_SIZE)); > +
Off by one, the last argument of kvm_zap_gfn_range is inclusive: Also, checking "activate" is a bit ugly when we have "new" available as well. Yes, they are the same if !!old != !!new, but we care about the global state, not the single bit.
Putting everything together, this could become something like
trace_kvm_apicv_update_request(activate, bit); if (!!old != !!new) { /* * Kick all CPUs out of guest mode. When * kvm_vcpu_update_apicv succeeds in taking * apicv_update_lock, it will see the * new apicv_inhibit_reasons that we set below. */ kvm_make_all_cpus_request(kvm, KVM_REQ_APICV_UPDATE);
if (new) { unsigned long gfn = gpa_to_gfn(APIC_DEFAULT_PHYS_BASE); kvm_zap_gfn_range(kvm, gfn, gfn); } } kvm->arch.apicv_inhibit_reasons = new; mutex_unlock(&kvm->arch.apicv_update_lock);
Paolo
| |