On 06/11/15 12:21, Marcel Apfelbaum wrote: > 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/pci-bridge@0/ethernet@2/ethernet-phy@0 >> >> 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: >> >> /pci-root@4/pci-bridge@0/ethernet@2/ethernet-phy@0 > Hi Laszlo, > > I applied your patches but I still get > /pci@i0cf8/ethernet@5/ethernet-phy@0 > in the boot list > > I checked and the code enters only once in sysbus_get_fw_dev_path > for pci@i0cf8 and goes for pio branch. > > Do you know maybe what I missed?
I think so, yes: you added the ...,bootorder=N property to a device that is *not* behind a PXB. :) You forgot the ...,bus=bridgeX property. Thanks Laszlo > > Thanks, > Marcel > > >> >> The original, initial "/pci" fragment has been replaced with >> "/pci-root@4", which (a) looks better, (b) provides all the necessary >> information. >> >> Cc: Markus Armbruster <arm...@redhat.com> >> Cc: Marcel Apfelbaum <mar...@redhat.com> >> Cc: Michael S. Tsirkin <m...@redhat.com> >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> --- >> >> 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; >> >