From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Hildenbrand Subject: Re: [PATCH v3 02/24] KVM: x86: new irqchip mode KVM_IRQCHIP_INIT_IN_PROGRESS Date: Wed, 12 Apr 2017 21:55:18 +0200 Message-ID: <6389097d-3393-ee12-ad6c-16a9050fcdf6@redhat.com> References: <20170407085041.5257-1-david@redhat.com> <20170407085041.5257-3-david@redhat.com> <20170412182638.GA20145@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, Paolo Bonzini , Peter Xu To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:58728 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754445AbdDLTzX (ORCPT ); Wed, 12 Apr 2017 15:55:23 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CADA519D22F for ; Wed, 12 Apr 2017 19:55:22 +0000 (UTC) In-Reply-To: <20170412182638.GA20145@potion> Sender: kvm-owner@vger.kernel.org List-ID: 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 >> --- >> 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