On Tue, Feb 07, 2012 at 05:32:07PM +1100, Michael Ellerman wrote:
> A test case which does the following:
>
> ioctl(vmfd, KVM_CREATE_VCPU, 0);
> ioctl(vmfd, KVM_CREATE_IRQCHIP);
> ioctl(cpufd, KVM_RUN);
>
> Can oops in kvm_apic_accept_pic_intr() because vcpu->arch.apic == NULL.
>
> Because irqchip_in_kernel() is false when we create the vcpu we leave
> vcpu->arch.apic uninitialised (in kvm_arch_vcpu_init()). Then when we run,
> irqchip_in_kernel() is true, but we didn't do the correct initialisation.
>
> The root of the problem seems to be that there is an assumption that
> KVM_CREATE_IRQCHIP will be called before any VCPUs are created. The
> documentation says "sets up future vcpus to have a local APIC".
>
> So the simplest fix seems to be to enforce that ordering in the code.
Ugh. With your patch below there is still the window for a race:
kvm_arch_vcpu_init can create a vcpu without vcpu->arch.apic,
block on mutex_lock(kvm->lock). Meanwhile a separate thread is on
KVM_CREATE_IRQCHIP holding kvm->lock, finds online_vcpus == 0 and
proceeds. Then kvm_arch_vcpu_init finishes.
Moving mutex_lock(kvm->lock) to the beginning of
kvm_vm_ioctl_create_vcpu should fix it?
>
> Signed-off-by: Michael Ellerman <michael@xxxxxxxxxxxxxx>
> ---
> arch/x86/kvm/x86.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 14d6cad..27dd380 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3110,6 +3110,9 @@ long kvm_arch_vm_ioctl(struct file *filp,
> r = -EEXIST;
> if (kvm->arch.vpic)
> goto create_irqchip_unlock;
> + r = -EINVAL;
> + if (atomic_read(&kvm->online_vcpus))
> + goto create_irqchip_unlock;
> r = -ENOMEM;
> vpic = kvm_create_pic(kvm);
> if (vpic) {
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
