qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 08/13] iommu: Introduce IOMMU emulation infrastr


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 08/13] iommu: Introduce IOMMU emulation infrastructure
Date: Mon, 14 May 2012 21:03:06 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1

On 05/14/2012 08:42 PM, David Gibson wrote:
On Mon, May 14, 2012 at 07:49:16PM -0500, Anthony Liguori wrote:
[snip]
+void iommu_wait_for_invalidated_maps(DMAContext *dma,
+                                     dma_addr_t addr, dma_addr_t len)
+{
+    DMAMemoryMap *map;
+    DMAInvalidationState is;
+
+    is.count = 0;
+    qemu_cond_init(&is.cond);
+
+    QLIST_FOREACH(map,&dma->memory_maps, list) {
+        if (ranges_overlap(addr, len, map->addr, map->len)) {
+            is.count++;
+            map->invalidate =&is;
+        }
+    }
+
+    if (is.count) {
+        qemu_cond_wait(&is.cond,&qemu_global_mutex);
+    }
+    assert(is.count == 0);
+}

I don't get what's going on here but I don't think it can possibly
be right. What is the purpose of this function?

So.  This is a function to be used by individual iommu
implementations.  When IOMMU mappings are updated on real hardware,
there may be some lag in th effect, particularly for in-flight DMAs,
due to shadow TLBs or other things.  But generally, there will be some
way to synchronize the IOMMU that once completed will ensure that no
further DMA access to the old translations may occur.  For the sPAPR
TCE MMU, this actually happens after every PUT_TCE hcall.

In our software implementation this is a problem if existing drivers
have done a dma_memory_map() and haven't yet done a
dma_memory_unmap(): they will have a real pointer to the translated
memory which can't be intercepted.  However, memory maps are supposed
to be transient, so this helper function invalidates memory maps based
on an IOVA address range, and blocks until they expire.  This function
would be called from CPU thread context, the dma_memory_unmap() would
come from the IO thread (in the only existing case from AIO completion
callbacks in the block IO code).

This gives the IOMMU implementation a way of blocking the CPU
initiating a sync operation until it really is safe to assume that no
further DMA operations may hit the invalidated mappings.  Note that if
we actually hit the blocking path here, that almost certainly
indicates the guest has done something wrong, or at least unusual -

So the CPU thread runs in lock-step with the I/O thread. Dropping the CPU thread lock to let the I/O thread run is a dangerous thing to do in a place like this.

Also, I think you'd effectively block the CPU until pending DMA operations complete? This could be many, many, milliseconds, no? That's going to make guests very upset.

Regards,

Anthony Liguori

DMA devices should be stopped before removing their IOMMU mappings
from under them.  However, one of the points of the IOMMU is the
ability to be able to forcibly stop DMAs, so we do need to implement
this behaviour for that case.

With difficulty, I've traced through qemu's difficult-to-follow thread
synchronization logic and I'm about 75% convinced this works correctly
with it.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]