[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag |
Date: |
Mon, 12 Sep 2016 13:13:41 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Mon, Sep 12, 2016 at 11:26:04AM +1000, David Gibson wrote:
> On Thu, Sep 08, 2016 at 05:07:32PM +0800, Peter Xu wrote:
> > On Wed, Sep 07, 2016 at 04:41:54PM +1000, David Gibson wrote:
> > > On Wed, Sep 07, 2016 at 02:34:19PM +0800, Peter Xu wrote:
> > > > On Wed, Sep 07, 2016 at 03:44:19PM +1000, David Gibson wrote:
> > > > > > For "CHANGE", it sounds like a unmap() + a map(). However I'd say
> > > > > > "ADDITION" is nowhere better...
> > > > >
> > > > > Right.. this brings up a good point.
> > > > >
> > > > > Changing a mapping (i.e. overwriting an existing mapping with a
> > > > > different one) would also need notification, even on x86, no? Since
> > > > > it implicitly invalidates the previous mapping.
> > > > >
> > > > > I'm guessing the guest will avoid this by always unmapping before it
> > > > > maps. We still need to consider this possibility when designing the
> > > > > notifier interface though.
> > > > >
> > > > > It seems the real notification triggers here are:
> > > > > * map - something is mapped which previously wasn't
> > > > > * unmap - something is no longer mapped which was before
> > > > >
> > > > > Note that whether the second needs to be triggered depends on the
> > > > > *previous* state of that IOBA range, *not* on the permissions of the
> > > > > new mapping (if any).
> > > > >
> > > > > A "change" - replacing one mapping with another should count as both a
> > > > > "map" and "unmap" event.
> > > >
> > > > Yeah... For MAP/UNMAP, it is strange in another way: e.g. for vhost,
> > > > it doesn't care about map/unmap, it cares about invalidated cache.
> > >
> > > I think caring about invalidated cache *is* caring about unmap. It
> > > doesn't matter whether the new mapping is something or nothing - if
> > > the old mapping is no longer valid, you need to invalidate the cache,
> > > yes?
> >
> > Yes, I think these two are exactly the same in implementation (vhost
> > needs UNMAP events of course). So that's why I called it "a naming
> > issue". :)
> >
> > >
> > > > So
> > > > IIUC this is a question about "naming" but not the implementations...
> > > > I suppose it is really a matter of taste, and both work for me (either
> > > > INVALIDATION/CHANGE or UNMAP/MAP).
> > >
> > > No.. it is a question of implementation. My point is that I don't
> > > think the new permission is sufficient information to let you know if
> > > a notification is necessary. You need to know if there was an
> > > existing mapping at that IOBA.
> >
> > My understanding is that we don't need to know that. Because IIUC
> > there are only map_page() and unmap_page() in guest IOMMU driver
> > (please check dma_map_ops in kernel). There is no chance for anyone to
> > "change" the content of the mapping, unless it calls unmap_page() then
> > with a map_page(). In that case, we'll have two IOTLB invalidation
> > requests.
>
> That's assuming a Linux guest using the current guest IOMMU model.
>
> I don't think we do so in practice, but the PAPR hypercall interface
> allows in-place changing of a mapping. The interface is just "set
> this IOPTE to this value".
I see. Even if so, QEMU IOMMU emulation codes can convert one CHANGE
request into UNMAP and a continuous MAP, right?
Thanks,
-- peterx
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, (continued)
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Paolo Bonzini, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Paolo Bonzini, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/07
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/07
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/07
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/08
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/11
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag,
Peter Xu <=
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/14
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/14
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/06
- Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, Peter Xu, 2016/09/06
Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag, David Gibson, 2016/09/06
[Qemu-devel] [PATCH 3/3] intel_iommu: allow IOMMU_NONE typed notifiers, Peter Xu, 2016/09/05
[Qemu-devel] [PATCH 1/3] memory: add one flag for IOMMU notifier, Peter Xu, 2016/09/05
Re: [Qemu-devel] [PATCH 0/3] memory: add IOMMU notifier type, David Gibson, 2016/09/06