Re: [PATCH v5 07/14] KVM: ARM: World-switch implementation

Christoffer Dall
2013-01-16T15:42:03Z

[...]

>
>> 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:

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