Re: [Qemu-devel] [PATCH 28/30] exec: change iotlb APIs to take AddressSpaceDispatch

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2013-06-28 20:26, Paolo Bonzini wrote:
> This makes it possible to start following RCU rules, which require
> not dereferencing as->dispatch more than once.  It is not covering
> the whole of TCG, since the TLB data structures are not RCU-friendly,
> but it is enough for exec.c.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cputlb.c              | 7 ++++---
>  exec.c                | 9 +++++----
>  include/exec/cputlb.h | 9 ++++++---
>  3 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/cputlb.c b/cputlb.c
> index 51381ae..82875b1 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -253,6 +253,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
>                    hwaddr paddr, int prot,
>                    int mmu_idx, target_ulong size)
>  {
> +    AddressSpaceDispatch *d;
>      MemoryRegionSection *section;
>      unsigned int index;
>      target_ulong address;
> @@ -267,8 +268,8 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
>      }
>  
>      sz = size;
> -    section = address_space_translate_for_iotlb(&address_space_memory, paddr,
> -                                                &xlat, &sz);
> +    d = address_space_memory.dispatch;
> +    section = address_space_translate_for_iotlb(d, paddr, &xlat, &sz);
>      assert(sz >= TARGET_PAGE_SIZE);
>  
>  #if defined(DEBUG_TLB)
> @@ -288,7 +289,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
>      }
>  
>      code_address = address;
> -    iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat,
> +    iotlb = memory_region_section_get_iotlb(d, env, section, vaddr, paddr, xlat,
>                                              prot, &address);
>  
>      index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> diff --git a/exec.c b/exec.c
> index 528c4d7..3e1a576 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -306,11 +306,11 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
>  }
>  
>  MemoryRegionSection *
> -address_space_translate_for_iotlb(AddressSpace *as, hwaddr addr, hwaddr *xlat,
> +address_space_translate_for_iotlb(AddressSpaceDispatch *d, hwaddr addr, hwaddr *xlat,
>                                    hwaddr *plen)
>  {
>      MemoryRegionSection *section;
> -    section = address_space_translate_internal(as->dispatch, addr, xlat, plen, false);
> +    section = address_space_translate_internal(d, addr, xlat, plen, false);
>  
>      assert(!section->mr->iommu_ops);
>      return section;
> @@ -726,7 +726,8 @@ static int cpu_physical_memory_set_dirty_tracking(int enable)
>      return ret;
>  }
>  
> -hwaddr memory_region_section_get_iotlb(CPUArchState *env,
> +hwaddr memory_region_section_get_iotlb(AddressSpaceDispatch *d,
> +                                       CPUArchState *env,
>                                         MemoryRegionSection *section,
>                                         target_ulong vaddr,
>                                         hwaddr paddr, hwaddr xlat,
> @@ -746,7 +747,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
>              iotlb |= PHYS_SECTION_ROM;
>          }
>      } else {
> -        iotlb = section - address_space_memory.dispatch->sections;
> +        iotlb = section - d->sections;
>          iotlb += xlat;
>      }
>  
> diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
> index e21cb60..968b6a4 100644
> --- a/include/exec/cputlb.h
> +++ b/include/exec/cputlb.h
> @@ -31,12 +31,15 @@ void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
>  extern int tlb_flush_count;
>  
>  /* exec.c */
> +typedef struct AddressSpaceDispatch AddressSpaceDispatch;
> +
>  void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr);
>  
>  MemoryRegionSection *
> -address_space_translate_for_iotlb(AddressSpace *as, hwaddr addr, hwaddr *xlat,
> -                                  hwaddr *plen);
> -hwaddr memory_region_section_get_iotlb(CPUArchState *env,
> +address_space_translate_for_iotlb(AddressSpaceDispatch *d, hwaddr addr,
> +                                  hwaddr *xlat, hwaddr *plen);
> +hwaddr memory_region_section_get_iotlb(AddressSpaceDispatch *d,
> +                                       CPUArchState *env,
>                                         MemoryRegionSection *section,
>                                         target_ulong vaddr,
>                                         hwaddr paddr, hwaddr xlat,
> 

Not sure if this version was also affected, but the current one in your
git requires this to build:

diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 968b6a4..bd7ff2f 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -19,6 +19,8 @@
 #ifndef CPUTLB_H
 #define CPUTLB_H
 
+#include <exec/memory-internal.h>
+
 #if !defined(CONFIG_USER_ONLY)
 /* cputlb.c */
 void tlb_protect_code(ram_addr_t ram_addr);
@@ -31,8 +33,6 @@ void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
 extern int tlb_flush_count;
 
 /* exec.c */
-typedef struct AddressSpaceDispatch AddressSpaceDispatch;
-
 void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr);
 
 MemoryRegionSection *


Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux