From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35895) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bAWZI-0000oA-1y for qemu-devel@nongnu.org; Wed, 08 Jun 2016 02:02:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bAWZE-0003J6-RC for qemu-devel@nongnu.org; Wed, 08 Jun 2016 02:02:20 -0400 Date: Wed, 8 Jun 2016 16:00:06 +1000 From: David Gibson Message-ID: <20160608060006.GY9226@voom.fritz.box> References: <1464771463-37214-1-git-send-email-aik@ozlabs.ru> <201606010902.u518wwmb029353@mx0a-001b2d01.pphosted.com> <20160602033558.GK15455@voom.fritz.box> <3901766f-6adc-b04d-f498-00bbbd0a3724@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="XiX5sJQOWZsNRXst" Content-Disposition: inline In-Reply-To: <3901766f-6adc-b04d-f498-00bbbd0a3724@redhat.com> Subject: Re: [Qemu-devel] [PATCH qemu v17 06/12] memory: Add reporting of supported page sizes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Alexey Kardashevskiy , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Alexander Graf , Alex Williamson --XiX5sJQOWZsNRXst Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 06, 2016 at 03:31:04PM +0200, Paolo Bonzini wrote: >=20 >=20 > On 02/06/2016 05:35, David Gibson wrote: > > On Wed, Jun 01, 2016 at 06:57:37PM +1000, Alexey Kardashevskiy wrote: > >> > Every IOMMU has some granularity which MemoryRegionIOMMUOps::transla= te > >> > uses when translating, however this information is not available out= side > >> > the translate context for various checks. > >> >=20 > >> > This adds a get_page_sizes callback to MemoryRegionIOMMUOps and > >> > a wrapper for it so IOMMU users (such as VFIO) can know the actual > >> > page size(s) used by an IOMMU. > >> >=20 > >> > As IOMMU MR represents a guest IOMMU, this uses TARGET_PAGE_SIZE > >> > as fallback. > >> >=20 > >> > This removes vfio_container_granularity() and uses new helper in > >> > memory_region_iommu_replay() when replaying IOMMU mappings on added > >> > IOMMU memory region. > >> >=20 > >> > Signed-off-by: Alexey Kardashevskiy > >> > Reviewed-by: David Gibson > > Paolo, > >=20 > > Looks like you were left off the CC for this one. > >=20 > > I think this is ready to go - do you want to merge, comment or ack and > > we'll take it either through my tree or Alex's? >=20 > It's okay for you to merge, but the callback should be called > "get_page_size" or "get_replay_granularity". The plural is weird. Hm, no, it really could return multiple page sizes if the logical (guest side) IOMMU supports them. That might be useful at some point in the future. For now, it's sufficient for the replay to use the smallest pagesize. >=20 > Thanks, >=20 > Paolo >=20 >=20 > >> > --- > >> > Changes: > >> > v16: > >> > * used memory_region_iommu_get_page_sizes() instead of > >> > mr->iommu_ops->get_page_sizes() in memory_region_iommu_replay() > >> >=20 > >> > v15: > >> > * s/qemu_real_host_page_size/TARGET_PAGE_SIZE/ in memory_region_iomm= u_get_page_sizes > >> >=20 > >> > v14: > >> > * removed vfio_container_granularity(), changed memory_region_iommu_= replay() > >> >=20 > >> > v4: > >> > * s/1< >> > --- > >> > hw/ppc/spapr_iommu.c | 8 ++++++++ > >> > hw/vfio/common.c | 6 ------ > >> > include/exec/memory.h | 18 ++++++++++++++---- > >> > memory.c | 16 +++++++++++++--- > >> > 4 files changed, 35 insertions(+), 13 deletions(-) > >> >=20 > >> > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c > >> > index a3cc572..90a45c0 100644 > >> > --- a/hw/ppc/spapr_iommu.c > >> > +++ b/hw/ppc/spapr_iommu.c > >> > @@ -149,6 +149,13 @@ static void spapr_tce_table_pre_save(void *opaq= ue) > >> > tcet->bus_offset, tcet->page_shift); > >> > } > >> > =20 > >> > +static uint64_t spapr_tce_get_page_sizes(MemoryRegion *iommu) > >> > +{ > >> > + sPAPRTCETable *tcet =3D container_of(iommu, sPAPRTCETable, iomm= u); > >> > + > >> > + return 1ULL << tcet->page_shift; > >> > +} > >> > + > >> > static int spapr_tce_table_post_load(void *opaque, int version_id) > >> > { > >> > sPAPRTCETable *tcet =3D SPAPR_TCE_TABLE(opaque); > >> > @@ -228,6 +235,7 @@ static const VMStateDescription vmstate_spapr_tc= e_table =3D { > >> > =20 > >> > static MemoryRegionIOMMUOps spapr_iommu_ops =3D { > >> > .translate =3D spapr_tce_translate_iommu, > >> > + .get_page_sizes =3D spapr_tce_get_page_sizes, > >> > }; > >> > =20 > >> > static int spapr_tce_table_realize(DeviceState *dev) > >> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > >> > index e51ed3a..f1a12b0 100644 > >> > --- a/hw/vfio/common.c > >> > +++ b/hw/vfio/common.c > >> > @@ -322,11 +322,6 @@ out: > >> > rcu_read_unlock(); > >> > } > >> > =20 > >> > -static hwaddr vfio_container_granularity(VFIOContainer *container) > >> > -{ > >> > - return (hwaddr)1 << ctz64(container->iova_pgsizes); > >> > -} > >> > - > >> > static void vfio_listener_region_add(MemoryListener *listener, > >> > MemoryRegionSection *section) > >> > { > >> > @@ -394,7 +389,6 @@ static void vfio_listener_region_add(MemoryListe= ner *listener, > >> > =20 > >> > memory_region_register_iommu_notifier(giommu->iommu, &giomm= u->n); > >> > memory_region_iommu_replay(giommu->iommu, &giommu->n, > >> > - vfio_container_granularity(conta= iner), > >> > false); > >> > =20 > >> > return; > >> > diff --git a/include/exec/memory.h b/include/exec/memory.h > >> > index f649697..bd9625f 100644 > >> > --- a/include/exec/memory.h > >> > +++ b/include/exec/memory.h > >> > @@ -149,6 +149,8 @@ typedef struct MemoryRegionIOMMUOps MemoryRegion= IOMMUOps; > >> > struct MemoryRegionIOMMUOps { > >> > /* Return a TLB entry that contains a given address. */ > >> > IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, bo= ol is_write); > >> > + /* Returns supported page sizes */ > >> > + uint64_t (*get_page_sizes)(MemoryRegion *iommu); > >> > }; > >> > =20 > >> > typedef struct CoalescedMemoryRange CoalescedMemoryRange; > >> > @@ -571,6 +573,15 @@ static inline bool memory_region_is_iommu(Memor= yRegion *mr) > >> > =20 > >> > =20 > >> > /** > >> > + * memory_region_iommu_get_page_sizes: get supported page sizes in = an iommu > >> > + * > >> > + * Returns %bitmap of supported page sizes for an iommu. > >> > + * > >> > + * @mr: the memory region being queried > >> > + */ > >> > +uint64_t memory_region_iommu_get_page_sizes(MemoryRegion *mr); > >> > + > >> > +/** > >> > * memory_region_notify_iommu: notify a change in an IOMMU translat= ion entry. > >> > * > >> > * @mr: the memory region that was changed > >> > @@ -594,16 +605,15 @@ void memory_region_register_iommu_notifier(Mem= oryRegion *mr, Notifier *n); > >> > =20 > >> > /** > >> > * memory_region_iommu_replay: replay existing IOMMU translations to > >> > - * a notifier > >> > + * a notifier with the minimum page granularity returned by > >> > + * mr->iommu_ops->get_page_sizes(). > >> > * > >> > * @mr: the memory region to observe > >> > * @n: the notifier to which to replay iommu mappings > >> > - * @granularity: Minimum page granularity to replay notifications f= or > >> > * @is_write: Whether to treat the replay as a translate "write" > >> > * through the iommu > >> > */ > >> > -void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, > >> > - hwaddr granularity, bool is_write); > >> > +void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool= is_write); > >> > =20 > >> > /** > >> > * memory_region_unregister_iommu_notifier: unregister a notifier f= or > >> > diff --git a/memory.c b/memory.c > >> > index 4e3cda8..761ae92 100644 > >> > --- a/memory.c > >> > +++ b/memory.c > >> > @@ -1500,12 +1500,22 @@ void memory_region_register_iommu_notifier(M= emoryRegion *mr, Notifier *n) > >> > notifier_list_add(&mr->iommu_notify, n); > >> > } > >> > =20 > >> > -void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, > >> > - hwaddr granularity, bool is_write) > >> > +uint64_t memory_region_iommu_get_page_sizes(MemoryRegion *mr) > >> > { > >> > - hwaddr addr; > >> > + assert(memory_region_is_iommu(mr)); > >> > + if (mr->iommu_ops && mr->iommu_ops->get_page_sizes) { > >> > + return mr->iommu_ops->get_page_sizes(mr); > >> > + } > >> > + return TARGET_PAGE_SIZE; > >> > +} > >> > + > >> > +void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, bool= is_write) > >> > +{ > >> > + hwaddr addr, granularity; > >> > IOMMUTLBEntry iotlb; > >> > =20 > >> > + granularity =3D (hwaddr)1 << ctz64(memory_region_iommu_get_page= _sizes(mr)); > >> > + > >> > for (addr =3D 0; addr < memory_region_size(mr); addr +=3D granu= larity) { >=20 --=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 --XiX5sJQOWZsNRXst Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXV7RmAAoJEGw4ysog2bOShUAP/0Z1soY8HyvGxeGA260ybNk5 sDLruFPX3Cl50mSeSAA361pSq5IxdBBx/El/N2DjKFLtjJNYsFJKZKQzYNZ4/2uU 6jQQ3X04yUSMbdB9pM11d97fcpMefK6d9sC00xwLVgAvF0DIFYIr/cP9ID6YCIiu X3Qiy/V3WscWvgg59J1etd8PErpQgOckMyLeR+XQK3wCpsolX/Rp7zqS0sDPNBNi NJILqEnfg+w1udmw56WRnwuYIehVoEJ33c4JmW745qGyapw4t2LePRt42d2H/R8x 6pwVlt2UytOD5twhABnfqkdxvILW7jYW0Fe7SuPg/wnsBRqXYGX9g7ClXNgUzSjd j/K/rR6uzgOJ1ldr2WCXRxqtVYK82mzFH2q4RerI+BMSxRWEuUb8z859Y5tHkFK7 hLRXXekkH592sprOoPZbpwdnEu1QcanWbQBLlEPtdNqgHhC2mlPFMcEYK0Ot3g4V OyyGmjKaluY85bAc0YXDbmqYLESr/iBhs2FGO0uY5jKUesTkCPE6bOCCAn6mrHmH MyLn9GS7+qIcfSY9IGrnuIhYnNa2WEmWCko6oiO1xu8x6IngJVVMBr4jKmIn6aoS SdD10BEHw5ll0QuMp/odkipLQbHr1bJ2pJEumQmxFcg2jAQUxibMq7rHG4JOQc4W qAEUxVWisSgi5frMfX9m =3Seq -----END PGP SIGNATURE----- --XiX5sJQOWZsNRXst--