On Wed, Jan 16, 2013 at 10:42:02AM -0500, Christoffer Dall wrote:
> [...]
>
> >
> >> read side RCU protects against is the memslots data structure as far
> >> as I can see, so the second patch pasted below fixes this for the code
> >> that actually accesses this data structure.
> > Many memory related functions that you call access memslots under the
> > hood and assume that locking is done by the caller. From the quick look
> > I found those that you've missed:
> > kvm_is_visible_gfn()
> > kvm_read_guest()
> > gfn_to_hva()
> > gfn_to_pfn_prot()
> > kvm_memslots()
> >
> > May be there are more. Can you enable RCU debugging in your kernel config
> > and check? This does not guaranty that it will catch all of the places,
> > but better than nothing.
> >
>
> yeah, I missed the call to is_visible_gfn and friends, this fixes it:
>
Thank you. One more kvm_read_guest() in emulate.c.
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index c806080..f30e131 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -591,7 +591,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
> struct kvm_run *run)
> struct kvm_memory_slot *memslot;
> bool is_iabt;
> gfn_t gfn;
> - int ret;
> + int ret, idx;
>
> hsr_ec = vcpu->arch.hsr >> HSR_EC_SHIFT;
> is_iabt = (hsr_ec == HSR_EC_IABT);
> @@ -608,33 +608,43 @@ int kvm_handle_guest_abort(struct kvm_vcpu
> *vcpu, struct kvm_run *run)
> return -EFAULT;
> }
>
> + idx = srcu_read_lock(&vcpu->kvm->srcu);
> +
> gfn = fault_ipa >> PAGE_SHIFT;
> if (!kvm_is_visible_gfn(vcpu->kvm, gfn)) {
> if (is_iabt) {
> /* Prefetch Abort on I/O address */
> kvm_inject_pabt(vcpu, vcpu->arch.hxfar);
> - return 1;
> + ret = 1;
> + goto out_unlock;
> }
>
> if (fault_status != FSC_FAULT) {
> kvm_err("Unsupported fault status on io memory: %#lx\n",
> fault_status);
> - return -EFAULT;
> + ret = -EFAULT;
> + goto out_unlock;
> }
>
> /* Adjust page offset */
> fault_ipa |= vcpu->arch.hxfar & ~PAGE_MASK;
> - return io_mem_abort(vcpu, run, fault_ipa);
> + ret = io_mem_abort(vcpu, run, fault_ipa);
> + goto out_unlock;
> }
>
> memslot = gfn_to_memslot(vcpu->kvm, gfn);
> if (!memslot->user_alloc) {
> kvm_err("non user-alloc memslots not supported\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out_unlock;
> }
>
> ret = user_mem_abort(vcpu, fault_ipa, gfn, memslot, fault_status);
> - return ret ? ret : 1;
> + if (ret == 0)
> + ret = 1;
> +out_unlock:
> + srcu_read_unlock(&vcpu->kvm->srcu, idx);
> + return ret;
> }
>
> static void handle_hva_to_gpa(struct kvm *kvm,
> --
>
> Thanks,
> -Christoffer
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
