Re: [Qemu-devel] [PATCH 2/9] exec: make iotlb RCU-friendly

[Fam Zheng]

On Tue, 02/03 13:52, Paolo Bonzini wrote:
> After the previous patch, TLBs will be flushed on every change to
> the memory mapping.  This patch augments that with synchronization
> of the MemoryRegionSections referred to in the iotlb array.
> 
> With this change, it is guaranteed that iotlb_to_region will access
> the correct memory map, even once the TLB will be accessed outside
> the BQL.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  cpu-exec.c              |  6 +++++-
>  cputlb.c                |  5 ++---
>  exec.c                  | 13 ++++++++-----
>  include/exec/cputlb.h   |  2 +-
>  include/exec/exec-all.h |  3 ++-
>  include/qom/cpu.h       |  1 +
>  softmmu_template.h      |  4 ++--
>  7 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 78fe382..98f968d 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -24,6 +24,8 @@
>  #include "qemu/atomic.h"
>  #include "sysemu/qtest.h"
>  #include "qemu/timer.h"
> +#include "exec/address-spaces.h"
> +#include "exec/memory-internal.h"
>  
>  /* -icount align implementation. */
>  
> @@ -144,7 +146,9 @@ void cpu_resume_from_signal(CPUState *cpu, void *puc)
>  
>  void cpu_reload_memory_map(CPUState *cpu)
>  {
> -    /* The TLB is protected by the iothread lock.  */
> +    /* The CPU and TLB are protected by the iothread lock.  */
> +    AddressSpaceDispatch *d = cpu->as->dispatch;
> +    cpu->memory_dispatch = d;
>      tlb_flush(cpu, 1);
>  }
>  #endif
> diff --git a/cputlb.c b/cputlb.c
> index 3b271d4..f92db5e 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -265,8 +265,7 @@ void tlb_set_page(CPUState *cpu, target_ulong vaddr,
>      }
>  
>      sz = size;
> -    section = address_space_translate_for_iotlb(cpu->as, paddr,
> -                                                &xlat, &sz);
> +    section = address_space_translate_for_iotlb(cpu, paddr, &xlat, &sz);
>      assert(sz >= TARGET_PAGE_SIZE);
>  
>  #if defined(DEBUG_TLB)
> @@ -347,7 +346,7 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, 
> target_ulong addr)
>          cpu_ldub_code(env1, addr);
>      }
>      pd = env1->iotlb[mmu_idx][page_index] & ~TARGET_PAGE_MASK;
> -    mr = iotlb_to_region(cpu->as, pd);
> +    mr = iotlb_to_region(cpu, pd);
>      if (memory_region_is_unassigned(mr)) {
>          CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> diff --git a/exec.c b/exec.c
> index 5a75909..1854c95 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -401,11 +401,12 @@ MemoryRegion *address_space_translate(AddressSpace *as, 
> hwaddr addr,
>  }
>  
>  MemoryRegionSection *
> -address_space_translate_for_iotlb(AddressSpace *as, hwaddr addr, hwaddr 
> *xlat,
> -                                  hwaddr *plen)
> +address_space_translate_for_iotlb(CPUState *cpu, hwaddr addr,
> +                                  hwaddr *xlat, hwaddr *plen)
>  {
>      MemoryRegionSection *section;
> -    section = address_space_translate_internal(as->dispatch, addr, xlat, 
> plen, false);
> +    section = address_space_translate_internal(cpu->memory_dispatch,
> +                                               addr, xlat, plen, false);
>  
>      assert(!section->mr->iommu_ops);
>      return section;
> @@ -1961,9 +1962,11 @@ static uint16_t dummy_section(PhysPageMap *map, 
> AddressSpace *as,
>      return phys_section_add(map, &section);
>  }
>  
> -MemoryRegion *iotlb_to_region(AddressSpace *as, hwaddr index)
>+MemoryRegion *iotlb_to_region(CPUState *cpu, hwaddr index)
>  {
> -    return as->dispatch->map.sections[index & ~TARGET_PAGE_MASK].mr;
> +    MemoryRegionSection *sections = cpu->memory_dispatch->map.sections;
> +
> +    return sections[index & ~TARGET_PAGE_MASK].mr;
>  }
>  
>  static void io_mem_init(void)
> diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
> index b8ecd6f..e0da9d7 100644
> --- a/include/exec/cputlb.h
> +++ b/include/exec/cputlb.h
> @@ -34,7 +34,7 @@ extern int tlb_flush_count;
>  void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
>  
>  MemoryRegionSection *
> -address_space_translate_for_iotlb(AddressSpace *as, hwaddr addr, hwaddr 
> *xlat,
> +address_space_translate_for_iotlb(CPUState *cpu, hwaddr addr, hwaddr *xlat,
>                                    hwaddr *plen);
>  hwaddr memory_region_section_get_iotlb(CPUState *cpu,
>                                         MemoryRegionSection *section,
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 1b30813..bb3fd37 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -338,7 +338,8 @@ extern uintptr_t tci_tb_ptr;
>  
>  void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align));
>  
> -struct MemoryRegion *iotlb_to_region(AddressSpace *as, hwaddr index);
> +struct MemoryRegion *iotlb_to_region(CPUState *cpu,
> +                                     hwaddr index);
>  bool io_mem_read(struct MemoryRegion *mr, hwaddr addr,
>                   uint64_t *pvalue, unsigned size);
>  bool io_mem_write(struct MemoryRegion *mr, hwaddr addr,
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 2098f1c..48fd6fb 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -256,6 +256,7 @@ struct CPUState {
>      sigjmp_buf jmp_env;
>  
>      AddressSpace *as;
> +    struct AddressSpaceDispatch *memory_dispatch;
>      MemoryListener *tcg_as_listener;
>  
>      void *env_ptr; /* CPUArchState */
> diff --git a/softmmu_template.h b/softmmu_template.h
> index 6b4e615..0e3dd35 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -149,7 +149,7 @@ static inline DATA_TYPE glue(io_read, 
> SUFFIX)(CPUArchState *env,
>  {
>      uint64_t val;
>      CPUState *cpu = ENV_GET_CPU(env);
> -    MemoryRegion *mr = iotlb_to_region(cpu->as, physaddr);
> +    MemoryRegion *mr = iotlb_to_region(cpu, physaddr);
>  
>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
>      cpu->mem_io_pc = retaddr;
> @@ -369,7 +369,7 @@ static inline void glue(io_write, SUFFIX)(CPUArchState 
> *env,
>                                            uintptr_t retaddr)
>  {
>      CPUState *cpu = ENV_GET_CPU(env);
> -    MemoryRegion *mr = iotlb_to_region(cpu->as, physaddr);
> +    MemoryRegion *mr = iotlb_to_region(cpu, physaddr);
>  
>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
>      if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu_can_do_io(cpu)) {
> -- 
> 1.8.3.1
> 
> 

Reviewed-by: Fam Zheng <address@hidden>