On Wed, Jan 16, 2013 at 10:52 AM, Gleb Natapov <gleb@xxxxxxxxxx> wrote:
> 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.
>
this one is going out for now (see the i/o discussion).
>> 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
