From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48072) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abIT2-00064T-VL for qemu-devel@nongnu.org; Wed, 02 Mar 2016 20:54:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abISz-0005A9-ML for qemu-devel@nongnu.org; Wed, 02 Mar 2016 20:54:16 -0500 Date: Thu, 3 Mar 2016 12:40:19 +1100 From: David Gibson Message-ID: <20160303014019.GD1620@voom.redhat.com> References: <1456823441-46757-1-git-send-email-aik@ozlabs.ru> <1456823441-46757-3-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="wLAMOaPNJ0fu1fTG" Content-Disposition: inline In-Reply-To: <1456823441-46757-3-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 02/16] spapr_pci: Move DMA window enablement to a helper 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 --wLAMOaPNJ0fu1fTG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 01, 2016 at 08:10:27PM +1100, Alexey Kardashevskiy wrote: > We are going to have multiple DMA windows soon so let's start preparing. >=20 > This adds a new helper to create a DMA window and makes use of it in > sPAPRPHBState::realize(). >=20 > Signed-off-by: Alexey Kardashevskiy > --- > hw/ppc/spapr_pci.c | 40 +++++++++++++++++++++++++++------------- > 1 file changed, 27 insertions(+), 13 deletions(-) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 3d1145e..248f20a 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -803,6 +803,29 @@ static char *spapr_phb_get_loc_code(sPAPRPHBState *s= phb, PCIDevice *pdev) > return buf; > } > =20 > +static int spapr_phb_dma_window_enable(sPAPRPHBState *sphb, > + uint32_t liobn, uint32_t page_shi= ft, > + uint64_t window_addr, > + uint64_t window_size) > +{ > + sPAPRTCETable *tcet; > + uint32_t nb_table =3D window_size >> page_shift; > + > + if (!nb_table) { > + return -1; > + } The caller shouldn't do this, so this probably makes more sense as an assert() than an error return. > + > + tcet =3D spapr_tce_new_table(DEVICE(sphb), liobn, window_addr, > + page_shift, nb_table, false); > + if (!tcet) { > + return -1; > + } Since you're adding error reporting, you might as well make it via the error API instead of a return code. That way if we wasnt to add more detailed error API reporting to spapr_tce_new_table() in future, there's less to change. > + > + memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset, > + spapr_tce_get_iommu(tcet)); > + return 0; > +} > + > /* Macros to operate with address in OF binding to PCI */ > #define b_x(x, p, l) (((x) & ((1<<(l))-1)) << (p)) > #define b_n(x) b_x((x), 31, 1) /* 0 if relocatable */ > @@ -1228,8 +1251,6 @@ static void spapr_phb_realize(DeviceState *dev, Err= or **errp) > int i; > PCIBus *bus; > uint64_t msi_window_size =3D 4096; > - sPAPRTCETable *tcet; > - uint32_t nb_table; > =20 > if (sphb->index !=3D (uint32_t)-1) { > hwaddr windows_base; > @@ -1381,18 +1402,11 @@ static void spapr_phb_realize(DeviceState *dev, E= rror **errp) > } > } > =20 > - nb_table =3D sphb->dma_win_size >> SPAPR_TCE_PAGE_SHIFT; > - tcet =3D spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn, > - 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false); > - if (!tcet) { > - error_setg(errp, "Unable to create TCE table for %s", > - sphb->dtbusname); > - return; > - } > - > /* Register default 32bit DMA window */ > - memory_region_add_subregion(&sphb->iommu_root, sphb->dma_win_addr, > - spapr_tce_get_iommu(tcet)); > + if (spapr_phb_dma_window_enable(sphb, sphb->dma_liobn, SPAPR_TCE_PAG= E_SHIFT, > + sphb->dma_win_addr, sphb->dma_win_si= ze)) { > + error_setg(errp, "Unable to create TCE table for %s", sphb->dtbu= sname); > + } > =20 > sphb->msi =3D g_hash_table_new_full(g_int_hash, g_int_equal, g_free,= g_free); > } --=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 --wLAMOaPNJ0fu1fTG Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW15YCAAoJEGw4ysog2bOSQbYQALYWFa4FH7Tz2lDdszjKyq/Q v1okgJMF7BNWOrud4D04o14n8EK5OYNn/kNPonh3mDDci57xIyX3qFpid7RP8APK UvsULvGIxmTLLMGMdhN1VelGHkjHtn8lcl0QgpirJatqtNNvBxo6BlAOnRPcuOMm 0rOKgn6KKLrYDjUTHnYg9LVhCopp6SidAm2nUI60s1UeFZ7KHxzmmSKHpuiVJHXL zsL4N6E/L/NYwT//om9yBo8iLZ16PNg+/X2daO/sA/IhM4ASuAtIXaMrFeffGDhO TmzHCgoJlVVQ7cxViD82VVPy/n5L14Xl/J3/tDOuaFvyXlpGWL3XxI5JV1n7qCwz 0WWMx7fSeo4ApLjg8cHIJOkmyZ6UJpVvReoYelav5edc40tUjIU2u82nENb4fn0b c7mh3pbqHKVloMHZM8SF/N3y07j1PADBILoAB9ARuf/XAUGHZa4Ik1MOG7j1ZV8b QEy0JVHbIKFux+xHJAEqdT9VoLwMyEAh7mMAMd72iwv69KLtESL4VSLB+UCnW+cS zKpu9divwVmoYM4SnVnLljzE2M58PXBqt/TUjeROgJjiKcL7zvftXb5iOfN5/2lf 9f2Z8n7Imzw2IntwVE6H81j0+9TArlkz8FDQggwlTzMvpkHWBSHpT7gR/8PBhYZA S7EBV7m07yyvQDTChYP4 =BWpv -----END PGP SIGNATURE----- --wLAMOaPNJ0fu1fTG--