From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758054AbcIND5Q (ORCPT ); Tue, 13 Sep 2016 23:57:16 -0400 Received: from mail-wm0-f48.google.com ([74.125.82.48]:37887 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752626AbcIND5O (ORCPT ); Tue, 13 Sep 2016 23:57:14 -0400 MIME-Version: 1.0 In-Reply-To: <992bf785-c4bb-b2c1-dd9b-87b74d9127c2@redhat.com> References: <20160913190145.GE15680@potion> <992bf785-c4bb-b2c1-dd9b-87b74d9127c2@redhat.com> From: Wanpeng Li Date: Wed, 14 Sep 2016 11:57:11 +0800 Message-ID: Subject: Re: kvm-unit-test fail for split irqchip To: Paolo Bonzini Cc: Radim Krcmar , kvm , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2016-09-14 4:43 GMT+08:00 Paolo Bonzini : > > > 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, Yeah, I just sent out a patch to fix the bug. Thanks for the long discussion with me and thanks Radim's proposal. :) > and change hw/intc/apic.c to use a new casting > macro > > APICCommonState *s = APIC(dev); > > instead of APIC_COMMON? > I'm not familiar with QOM too much, what APIC macro do you like? Regards, Wanpeng Li