On 02/08/2016 08:37, Alex Bennée wrote:
>> - in notdirty_mem_write, care must be put in the ordering of
>> tb_invalidate_phys_page_fast (which itself calls tlb_unprotect_code and
>> takes the tb_lock in tb_invalidate_phys_page_range) and tlb_set_dirty.
>> At least it seems to me that the call to tb_invalidate_phys_page_fast
>> should be after the write, but that's not all. Perhaps merge this part
>> of notdirty_mem_write:
I looked at it again and you are already doing the right thing in patch 19.
It's possible to simplify it a bit though like this:
diff --git a/exec.c b/exec.c
index c8389f9..7850c39 100644
--- a/exec.c
+++ b/exec.c
@@ -1944,9 +1944,6 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr)
static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
uint64_t val, unsigned size)
{
- if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
- tb_invalidate_phys_page_fast(ram_addr, size);
- }
switch (size) {
case 1:
stb_p(qemu_map_ram_ptr(NULL, ram_addr), val);
@@ -1960,11 +1957,19 @@ static void notdirty_mem_write(void *opaque, hwaddr
ram_addr,
*/
cpu_physical_memory_set_dirty_range(ram_addr, size,
DIRTY_CLIENTS_NOCODE);
+ tb_lock();
+ if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
+ /* tb_invalidate_phys_page_range will call tlb_unprotect_code
+ * once the last TB in this page is gone.
+ */
+ tb_invalidate_phys_page_fast(ram_addr, size);
+ }
/* we remove the notdirty callback only if the code has been
flushed */
if (!cpu_physical_memory_is_clean(ram_addr)) {
tlb_set_dirty(current_cpu, current_cpu->mem_io_vaddr);
}
+ tb_unlock();
}
static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
Anyhow, the next step is to merge either cmpxchg-based atomics
or iothread-free single-threaded TCG. Either will do. :)
I think that even iothread-free single-threaded TCG requires this
TLB stuff, because the iothread's address_space_write (and hence
invalidate_and_set_dirty) can race against the TCG thread's
code generation.
Thanks,
Paolo
