From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMB2w-0002Q0-7b for qemu-devel@nongnu.org; Tue, 26 Aug 2014 03:20:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XMB2q-0006P2-2b for qemu-devel@nongnu.org; Tue, 26 Aug 2014 03:20:02 -0400 Date: Tue, 26 Aug 2014 17:14:31 +1000 From: David Gibson Message-ID: <20140826071431.GW9923@voom.redhat.com> References: <1408097555-28126-1-git-send-email-aik@ozlabs.ru> <1408097555-28126-9-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Cf1qy2gtPj5yoBMh" Content-Disposition: inline In-Reply-To: <1408097555-28126-9-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [RFC PATCH v2 08/13] 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, Gavin Shan , Alexander Graf --Cf1qy2gtPj5yoBMh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 15, 2014 at 08:12:30PM +1000, Alexey Kardashevskiy wrote: > This implements DDW for emulated PHB. >=20 > This advertises DDW in device tree. >=20 > Since QEMU does not implement any 64bit DMA capable device, this hack > has been used to enable 64bit DMA on E1000: >=20 > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index 0fc29a0..131f80a 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -240,6 +240,7 @@ static const uint32_t mac_reg_init[] =3D { > [STATUS] =3D 0x80000000 | E1000_STATUS_GIO_MASTER_ENABLE | > E1000_STATUS_ASDV | E1000_STATUS_MTXCKOK | > E1000_STATUS_SPEED_1000 | E1000_STATUS_FD | > + E1000_STATUS_PCIX_MODE | > E1000_STATUS_LU, > [MANC] =3D E1000_MANC_EN_MNG2HOST | E1000_MANC_RCV_TCO_EN | > E1000_MANC_ARP_EN | E1000_MANC_0298_EN | Are you planning to send a patch to enable 64-bit DMA in the e1000 device properly? >=20 > Signed-off-by: Alexey Kardashevskiy > --- > Changes: > v2: > * tested on hacked emulated E1000 > * implemented DDW reset on the PHB reset > * spapr_pci_ddw_remove/spapr_pci_ddw_reset are public for reuse by VFIO > --- > hw/ppc/spapr_pci.c | 96 +++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/pci-host/spapr.h | 7 ++++ > 2 files changed, 103 insertions(+) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 9b03d0d..cba8d9d 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" > @@ -498,6 +499,70 @@ void spapr_pci_msi_init(sPAPREnvironment *spapr, hwa= ddr addr) > } > =20 > /* > + * Dynamic DMA windows > + */ > +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, > + 1ULL << (window_shift - page_shift), > + true); Does anything actually validate that the specified page_shift is one of the permitted/advertised ones? > + if (!*ptcet) { > + return -1; > + } > + memory_region_add_subregion(&sphb->iommu_root, (*ptcet)->bus_offset, > + spapr_tce_get_iommu(*ptcet)); > + > + return 0; > +} > + > +int spapr_pci_ddw_remove(sPAPRPHBState *sphb, sPAPRTCETable *tcet) > +{ > + memory_region_del_subregion(&sphb->iommu_root, > + spapr_tce_get_iommu(tcet)); > + spapr_tce_free_table(tcet); Ok, relating to my comment in the previous patch, ddw_num doesn't seem to be decremented here either. > + > + return 0; > +} > + > +static int spapr_pci_remove_ddw_cb(Object *child, void *opaque) > +{ > + sPAPRTCETable *tcet; > + > + tcet =3D (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE= _TABLE); > + > + /* Delete all dynamic windows, i.e. every except the default one wit= h #0 */ > + if (tcet && SPAPR_PCI_DMA_WINDOW_NUM(tcet->liobn)) { > + sPAPRPHBState *sphb =3D opaque; > + sPAPRPHBClass *spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + > + spc->ddw_remove(sphb, tcet); > + } > + > + return 0; > +} > + > +int spapr_pci_ddw_reset(sPAPRPHBState *sphb) > +{ > + object_child_foreach(OBJECT(sphb), spapr_pci_remove_ddw_cb, sphb); > + sphb->ddw_num =3D 0; So, you do reset ddw_num here, but since it is incremented in the generic RTAS code, this smells like a layering violation. > + > + return 0; > +} > + > +/* > * PHB PCI device > */ > static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int = devfn) > @@ -671,6 +736,12 @@ static int spapr_phb_children_reset(Object *child, v= oid *opaque) > =20 > static void spapr_phb_reset(DeviceState *qdev) > { > + sPAPRPHBClass *spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(qdev); > + > + if (spc->ddw_reset) { > + spc->ddw_reset(SPAPR_PCI_HOST_BRIDGE(qdev)); > + } > + > /* Reset the IOMMU state */ > object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL); > } > @@ -685,6 +756,7 @@ static Property spapr_phb_properties[] =3D { > DEFINE_PROP_UINT64("io_win_addr", sPAPRPHBState, io_win_addr, -1), > DEFINE_PROP_UINT64("io_win_size", sPAPRPHBState, io_win_size, > SPAPR_PCI_IO_WIN_SIZE), > + DEFINE_PROP_BOOL("ddw", sPAPRPHBState, ddw_enabled, false), > DEFINE_PROP_END_OF_LIST(), > }; > =20 > @@ -802,6 +874,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 { > @@ -885,6 +961,13 @@ 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); > =20 > /* Start populating the FDT */ > sprintf(nodename, "pci@%" PRIx64, phb->buid); > @@ -914,6 +997,19 @@ 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 (phb->ddw_enabled && > + 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 */ I guess it's not really relevant here, but the reason for availability of reset causing the guest to remove the default window seems unclear. > + _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 a1fdbb2..0b40fcf 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -104,6 +104,8 @@ struct sPAPRPHBState { > int32_t msi_devs_num; > spapr_pci_msi_mig *msi_devs; > =20 > + bool ddw_enabled; > + > QLIST_ENTRY(sPAPRPHBState) list; > }; > =20 > @@ -126,6 +128,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); > @@ -144,5 +149,7 @@ void spapr_pci_rtas_init(void); > sPAPRPHBState *spapr_pci_find_phb(sPAPREnvironment *spapr, uint64_t buid= ); > PCIDevice *spapr_pci_find_dev(sPAPREnvironment *spapr, uint64_t buid, > uint32_t config_addr); > +int spapr_pci_ddw_remove(sPAPRPHBState *sphb, sPAPRTCETable *tcet); > +int spapr_pci_ddw_reset(sPAPRPHBState *sphb); > =20 > #endif /* __HW_SPAPR_PCI_H__ */ --=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 --Cf1qy2gtPj5yoBMh Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJT/DPXAAoJEGw4ysog2bOSr2AP/jodhyOG8P57FWjnYPQhXKmC gJ4yzajAPSyCmERYu9rwp/6UmuRbF/o3v7oMOKdZqY4M+O8zgpxJu5LIpEWiE4hY ugFoBMWOsyg/nMQw0BsC96U+nAySsMUw+YkmVsUbIVf5vZbPTLBRyz8u8ezZ7TpK yTzmhx323aoqW32mSGzQU+9dHc5EZ/Re8e8L62MOLS5CTtqPcgltCxpknPIy3btJ 3Yt+lx7DLqsJfRtUbKiZzu/k2HoUnFMIx2AxYJPlNHZDayqtM1lw608gwbqC4vHk VWiS+Tzyh8lR7BAJ2Uu3+ZibgL1rSknCdr4VEkW37ubb77hRPwtJ1r2Lj1NlEQ42 Anp/nKTaHxTh9kf5l7BCxbWU2bypBVzR5lZRVBAlbeOyHJbZluoBd01YRUISYQDY oqSHaz0nyw5+ADYuit5a0q0G4bN9DWnmm32N/QV/3ptZ9nVBO10AUGcOO4mykZks K0T9i43EY6DoZHJLC5781383sDMVYuYLDGqvLwhFWreR5jRMlrwyJy4p1OZHQTow QzpC//doHAA9V7Hx0Mq/duDiA+92DEWRbeH5kTZSOK5h/FZEImtv10CYFWOxv0hi AGMuNGMenY7fbhkyYOfZkqgUoIQUSe0KYHHO18zUVat4J4mUKj27NU9aCr9UR9EJ /YeRYFSIQlcSCp3T3otq =E3uj -----END PGP SIGNATURE----- --Cf1qy2gtPj5yoBMh--