Re: [Qemu-devel] [PATCH v3 6/6] hw/pci-bridge: set explicit OFW unit address for TYPE_PXB_HOST

[Marcel Apfelbaum]

On 06/11/2015 03:38 AM, Laszlo Ersek wrote:
> The PXB implementation doesn't allow firmware (SeaBIOS or OVMF) to boot
> off devices behind the PXB. This happens because the
> sysbus_get_fw_dev_path() function in "hw/core/sysbus.c" doesn't have
> enough information to format a unique identifier for the PXB in question,
> and consequently the OpenFirmware device path passed down to the guest
> firmware in the "bootorder" fw_cfg file is unusable for identifying the
> boot device.
>
> For example, the command line fragment
>
>    -device pxb,id=bridge1,bus_nr=4 \
>    \
>    -netdev user,id=netdev0 \
>    -device e1000,netdev=netdev0,bus=bridge1,addr=2,bootindex=0
>
> results in the following "bootorder" entry:
>
>    /pci/address@hidden/address@hidden/address@hidden
>
> The initial "pci" node is formatted by sysbus_get_fw_dev_path(), and the
> resultant OpenFirmware device path is independent of bus_nr=4 -- and
> therefore it is useless for identifying the device.
>
> In this patch we change the fw_name device class member of TYPE_PXB_HOST
> from "pci" to "pci-root", and set each instance's explicit OFW unit
> address to the PXB bus number. The same command line fragment results in
> the following OpenFirmware device path in the "bootorder" fw_cfg file:
>
>    /address@hidden/address@hidden/address@hidden/address@hidden
Clean

> The original, initial "/pci" fragment has been replaced with
> "/address@hidden", which (a) looks better, (b) provides all the necessary
> information.
>
> Cc: Markus Armbruster <address@hidden>
> Cc: Marcel Apfelbaum <address@hidden>
> Cc: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
>
> Notes:
>      v3:
>      - using new sysbus device property, not inserting additional bus
>
>   hw/pci-bridge/pci_expander_bridge.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci-bridge/pci_expander_bridge.c 
> b/hw/pci-bridge/pci_expander_bridge.c
> index c7a085d..d468670 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -93,7 +93,7 @@ static void pxb_host_class_init(ObjectClass *class, void 
> *data)
>       DeviceClass *dc = DEVICE_CLASS(class);
>       PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
>
> -    dc->fw_name = "pci";
> +    dc->fw_name = "pci-root";
>       hc->root_bus_path = pxb_host_root_bus_path;
>   }
>
> @@ -152,6 +152,7 @@ static int pxb_dev_initfn(PCIDevice *dev)
>   {
>       PXBDev *pxb = PXB_DEV(dev);
>       DeviceState *ds, *bds;
> +    char *bus_nr_str;
>       PCIBus *bus;
>       const char *dev_name = NULL;
>
> @@ -166,6 +167,10 @@ static int pxb_dev_initfn(PCIDevice *dev)
>       }
>
>       ds = qdev_create(NULL, TYPE_PXB_HOST);
> +    bus_nr_str = g_strdup_printf("%x", pxb->bus_nr);
> +    qdev_prop_set_string(ds, "explicit_ofw_unit_address", bus_nr_str);
> +    g_free(bus_nr_str);

This is the best approach yet.
I think it will not affect migration, because on the target site
it will have the same bus number, so the property doesn't need to be
passed.

Is my time to see how it will work on Seabios.
Thanks a lot for your help!!

Reviewed-by: Marcel Apfelbaum <address@hidden>


> +
>       bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
>
>       bus->parent_dev = dev;