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::translate
>> > uses when translating, however this information is not available outside
>> > the translate context for various checks.
>> >
>> > 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.
>> >
>> > As IOMMU MR represents a guest IOMMU, this uses TARGET_PAGE_SIZE
>> > as fallback.
>> >
>> > This removes vfio_container_granularity() and uses new helper in
>> > memory_region_iommu_replay() when replaying IOMMU mappings on added
>> > IOMMU memory region.
>> >
>> > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>> > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> Paolo,
>
> Looks like you were left off the CC for this one.
>
> 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()
>> >
>> > v15:
>> > * s/qemu_real_host_page_size/TARGET_PAGE_SIZE/ in
>> > memory_region_iommu_get_page_sizes
>> >
>> > v14:
>> > * removed vfio_container_granularity(), changed
>> > memory_region_iommu_replay()
>> >
>> > v4:
>> > * s/1<<TARGET_PAGE_BITS/qemu_real_host_page_size/
>> > ---
>> > 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(-)
>> >
>> > 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 *opaque)
>> > tcet->bus_offset, tcet->page_shift);
>> > }
>> >
>> > +static uint64_t spapr_tce_get_page_sizes(MemoryRegion *iommu)
>> > +{
>> > + sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
>> > +
>> > + return 1ULL << tcet->page_shift;
>> > +}
>> > +
>> > static int spapr_tce_table_post_load(void *opaque, int version_id)
>> > {
>> > sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque);
>> > @@ -228,6 +235,7 @@ static const VMStateDescription
>> > vmstate_spapr_tce_table = {
>> >
>> > static MemoryRegionIOMMUOps spapr_iommu_ops = {
>> > .translate = spapr_tce_translate_iommu,
>> > + .get_page_sizes = spapr_tce_get_page_sizes,
>> > };
>> >
>> > 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();
>> > }
>> >
>> > -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(MemoryListener
>> > *listener,
>> >
>> > memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
>> > memory_region_iommu_replay(giommu->iommu, &giommu->n,
>> > - vfio_container_granularity(container),
>> > false);
>> >
>> > 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
>> > MemoryRegionIOMMUOps;
>> > struct MemoryRegionIOMMUOps {
>> > /* Return a TLB entry that contains a given address. */
>> > IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, bool
>> > is_write);
>> > + /* Returns supported page sizes */
>> > + uint64_t (*get_page_sizes)(MemoryRegion *iommu);
>> > };
>> >
>> > typedef struct CoalescedMemoryRange CoalescedMemoryRange;
>> > @@ -571,6 +573,15 @@ static inline bool
>> > memory_region_is_iommu(MemoryRegion *mr)
>> >
>> >
>> > /**
>> > + * 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 translation
>> > entry.
>> > *
>> > * @mr: the memory region that was changed
>> > @@ -594,16 +605,15 @@ void
>> > memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n);
>> >
>> > /**
>> > * 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 for
>> > * @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);
>> >
>> > /**
>> > * memory_region_unregister_iommu_notifier: unregister a notifier for
>> > 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(MemoryRegion *mr, Notifier *n)
>> > notifier_list_add(&mr->iommu_notify, n);
>> > }
>> >
>> > -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;
>> >
>> > + granularity = (hwaddr)1 <<
>> > ctz64(memory_region_iommu_get_page_sizes(mr));
>> > +
>> > for (addr = 0; addr < memory_region_size(mr); addr += granularity) {
