From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34924) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2zji-00039s-7a for qemu-devel@nongnu.org; Thu, 11 Jun 2015 06:29:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2zjb-000533-BW for qemu-devel@nongnu.org; Thu, 11 Jun 2015 06:29:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40344) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2zjb-00052t-4K for qemu-devel@nongnu.org; Thu, 11 Jun 2015 06:29:19 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id A63CE3A62AC for ; Thu, 11 Jun 2015 10:29:18 +0000 (UTC) Message-ID: <557962FA.4010208@redhat.com> Date: Thu, 11 Jun 2015 13:29:14 +0300 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1433983083-4636-1-git-send-email-lersek@redhat.com> <1433983083-4636-7-git-send-email-lersek@redhat.com> <55796140.1040403@redhat.com> <55796240.5080802@redhat.com> In-Reply-To: <55796240.5080802@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 6/6] hw/pci-bridge: set explicit OFW unit address for TYPE_PXB_HOST List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek , qemu-devel@nongnu.org Cc: Markus Armbruster , "Michael S. Tsirkin" On 06/11/2015 01:26 PM, Laszlo Ersek wrote: > 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. Actually: -device pxb,id=bridge1,bus_nr=4 -netdev user,id=u \ -device e1000,id=net2,bus=bridge1,netdev=u,addr=0x5,bootindex=0,romfile=../pc-bios/efi-e1000.rom,bus=pci.0 Hmm :( > > 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 >>> Cc: Marcel Apfelbaum >>> Cc: Michael S. Tsirkin >>> Signed-off-by: Laszlo Ersek >>> --- >>> >>> 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; >>> >> >