From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38159) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c8lUo-000441-T8 for qemu-devel@nongnu.org; Mon, 21 Nov 2016 05:06:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c8lUk-0007DV-S0 for qemu-devel@nongnu.org; Mon, 21 Nov 2016 05:06:42 -0500 References: <20161120231605.D90DD745789@zero.eik.bme.hu> From: Thomas Huth Message-ID: <897afd71-0644-a9e7-a15d-0e457aade67c@redhat.com> Date: Mon, 21 Nov 2016 11:06:34 +0100 MIME-Version: 1.0 In-Reply-To: <20161120231605.D90DD745789@zero.eik.bme.hu> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: Make uninorth interrupt swizzling identical to Grackle List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: BALATON Zoltan , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, David Gibson , Benjamin Herrenschmidt On 20.11.2016 15:12, BALATON Zoltan wrote: > From: Benjamin Herrenschmidt > > It's currently broken as it uses an incorrect shift, it tries > to use the slot number but uses the top bits of the bus number > instead. > > Note: Neither implementation matches what OpenBIOS ends up putting > in the device-tree either, which will have to be fixed separately. > > This is not quite correct for modelling a real Mac since Apple > tend to tie all 4 interrupt lines of a slot together and have > separate interrupts for every slot and every motherboard devices > going straight to the PIC but we'll sort that out later. > > Signed-off-by: Benjamin Herrenschmidt > --- > hw/pci-host/uninorth.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > This needs a corresponding fix in OpenBIOS but this has to be > committed first for that. As this is already broken making this change > should not make things worse as they are now. Could we get this in now > as a bugfix commit? > > diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c > index 7aac4d6..df342ac 100644 > --- a/hw/pci-host/uninorth.c > +++ b/hw/pci-host/uninorth.c > @@ -62,9 +62,7 @@ typedef struct UNINState { > > static int pci_unin_map_irq(PCIDevice *pci_dev, int irq_num) > { > - int devfn = pci_dev->devfn & 0x00FFFFFF; > - > - return (((devfn >> 11) & 0x1F) + irq_num) & 3; > + return (irq_num + (pci_dev->devfn >> 3)) & 3; > } > > static void pci_unin_set_irq(void *opaque, int irq_num, int level) > Looks reasonable. Reviewed-by: Thomas Huth