| Message ID | 4E688E32.7060207@redhat.com |
|---|---|
| State | New |
| Headers | show |
At 09/08/2011 05:43 PM, Gerd Hoffmann Write: > Hi, > >> I modify the code like this, and the PCI_INTERRUPT_LINE register is >> set, and I can bind >> it to uio_pci_generic: > >> --- a/src/pciinit.c >> +++ b/src/pciinit.c >> @@ -575,6 +575,8 @@ static int pci_bios_init_root_regions(u32 start, >> u32 end) > >> pci_bios_init_bus_bases(&busses[0]); >> - pci_bios_map_device_in_bus(0 /* host bus */); >> + for (bus = 0; bus<= MaxPCIBus; bus++) { >> + pci_bios_map_device_in_bus(bus /* host bus */); > > No. pci_bios_map_device_in_bus goes down recursively when it finds a > bridge, so it should cover all devices already. Yes, pci_bios_map_device() goes down recursively. > >> - pci_bios_init_device_in_bus(0 /* host bus */); >> + pci_bios_init_device_in_bus(bus /* host bus */); >> + } > > That is correct. Can be done easier though by just not limiting device > initialization to a specific bus like in the attached patch. Does that > one work for you? I test it, and it works for me. Thanks Wen Congyang > > cheers, > Gerd
On Thu, Sep 08, 2011 at 05:58:12PM +0800, Wen Congyang wrote: > At 09/08/2011 05:43 PM, Gerd Hoffmann Write: > > Hi, > > > >> I modify the code like this, and the PCI_INTERRUPT_LINE register is > >> set, and I can bind > >> it to uio_pci_generic: > > > >> --- a/src/pciinit.c > >> +++ b/src/pciinit.c > >> @@ -575,6 +575,8 @@ static int pci_bios_init_root_regions(u32 start, > >> u32 end) > > > >> pci_bios_init_bus_bases(&busses[0]); > >> - pci_bios_map_device_in_bus(0 /* host bus */); > >> + for (bus = 0; bus<= MaxPCIBus; bus++) { > >> + pci_bios_map_device_in_bus(bus /* host bus */); > > > > No. pci_bios_map_device_in_bus goes down recursively when it finds a > > bridge, so it should cover all devices already. > > Yes, pci_bios_map_device() goes down recursively. The value seems to be wrong though, I think. It seems to simply use the interrupt pin as array index. Instead, each bridge should interrupts as follows: /* Mapping mandated by PCI-to-PCI Bridge architecture specification, * revision 1.2 */ /* Table 9-1: Interrupt Binding for Devices Behind a Bridge */ static int pci_bridge_dev_map_irq_fn(PCIDevice *dev, int irq_num) { return (irq_num + PCI_SLOT(dev->devfn)) % PCI_NUM_PINS; } until we get to the host bridge. > > > >> - pci_bios_init_device_in_bus(0 /* host bus */); > >> + pci_bios_init_device_in_bus(bus /* host bus */); > >> + } > > > > That is correct. Can be done easier though by just not limiting device > > initialization to a specific bus like in the attached patch. Does that > > one work for you? > > I test it, and it works for me. > > Thanks > Wen Congyang > > > > > cheers, > > Gerd
At 09/08/2011 06:42 PM, Michael S. Tsirkin Write: > On Thu, Sep 08, 2011 at 05:58:12PM +0800, Wen Congyang wrote: >> At 09/08/2011 05:43 PM, Gerd Hoffmann Write: >>> Hi, >>> >>>> I modify the code like this, and the PCI_INTERRUPT_LINE register is >>>> set, and I can bind >>>> it to uio_pci_generic: >>> >>>> --- a/src/pciinit.c >>>> +++ b/src/pciinit.c >>>> @@ -575,6 +575,8 @@ static int pci_bios_init_root_regions(u32 start, >>>> u32 end) >>> >>>> pci_bios_init_bus_bases(&busses[0]); >>>> - pci_bios_map_device_in_bus(0 /* host bus */); >>>> + for (bus = 0; bus<= MaxPCIBus; bus++) { >>>> + pci_bios_map_device_in_bus(bus /* host bus */); >>> >>> No. pci_bios_map_device_in_bus goes down recursively when it finds a >>> bridge, so it should cover all devices already. >> >> Yes, pci_bios_map_device() goes down recursively. > > The value seems to be wrong though, I think. > It seems to simply use the interrupt pin as array index. > Instead, each bridge should interrupts as follows: > > /* Mapping mandated by PCI-to-PCI Bridge architecture specification, > * revision 1.2 */ > /* Table 9-1: Interrupt Binding for Devices Behind a Bridge */ > static int pci_bridge_dev_map_irq_fn(PCIDevice *dev, int irq_num) > { > return (irq_num + PCI_SLOT(dev->devfn)) % PCI_NUM_PINS; > } > > until we get to the host bridge. I use gdb to debug, and find that this function is never called. Thanks Wen Congyang > > >>> >>>> - pci_bios_init_device_in_bus(0 /* host bus */); >>>> + pci_bios_init_device_in_bus(bus /* host bus */); >>>> + } >>> >>> That is correct. Can be done easier though by just not limiting device >>> initialization to a specific bus like in the attached patch. Does that >>> one work for you? >> >> I test it, and it works for me. >> >> Thanks >> Wen Congyang >> >>> >>> cheers, >>> Gerd >
On Thu, Sep 08, 2011 at 07:03:10PM +0800, Wen Congyang wrote: > At 09/08/2011 06:42 PM, Michael S. Tsirkin Write: > > On Thu, Sep 08, 2011 at 05:58:12PM +0800, Wen Congyang wrote: > >> At 09/08/2011 05:43 PM, Gerd Hoffmann Write: > >>> Hi, > >>> > >>>> I modify the code like this, and the PCI_INTERRUPT_LINE register is > >>>> set, and I can bind > >>>> it to uio_pci_generic: > >>> > >>>> --- a/src/pciinit.c > >>>> +++ b/src/pciinit.c > >>>> @@ -575,6 +575,8 @@ static int pci_bios_init_root_regions(u32 start, > >>>> u32 end) > >>> > >>>> pci_bios_init_bus_bases(&busses[0]); > >>>> - pci_bios_map_device_in_bus(0 /* host bus */); > >>>> + for (bus = 0; bus<= MaxPCIBus; bus++) { > >>>> + pci_bios_map_device_in_bus(bus /* host bus */); > >>> > >>> No. pci_bios_map_device_in_bus goes down recursively when it finds a > >>> bridge, so it should cover all devices already. > >> > >> Yes, pci_bios_map_device() goes down recursively. > > > > The value seems to be wrong though, I think. > > It seems to simply use the interrupt pin as array index. > > Instead, each bridge should interrupts as follows: > > > > /* Mapping mandated by PCI-to-PCI Bridge architecture specification, > > * revision 1.2 */ > > /* Table 9-1: Interrupt Binding for Devices Behind a Bridge */ > > static int pci_bridge_dev_map_irq_fn(PCIDevice *dev, int irq_num) > > { > > return (irq_num + PCI_SLOT(dev->devfn)) % PCI_NUM_PINS; > > } > > > > until we get to the host bridge. > > I use gdb to debug, and find that this function is never called. > > Thanks > Wen Congyang No, I mean that bios must implement this logic. You don't see it called probably because ivshmem does not cause interrupts for you. > > > > > >>> > >>>> - pci_bios_init_device_in_bus(0 /* host bus */); > >>>> + pci_bios_init_device_in_bus(bus /* host bus */); > >>>> + } > >>> > >>> That is correct. Can be done easier though by just not limiting device > >>> initialization to a specific bus like in the attached patch. Does that > >>> one work for you? > >> > >> I test it, and it works for me. > >> > >> Thanks > >> Wen Congyang > >> > >>> > >>> cheers, > >>> Gerd > >
diff --git a/src/pciinit.c b/src/pciinit.c index 597c8ea..676e35e 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -45,7 +45,7 @@ static struct pci_bus { } *busses; static int busses_count; -static void pci_bios_init_device_in_bus(int bus); +static void pci_bios_init_device_all(void); static void pci_bios_check_device_in_bus(int bus); static void pci_bios_init_bus_bases(struct pci_bus *bus); static void pci_bios_map_device_in_bus(int bus); @@ -254,15 +254,10 @@ static void pci_bios_init_device(struct pci_device *pci) pci_init_device(pci_device_tbl, pci, NULL); } -static void pci_bios_init_device_in_bus(int bus) +static void pci_bios_init_device_all(void) { struct pci_device *pci; foreachpci(pci) { - u8 pci_bus = pci_bdf_to_bus(pci->bdf); - if (pci_bus < bus) - continue; - if (pci_bus > bus) - break; pci_bios_init_device(pci); } } @@ -605,7 +600,7 @@ pci_setup(void) pci_bios_init_bus_bases(&busses[0]); pci_bios_map_device_in_bus(0 /* host bus */); - pci_bios_init_device_in_bus(0 /* host bus */); + pci_bios_init_device_all(); struct pci_device *pci; foreachpci(pci) {