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
pgpPKnF40JKEA.pgp
Description: PGP signature
