From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37271) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMK34-0002Ne-B9 for qemu-devel@nongnu.org; Thu, 11 Oct 2012 10:47:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMK2u-00043X-7M for qemu-devel@nongnu.org; Thu, 11 Oct 2012 10:47:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4397) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMK2t-00043Q-VQ for qemu-devel@nongnu.org; Thu, 11 Oct 2012 10:47:32 -0400 Date: Thu, 11 Oct 2012 16:49:16 +0200 From: "Michael S. Tsirkin" Message-ID: <20121011144916.GH8983@redhat.com> References: <1989bebbf4a990b0c401b18815d8f6dbbdbcd4b6.1349749915.git.jbaron@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1989bebbf4a990b0c401b18815d8f6dbbdbcd4b6.1349749915.git.jbaron@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 18/21] q35: Fix irr initialization for slots 25..31 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Baron Cc: agraf@suse.de, aliguori@us.ibm.com, juzhang@redhat.com, jan.kiszka@siemens.com, qemu-devel@nongnu.org, armbru@redhat.com, blauwirbel@gmail.com, yamahata@valinux.co.jp, alex.williamson@redhat.com, kevin@koconnor.net, avi@redhat.com, mkletzan@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, afaerber@suse.de, kraxel@redhat.com On Mon, Oct 08, 2012 at 11:30:37PM -0400, Jason Baron wrote: > From: Isaku Yamahata > > This was totally off: The CC registers are 16 bit (stored as little > endian), their offsets run in reverse order, and D26IR as well as D25IR > have 4 bytes offset to their successors. > > Reported-by: Jan Kiszka > Signed-off-by: Isaku Yamahata > Signed-off-by: Jason Baron Probably should be smashed into the original patch too? More contained so less critical. > --- > hw/q35.c | 29 ++++++++++++++++++++--------- > 1 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/hw/q35.c b/hw/q35.c > index 5d256cb..e4f313e 100644 > --- a/hw/q35.c > +++ b/hw/q35.c > @@ -480,7 +480,7 @@ static void ich9_lpc_reset(DeviceState *qdev); > * Although it's not pci configuration space, it's little endian as Intel. > */ > > -static void ich9_cc_update_ir(uint8_t irr[PCI_NUM_PINS], uint32_t ir) > +static void ich9_cc_update_ir(uint8_t irr[PCI_NUM_PINS], uint16_t ir) > { > int intx; > for (intx = 0; intx < PCI_NUM_PINS; intx++) { > @@ -491,15 +491,26 @@ static void ich9_cc_update_ir(uint8_t irr[PCI_NUM_PINS], uint32_t ir) > static void ich9_cc_update(ICH9LPCState *lpc) > { > int slot; > - int reg_offset; > - int intx; > + int pci_intx; > + > + const int reg_offsets[] = { > + ICH9_CC_D25IR, > + ICH9_CC_D26IR, > + ICH9_CC_D27IR, > + ICH9_CC_D28IR, > + ICH9_CC_D29IR, > + ICH9_CC_D30IR, > + ICH9_CC_D31IR, > + }; > + const int *offset; > > /* D{25 - 31}IR, but D30IR is read only to 0. */ > - for (slot = 25, reg_offset = 0; slot < 32; slot++, reg_offset++) { > - if (slot != 30) { > - ich9_cc_update_ir(lpc->irr[slot], > - lpc->chip_config[ICH9_CC_D31IR + reg_offset]); > + for (slot = 25, offset = reg_offsets; slot < 32; slot++, offset++) { > + if (slot == 30) { > + continue; > } > + ich9_cc_update_ir(lpc->irr[slot], > + pci_get_word(lpc->chip_config + *offset)); > } > > /* > @@ -508,8 +519,8 @@ static void ich9_cc_update(ICH9LPCState *lpc) > * are connected to pirq lines. Our choice is PIRQ[E-H]. > * INT[A-D] are connected to PIRQ[E-H] > */ > - for (intx = 0; intx < PCI_NUM_PINS; intx++) { > - lpc->irr[30][intx] = intx + 4; > + for (pci_intx = 0; pci_intx < PCI_NUM_PINS; pci_intx++) { > + lpc->irr[30][pci_intx] = pci_intx + 4; > } > } > > -- > 1.7.1