[PATCH 3/4 v3] arch/ppc/qemu: Add a node for the other (empty) PCI bus to the device tree
QEMU emulates two of the three PCI buses found on real hardware because some clients seem to need both and fail with only one present, but OpenBIOS only handles a single PCI bus and initialises and puts in the device tree only one of these: the second one which is where devices are connected and also marks it bus 0. However, clients getting info from the device tree may not know about this and thinking there is only one PCI bus they erroneously use the address of the first bus to access PCI config registers for devices on the second bus which silently fails as these requests will go to the other empty bus emulated and return invalid values. Devices mapped via MMIO still appear to work but they may not be correctly initialised and some cards are not detected because of this.
Until support for multiple PCI buses is implemented add an empty node in the device tree for the uninitialised bus to let clients know about it. This is still not entirely correct as bus-range property does not match real hardware but this fixes detecting PCI devices (such as USB) under MorphOS and may also fix enabling the bus master bit needed with some network cards and allow the workaround for this to be reverted.
Signed-off-by: BALATON Zoltan balaton@eik.bme.hu --- arch/ppc/qemu/init.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
v3: Move device tree patching in its own function and call it from init (otherwise same as v2, take which you like)
v2: Move to init.c from tree.fs and only add it for mac99 Patches 1 and 2 from original series https://www.coreboot.org/pipermail/openbios/2016-November/009825.html https://www.coreboot.org/pipermail/openbios/2016-November/009824.html are still valid and needed, patch 4 was already applied so only this one (3/4) is resent but 1 and 2 should be applied as well with this.
diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ffbf08d..46a56a0 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -668,6 +668,48 @@ static void kvm_of_init(void) fword("finish-device"); }
+static void empty_pci_bus_init(void) +{ + if (machine_id == ARCH_MAC99) { + /* Add empty node for first pci bus */ + /* Remove this when pci driver is fixed to handle multiple buses */ + activate_device("/"); + fword("new-device"); + push_str("pci"); + fword("device-name"); + push_str("pci"); + fword("device-type"); + PUSH(0xf0000000); + fword("encode-int"); + PUSH(0x02000000); + fword("encode-int"); + fword("encode+"); + push_str("reg"); + fword("property"); + PUSH(3); + fword("encode-int"); + push_str("#address-cells"); + fword("property"); + PUSH(2); + fword("encode-int"); + push_str("#size-cells"); + fword("property"); + PUSH(1); + fword("encode-int"); + push_str("#interrupt-cells"); + fword("property"); + PUSH(0); + fword("encode-int"); + PUSH(0); + fword("encode-int"); + fword("encode+"); + push_str("bus-range"); + fword("property"); + fword("finish-device"); + device_end(); + } +} + /* * filll ( addr bytes quad -- ) */ @@ -755,6 +797,7 @@ arch_of_init(void) openbios_init(); modules_init(); setup_timers(); + empty_pci_bus_init(); #ifdef CONFIG_DRIVER_PCI ob_pci_init(); #endif
On 31/01/17 13:50, BALATON Zoltan wrote:
QEMU emulates two of the three PCI buses found on real hardware because some clients seem to need both and fail with only one present, but OpenBIOS only handles a single PCI bus and initialises and puts in the device tree only one of these: the second one which is where devices are connected and also marks it bus 0. However, clients getting info from the device tree may not know about this and thinking there is only one PCI bus they erroneously use the address of the first bus to access PCI config registers for devices on the second bus which silently fails as these requests will go to the other empty bus emulated and return invalid values. Devices mapped via MMIO still appear to work but they may not be correctly initialised and some cards are not detected because of this.
Until support for multiple PCI buses is implemented add an empty node in the device tree for the uninitialised bus to let clients know about it. This is still not entirely correct as bus-range property does not match real hardware but this fixes detecting PCI devices (such as USB) under MorphOS and may also fix enabling the bus master bit needed with some network cards and allow the workaround for this to be reverted.
Signed-off-by: BALATON Zoltan balaton@eik.bme.hu
arch/ppc/qemu/init.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
v3: Move device tree patching in its own function and call it from init (otherwise same as v2, take which you like)
v2: Move to init.c from tree.fs and only add it for mac99 Patches 1 and 2 from original series https://www.coreboot.org/pipermail/openbios/2016-November/009825.html https://www.coreboot.org/pipermail/openbios/2016-November/009824.html are still valid and needed, patch 4 was already applied so only this one (3/4) is resent but 1 and 2 should be applied as well with this.
diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index ffbf08d..46a56a0 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -668,6 +668,48 @@ static void kvm_of_init(void) fword("finish-device"); }
+static void empty_pci_bus_init(void) +{
- if (machine_id == ARCH_MAC99) {
/* Add empty node for first pci bus *//* Remove this when pci driver is fixed to handle multiple buses */activate_device("/");fword("new-device");push_str("pci");fword("device-name");push_str("pci");fword("device-type");PUSH(0xf0000000);fword("encode-int");PUSH(0x02000000);fword("encode-int");fword("encode+");push_str("reg");fword("property");PUSH(3);fword("encode-int");push_str("#address-cells");fword("property");PUSH(2);fword("encode-int");push_str("#size-cells");fword("property");PUSH(1);fword("encode-int");push_str("#interrupt-cells");fword("property");PUSH(0);fword("encode-int");PUSH(0);fword("encode-int");fword("encode+");push_str("bus-range");fword("property");fword("finish-device");device_end();- }
+}
/*
- filll ( addr bytes quad -- )
*/ @@ -755,6 +797,7 @@ arch_of_init(void) openbios_init(); modules_init(); setup_timers();
- empty_pci_bus_init();
#ifdef CONFIG_DRIVER_PCI ob_pci_init(); #endif
I've now had a chance to give patches 1 and 2 a thorough testing locally and these look good, so I've pushed these to master.
So that only leaves this patch. I am fairly happy that this version of the patch is the better solution until OpenBIOS can support multiple PCI domains, however I still don't want to use this in its current form because of the warnings generated (not at least because it seems that enumerating the devices on the new empty PCI bus still seems to be accessing devices on the current PCI bus).
Having a quick browse over the error messages, I think it should be possible to make these warnings go away if you both add the new empty PCI bus to QEMU's mac99 machine and then include a fake "ranges" property for the new PCI DT node - can you try this and see if that still fixes your use case?
ATB,
Mark.
On Mon, 13 Feb 2017, Mark Cave-Ayland wrote:
So that only leaves this patch. I am fairly happy that this version of the patch is the better solution until OpenBIOS can support multiple PCI domains, however I still don't want to use this in its current form because of the warnings generated (not at least because it seems that enumerating the devices on the new empty PCI bus still seems to be accessing devices on the current PCI bus).
Having a quick browse over the error messages, I think it should be possible to make these warnings go away if you both add the new empty PCI bus to QEMU's mac99 machine and then include a fake "ranges" property for the new PCI DT node - can you try this and see if that still fixes your use case?
Not sure I understand this. QEMU already emulates the empty PCI bus, see in pci_pmac_init() in uninorth.c. What I think is happening that before this patch you get no warnings because the client thinks there's only one PCI bus and does not enumerate the second bus (where devices are) and finds nothing on the first bus, hence no warnings but devices on second bus are not initialised either (this is why bus master bit was not set for rtl8139). They may still appear to work because they are memory mapped and if the client writes the registers at the memory address then can talk to the device but PCI config may be wrong.
After this patch it actually finds both buses. There's one warning for overlapping bus-range but this is probably harmless as there are no devices on the first bus. Then enumerates the second bus and finds OHCI device which may have some other problem that cause the warnings but now at least it is found and the client could talk to it. At least this is what happened on MorphOS. If this is the case then fixing these warnings would need another patch independent of this one but I'd leave that to someone else as I have no time now to try to fix that. This patch alone fixes USB on MorphOS and if it does not break anything else then it's an improvement, even if not a complete fix for the old OpenSUSE image you get these warnings with.
Have you found a regression? If it works as good as before (i.e. did not work before on OpenSUSE and still not working but all other OSes are the same and MorphOS is better) then you might as well apply this patch and then look to fix OHCI with OpenSUSE later which now at least issues warnings instead of silently failing that may be a step forward.
I don't have time to experiment with this now but if you find some properties that help OpenSUSE I can test the patch.
Regards, BALATON Zoltan
On 13/02/17 21:19, BALATON Zoltan wrote:
Not sure I understand this. QEMU already emulates the empty PCI bus, see in pci_pmac_init() in uninorth.c. What I think is happening that before this patch you get no warnings because the client thinks there's only one PCI bus and does not enumerate the second bus (where devices are) and finds nothing on the first bus, hence no warnings but devices on second bus are not initialised either (this is why bus master bit was not set for rtl8139). They may still appear to work because they are memory mapped and if the client writes the registers at the memory address then can talk to the device but PCI config may be wrong.
After this patch it actually finds both buses. There's one warning for overlapping bus-range but this is probably harmless as there are no devices on the first bus. Then enumerates the second bus and finds OHCI device which may have some other problem that cause the warnings but now at least it is found and the client could talk to it. At least this is what happened on MorphOS. If this is the case then fixing these warnings would need another patch independent of this one but I'd leave that to someone else as I have no time now to try to fix that. This patch alone fixes USB on MorphOS and if it does not break anything else then it's an improvement, even if not a complete fix for the old OpenSUSE image you get these warnings with.
Have you found a regression? If it works as good as before (i.e. did not work before on OpenSUSE and still not working but all other OSes are the same and MorphOS is better) then you might as well apply this patch and then look to fix OHCI with OpenSUSE later which now at least issues warnings instead of silently failing that may be a step forward.
I don't have time to experiment with this now but if you find some properties that help OpenSUSE I can test the patch.
Indeed, I've had a look and it does seem that the second bus does exist in QEMU but AFAICT it's wired up incorrectly (or at least the PCI data space is shared between both buses which is why I suspect these errors show up). It's almost as if the UniNorth AGP was supposed to be a subdevice on an existing PCI bus rather than a self-contained bus of its own.
Given the mix-up between the two buses I'm inclined to leave this patch for the moment until someone can also untangle the QEMU side to make sure that both distinct buses are addressed separately. Perhaps this is something that Ben managed to resolve in his branches from last year?
ATB,
Mark.
-
BALATON Zoltan -
Mark Cave-Ayland