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@xxxxxxxxxx>
>> ---
>> 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
