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
Hi Laszlo,

I applied your patches but I still get  
/address@hidden/address@hidden/address@hidden
in the boot list

I checked and  the code enters only once in sysbus_get_fw_dev_path
for address@hidden and goes for pio branch.

Do you know maybe what I missed?

Thanks,
Marcel


> 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);
> +
>       bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
>
>       bus->parent_dev = dev;