From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44397) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9ucf-0008Nj-CG for qemu-devel@nongnu.org; Mon, 06 Jun 2016 09:31:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9ucb-0007aQ-6P for qemu-devel@nongnu.org; Mon, 06 Jun 2016 09:31:16 -0400 References: <1464771463-37214-1-git-send-email-aik@ozlabs.ru> <201606010902.u518wwmb029353@mx0a-001b2d01.pphosted.com> <20160602033558.GK15455@voom.fritz.box> From: Paolo Bonzini Message-ID: <3901766f-6adc-b04d-f498-00bbbd0a3724@redhat.com> Date: Mon, 6 Jun 2016 15:31:04 +0200 MIME-Version: 1.0 In-Reply-To: <20160602033558.GK15455@voom.fritz.box> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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: David Gibson , Alexey Kardashevskiy Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Alexander Graf , Alex Williamson 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? It's okay for you to merge, but the callback should be called "get_page_size" or "get_replay_granularity". The plural is weird. Thanks, Paolo >> > --- >> > 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 t= o >> > - * 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) {