Re: [Qemu-devel] [PATCH RFC v4 19/20] intel_iommu: unmap existing pages before replay

[Peter Xu <peterx@redhat.com>]

On Wed, Jan 25, 2017 at 11:11:30AM +0800, Jason Wang wrote:
> 
> 
> On 2017年01月24日 15:31, Peter Xu wrote:
> >On Mon, Jan 23, 2017 at 06:40:12PM +0800, Jason Wang wrote:
> >>On 2017年01月20日 21:08, Peter Xu wrote:
> >>>  static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private)
> >>>  {
> >>>      memory_region_notify_one((IOMMUNotifier *)private, entry);
> >>>@@ -2711,13 +2768,16 @@ static void vtd_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n)
> >>>      if (vtd_dev_to_context_entry(s, bus_n, vtd_as->devfn, &ce) == 0) {
> >>>          /*
> >>>-         * Scanned a valid context entry, walk over the pages and
> >>>-         * notify when needed.
> >>>+         * Scanned a valid context entry, we first make sure to remove
> >>>+         * all existing mappings in old domain, by sending UNMAP to
> >>>+         * all the notifiers. Then, we walk over the pages and notify
> >>>+         * with existing mapped new entries in the new domain.
> >>>           */
> >>A question is what if the context cache was invalidated but the device were
> >>not moved to a new domain. Then the code here does not do anything I
> >>believe?
> >Yes, it'll unmap all the stuffs and remap them all. I think that's my
> >intention, and can we really avoid this?
> >
> >>I think we should move vtd_address_space_unmap() in the context
> >>entry invalidation processing.
> >IMHO we need this "whole umap" thing not only for context entry
> >invalidation, but all the places that need this replay, no? For
> >example, when we receive domain flush.
> >
> >Thanks,
> >
> >-- peterx
> >
> 
> Consider the case that we move device from domain A to no domain. Looks like
> current code did nothing since it can not get a valid context entry during
> replay?

Right. I should do the "whole region unmap" thing even without a valid
context entry. Will fix it in next post. Thanks,

-- peterx