On 06/02/12 12:53, Carsten Otte wrote:
> Am 06.02.2012 10:59, schrieb Christian Borntraeger:
>> +static int __sigp_restart(struct kvm_vcpu *vcpu, u16 cpu_addr)
>> +{
>> + int rc = 0;
>> + struct kvm_s390_float_interrupt *fi =&vcpu->kvm->arch.float_int;
>> +
>> + spin_lock(&fi->lock);
>> + if (fi->local_int[cpu_addr]->action_bits& ACTION_STOP_ON_STOP)
>> + rc = 2; /* busy */
>> + else
>> + VCPU_EVENT(vcpu, 4, "sigp restart %x to handle userspace",
>> + cpu_addr);
>> + spin_unlock(&fi->lock);
>> +
>> + return rc;
>> +}
>>
> local_int->action_bits is protected by the local int lock of subject CPU,
> as one can see in patch #2 of this series. This is racy.
Indeed the inner lock is missing (something like the addon-patch below)
Jens, can you update and test the patch accordingly?
--- a/arch/s390/kvm/sigp.c
+++ b/arch/s390/kvm/sigp.c
@@ -316,13 +316,26 @@ static int __sigp_restart(struct kvm_vcpu *vcpu, u16 cpu_addr)
{
int rc = 0;
struct kvm_s390_float_interrupt *fi = &vcpu->kvm->arch.float_int;
+ struct kvm_s390_local_interrupt *li;
+
+ if (cpu_addr >= KVM_MAX_VCPUS)
+ return 3; /* not operational */
spin_lock(&fi->lock);
- if (fi->local_int[cpu_addr]->action_bits & ACTION_STOP_ON_STOP)
+ li = fi->local_int[cpu_addr];
+ if (li == NULL) {
+ rc = 3; /* not operational */
+ goto out;
+ }
+
+ spin_lock_bh(&li->lock);
+ if (li->action_bits & ACTION_STOP_ON_STOP)
rc = 2; /* busy */
else
VCPU_EVENT(vcpu, 4, "sigp restart %x to handle userspace",
cpu_addr);
+ spin_unlock_bh(&li->lock);
+out:
spin_unlock(&fi->lock);
return rc;
Christian
--
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
