From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33997) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7en4-0001S1-CT for qemu-devel@nongnu.org; Wed, 24 Jun 2015 03:08:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7en0-0007QJ-Oj for qemu-devel@nongnu.org; Wed, 24 Jun 2015 03:08:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35777) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7en0-0007Q6-Hr for qemu-devel@nongnu.org; Wed, 24 Jun 2015 03:08:06 -0400 Message-ID: <558A5752.3010704@redhat.com> Date: Wed, 24 Jun 2015 10:08:02 +0300 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1434681617-15539-1-git-send-email-lersek@redhat.com> <1434681617-15539-11-git-send-email-lersek@redhat.com> <20150624054005.GA30653@redhat.com> <558A55D2.1030700@redhat.com> In-Reply-To: <558A55D2.1030700@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7 10/10] hw/pci-bridge: format special OFW unit address for PXB host List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: Kevin O'Connor , qemu-devel@nongnu.org, "Michael S. Tsirkin" On 06/24/2015 10:01 AM, Laszlo Ersek wrote: > On 06/24/15 07:40, Michael S. Tsirkin wrote: >> On Fri, Jun 19, 2015 at 04:40:17AM +0200, Laszlo Ersek wrote: >>> We have agreed that OpenFirmware device paths in the "bootorder" fw_cfg >>> file should follow the pattern >>> >>> /pci@i0cf8,%x/... >>> >>> for devices that live behind an extra root bus. The extra root bus in >>> question is the %x'th among the extra root buses. (In other words, %x >>> gives the position of the affected extra root bus relative to the other >>> extra root buses, in bus_nr order.) %x starts at 1, and is formatted in >>> hex. >>> >>> The portion of the unit address that comes before the comma is dynamically >>> taken from the main host bridge, similarly to sysbus_get_fw_dev_path(). >>> >>> Cc: Kevin O'Connor >>> Cc: Michael S. Tsirkin >>> Cc: Marcel Apfelbaum >>> Signed-off-by: Laszlo Ersek >> >> So as I said, a global pxb_dev_list is ugly, and we do not want >> duplicate data structures - please scan child devices of parent instead. >> Functionally correct, so can be a patch on top. > > Marcel, can you pick this up please? Sure thing, thank you for your patches! I'll take care of this, Marcel > > Thanks > Laszlo > >> >> >>> --- >>> >>> Notes: >>> v7: >>> - implement the format that both Kevin and Michael agreed with. Example: >>> /pci@i0cf8,1/pci-bridge@0/scsi@0/channel@0/disk@0,0 >>> - I updated the OVMF patchset accordingly, but I won't post it until >>> this QEMU patch is applied >>> - Someone please write the SeaBIOS patch >>> >>> 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 | 53 +++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 53 insertions(+) >>> >>> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c >>> index 70708ef..57f8a37 100644 >>> --- a/hw/pci-bridge/pci_expander_bridge.c >>> +++ b/hw/pci-bridge/pci_expander_bridge.c >>> @@ -43,6 +43,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) >>> @@ -89,12 +91,45 @@ 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 *pxb_host; >>> + const PCIBus *pxb_bus; >>> + const PXBDev *pxb_dev; >>> + int position; >>> + const DeviceState *pxb_dev_base; >>> + const PCIHostState *main_host; >>> + const SysBusDevice *main_host_sbd; >>> + >>> + pxb_host = PCI_HOST_BRIDGE(dev); >>> + pxb_bus = pxb_host->bus; >>> + pxb_dev = PXB_DEV(pxb_bus->parent_dev); >>> + position = g_list_index(pxb_dev_list, pxb_dev); >>> + assert(position >= 0); >>> + >>> + pxb_dev_base = DEVICE(pxb_dev); >>> + main_host = PCI_HOST_BRIDGE(pxb_dev_base->parent_bus->parent); >>> + main_host_sbd = SYS_BUS_DEVICE(main_host); >>> + >>> + if (main_host_sbd->num_mmio > 0) { >>> + return g_strdup_printf(TARGET_FMT_plx ",%x", >>> + main_host_sbd->mmio[0].addr, position + 1); >>> + } >>> + if (main_host_sbd->num_pio > 0) { >>> + return g_strdup_printf("i%04x,%x", >>> + main_host_sbd->pio[0], position + 1); >>> + } >>> + return NULL; >>> +} >>> + >>> 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"; >>> + sbc->explicit_ofw_unit_address = pxb_host_ofw_unit_address; >>> hc->root_bus_path = pxb_host_root_bus_path; >>> } >>> >>> @@ -149,6 +184,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); >>> @@ -192,9 +236,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), >>> @@ -208,6 +260,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 >