Re: [Qemu-devel] [RFC PATCH qemu 1/3] memory: Add get_fd() hook for IOMMU MR

[David Gibson]

On Wed, Mar 29, 2017 at 10:04:47AM +0200, Paolo Bonzini wrote:
> 
> 
> On 29/03/2017 05:04, Alex Williamson wrote:
> >>> What if we used this as a prototype:
> >>>
> >>> int (*get_fd)(IOMMUFdType type, MemoryRegion *iommu);
> >>>
> >>> And then we defined:
> >>>
> >>> typedef enum {
> >>>     SPAPR_IOMMU_TABLE_FD = 0,
> >>> } IOMMUFdType;  
> >>
> >> Where do I put this enum definition? include/exec/memory.h? It does not
> >> have any mention of any platform yet...
> >  
> > I would assume memory.h, yes.  It seems like the enum is just an
> > abstraction, what does "get fd" mean generically to an IOMMU
> > MemoryRegion?  How can anyone else ever implement that callback if the
> > initial user is assuming that the returned fd is a specific, yet
> > unspecified type.  If the API is "give me an fd for this type of thing"
> > then the IOMMU driver can either provide it or indicate that type is not
> > supported.  There's really no platform knowledge at the memory API
> > level, it's just a type of thing that means something to the driver
> > providing the MemoryRegionIOMMUOps and the caller.
> 
> I think we should move to a QOM hierarchy like
> 
>    AbstractMemoryRegion
>       MemoryRegion                  << adds MemoryRegionOps
>       IOMMUMemoryRegion             << adds MemoryRegionIOMMUOps
>         sPAPR_IOMMUMemoryRegion     << adds get_fd

Right, this makes more sense to me than the enum as well.

> but for now I'm okay with Alex's proposal.

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

signature.asc
Description: PGP signature