From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759987AbcINJmd (ORCPT ); Wed, 14 Sep 2016 05:42:33 -0400 Received: from mail-wm0-f49.google.com ([74.125.82.49]:36052 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750975AbcINJmb (ORCPT ); Wed, 14 Sep 2016 05:42:31 -0400 Subject: Re: kvm-unit-test fail for split irqchip To: Wanpeng Li References: <20160913190145.GE15680@potion> <992bf785-c4bb-b2c1-dd9b-87b74d9127c2@redhat.com> Cc: Radim Krcmar , kvm , "linux-kernel@vger.kernel.org" From: Paolo Bonzini Message-ID: <5c70df87-a6ea-4269-02a1-a3d4b77a40c8@redhat.com> Date: Wed, 14 Sep 2016 11:42:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14/09/2016 05:57, Wanpeng Li wrote: > 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? The macro would be defined like #define TYPE_APIC "apic" #define APIC(obj) \ OBJECT_CHECK(APICCommonState, (obj), TYPE_APIC) With this change and without your other patch, you should get an assertion failure in eventinj.flat (which would have pointed immediately at the bug!). Thanks, Paolo > Regards, > Wanpeng Li > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >