Re: [Qemu-ppc] [PATCH v4 08/18] vfio: Add DMA memory registering

[David Gibson]

On Tue, Feb 17, 2015 at 01:14:33PM +1100, Alexey Kardashevskiy wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 02/02/2015 06:04 PM, David Gibson wrote:
> > On Thu, Jan 29, 2015 at 08:27:20PM +1100, Alexey Kardashevskiy
> wrote:
[snip]
> >> +    vfio_mem_unregister(container, vaddr, end - iova); +} + +const
> >> MemoryListener vfio_ram_listener = { +    .region_add =
> >> vfio_ram_region_add, +    .region_del = vfio_ram_region_del, +}; + 
> >> +static void vfio_spapr_listener_release(VFIOContainer *container) 
> >> +{ +
> >> memory_listener_unregister(&container->iommu_data.type1.listener); +
> >> memory_listener_unregister(&container->iommu_data.type1.ramlistener);
> >
> >> 
> > Accessing fields within type1 from a function whose name says it's 
> > spapr specific seems very wrong.
> 
> 
> Kind of ugly, yes. But we share the common memory listener with Type1 so...
> 
> 
> >> +} + int vfio_mmap_region(Object *obj, VFIORegion *region, 
> >> MemoryRegion *mem, MemoryRegion *submem, void **map, size_t size,
> >> off_t offset, @@ -705,6 +798,10 @@ static int
> >> vfio_connect_container(VFIOGroup *group, AddressSpace *as) goto
> >> free_container_exit; }
> >> 
> >> +        container->iommu_data.type1.ramlistener =
> >> vfio_ram_listener; +
> >> memory_listener_register(&container->iommu_data.type1.ramlistener, +
> >> &address_space_memory);
> > 
> > Why two separate listeners, rather than doing both jobs from a single
> > listener?
> 
> ... I actually like the idea to have this separated from the rest of the
> code. Furthermore, now I think we better have separate memory listeners
> for Type1 and SPAPR as the current vfio_listener_region_add()/del() look
> quite ugly trying to do different things depending on
> memory_region_is_iommu().
> 
> Any objection to separating SPAPR's listener (and merging it with the one
> introduced by this patch)?

Not from here, I think that sounds like a good idea.

-- 
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

pgpEWNX2TPA4A.pgp
Description: PGP signature