Re: [Qemu-devel] [PATCH RFC v4 15/20] intel_iommu: provide its own replay() callback

[Peter Xu <peterx@redhat.com>]

On Mon, Jan 23, 2017 at 12:34:29PM -0700, Alex Williamson wrote:
> On Mon, 23 Jan 2017 10:54:49 +0800
> Peter Xu <peterx@redhat.com> wrote:
> 
> > On Mon, Jan 23, 2017 at 09:48:48AM +0800, Jason Wang wrote:
> > > 
> > > 
> > > On 2017年01月22日 16:51, Peter Xu wrote:  
> > > >On Sun, Jan 22, 2017 at 03:56:10PM +0800, Jason Wang wrote:
> > > >
> > > >[...]
> > > >  
> > > >>>+/**
> > > >>>+ * vtd_page_walk_level - walk over specific level for IOVA range
> > > >>>+ *
> > > >>>+ * @addr: base GPA addr to start the walk
> > > >>>+ * @start: IOVA range start address
> > > >>>+ * @end: IOVA range end address (start <= addr < end)
> > > >>>+ * @hook_fn: hook func to be called when detected page
> > > >>>+ * @private: private data to be passed into hook func
> > > >>>+ * @read: whether parent level has read permission
> > > >>>+ * @write: whether parent level has write permission
> > > >>>+ * @skipped: accumulated skipped ranges  
> > > >>What's the usage for this parameter? Looks like it was never used in this
> > > >>series.  
> > > >This was for debugging purpose before, and I kept it in case one day
> > > >it can be used again, considering that will not affect much on the
> > > >overall performance.  
> > > 
> > > I think we usually do not keep debugging codes outside debug macros.  
> > 
> > I'll remove it.
> 
> While you're at it, what's the value in using a void* private rather
> than just passing around an IOMMUNotifier*.  Seems like unnecessary
> abstraction.  Thanks,

When handling PSIs (in continuous patches, not this one), we were
passing in MemoryRegion* rather than IOMMUNotifier*:

        vtd_page_walk(&ce, addr, addr + (1 << am) * VTD_PAGE_SIZE,
                        vtd_page_invalidate_notify_hook,
                        (void *)&vtd_as->iommu, true);

So a void* might still be required. Thanks,

-- peterx