From nobody Mon Apr 29 01:11:18 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 148776128134490.72814278785415; Wed, 22 Feb 2017 03:01:21 -0800 (PST) Received: from localhost ([::1]:51422 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cgUfg-0003H3-1E for importer@patchew.org; Wed, 22 Feb 2017 06:01:20 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46175) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cgUbh-0000Tf-Px for qemu-devel@nongnu.org; Wed, 22 Feb 2017 05:57:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cgUbe-0007PX-M5 for qemu-devel@nongnu.org; Wed, 22 Feb 2017 05:57:13 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:51183) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cgUbe-0007O8-Cs for qemu-devel@nongnu.org; Wed, 22 Feb 2017 05:57:10 -0500 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v1MArqMn139731 for ; Wed, 22 Feb 2017 05:57:02 -0500 Received: from e36.co.us.ibm.com (e36.co.us.ibm.com [32.97.110.154]) by mx0a-001b2d01.pphosted.com with ESMTP id 28s411v0vx-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 22 Feb 2017 05:57:01 -0500 Received: from localhost by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 22 Feb 2017 03:57:00 -0700 Received: from d03dlp01.boulder.ibm.com (9.17.202.177) by e36.co.us.ibm.com (192.168.1.136) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 22 Feb 2017 03:56:57 -0700 Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 313701FF001E; Wed, 22 Feb 2017 03:56:34 -0700 (MST) Received: from b01ledav03.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v1MAuuSG55509174; Wed, 22 Feb 2017 10:56:56 GMT Received: from b01ledav03.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 34BDFB2052; Wed, 22 Feb 2017 05:56:55 -0500 (EST) Received: from [192.168.66.23] (unknown [9.167.246.203]) by b01ledav03.gho.pok.ibm.com (Postfix) with ESMTP id DDFE8B2046; Wed, 22 Feb 2017 05:56:53 -0500 (EST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Wed, 22 Feb 2017 11:56:53 +0100 User-Agent: StGit/0.17.1-20-gc0b1b-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17022210-0020-0000-0000-00000B7110B2 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006662; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000204; SDB=6.00825453; UDB=6.00404173; IPR=6.00602908; BA=6.00005164; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014388; XFM=3.00000011; UTC=2017-02-22 10:56:59 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17022210-0021-0000-0000-00005A51C240 Message-Id: <148776029578.5865.5785337570950575739.stgit@bahia> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-02-22_08:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702220108 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 148.163.156.1 Subject: [Qemu-devel] [PATCH] spapr/pci: populate PCI DT in reverse order X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Thomas Huth , Nikunj A Dadhania , "Michael S. Tsirkin" , qemu-ppc@nongnu.org, Marcel Apfelbaum , David Gibson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 From: Greg Kurz Since commit 1d2d974244c6 "spapr_pci: enumerate and add PCI device tree", Q= EMU populates the PCI device tree in the opposite order compared to SLOF. Before 1d2d974244c6: Populating /pci@800000020000000 00 0000 (D) : 1af4 1000 virtio [ net ] 00 0800 (D) : 1af4 1001 virtio [ block ] 00 1000 (D) : 1af4 1009 virtio [ network ] Populating /pci@800000020000000/unknown-legacy-device@2 7e5294b8 : /pci@800000020000000 7e52b998 : |-- ethernet@0 7e52c0c8 : |-- scsi@1 7e52c7e8 : +-- unknown-legacy-device@2 ok Since 1d2d974244c6: Populating /pci@800000020000000 00 1000 (D) : 1af4 1009 virtio [ network ] Populating /pci@800000020000000/unknown-legacy-device@2 00 0800 (D) : 1af4 1001 virtio [ block ] 00 0000 (D) : 1af4 1000 virtio [ net ] 7e5e8118 : /pci@800000020000000 7e5ea6a0 : |-- unknown-legacy-device@2 7e5eadb8 : |-- scsi@1 7e5eb4d8 : +-- ethernet@0 ok This behaviour change is not actually a bug since no assumptions should be made on DT ordering. But it has no real justification either, other than being the consequence of the way fdt_add_subnode() inserts new elements to the front of the FDT rather than adding them to the tail. This patch reverts to the historical SLOF ordering by walking PCI devices in reverse order. This reconciles pseries with x86 machine types behavior. It is expected to make things easier when porting existing applications to power. Signed-off-by: Greg Kurz Tested-by: Thomas Huth Reviewed-by: Nikunj A Dadhania (slight update to the changelog) Signed-off-by: Greg Kurz --- hw/pci/pci.c | 28 ++++++++++++++++++++++++++++ hw/ppc/spapr_pci.c | 12 ++++++------ include/hw/pci/pci.h | 4 ++++ 3 files changed, 38 insertions(+), 6 deletions(-) David, This patch was posted and already discussed during 2.5 development: http://patchwork.ozlabs.org/patch/549925/ The "consensus" at the time was that guests should not rely on device ordering (i.e. use persistent naming instead). I got recently contacted by OpenStack people who had several complaints about the reverse ordering of PCI devices in pseries: different behavior between ppc64 and x86, lots of time spent in debugging when porting applications from x86 to ppc64 before realizing that it is caused by the reverse ordering, necessity to carry hacky workarounds... One strong argument against handling this properly with persistent naming is that it requires systemd/udev. This option is considered as painful with CirrOS, which aims at remaining as minimal as possible and is widely used in the OpenStack ecosystem. Would you re-consider your position and apply this patch ? Cheers. diff --git a/hw/pci/pci.c b/hw/pci/pci.c index a563555e7da7..273f1e46025a 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1530,6 +1530,34 @@ static const pci_class_desc pci_class_descriptions[]= =3D { 0, NULL} }; =20 +static void pci_for_each_device_under_bus_reverse(PCIBus *bus, + void (*fn)(PCIBus *b, + PCIDevice *d, + void *opaque), + void *opaque) +{ + PCIDevice *d; + int devfn; + + for (devfn =3D 0; devfn < ARRAY_SIZE(bus->devices); devfn++) { + d =3D bus->devices[ARRAY_SIZE(bus->devices) - 1 - devfn]; + if (d) { + fn(bus, d, opaque); + } + } +} + +void pci_for_each_device_reverse(PCIBus *bus, int bus_num, + void (*fn)(PCIBus *b, PCIDevice *d, void *opaque), + void *opaque) +{ + bus =3D pci_find_bus_nr(bus, bus_num); + + if (bus) { + pci_for_each_device_under_bus_reverse(bus, fn, opaque); + } +} + static void pci_for_each_device_under_bus(PCIBus *bus, void (*fn)(PCIBus *b, PCIDevice = *d, void *opaque), diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index fd6fc1d95344..2a20c2a140fc 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1782,9 +1782,9 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus= , PCIDevice *pdev, s_fdt.fdt =3D p->fdt; s_fdt.node_off =3D offset; s_fdt.sphb =3D p->sphb; - pci_for_each_device(sec_bus, pci_bus_num(sec_bus), - spapr_populate_pci_devices_dt, - &s_fdt); + pci_for_each_device_reverse(sec_bus, pci_bus_num(sec_bus), + spapr_populate_pci_devices_dt, + &s_fdt); } =20 static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, @@ -1953,9 +1953,9 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, s_fdt.fdt =3D fdt; s_fdt.node_off =3D bus_off; s_fdt.sphb =3D phb; - pci_for_each_device(bus, pci_bus_num(bus), - spapr_populate_pci_devices_dt, - &s_fdt); + pci_for_each_device_reverse(bus, pci_bus_num(bus), + spapr_populate_pci_devices_dt, + &s_fdt); =20 ret =3D spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), SPAPR_DR_CONNECTOR_TYPE_PCI); diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 6983f13745a5..9349acbfb278 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -429,6 +429,10 @@ int pci_bus_numa_node(PCIBus *bus); void pci_for_each_device(PCIBus *bus, int bus_num, void (*fn)(PCIBus *bus, PCIDevice *d, void *opaqu= e), void *opaque); +void pci_for_each_device_reverse(PCIBus *bus, int bus_num, + void (*fn)(PCIBus *bus, PCIDevice *d, + void *opaque), + void *opaque); void pci_for_each_bus_depth_first(PCIBus *bus, void *(*begin)(PCIBus *bus, void *parent= _state), void (*end)(PCIBus *bus, void *state),