From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56131) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5Dta-0005cE-Db for qemu-devel@nongnu.org; Wed, 17 Jun 2015 10:00:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5DtT-0007fQ-IJ for qemu-devel@nongnu.org; Wed, 17 Jun 2015 10:00:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44999) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5DtT-0007eW-AV for qemu-devel@nongnu.org; Wed, 17 Jun 2015 10:00:43 -0400 Message-ID: <55817D86.8090808@redhat.com> Date: Wed, 17 Jun 2015 16:00:38 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1434545105-5811-1-git-send-email-lersek@redhat.com> <1434545105-5811-8-git-send-email-lersek@redhat.com> <20150617155237-mutt-send-email-mst@redhat.com> In-Reply-To: <20150617155237-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 7/7] hw/pci-bridge: format SeaBIOS-compliant OFW device node for PXB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Marcel Apfelbaum , Kevin O'Connor , qemu-devel@nongnu.org On 06/17/15 15:57, Michael S. Tsirkin wrote: > On Wed, Jun 17, 2015 at 02:45:05PM +0200, Laszlo Ersek wrote: >> SeaBIOS expects OpenFirmware device paths in the "bootorder" fw_cfg file >> to follow the pattern >> >> /pci-root@N/pci@i0cf8/... > > It's kind of crazy, isn't it? It's... idiosyncratic. > /pci@i0cf8/pci-root@N would make some sense: access rootN through cf8. > > But if bios needs to keep this for compatibility, maybe > we have too, to. Kevin? The only reason I'm doing this is because SeaBIOS expects precisely this OFW devpath fragment. Thanks Laszlo > >> >> for devices that live behind an extra root bus. The extra root bus in >> question is the N'th among the extra root bridges. (In other words, N >> gives the position of the affected extra root bus relative to the other >> extra root buses, in bus_nr order.) N starts at 1, and is formatted in >> hex. >> >> The "pci@i0cf8" node text is hardcoded in SeaBIOS (see the macro >> FW_PCI_DOMAIN). >> >> Cc: Kevin O'Connor >> Cc: Marcel Apfelbaum >> Cc: Michael S. Tsirkin >> Signed-off-by: Laszlo Ersek >> Tested-by: Marcel Apfelbaum >> Reviewed-by: Marcel Apfelbaum > > This makes this a PC-specific device, which is pretty ugly. > You need to hardcode /pci-root@N here, but why not get > /pci@i0cf8 from the real root? > >> --- >> >> Notes: >> v6: >> - no changes >> >> v5: >> - constify parameter and local variables of pxb_host_ofw_unit_address(), >> in accord with the previous patch >> >> v4: >> - new in v4 >> >> hw/pci-bridge/pci_expander_bridge.c | 39 ++++++++++++++++++++++++++++++++++++- >> 1 file changed, 38 insertions(+), 1 deletion(-) >> >> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c >> index 4398d98..37574ec 100644 >> --- a/hw/pci-bridge/pci_expander_bridge.c >> +++ b/hw/pci-bridge/pci_expander_bridge.c >> @@ -42,6 +42,8 @@ typedef struct PXBDev { >> uint16_t numa_node; >> } PXBDev; >> >> +static GList *pxb_dev_list; >> + >> #define TYPE_PXB_HOST "pxb-host" >> >> static int pxb_bus_num(PCIBus *bus) >> @@ -88,12 +90,29 @@ static const char *pxb_host_root_bus_path(PCIHostState *host_bridge, >> return bus->bus_path; >> } >> >> +static char *pxb_host_ofw_unit_address(const SysBusDevice *dev) >> +{ >> + const PCIHostState *host = PCI_HOST_BRIDGE(dev); >> + const PCIBus *bus; >> + const PXBDev *pxb; >> + int position; >> + >> + bus = host->bus; >> + pxb = PXB_DEV(bus->parent_dev); >> + position = g_list_index(pxb_dev_list, pxb); >> + assert(position >= 0); >> + >> + return g_strdup_printf("%x/pci@i0cf8", position + 1); > > Please document that this reverse order is to match > existing bios behaviour. > > >> +} >> + >> static void pxb_host_class_init(ObjectClass *class, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(class); >> + SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(class); >> PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class); >> >> - dc->fw_name = "pci"; >> + dc->fw_name = "pci-root"; >> + sbc->explicit_ofw_unit_address = pxb_host_ofw_unit_address; >> hc->root_bus_path = pxb_host_root_bus_path; >> } >> >> @@ -148,6 +167,15 @@ static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin) >> return pin - PCI_SLOT(pxb->devfn); >> } >> >> +static gint pxb_compare(gconstpointer a, gconstpointer b) >> +{ >> + const PXBDev *pxb_a = a, *pxb_b = b; >> + >> + return pxb_a->bus_nr < pxb_b->bus_nr ? -1 : >> + pxb_a->bus_nr > pxb_b->bus_nr ? 1 : >> + 0; >> +} >> + >> static int pxb_dev_initfn(PCIDevice *dev) >> { >> PXBDev *pxb = PXB_DEV(dev); >> @@ -191,9 +219,17 @@ static int pxb_dev_initfn(PCIDevice *dev) >> PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK); >> pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_HOST); >> >> + pxb_dev_list = g_list_insert_sorted(pxb_dev_list, pxb, pxb_compare); >> return 0; >> } >> >> +static void pxb_dev_exitfn(PCIDevice *pci_dev) >> +{ >> + PXBDev *pxb = PXB_DEV(pci_dev); >> + >> + pxb_dev_list = g_list_remove(pxb_dev_list, pxb); >> +} >> + >> static Property pxb_dev_properties[] = { >> /* Note: 0 is not a legal a PXB bus number. */ >> DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0), >> @@ -207,6 +243,7 @@ static void pxb_dev_class_init(ObjectClass *klass, void *data) >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> >> k->init = pxb_dev_initfn; >> + k->exit = pxb_dev_exitfn; >> k->vendor_id = PCI_VENDOR_ID_REDHAT; >> k->device_id = PCI_DEVICE_ID_REDHAT_PXB; >> k->class_id = PCI_CLASS_BRIDGE_HOST; >> -- >> 1.8.3.1