On 19/11/17 11:06, Mark Cave-Ayland wrote:
On 17/11/17 14:33, Artyom Tarasenko wrote:
On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
<mark.cave-ayl...@ilande.co.uk> wrote:
After the previous refactoring it is now possible to use separate functions
to improve clarity of the interrupt paths. Similarly by checking the PCI
devnfn to identify busA during apb_pci_bridge_realize() it becomes possible
to completely remove the busA property from the PBMPCIBridge state.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
hw/pci-host/apb.c | 54 ++++++++++++++++++---------------------------
include/hw/pci-host/apb.h | 3 ---
2 files changed, 21 insertions(+), 36 deletions(-)
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 6c20285..268100e 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -517,32 +517,27 @@ static int pci_apb_map_irq(PCIDevice *pci_dev, int
irq_num)
return irq_num;
}
-static int pci_pbm_map_irq(PCIDevice *pci_dev, int irq_num)
+static int pci_pbmA_map_irq(PCIDevice *pci_dev, int irq_num)
{
- PBMPCIBridge *br = PBM_PCI_BRIDGE(pci_bridge_get_device(
- PCI_BUS(qdev_get_parent_bus(DEVICE(pci_dev)))));
-
- int bus_offset;
- if (br->busA) {
- bus_offset = 0x0;
+ /* The on-board devices have fixed (legacy) OBIO intnos */
+ switch (PCI_SLOT(pci_dev->devfn)) {
+ case 1:
+ /* Onboard NIC */
+ return 0x21;
+ case 3:
+ /* Onboard IDE */
+ return 0x20;
+ default:
+ /* Normal intno, fall through */
+ break;
+ }
- /* The on-board devices have fixed (legacy) OBIO intnos */
- switch (PCI_SLOT(pci_dev->devfn)) {
- case 1:
- /* Onboard NIC */
- return 0x21;
- case 3:
- /* Onboard IDE */
- return 0x20;
+ return ((PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
+}
- default:
- /* Normal intno, fall through */
- break;
- }
- } else {
- bus_offset = 0x10;
- }
- return (bus_offset + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
+static int pci_pbmB_map_irq(PCIDevice *pci_dev, int irq_num)
+{
+ return (0x10 + (PCI_SLOT(pci_dev->devfn) << 2) + irq_num) & 0x1f;
}
static void pci_apb_set_irq(void *opaque, int irq_num, int level)
@@ -593,7 +588,7 @@ static void apb_pci_bridge_realize(PCIDevice *dev, Error
**errp)
/* If initialising busA, ensure that we allow IO transactions so that
we get the early serial console until OpenBIOS configures the bridge */
- if (br->busA) {
+ if (dev->devfn == PCI_DEVFN(1, 1)) {
I think the previous syntax was more explicit here. A comment would be nice.
Yes it's definitely something that isn't immediately obvious, which is
why I left the above comment in place explaining what the if() branch is
doing. Is there something in the comment that isn't particularly clear?
Note one of the reasons for wanting to remove the busA property is that
where possible I'd like to reduce the code in the IRQ path, and while
the existing code works I am still unsure of the additional overhead of
the 2 levels of QOM type checking that the current approach requires for
each IRQ.
Hi Artyom,
Thinking about this a bit more during freeze, this is actually doing the
opposite of what we want, as it requires the device realise function to
behave differently depending upon how it is related to the PCI bus.
How about swapping this out for a qdev bool property for APB named
"enable-early-pci-io-access", setting it just for the PCI_DEVFN(1, 1)
device containing the ebus and then alter the if() statement above to
enable PCI IO access if the qdev property is set?
ATB,
Mark.
