From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38741) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH1e7-0001WI-Pn for qemu-devel@nongnu.org; Mon, 11 Aug 2014 22:17:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XH1e3-0003rA-Mr for qemu-devel@nongnu.org; Mon, 11 Aug 2014 22:17:07 -0400 Date: Tue, 12 Aug 2014 12:10:45 +1000 From: David Gibson Message-ID: <20140812021045.GA23065@voom.redhat.com> References: <1406799254-25223-1-git-send-email-aik@ozlabs.ru> <1406799254-25223-9-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="SLDf9lqlvOQaIe6s" Content-Disposition: inline In-Reply-To: <1406799254-25223-9-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [RFC PATCH 08/10] spapr_pci: Enable DDW List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Alex Williamson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf --SLDf9lqlvOQaIe6s Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 31, 2014 at 07:34:12PM +1000, Alexey Kardashevskiy wrote: > This implements DDW for emulated PHB. >=20 > This advertises DDW in device tree. >=20 > Signed-off-by: Alexey Kardashevskiy > --- >=20 > The DDW has not been tested as QEMU does not implement any 64bit DMA capa= ble > device and existing linux guests do not use DDW for 32bit DMA. > --- > hw/ppc/spapr_pci.c | 65 +++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/pci-host/spapr.h | 5 ++++ > 2 files changed, 70 insertions(+) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 230b59c..d1f4c86 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -22,6 +22,7 @@ > * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALING= S IN > * THE SOFTWARE. > */ > +#include "sysemu/sysemu.h" > #include "hw/hw.h" > #include "hw/pci/pci.h" > #include "hw/pci/msi.h" > @@ -650,6 +651,8 @@ static void spapr_phb_finish_realize(sPAPRPHBState *s= phb, Error **errp) > /* Register default 32bit DMA window */ > memory_region_add_subregion(&sphb->iommu_root, 0, > spapr_tce_get_iommu(tcet)); > + > + sphb->ddw_supported =3D true; > } > =20 > static int spapr_phb_children_reset(Object *child, void *opaque) > @@ -781,6 +784,42 @@ static const char *spapr_phb_root_bus_path(PCIHostSt= ate *host_bridge, > return sphb->dtbusname; > } > =20 > +static int spapr_pci_ddw_query(sPAPRPHBState *sphb, > + uint32_t *windows_available, > + uint32_t *page_size_mask) > +{ > + *windows_available =3D 1; > + *page_size_mask =3D DDW_PGSIZE_16M; > + > + return 0; > +} > + > +static int spapr_pci_ddw_create(sPAPRPHBState *sphb, uint32_t page_shift, > + uint32_t window_shift, uint32_t liobn, > + sPAPRTCETable **ptcet) > +{ > + *ptcet =3D spapr_tce_new_table(DEVICE(sphb), liobn, SPAPR_PCI_TCE64_= START, > + page_shift, 1 << (window_shift - page_s= hift), > + true); > + if (!*ptcet) { > + return -1; > + } > + memory_region_add_subregion(&sphb->iommu_root, (*ptcet)->bus_offset, > + spapr_tce_get_iommu(*ptcet)); > + > + return 0; > +} > + > +static int spapr_pci_ddw_remove(sPAPRPHBState *sphb, sPAPRTCETable *tcet) > +{ > + return 0; > +} > + > +static int spapr_pci_ddw_reset(sPAPRPHBState *sphb) > +{ > + return 0; > +} > + > static void spapr_phb_class_init(ObjectClass *klass, void *data) > { > PCIHostBridgeClass *hc =3D PCI_HOST_BRIDGE_CLASS(klass); > @@ -795,6 +834,10 @@ static void spapr_phb_class_init(ObjectClass *klass,= void *data) > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > dc->cannot_instantiate_with_device_add_yet =3D false; > spc->finish_realize =3D spapr_phb_finish_realize; > + spc->ddw_query =3D spapr_pci_ddw_query; > + spc->ddw_create =3D spapr_pci_ddw_create; > + spc->ddw_remove =3D spapr_pci_ddw_remove; > + spc->ddw_reset =3D spapr_pci_ddw_reset; > } > =20 > static const TypeInfo spapr_phb_info =3D { > @@ -878,6 +921,14 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > uint32_t interrupt_map_mask[] =3D { > cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; > uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; > + uint32_t ddw_applicable[] =3D { > + RTAS_IBM_QUERY_PE_DMA_WINDOW, > + RTAS_IBM_CREATE_PE_DMA_WINDOW, > + RTAS_IBM_REMOVE_PE_DMA_WINDOW > + }; > + uint32_t ddw_extensions[] =3D { 1, RTAS_IBM_RESET_PE_DMA_WINDOW }; > + sPAPRPHBClass *spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(phb); > + QemuOpts *machine_opts =3D qemu_get_machine_opts(); > =20 > /* Start populating the FDT */ > sprintf(nodename, "pci@%" PRIx64, phb->buid); > @@ -907,6 +958,20 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1= )); > _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", XICS_IRQS)); > =20 > + /* Dynamic DMA window */ > + if (qemu_opt_get_bool(machine_opts, "ddw", true) && So, I think this is a rephrasing of agraf's objection. This feels like the wrong place to be checking the machine option. I think the machine option should actually disable the DDW support on the PHBs (i.e. cause the DDW RTAS calls to fail if attempted), rather than just turn off advertisement in the device tree. In fact it should probably stop the RTAS calls being registered at all, at the moment the RTAS tokens will still be advertised in the device tree, even if ddw is "disabled" > + phb->ddw_supported && > + spc->ddw_query && spc->ddw_create && spc->ddw_remove) { > + _FDT(fdt_setprop(fdt, bus_off, "ibm,ddw-applicable", &ddw_applic= able, > + sizeof(ddw_applicable))); > + > + if (spc->ddw_reset) { > + /* When enabled, the guest will remove the default 32bit win= dow */ > + _FDT(fdt_setprop(fdt, bus_off, "ibm,ddw-extensions", > + &ddw_extensions, sizeof(ddw_extensions))); > + } > + } > + > /* Build the interrupt-map, this must matches what is done > * in pci_spapr_map_irq > */ > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index 119d326..2046356 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -103,6 +103,8 @@ struct sPAPRPHBState { > int32_t msi_devs_num; > spapr_pci_msi_mig *msi_devs; > =20 > + bool ddw_supported; > + > QLIST_ENTRY(sPAPRPHBState) list; > }; > =20 > @@ -125,6 +127,9 @@ struct sPAPRPHBVFIOState { > =20 > #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL > =20 > +/* Default 64bit dynamic window offset */ > +#define SPAPR_PCI_TCE64_START 0x8000000000000000ULL > + > static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int= pin) > { > return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq); Also, I think there might be a leak on system_reset here. I can't see anything that will clean up any secondary DDW TCE tables when the whole system is reset. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --SLDf9lqlvOQaIe6s Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJT6XelAAoJEGw4ysog2bOS/ecP/1bPw4jut5TBmpUb3qb03fg/ ZvlE78C1AajMb0NycPPmMpL2Kdcq0qnkRr+lV/nrdeiW5YB/0XzRnTMl1DWZnUwx T6M6+AZsBuHf+e5O3qAG6EYrCZg6oDl8MOj/MklI5jVn8hIct9wut6guCZ7tTAv4 b0ONJ2m/DlX5DEiEc+kyUEc6LICm26wV8DayePZocbxgf1O2bBnhXulHMmcPMOsl oHwZ+D2OlNSV/9aY8N4/7spXY0hFVOCL3eUukljdwDYwLLpEis7IAa3KhvPlBxgd R81in9NEdKfuriXfyXp9IgLIB/pofKIjXyaiIfkNFkSuIZBdGqXJOkj8aIw03B84 iOFOO9e+BMzZoAL82rILXZ4rje84WPLGb+ihHpq1zLWSZ5LgccR35O06AW8/GzFC BUq0BhJn14pZFoBj7uQvd4l188rnHUiYhlBt0q9UX4MJPWJQaGQPqcybKGe8j1l4 ogjeE83z+8CCee3kPHmVwnzPxEb90TIWDxz6Wkyju05yxkI46Oz5AbEbVDye4yIN BYUen3Lw9cqmcf+G583hQoDglXsIT/tMMxWhQR4FOQWM7NzAefPEeOIOiQE+3z5j Rhto6T7Wx90xJfN7uBSfVuJycOPvzfmMtA3AjIDiHcTVi0RVkzTYtXNYHl2bKlxK 4BBlfLsfHwbRxAECUHwa =ZJ0f -----END PGP SIGNATURE----- --SLDf9lqlvOQaIe6s--