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

[Christoffer Dall]

On Wed, Jan 16, 2013 at 10:52 AM, Gleb Natapov <gleb at redhat.com> 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.