From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38270) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XI56y-0001wb-Ru for qemu-devel@nongnu.org; Thu, 14 Aug 2014 20:11:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XI56u-0000Dh-PA for qemu-devel@nongnu.org; Thu, 14 Aug 2014 20:11:16 -0400 Date: Fri, 15 Aug 2014 10:04:07 +1000 From: David Gibson Message-ID: <20140815000407.GK7628@voom.redhat.com> References: <1406799254-25223-1-git-send-email-aik@ozlabs.ru> <1406799254-25223-7-git-send-email-aik@ozlabs.ru> <20140812014533.GE7628@voom.redhat.com> <53E9C169.5060809@ozlabs.ru> <20140813032751.GG7628@voom.redhat.com> <53EC737E.4080203@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="5mZBmBd1ZkdwT1ny" Content-Disposition: inline In-Reply-To: <53EC737E.4080203@ozlabs.ru> Subject: Re: [Qemu-devel] [RFC PATCH 06/10] spapr_rtas: Add Dynamic DMA windows (DDW) RTAS calls support 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 --5mZBmBd1ZkdwT1ny Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Aug 14, 2014 at 06:29:50PM +1000, Alexey Kardashevskiy wrote: > On 08/13/2014 01:27 PM, David Gibson wrote: > > On Tue, Aug 12, 2014 at 05:25:29PM +1000, Alexey Kardashevskiy wrote: > >> On 08/12/2014 11:45 AM, David Gibson wrote: > >>> On Thu, Jul 31, 2014 at 07:34:10PM +1000, Alexey Kardashevskiy > >> wrote: > > [snip] > >>> The function of this is kind of unclear. I'm assuming this is > >>> filtering the supported page sizes reported by the PHB by the possible > >>> page sizes based on host page size or other constraints. Is that > >>> right? > >>> > >>> I think you'd be better off folding the whole double loop into the > >>> fixmask function. > >>> > >>>> + > >>>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > >>>> + rtas_st(rets, 1, windows_available); > >>>> + /* Return maximum number as all RAM was 4K pages */ > >>>> + rtas_st(rets, 2, ram_size >> SPAPR_TCE_PAGE_SHIFT); > >>> > >>> I'm assuming this is the allowed size of the dynamic windows. > >>> Shouldn't that be reported by a PHB callback, rather than hardcoded > >>> here? > >> > >> Why PHB? This is DMA memory. @ram_size is the upper limit, we can make= more > >> only when we have memory hotplug (which we do not have) and the guest = can > >> create smaller windows if it wants so I do not really follow you here. > >=20 > > What I'm not clear on is what this RTAS return actually means. Is it > > saying the maximum size of the DMA window, or the maximum address > > which can be mapped by that window? Remember I don't have access to > > PAPR documentation any more - nor do others reading these patches. >=20 >=20 > It is literally "Largest contiguous block of TCEs allocated specifically > for (that is, are reserved for) this PE". Which I understand as the maxim= um > number of TCEs. Ok, so essentially it's a property of the IOMMU. Hrm, I guess ram_size is good enough for now then. [snip] > > [snip] > >>>> +static void rtas_ibm_create_pe_dma_window(PowerPCCPU *cpu, > >>>> + sPAPREnvironment *spapr, > >>>> + uint32_t token, uint32_t = nargs, > >>>> + target_ulong args, > >>>> + uint32_t nret, target_ulo= ng rets) > >>>> +{ > >>>> + sPAPRPHBState *sphb; > >>>> + sPAPRPHBClass *spc; > >>>> + sPAPRTCETable *tcet =3D NULL; > >>>> + uint32_t addr, page_shift, window_shift, liobn; > >>>> + uint64_t buid; > >>>> + long ret; > >>>> + > >>>> + if ((nargs !=3D 5) || (nret !=3D 4)) { > >>>> + goto param_error_exit; > >>>> + } > >>>> + > >>>> + buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > >>>> + addr =3D rtas_ld(args, 0); > >>>> + sphb =3D spapr_pci_find_phb(spapr, buid); > >>>> + if (!sphb) { > >>>> + goto param_error_exit; > >>>> + } > >>>> + > >>>> + spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > >>>> + if (!spc->ddw_create) { > >>>> + goto hw_error_exit; > >>>> + } > >>>> + > >>>> + page_shift =3D rtas_ld(args, 3); > >>>> + window_shift =3D rtas_ld(args, 4); > >>>> + liobn =3D sphb->dma_liobn + 0x10000; > >>> > >>> Isn't using a fixed LIOBN here assuming you can only have a single DDW > >>> per PHB? That's true for now, but in theory shouldn't it be reported > >>> by the PHB code itself? > >> > >> > >> This should be a unique LIOBN so it is not up to PHB to choose. And we > >> cannot make it completely random for migration purposes. I'll make it > >> something like > >> > >> #define SPAPR_DDW_LIOBN(sphb, windownum) ((sphb)->dma_liobn | windownu= m) > >=20 > > Ok. > >=20 > > Really, the assigned liobns should be included in the migration stream > > if they're not already. >=20 > LIOBNs already migrate, liobn itself is an instance id of a TCE table > object in the migration stream. Ok, so couldn't we just add an alloc_liobn() function instead of hardcoding how the liobns are constructed? --=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 --5mZBmBd1ZkdwT1ny Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJT7U53AAoJEGw4ysog2bOS2JgQAOOToRwFYiX+0i+M+Cd0SYIT jRGG1YaP4YDbxnipZFgJ/omobJS0Sks21Q67V96uSHb0EY8hJYXfK84Go474aHoK c1WaAAQ/+4z5iTglIk7/YCNCa5gDyNPBtXYNVf2zTNg1r9Q95ENaS3m2z9YgYXT8 kJmWi/ik3rt4XPfl8tSBG9wZZSQBwCMycgQzLprHaBkDlxe4HMVBfGutcrh3W7yd UzyGAfvxCcML+gqa0nEII1iFY8ztRq2hkAR5c/mo5M1TKJyGhp1V7Evy6qvEGT44 RedIlhyg/zU2ZVRkFJUn7s3qdL0uNDSlOg1hiOi/ywGdX2r3UZTV4O1xL/czBP5E MFyXyfbdY/P77PQFwIQJkggLfix4WYhldclczbNSGyRzWuojted89MVmR4z3aeNt rCi66c5+wYsiPeuVjns+4t+WpM00KzqObZ8lnOffh6QTDdNIiRr9LV/OVGqD7Kwz fvZS1pu0dA+hh26PqcaTUw7pFHqEjVOF13Ae7HLuDQUzVESJH4v5GFtaGc4rLXyS HnhWueBx1K5QMTkhySwgKluHj2Bk+siERQAKbUcwIhDCwiVubMf40hAFXexjSKk1 UnAO8chRaAZe4IzBOVUsCbflI/cqnTMMEECpvh9sTcJxk1FaH4rMEGXTQdT1ikno MprCbjOYM0es3/fTBfmc =/KBU -----END PGP SIGNATURE----- --5mZBmBd1ZkdwT1ny--