Re: [PATCH v3 02/24] KVM: x86: new irqchip mode KVM_IRQCHIP_INIT_IN_PROGRESS

[David Hildenbrand <david@redhat.com>]

On 12.04.2017 20:26, Radim Krčmář wrote:
> 2017-04-07 10:50+0200, David Hildenbrand:
>> Let's add a new mode and set it while we create the irqchip via
>> KVM_CREATE_IRQCHIP and KVM_CAP_SPLIT_IRQCHIP.
>>
>> This mode will be used later to test if adding routes
>> (in kvm_set_routing_entry()) is already allowed.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> @@ -3934,9 +3934,14 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>>  			goto split_irqchip_unlock;
>>  		if (kvm->created_vcpus)
>>  			goto split_irqchip_unlock;
>> +		kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
>>  		r = kvm_setup_empty_irq_routing(kvm);
>> -		if (r)
>> +		if (r) {
>> +			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
>> +			/* Pairs with smp_rmb() when reading irqchip_mode */
>> +			smp_wmb();
>>  			goto split_irqchip_unlock;
>> +		}
>>  		/* Pairs with irqchip_in_kernel. */
>>  		smp_wmb();
>>  		kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
>> @@ -4024,8 +4029,12 @@ long kvm_arch_vm_ioctl(struct file *filp,
>>  			goto create_irqchip_unlock;
>>  		}
>>  
>> +		kvm->arch.irqchip_mode = KVM_IRQCHIP_INIT_IN_PROGRESS;
>>  		r = kvm_setup_default_irq_routing(kvm);
>>  		if (r) {
>> +			kvm->arch.irqchip_mode = KVM_IRQCHIP_NONE;
>> +			/* Pairs with smp_rmb() when reading irqchip_mode */
>> +			smp_wmb();
> 
> I think these two barriers are pointless.
> 
> KVM_IRQCHIP_INIT_IN_PROGRESS and KVM_IRQCHIP_NONE have the same result
> for readers of irqchip mode and we don't even have a barrier after
> setting KVM_IRQCHIP_INIT_IN_PROGRESS mode, so it looks weird.
> 
> I can remove them if you agree.

Paolo requested this, and I agreed to rather have a unified handling for
memory barriers with kvm->arch.irqchip_mode, then trying to optimize for
special cases.

(implicit knowledge here is e.g. that this is protected by kvm->lock)

If you think we can drop this, feel free to drop.

Thanks!

> 
> Thanks.
> 


-- 

Thanks,

David