Re: [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()

From: Paolo Bonzini <pbonzini@redhat.com>
To: David Hildenbrand <david@redhat.com>, kvm@vger.kernel.org
Cc: rkrcmar@redhat.com
Subject: Re: [PATCH RFC 02/21] KVM: x86: check against irqchip_mode in kvm_set_routing_entry()
Date: Mon, 6 Mar 2017 19:08:37 +0100	[thread overview]
Message-ID: <b54970b6-5b67-4b4a-b8b4-81bdf068f259@redhat.com> (raw)
In-Reply-To: <20170306131815.12033-3-david@redhat.com>

On 06/03/2017 14:17, David Hildenbrand wrote:
>  	KVM_IRQCHIP_NONE,
> +	KVM_IRQCHIP_KERNEL_INIT,  /* KVM_CREATE_IRQCHIP in progress */

Maybe KVM_IRQCHIP_INIT_IN_PROGRESS?

> -	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
> +	bool ret = kvm->arch.irqchip_mode == KVM_IRQCHIP_KERNEL ||
> +		   kvm->arch.irqchip_mode == KVM_IRQCHIP_SPLIT;

I suspect that if you phrase it the other way round (!= NONE && !=
KERNEL_INIT) you'll get infinitesimally better code, because it can be
compiled to an unsigned comparison with 1.

>  	/* Matches with wmb after initializing kvm->irq_routing. */
>  	smp_rmb();
> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> index b96d389..4e4a67a 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -282,22 +282,18 @@ int kvm_set_routing_entry(struct kvm *kvm,
>  
>  	switch (ue->type) {
>  	case KVM_IRQ_ROUTING_IRQCHIP:
> +		if (!irqchip_kernel(kvm) && !irqchip_kernel_init(kvm))
> +			goto out;
>  		delta = 0;

This can be irqchip_in_kernel, after which irqchip_kernel_init can be
removed.

Should the code to enable split irqchip also use KVM_IRQCHIP_KERNEL_INIT?

Paolo