[OpenBIOS] [PATCH 3/4 v3] arch/ppc/qemu: Add a node for the other (empty) PCI bus to the device tree

[Mark Cave-Ayland]

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 at 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.