Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Wed, Sep 14, 2016 at 02:00:29PM +1000, David Gibson wrote: [...] > > > > > > > > > > > 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 > > > > > >

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Mon, Sep 12, 2016 at 01:13:41PM +0800, Peter Xu wrote: > 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

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

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

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

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

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

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

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

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

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

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)

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Tue, Sep 06, 2016 at 06:31:42PM +0800, Peter Xu wrote: > On Tue, Sep 06, 2016 at 10:19:14AM +0200, Paolo Bonzini wrote: > > > > > > On 06/09/2016 10:17, Peter Xu wrote: > > > After knowing the possibility that the two consumers might be > > > mixturely used in the future (as David has

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Tue, Sep 06, 2016 at 10:19:14AM +0200, Paolo Bonzini wrote: > > > On 06/09/2016 10:17, Peter Xu wrote: > > After knowing the possibility that the two consumers might be > > mixturely used in the future (as David has mentioned), I'd vote for a > > bitmask for notification type: > > > >

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On 06/09/2016 10:17, Peter Xu wrote: > After knowing the possibility that the two consumers might be > mixturely used in the future (as David has mentioned), I'd vote for a > bitmask for notification type: > > IOMMU_NOTIFIER_NONE = 0, > IOMMU_NOTIFIER_INVALIDATION = 1, >

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Tue, Sep 06, 2016 at 09:51:28AM +0200, Paolo Bonzini wrote: > > > On 06/09/2016 07:27, Peter Xu wrote: > > Maybe I haven't explained the idea very clearly, but device-IOTLB is > > not a "flush" of whole device cache. It still needs a IOMMUTLBEntry, > > and works just like how general IOMMU

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On 06/09/2016 07:27, Peter Xu wrote: > Maybe I haven't explained the idea very clearly, but device-IOTLB is > not a "flush" of whole device cache. It still needs a IOMMUTLBEntry, > and works just like how general IOMMU invalidations. E.g., we can do > device-IOTLB invalidation for a single 4K

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Tue, Sep 06, 2016 at 03:12:26PM +1000, David Gibson wrote: > > /** > > @@ -611,9 +613,11 @@ uint64_t > > memory_region_iommu_get_min_page_size(MemoryRegion *mr); > > * @entry: the new entry in the IOMMU translation table. The entry > > * replaces all old entries for the same

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Mon, Sep 05, 2016 at 03:21:20PM +0800, Peter Xu wrote: > When there are active IOMMU notify registers, the iommu_notify_flag will > cache existing notify flag. When notifiers are triggered, flag will be > checked against the cached result. > > Signed-off-by: Peter Xu > ---

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Mon, Sep 05, 2016 at 04:38:04PM +0800, Peter Xu wrote: > On Mon, Sep 05, 2016 at 10:04:42AM +0200, Paolo Bonzini wrote: > > > > > > On 05/09/2016 09:21, Peter Xu wrote: > > > void memory_region_notify_iommu(MemoryRegion *mr, > > > -IOMMUTLBEntry entry) > > > +

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Tue, Sep 06, 2016 at 03:18:46PM +1000, David Gibson wrote: > > Maybe here naming the flags as IOMMU_{RW_NONE} is a little bit > > confusing (just to leverage existing access flags), but what I was > > trying to do is to make the two things not overlapped at all, since I > > didn't find a

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Mon, Sep 05, 2016 at 11:56:12AM +0200, Paolo Bonzini wrote: > Yeah, if you really want to have these semantics, you need to define an > enum like this: > > IOMMU_NOTIFIER_NONE = -1, > IOMMU_NOTIFIER_FLUSH = 0, > IOMMU_NOTIFIER_CHANGED_ENTRY = 1, > > But I'm still not

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On 05/09/2016 10:38, Peter Xu wrote: > However in this patch I was not meant to do that. I made it an > exclusive flag to identify two different use cases. I don't know > whether this is good, but at least for Intel IOMMU's current use case, > these two types should be totally isolated from each

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On Mon, Sep 05, 2016 at 10:04:42AM +0200, Paolo Bonzini wrote: > > > On 05/09/2016 09:21, Peter Xu wrote: > > void memory_region_notify_iommu(MemoryRegion *mr, > > -IOMMUTLBEntry entry) > > +IOMMUTLBEntry entry, IOMMUAccessFlags

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

On 05/09/2016 09:21, Peter Xu wrote: > void memory_region_notify_iommu(MemoryRegion *mr, > -IOMMUTLBEntry entry) > +IOMMUTLBEntry entry, IOMMUAccessFlags flag) > { > assert(memory_region_is_iommu(mr)); > +assert(flag ==

[Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

When there are active IOMMU notify registers, the iommu_notify_flag will cache existing notify flag. When notifiers are triggered, flag will be checked against the cached result. Signed-off-by: Peter Xu --- hw/ppc/spapr_iommu.c | 2 +- hw/s390x/s390-pci-inst.c | 2 +-