Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1529560

Re: [PATCH] KVM: x86: check for pic and ioapic presence before use

From Paolo Bonzini <pbonzini@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH] KVM: x86: check for pic and ioapic presence before use
Date 2016-11-24 17:40 +0100
Message-ID <sH5e2-5eA-45@gated-at.bofh.it> (permalink)
References <sGMl4-1a7-3@gated-at.bofh.it> <sGNK9-1Sn-7@gated-at.bofh.it> <sH1Dr-2Hg-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


> Oops, I wrote the race with wrong IOCTL -- it should be KVM_IRQ_LINE.
> 
>  1) set KVM_CAP_SPLIT_IRQCHIP (unlocks KVM_IRQ_LINE)
>  a) call KVM_CREATE_IRQCHIP (creates routes while !kvm->arch.vpic)
>  b) concurrently call KVM_IRQ_LINE for PIO routes (dereferences NULL)
> 
> The problem is that we use pic_in_kernel() as irqchip_in_kernel(), so it
> cannot be set before we set up routes, but we then cannot reject routes
> when pic is not in use.  The best effort is to do this for pic routes in
> kvm_set_routing_entry():
> 
>  // initialization is the only place where pic_in_kernel() !=
>  ioapic_in_kernel()
>  if (!pic_in_kernel(kvm) && !ioapic_in_kernel(kvm))
>  	goto out;
> 
> and similar for ioapic routes:
> 
>  if (!ioapic_in_kernel(kvm))
>  	goto out;
> 
> I think it would work if we forbade KVM_CREATE_IRQCHIP after
> KVM_CAP_SPLIT_IRQCHIP (which we want to do anyway).

Yeah, definitely.

> And adding a new
> variable for irqchip_in_kernel() would allow us to make the pic
> condition reasonabled.

Or change kvm->arch.irqchip_split to an enum.

> I'll do something like that for 4.10, but the current patch is better
> suited for stable.
> 
> Would fixing the comment be enough?

Yes, fine!

> Do you want the following hunk already in 4.9?
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6f9c9ad13f88..dbed51045c37 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3901,7 +3901,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  
>  		mutex_lock(&kvm->lock);
>  		r = -EEXIST;
> -		if (kvm->arch.vpic)
> +		if (irqchip_in_kernel(kvm))
>  			goto create_irqchip_unlock;
>  		r = -EINVAL;
>  		if (kvm->created_vcpus)

No, it's unnecessary.

Paolo

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread


Thread

[PATCH] KVM: x86: check for pic and ioapic presence before use Radim Krčmář <rkrcmar@redhat.com> - 2016-11-23 21:30 +0100
  Re: [PATCH] KVM: x86: check for pic and ioapic presence before use Paolo Bonzini <pbonzini@redhat.com> - 2016-11-23 23:00 +0100
    Re: [PATCH] KVM: x86: check for pic and ioapic presence before use Radim Krčmář <rkrcmar@redhat.com> - 2016-11-24 13:50 +0100
      Re: [PATCH] KVM: x86: check for pic and ioapic presence before use Paolo Bonzini <pbonzini@redhat.com> - 2016-11-24 17:40 +0100

csiph-web