From patchwork Tue Dec 6 12:27:53 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 129664 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 150431007D4 for ; Tue, 6 Dec 2011 23:26:30 +1100 (EST) Received: from localhost ([::1]:51596 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RXu6G-0007h9-DQ for incoming@patchwork.ozlabs.org; Tue, 06 Dec 2011 07:26:20 -0500 Received: from eggs.gnu.org ([140.186.70.92]:34679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RXu6A-0007gt-QD for qemu-devel@nongnu.org; Tue, 06 Dec 2011 07:26:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RXu64-0004gI-DC for qemu-devel@nongnu.org; Tue, 06 Dec 2011 07:26:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51109) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RXu64-0004g5-6H for qemu-devel@nongnu.org; Tue, 06 Dec 2011 07:26:08 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pB6CQ6re015499 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 6 Dec 2011 07:26:06 -0500 Received: from redhat.com (dhcp-4-49.tlv.redhat.com [10.35.4.49]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id pB6CQ3rC013618; Tue, 6 Dec 2011 07:26:04 -0500 Date: Tue, 6 Dec 2011 14:27:53 +0200 From: "Michael S. Tsirkin" To: Michael Tokarev Message-ID: <20111206122752.GA31385@redhat.com> References: <4EDC8D06.20308@msgid.tls.msk.ru> <4EDCC6FE.8040702@redhat.com> <4EDD2763.8010808@msgid.tls.msk.ru> <4EDDEF26.9030403@redhat.com> <4EDDF659.6040701@msgid.tls.msk.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4EDDF659.6040701@msgid.tls.msk.ru> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 209.132.183.28 Cc: Isaku Yamahata , Avi Kivity , KVM list , qemu-devel@nongnu.org Subject: Re: [Qemu-devel] winXP "Standard PC" HAL and qemu-kvm >= 0.15 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Tue, Dec 06, 2011 at 03:02:49PM +0400, Michael Tokarev wrote: > On 06.12.2011 14:32, Avi Kivity wrote: > > On 12/05/2011 10:19 PM, Michael Tokarev wrote: > >> On 05.12.2011 17:28, Avi Kivity wrote: > >> [] > >>>> I haven't debugged further yet, -- because it were > >>>> not easy to find out what was causing the regression > >>>> and how to reproduce it, and also because I don't think > >>>> it is the right HAL for qemu-kvm guest anyway. > >>> > >>> It's not, but the regression indicates we broke something. It would be > >>> good to know what that is. > >> > >> So today I gave it a chance with git bisect, and here's what it found: > > >> First bad commit ef390067a72fe09977bb4ac8211313e1503302ea > >> Merge: c7b3e90 0fd542f > >> Author: Avi Kivity > >> Date: Sun May 15 04:48:05 2011 -0400 > >> > >> Merge commit '0fd542fb7d13ddf12f897bb27c5950f31638b1df' into upstream-merge > > And after applying Avi's instructions here's the real bisect > result: > > ab431c283e7055bcd6fb622f212bb29e84a6a134 is the first bad commit > commit ab431c283e7055bcd6fb622f212bb29e84a6a134 > Author: Isaku Yamahata > Date: Fri Apr 1 20:43:23 2011 +0900 > > piix_pci: optimize set irq path Could you try with this commit reverted please? Reverting patch below. Warning: compiled only. commit 8f40db3918a0618a3beb9a771a569d20fe9c1bb3 Author: Michael S. Tsirkin Date: Tue Dec 6 14:24:32 2011 +0200 Revert "piix_pci: optimize set irq path" This reverts commit ab431c283e7055bcd6fb622f212bb29e84a6a134. Conflicts: hw/piix_pci.c diff --git a/hw/piix_pci.c b/hw/piix_pci.c index ee11ff2..5b35c01 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -38,28 +38,12 @@ typedef PCIHostState I440FXState; -#define PIIX_NUM_PIC_IRQS 16 /* i8259 * 2 */ #define PIIX_NUM_PIRQS 4ULL /* PIRQ[A-D] */ #define XEN_PIIX_NUM_PIRQS 128ULL #define PIIX_PIRQC 0x60 typedef struct PIIX3State { PCIDevice dev; - - /* - * bitmap to track pic levels. - * The pic level is the logical OR of all the PCI irqs mapped to it - * So one PIC level is tracked by PIIX_NUM_PIRQS bits. - * - * PIRQ is mapped to PIC pins, we track it by - * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with - * pic_irq * PIIX_NUM_PIRQS + pirq - */ -#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64 -#error "unable to encode pic state in 64bit in pic_levels." -#endif - uint64_t pic_levels; - qemu_irq *pic; /* This member isn't used. Just for save/load compatibility */ @@ -365,61 +349,24 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, return b; } -/* PIIX3 PCI to ISA bridge */ -static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) -{ - qemu_set_irq(piix3->pic[pic_irq], - !!(piix3->pic_levels & - (((1ULL << PIIX_NUM_PIRQS) - 1) << - (pic_irq * PIIX_NUM_PIRQS)))); -} - -static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) -{ - int pic_irq; - uint64_t mask; - - pic_irq = piix3->dev.config[PIIX_PIRQC + pirq]; - if (pic_irq >= PIIX_NUM_PIC_IRQS) { - return; - } - - mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq); - piix3->pic_levels &= ~mask; - piix3->pic_levels |= mask * !!level; - - piix3_set_irq_pic(piix3, pic_irq); -} - -static void piix3_set_irq(void *opaque, int pirq, int level) +static void piix3_set_irq(void *opaque, int irq_num, int level) { + int i, pic_irq, pic_level; PIIX3State *piix3 = opaque; - piix3_set_irq_level(piix3, pirq, level); -} - -/* irq routing is changed. so rebuild bitmap */ -static void piix3_update_irq_levels(PIIX3State *piix3) -{ - int pirq; - - piix3->pic_levels = 0; - for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) { - piix3_set_irq_level(piix3, pirq, - pci_bus_get_irq_level(piix3->dev.bus, pirq)); - } -} -static void piix3_write_config(PCIDevice *dev, - uint32_t address, uint32_t val, int len) -{ - pci_default_write_config(dev, address, val, len); - if (ranges_overlap(address, len, PIIX_PIRQC, 4)) { - PIIX3State *piix3 = DO_UPCAST(PIIX3State, dev, dev); - int pic_irq; - piix3_update_irq_levels(piix3); - for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) { - piix3_set_irq_pic(piix3, pic_irq); + /* now we change the pic irq level according to the piix irq mappings */ + /* XXX: optimize */ + pic_irq = piix3->dev.config[0x60 + irq_num]; + if (pic_irq < 16) { + /* The pic level is the logical OR of all the PCI irqs mapped + to it */ + pic_level = 0; + for (i = 0; i < 4; i++) { + if (pic_irq == piix3->dev.config[0x60 + i]) { + pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i); + } } + qemu_set_irq(piix3->pic[pic_irq], pic_level); } } @@ -434,7 +381,7 @@ static void piix3_write_config_xen(PCIDevice *dev, uint32_t address, uint32_t val, int len) { xen_piix_pci_write_config_client(address, val, len); - piix3_write_config(dev, address, val, len); + pci_default_write_config(dev, address, val, len); } static void piix3_reset(void *opaque) @@ -473,15 +420,6 @@ static void piix3_reset(void *opaque) pci_conf[0xab] = 0x00; pci_conf[0xac] = 0x00; pci_conf[0xae] = 0x00; - - d->pic_levels = 0; -} - -static int piix3_post_load(void *opaque, int version_id) -{ - PIIX3State *piix3 = opaque; - piix3_update_irq_levels(piix3); - return 0; } static void piix3_pre_save(void *opaque) @@ -500,7 +438,6 @@ static const VMStateDescription vmstate_piix3 = { .version_id = 3, .minimum_version_id = 2, .minimum_version_id_old = 2, - .post_load = piix3_post_load, .pre_save = piix3_pre_save, .fields = (VMStateField []) { VMSTATE_PCI_DEVICE(dev, PIIX3State), @@ -541,7 +478,6 @@ static PCIDeviceInfo i440fx_info[] = { .qdev.no_user = 1, .no_hotplug = 1, .init = piix3_initfn, - .config_write = piix3_write_config, .vendor_id = PCI_VENDOR_ID_INTEL, .device_id = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 PCI-to-ISA bridge (Step A1) .class_id = PCI_CLASS_BRIDGE_ISA,