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

Groups > linux.kernel > #1482773

Re: kvm-unit-test fail for split irqchip

Show all headers | View raw

On 13/09/2016 21:01, Radim Krcmar wrote:
> kvm_handle_interrupt() does
> 
>   interrupt_request |= CPU_INTERRUPT_HARD
> 
> which later calls cpu_get_pic_interrupt() in kvm_arch_pre_run(), but
> that function uses stale information from APIC and injects 62 again.
> If we synchronized the APIC, then the test would #GP, because there
> would be no injectable interrupt in LAPIC or PIC, so pic_read_irq()
> would return 15, thinking it was spurious.
> 
> I think the bug starts in pic_irq_request(), which should not touch
> LAPIC.  The patch below makes it work (just the second hunk is
> sufficient), but it's still far from sane code ...

This makes sense.  Most of the functions exported by hw/intc/apic.c
should only be used with a userspace APIC:

0000000000000b70 T apic_accept_pic_intr
00000000000010f0 T apic_deliver_irq
00000000000011e0 T apic_deliver_pic_intr
0000000000001310 T apic_get_interrupt
0000000000001270 T apic_poll_irq
0000000000000a40 T apic_sipi

The patch is okay, though for consistency with other code I'd use
!kvm_irqchip_in_kernel() rather than !kvm_irqchip_is_split().

Wanpeng, can you do that, and change hw/intc/apic.c to use a new casting
macro

    APICCommonState *s = APIC(dev);

instead of APIC_COMMON?

Thanks,

Paolo


> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 47593b741a5b..6983e9f13813 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -161,13 +161,16 @@ int cpu_get_pic_interrupt(CPUX86State *env)
>      X86CPU *cpu = x86_env_get_cpu(env);
>      int intno;
>  
> -    intno = apic_get_interrupt(cpu->apic_state);
> -    if (intno >= 0) {
> -        return intno;
> -    }
> -    /* read the irq from the PIC */
> -    if (!apic_accept_pic_intr(cpu->apic_state)) {
> -        return -1;
> +    if (!kvm_irqchip_is_split()) {
> +        /* XXX: why query APIC at all? */
> +        intno = apic_get_interrupt(cpu->apic_state);
> +        if (intno >= 0) {
> +            return intno;
> +        }
> +        /* read the irq from the PIC */
> +        if (!apic_accept_pic_intr(cpu->apic_state)) {
> +            return -1;
> +        }
>      }
>  
>      intno = pic_read_irq(isa_pic);
> @@ -180,7 +183,7 @@ static void pic_irq_request(void *opaque, int irq, int level)
>      X86CPU *cpu = X86_CPU(cs);
>  
>      DPRINTF("pic_irqs: %s irq %d\n", level? "raise" : "lower", irq);
> -    if (cpu->apic_state) {
> +    if (cpu->apic_state && !kvm_irqchip_is_split()) {
>          CPU_FOREACH(cs) {
>              cpu = X86_CPU(cs);
>              if (apic_accept_pic_intr(cpu->apic_state)) {
> 
> 

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

Thread

kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-13 13:10 +0200
  Re: kvm-unit-test fail for split irqchip Radim Krcmar <rkrcmar@redhat.com> - 2016-09-13 21:10 +0200
    Re: kvm-unit-test fail for split irqchip Paolo Bonzini <pbonzini@redhat.com> - 2016-09-13 22:50 +0200
      Re: kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-14 06:00 +0200
        Re: kvm-unit-test fail for split irqchip Paolo Bonzini <pbonzini@redhat.com> - 2016-09-14 11:50 +0200
          Re: kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-15 03:00 +0200
    Re: kvm-unit-test fail for split irqchip Wanpeng Li <kernellwp@gmail.com> - 2016-09-14 06:00 +0200

csiph-web