From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56587) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c13vf-0008Cy-1s for qemu-devel@nongnu.org; Mon, 31 Oct 2016 00:10:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c13vb-0001i9-Rb for qemu-devel@nongnu.org; Mon, 31 Oct 2016 00:10:35 -0400 Received: from mail-pf0-x242.google.com ([2607:f8b0:400e:c00::242]:34330) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c13vb-0001ht-I2 for qemu-devel@nongnu.org; Mon, 31 Oct 2016 00:10:31 -0400 Received: by mail-pf0-x242.google.com with SMTP id y68so1733756pfb.1 for ; Sun, 30 Oct 2016 21:10:30 -0700 (PDT) References: <1477641400-23292-1-git-send-email-aik@ozlabs.ru> <20161028120712.4a911866@bahia> <20161031025314.GJ18226@umbus.fritz.box> From: Alexey Kardashevskiy Message-ID: Date: Mon, 31 Oct 2016 15:10:23 +1100 MIME-Version: 1.0 In-Reply-To: <20161031025314.GJ18226@umbus.fritz.box> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="nJc45s3nUbN3d69XXfxasGmN90IkF8jk1" Subject: Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , Greg Kurz Cc: qemu-devel@nongnu.org, Michael Roth , Alex Williamson , qemu-ppc@nongnu.org, Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nJc45s3nUbN3d69XXfxasGmN90IkF8jk1 From: Alexey Kardashevskiy To: David Gibson , Greg Kurz Cc: qemu-devel@nongnu.org, Michael Roth , Alex Williamson , qemu-ppc@nongnu.org, Paolo Bonzini Message-ID: Subject: Re: [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default References: <1477641400-23292-1-git-send-email-aik@ozlabs.ru> <20161028120712.4a911866@bahia> <20161031025314.GJ18226@umbus.fritz.box> In-Reply-To: <20161031025314.GJ18226@umbus.fritz.box> Content-Type: text/plain; charset=koi8-r Content-Transfer-Encoding: quoted-printable On 31/10/16 13:53, David Gibson wrote: > On Fri, Oct 28, 2016 at 12:07:12PM +0200, Greg Kurz wrote: >> On Fri, 28 Oct 2016 18:56:40 +1100 >> Alexey Kardashevskiy wrote: >> >>> At the moment sPAPR PHB creates a root buf of TYPE_PCI_BUS type. >>> This means that vfio-pci devices attached to it (and this is >>> a default behaviour) hide PCIe extended capabilities as >>> the bus does not pass a pci_bus_is_express(pdev->bus) check. >>> >>> This changes adds a default PCI bus type property to sPAPR PHB >>> and uses TYPE_PCIE_BUS if none passed; older machines get TYPE_PCI_BU= S >>> for backward compatibility as a bus type is used in the bus name >>> so the root bus name becomes "pcie.0" instead of "pci.0". >>> >>> Signed-off-by: Alexey Kardashevskiy >>> --- >>> >>> What can possibly go wrong with such change of a name? >>> From devices prospective, I cannot see any. >>> >>> libvirt might get upset as "pci.0" will not be available, >>> will it make sense to create pcie.0 as a root bus and always >>> add a PCIe->PCI bridge and name its bus "pci.0"? >>> >>> Or create root bus from TYPE_PCIE_BUS and force name to "pci.0"? >>> pci_register_bus() can do this. >>> >>> >>> --- >>> hw/ppc/spapr.c | 5 +++++ >>> hw/ppc/spapr_pci.c | 5 ++++- >>> include/hw/pci-host/spapr.h | 1 + >>> 3 files changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index 0b3820b..a268511 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -2541,6 +2541,11 @@ DEFINE_SPAPR_MACHINE(2_8, "2.8", true); >>> .driver =3D TYPE_SPAPR_PCI_HOST_BRIDGE, \ >>> .property =3D "mem64_win_size", \ >>> .value =3D "0", \ >>> + }, \ >>> + { \ >>> + .driver =3D TYPE_SPAPR_PCI_HOST_BRIDGE, \ >>> + .property =3D "root_bus_type", \ >>> + .value =3D TYPE_PCI_BUS, \ >>> }, >>> =20 >>> static void phb_placement_2_7(sPAPRMachineState *spapr, uint32_t ind= ex, >>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>> index 7cde30e..2fa1f22 100644 >>> --- a/hw/ppc/spapr_pci.c >>> +++ b/hw/ppc/spapr_pci.c >>> @@ -1434,7 +1434,9 @@ static void spapr_phb_realize(DeviceState *dev,= Error **errp) >>> bus =3D pci_register_bus(dev, NULL, >>> pci_spapr_set_irq, pci_spapr_map_irq, sph= b, >>> &sphb->memspace, &sphb->iospace, >>> - PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_B= US); >>> + PCI_DEVFN(0, 0), PCI_NUM_PINS, >>> + sphb->root_bus_type ? sphb->root_bus_type= : >>> + TYPE_PCIE_BUS); >> >> Shouldn't we ensure that sphb->root_bus_type is either TYPE_PCIE_BUS o= r >> TYPE_PCI_BUS ? >=20 > Yes, I think so. In fact, I think it would be better to make the > property a boolean that just selects PCI-E, rather than this which > exposes qemu (semi-)internal type names on the comamnd line. Sure, a "pcie-root" boolean property should do. However this is not my main concern, I rather wonder if we have to have pci.0 when we pick PCIe for the root. >> Otherwise, if we pass something like: >> >> -global spapr-pci-host-bridge.root_bus_type=3Dpcie >> >> we get the following not really explicit error: >> >> ERROR:/home/greg/Work/qemu/qemu-spapr/qom/object.c:474:object_new_with= _type: assertion failed: (type !=3D NULL) >> >>> phb->bus =3D bus; >>> qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL); >>> =20 >>> @@ -1590,6 +1592,7 @@ static Property spapr_phb_properties[] =3D { >>> DEFINE_PROP_UINT64("pgsz", sPAPRPHBState, page_size_mask, >>> (1ULL << 12) | (1ULL << 16)), >>> DEFINE_PROP_UINT32("numa_node", sPAPRPHBState, numa_node, -1), >>> + DEFINE_PROP_STRING("root_bus_type", sPAPRPHBState, root_bus_type= ), >>> DEFINE_PROP_END_OF_LIST(), >>> }; >>> =20 >>> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.= h >>> index b92c1b5..a4ee873 100644 >>> --- a/include/hw/pci-host/spapr.h >>> +++ b/include/hw/pci-host/spapr.h >>> @@ -79,6 +79,7 @@ struct sPAPRPHBState { >>> uint64_t dma64_win_addr; >>> =20 >>> uint32_t numa_node; >>> + char *root_bus_type; >>> }; >>> =20 >>> #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL >> >=20 --=20 Alexey --nJc45s3nUbN3d69XXfxasGmN90IkF8jk1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIrBAEBCAAVBQJYFsQvDhxhaWtAb3psYWJzLnJ1AAoJEIYTPdgrwSC5uFYQALja ozxu790o+PAvEPWb0LmyiPC1rVRUis5tvT1aSOB5QbfKrKVjWGjXKUy8NN5ngowM HHqVM4wiy8oLHMhm5/vpo29m4wKAF5ZD5JtR2y7nw+mRSlpGQ4JhUX4c7E/F5IKX pcDywVj8V1GLhG48nD93OaLHrGjd9ftbVmzE16slAA7PegBZR2J1mImkk2eTyPUM TVau6giYOu+28sSq5Qh5pC1I2WA9uBwRC2SD8tbwT/RdeGq0Xq+rpex1uJMMXOhx QVcHML4M+Eb+tL8hitJ5iDe3cpKjbEjZgEXnpoo2zPdHdKcQcX6oMexUNGXpp1iu 6k8RoOFPboubc03jNMMnTDH4voGxm78w3BPS1yn4gCWqT/j1JPHKkFYva3+iTjij 7vOAancH8LOrdZmplTB2wkZ8XNW66z2IHcEAQOw967K3X7RixeRp7lRkzhxx8g8V hFsNEJlMzWXw9O4u8lRRyhGASt2Gc/7zTqMsfRcVt27U4Y3zspErpVNMbu02RmNF 921uL4BJvykqSkFfpM4ufar4ucmOU90SC1OkQnT3m409sM1mw6MZwgqXNxihRCHq NrMFF335Sd/AfNYYTWXNTIZNUmKriVbzHKb/BObTl7Bo9yYusFzz7vYPwH/gAQ0x YRvJ73vMSU8JzYwcEm3lFCdKobnXN+Splq+fx5AY =ad0L -----END PGP SIGNATURE----- --nJc45s3nUbN3d69XXfxasGmN90IkF8jk1--