Re: [PATCH] KVM: PPC: Take the kvm->srcu lock in kvmppc_h_logical_ci_load/store()

[Paolo Bonzini <pbonzini@redhat.com>]

On 21/09/2015 03:37, David Gibson wrote:
> On Fri, Sep 18, 2015 at 08:57:28AM +0200, Thomas Huth wrote:
>> Access to the kvm->buses (like with the kvm_io_bus_read() and
>> -write() functions) has to be protected via the kvm->srcu lock. 
>> The kvmppc_h_logical_ci_load() and -store() functions are
>> missing this lock so far, so let's add it there, too. This fixes
>> the problem that the kernel reports "suspicious RCU usage" when
>> lock debugging is enabled.
>> 
>> Fixes: 99342cf8044420eebdf9297ca03a14cb6a7085a1 Signed-off-by:
>> Thomas Huth <thuth@redhat.com>
> 
> Nice catch.  Looks like I missed this because the places 
> kvm_io_bus_{read,write}() are called on x86 are buried about 5
> layers below where the srcu lock is taken :/.
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
>> --- arch/powerpc/kvm/book3s.c | 6 ++++++ 1 file changed, 6
>> insertions(+)
>> 
>> diff --git a/arch/powerpc/kvm/book3s.c
>> b/arch/powerpc/kvm/book3s.c index d75bf32..096e5eb 100644 ---
>> a/arch/powerpc/kvm/book3s.c +++ b/arch/powerpc/kvm/book3s.c @@
>> -828,12 +828,15 @@ int kvmppc_h_logical_ci_load(struct kvm_vcpu
>> *vcpu) unsigned long size = kvmppc_get_gpr(vcpu, 4); unsigned
>> long addr = kvmppc_get_gpr(vcpu, 5); u64 buf; +	int srcu_idx; int
>> ret;
>> 
>> if (!is_power_of_2(size) || (size > sizeof(buf))) return
>> H_TOO_HARD;
>> 
>> +	srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); ret =
>> kvm_io_bus_read(vcpu, KVM_MMIO_BUS, addr, size, &buf); +
>> srcu_read_unlock(&vcpu->kvm->srcu, srcu_idx); if (ret != 0) 
>> return H_TOO_HARD;
>> 
>> @@ -868,6 +871,7 @@ int kvmppc_h_logical_ci_store(struct kvm_vcpu
>> *vcpu) unsigned long addr = kvmppc_get_gpr(vcpu, 5); unsigned
>> long val = kvmppc_get_gpr(vcpu, 6); u64 buf; +	int srcu_idx; int
>> ret;
>> 
>> switch (size) { @@ -891,7 +895,9 @@ int
>> kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu) return
>> H_TOO_HARD; }
>> 
>> +	srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); ret =
>> kvm_io_bus_write(vcpu, KVM_MMIO_BUS, addr, size, &buf); +
>> srcu_read_unlock(&vcpu->kvm->srcu, srcu_idx); if (ret != 0) 
>> return H_TOO_HARD;
>> 
> 

Paul,

shall I take this directly into my tree for -rc3?

Paolo