From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51270) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHREX-0007kD-G9 for qemu-devel@nongnu.org; Wed, 13 Aug 2014 01:36:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHRES-0000GH-NB for qemu-devel@nongnu.org; Wed, 13 Aug 2014 01:36:25 -0400 Date: Wed, 13 Aug 2014 13:27:51 +1000 From: David Gibson Message-ID: <20140813032751.GG7628@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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Cgrdyab2wu3Akvjd" Content-Disposition: inline In-Reply-To: <53E9C169.5060809@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 --Cgrdyab2wu3Akvjd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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? > >=20 > > I think you'd be better off folding the whole double loop into the > > fixmask function. > >=20 > >> + > >> + 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); > >=20 > > 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? >=20 > Why PHB? This is DMA memory. @ram_size is the upper limit, we can make mo= re > 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. 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. [snip] > >> +static void rtas_ibm_create_pe_dma_window(PowerPCCPU *cpu, > >> + sPAPREnvironment *spapr, > >> + uint32_t token, uint32_t na= rgs, > >> + target_ulong args, > >> + uint32_t nret, target_ulong= 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; > >=20 > > 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? >=20 >=20 > 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 >=20 > #define SPAPR_DDW_LIOBN(sphb, windownum) ((sphb)->dma_liobn | windownum) Ok. Really, the assigned liobns should be included in the migration stream if they're not already. Relying them on them being set consistently at startup is going to be really fragile. --=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 --Cgrdyab2wu3Akvjd Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJT6ts3AAoJEGw4ysog2bOSvygQAMcd1AQY+pQnQotZhkHETh1o nTpMmEHqdCE4DzhNyWe8sAYuTD0aO+UZxaMrAAQYvcNFGB9WdSYSTvma3hd4fCfF 9LPdqAczK0UG4KCJ9AJnc401QZbwc1bgN+CSY/T+aT6SJkuJcT0nwe8XAbaUUP4e wgOdMRYlTwGq1rP4DvLAJyPOWzIlIMnEfPtNnrJw8IYVPIpt7fGadgevXxcilat3 JFIj2ihnE9pNqNs2mt4w8eRbrrGi4S+yqrH/ok2MzPkMAfp1DaNPEB3v44WmbtcZ ufUVPiIYxEY+tol8LPyc6s2++/oSrtwfIWdosR+Cf2FECzEqNeuq+LToAvEThR8B pEdFKpa1VytaJn9jwYgNssuIWUjRiAPnQJCFdWDjLqHcYKuL9RlrXLoNzNY0jXtH mvEHbqtN4O7BqaJb2xXqqsBb2EWsP3p5yRCr552ABsv4q2Tdw+ORTHnSqANgVZXd CFGU0l1uumdldKT5dO+NXIhCmZg5vjWjzQw3a/NGCmstVwJ+vzUInPbea+JNnbZ/ l8o5NAppLcNy3/JZkmYDnu6jQifJoZAdzwp8HXYiq+FhLe2u5Z60N1O9UM0KRfnl 5KLwhDafavCGRNCW4ue2o27ThbYr8aT/loAULqCoYTkMzuzm1V40iJgRMOX1eQFZ /9zUWYgEvTZHtspFFL5e =kMO/ -----END PGP SIGNATURE----- --Cgrdyab2wu3Akvjd--