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

[Peter Xu <peterx@redhat.com>]

On Tue, Mar 14, 2017 at 02:34:30PM +0100, David Hildenbrand wrote:
> Let's replace the checks for pic_in_kernel() and ioapic_in_kernel() by
> checks against irqchip_mode. Add another state, indicating that the
> caller is currently initializing the irqchip.
> 
> This is necessary to switch pic_in_kernel() and ioapic_in_kernel() to
> irqchip_mode, too.
> 
> Also make sure that creation of any route is only possible if we have
> a lapic in kernel (irqchip_in_kernel()) or if we are currently
> inititalizing the irqchip.

The subject of the patch is:

  KVM: x86: check against irqchip_mode in kvm_set_routing_entry()

IMHO the most significant change this patch brought is the new irqchip
mode, but we just didn't mention it in the subject...

If we really want this new mode, maybe split current patch into two?
One for the new mode, one for the change in kvm_set_routing_entry()?

Thanks,

> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/irq.h              |  2 +-
>  arch/x86/kvm/irq_comm.c         | 12 ++++++------
>  arch/x86/kvm/x86.c              |  7 ++++++-
>  4 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 74ef58c..164e544 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -727,6 +727,7 @@ struct kvm_hv {
>  
>  enum kvm_irqchip_mode {
>  	KVM_IRQCHIP_NONE,
> +	KVM_IRQCHIP_INIT_IN_PROGRESS, /* temporarily set during creation */
>  	KVM_IRQCHIP_KERNEL,       /* created with KVM_CREATE_IRQCHIP */
>  	KVM_IRQCHIP_SPLIT,        /* created with KVM_CAP_SPLIT_IRQCHIP */
>  };
> diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
> index 40d5b2c..7fa2480 100644
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -103,7 +103,7 @@ static inline int irqchip_kernel(struct kvm *kvm)
>  
>  static inline int irqchip_in_kernel(struct kvm *kvm)
>  {
> -	bool ret = kvm->arch.irqchip_mode != KVM_IRQCHIP_NONE;
> +	bool ret = kvm->arch.irqchip_mode > KVM_IRQCHIP_INIT_IN_PROGRESS;
>  
>  	/* 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 6825cd3..0a026f8 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -282,24 +282,24 @@ int kvm_set_routing_entry(struct kvm *kvm,
>  	int delta;
>  	unsigned max_pin;
>  
> +	/* also allow creation of routes during KVM_IRQCHIP_INIT_IN_PROGRESS */
> +	if (kvm->arch.irqchip_mode == KVM_IRQCHIP_NONE)
> +		goto out;
> +
>  	switch (ue->type) {
>  	case KVM_IRQ_ROUTING_IRQCHIP:
> +		if (irqchip_split(kvm))
> +			goto out;
>  		delta = 0;
>  		switch (ue->u.irqchip.irqchip) {
>  		case KVM_IRQCHIP_PIC_SLAVE:
>  			delta = 8;
>  			/* fall through */
>  		case KVM_IRQCHIP_PIC_MASTER:
> -			if (!pic_in_kernel(kvm))
> -				goto out;
> -
>  			e->set = kvm_set_pic_irq;
>  			max_pin = PIC_NUM_PINS;
>  			break;
>  		case KVM_IRQCHIP_IOAPIC:
> -			if (!ioapic_in_kernel(kvm))
> -				goto out;
> -
>  			max_pin = KVM_IOAPIC_NUM_PINS;
>  			e->set = kvm_set_ioapic_irq;
>  			break;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1faf620..fa8cceb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3934,9 +3934,12 @@ 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;
>  			goto split_irqchip_unlock;
> +		}
>  		/* Pairs with irqchip_in_kernel. */
>  		smp_wmb();
>  		kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
> @@ -4024,8 +4027,10 @@ 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;
>  			mutex_lock(&kvm->slots_lock);
>  			mutex_lock(&kvm->irq_lock);
>  			kvm_ioapic_destroy(kvm);
> -- 
> 2.9.3
> 

-- peterx