qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] ppc-e500: implement PCI INTx routing


From: address@hidden
Subject: Re: [Qemu-devel] [PATCH 2/2] ppc-e500: implement PCI INTx routing
Date: Fri, 20 Dec 2013 04:15:09 +0000


> -----Original Message-----
> From: Michael S. Tsirkin [mailto:address@hidden
> Sent: Friday, December 20, 2013 12:02 AM
> To: Alexander Graf
> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers; qemu-ppc
> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> 
> On Thu, Dec 19, 2013 at 05:28:17PM +0100, Alexander Graf wrote:
> >
> > On 19.12.2013, at 16:39, address@hidden wrote:
> >
> > >
> > >
> > >> -----Original Message-----
> > >> From: Michael S. Tsirkin [mailto:address@hidden
> > >> Sent: Thursday, December 19, 2013 3:55 AM
> > >> To: Alexander Graf
> > >> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; QEMU Developers;
> > >> qemu-ppc; Bhushan
> > >> Bharat-R65777
> > >> Subject: Re: [PATCH 2/2] ppc-e500: implement PCI INTx routing
> > >>
> > >> On Wed, Dec 18, 2013 at 10:53:32PM +0100, Alexander Graf wrote:
> > >>>
> > >>> On 28.11.2013, at 07:35, Bharat Bhushan <address@hidden> wrote:
> > >>>
> > >>>> This patch adds pci pin to irq_num routing callback Without this
> > >>>> patch we gets below warning
> > >>>>
> > >>>> "
> > >>>> PCI: Bug - unimplemented PCI INTx routing (e500-pcihost)
> > >>>> qemu-system-ppc64: PCI: Bug - unimplemented PCI INTx routing
> > >>>> (e500-pcihost) "
> > >>>>
> > >>>> Signed-off-by: Bharat Bhushan <address@hidden>
> > >>>> ---
> > >>>> hw/pci-host/ppce500.c |   20 ++++++++++++++++++--
> > >>>> 1 files changed, 18 insertions(+), 2 deletions(-)
> > >>>>
> > >>>> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c index
> > >>>> 49bfcc6..3c4cf9e 100644
> > >>>> --- a/hw/pci-host/ppce500.c
> > >>>> +++ b/hw/pci-host/ppce500.c
> > >>>> @@ -88,6 +88,7 @@ struct PPCE500PCIState {
> > >>>>    struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
> > >>>>    uint32_t gasket_time;
> > >>>>    qemu_irq irq[PCI_NUM_PINS];
> > >>>> +    uint32_t irq_num[PCI_NUM_PINS];
> > >>>>    uint32_t first_slot;
> > >>>>    /* mmio maps */
> > >>>>    MemoryRegion container;
> > >>>> @@ -267,13 +268,26 @@ static int mpc85xx_pci_map_irq(PCIDevice
> > >>>> *pci_dev, int pin)
> > >>>>
> > >>>> static void mpc85xx_pci_set_irq(void *opaque, int pin, int level) {
> > >>>> -    qemu_irq *pic = opaque;
> > >>>> +    PPCE500PCIState *s = opaque;
> > >>>> +    qemu_irq *pic = s->irq;;
> > >>>
> > >>> Double semicolon?
> > >
> > > Ok, will correct.
> > >
> > >>>
> > >>>>
> > >>>>    pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin ,
> > >>>> level);
> > >>>>
> > >>>>    qemu_set_irq(pic[pin], level); }
> > >>>>
> > >>>> +static PCIINTxRoute e500_route_intx_pin_to_irq(void *opaque, int
> > >>>> +pin) {
> > >>>> +    PCIINTxRoute route;
> > >>>> +    PPCE500PCIState *s = opaque;
> > >>>> +
> > >>>> +    route.mode = PCI_INTX_ENABLED;
> > >>>> +    route.irq = s->irq_num[pin];
> > >>>> +
> > >>>> +    pci_debug("%s: PCI irq-pin = %d, irq_num= %d\n", __func__,
> > >>>> + pin,
> > >> route.irq);
> > >>>> +    return route;
> > >>>> +}
> > >>>> +
> > >>>> static const VMStateDescription vmstate_pci_outbound = {
> > >>>>    .name = "pci_outbound",
> > >>>>    .version_id = 0,
> > >>>> @@ -350,12 +364,13 @@ static int e500_pcihost_initfn(SysBusDevice
> > >>>> *dev)
> > >>>>
> > >>>>    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
> > >>>>        sysbus_init_irq(dev, &s->irq[i]);
> > >>>> +        s->irq_num[i] = i + 1;
> > >>>
> > >>> Doesn't this duplicate the logic from ppce500_pci_map_irq_slot()?
> > >>> I don't
> > >> understand the purpose of this whole exercise to be honest.
> > >>>
> > >>> Michael, could you please shed some light on this?
> > >>>
> > >>>
> > >>> Alex
> > >>
> > >> This is printed by pci_device_route_intx_to_irq - it's used by
> > >> device assignment and vfio to figure out which irq does a given pci 
> > >> device
> drive.
> > >
> > > Yes, exactly same reason.
> >
> > Is there any way we could get rid of the information duplication? The fact
> that INTA/B/C/D are mapped to 1,2,3,4 is really a configuration parameter that
> should only live at a single spot.
> >
> >
> > Alex
> 
> Yes. In fact I had the idea to only have something like
> pci_device_route_intx_to_irq and call it once for all interrupts and cache 
> that,
> then use this to send interrupts directly to apic.
> Redo this each time routing changes.
> I had a patch like this (and I think Jan had one too), but Anthony said he'll
> rewrite all interrupt routing using QOM so I dropped it. I'll try to resurrect
> it.

So do we want to have this patch almost in this shape and hope Anthony's 
changes will handle this well or wait for Anthony patches first ?

Thanks
-Bharat

> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]