[Qemu-devel] [PATCH 0/5] Remove cpu_register_io_memory

[Qemu-devel] [PATCH 0/5] Remove cpu_register_io_memory

[Avi Kivity]

The current code allows us direct lookup from physical address (or TLB entry)
to a MemoryRegion, yet we still dispatch accesses via the ram_addr returned
by cpu_register_io_memory().  This is clumsy, so remove this extra indirection.

Avi Kivity (5):
  memory: make phys_page_find() return an unadjusted section
  memory: store section indices in iotlb instead of io indices
  exec: fix code tlb entry misused as iotlb in get_page_addr_code()
  memory: dispatch directly via MemoryRegion
  memory: get rid of cpu_register_io_memory()

 cpu-all.h          |    8 -
 exec-all.h         |    9 +-
 exec-obsolete.h    |    3 -
 exec.c             |  362 ++++++++++++++++++++--------------------------------
 memory.c           |   13 +-
 softmmu_template.h |   40 +++---
 6 files changed, 168 insertions(+), 267 deletions(-)

-- 
1.7.9

[Qemu-devel] [PATCH 1/5] memory: make phys_page_find() return an unadjusted section

[Avi Kivity]

We'd like to store the section index in the iotlb, so we can't
adjust it before returning.  Return an unadjusted section and
instead introduce section_addr(), which does the adjustment later.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec.c |  218 ++++++++++++++++++++++++++++++----------------------------------
 1 files changed, 102 insertions(+), 116 deletions(-)

diff --git a/exec.c b/exec.c
index 1e5bbd6..1fb0158 100644
--- a/exec.c
+++ b/exec.c
@@ -480,13 +480,11 @@ static void phys_page_set(target_phys_addr_t index, target_phys_addr_t nb,
     phys_page_set_level(&phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
 }
 
-static MemoryRegionSection phys_page_find(target_phys_addr_t index)
+static MemoryRegionSection *phys_page_find(target_phys_addr_t index)
 {
     PhysPageEntry lp = phys_map;
     PhysPageEntry *p;
     int i;
-    MemoryRegionSection section;
-    target_phys_addr_t delta;
     uint16_t s_index = phys_section_unassigned;
 
     for (i = P_L2_LEVELS - 1; i >= 0 && !lp.is_leaf; i--) {
@@ -499,15 +497,15 @@ static MemoryRegionSection phys_page_find(target_phys_addr_t index)
 
     s_index = lp.ptr;
 not_found:
-    section = phys_sections[s_index];
-    index <<= TARGET_PAGE_BITS;
-    assert(section.offset_within_address_space <= index
-           && index <= section.offset_within_address_space + section.size-1);
-    delta = index - section.offset_within_address_space;
-    section.offset_within_address_space += delta;
-    section.offset_within_region += delta;
-    section.size -= delta;
-    return section;
+    return &phys_sections[s_index];
+}
+
+static target_phys_addr_t section_addr(MemoryRegionSection *section,
+                                       target_phys_addr_t addr)
+{
+    addr -= section->offset_within_address_space;
+    addr += section->offset_within_region;
+    return addr;
 }
 
 static void tlb_protect_code(ram_addr_t ram_addr);
@@ -1468,17 +1466,16 @@ static void breakpoint_invalidate(CPUState *env, target_ulong pc)
 {
     target_phys_addr_t addr;
     ram_addr_t ram_addr;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     addr = cpu_get_phys_page_debug(env, pc);
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
-    if (!(memory_region_is_ram(section.mr)
-          || (section.mr->rom_device && section.mr->readable))) {
+    if (!(memory_region_is_ram(section->mr)
+          || (section->mr->rom_device && section->mr->readable))) {
         return;
     }
-    ram_addr = (memory_region_get_ram_addr(section.mr)
-                + section.offset_within_region) & TARGET_PAGE_MASK;
-    ram_addr |= (pc & ~TARGET_PAGE_MASK);
+    ram_addr = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
+        + section_addr(section, addr);
     tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0);
 }
 #endif
@@ -2181,7 +2178,7 @@ void tlb_set_page(CPUState *env, target_ulong vaddr,
                   target_phys_addr_t paddr, int prot,
                   int mmu_idx, target_ulong size)
 {
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
     unsigned int index;
     target_ulong address;
     target_ulong code_address;
@@ -2202,21 +2199,21 @@ void tlb_set_page(CPUState *env, target_ulong vaddr,
 #endif
 
     address = vaddr;
-    if (!is_ram_rom_romd(&section)) {
+    if (!is_ram_rom_romd(section)) {
         /* IO memory case (romd handled later) */
         address |= TLB_MMIO;
     }
-    if (is_ram_rom_romd(&section)) {
-        addend = (unsigned long)(memory_region_get_ram_ptr(section.mr)
-                                 + section.offset_within_region);
+    if (is_ram_rom_romd(section)) {
+        addend = (unsigned long)memory_region_get_ram_ptr(section->mr)
+                                 + section_addr(section, paddr);
     } else {
         addend = 0;
     }
-    if (is_ram_rom(&section)) {
+    if (is_ram_rom(section)) {
         /* Normal RAM.  */
-        iotlb = (memory_region_get_ram_addr(section.mr)
-                 + section.offset_within_region) & TARGET_PAGE_MASK;
-        if (!section.readonly)
+        iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
+            + section_addr(section, paddr);
+        if (!section->readonly)
             iotlb |= io_mem_notdirty.ram_addr;
         else
             iotlb |= io_mem_rom.ram_addr;
@@ -2227,8 +2224,8 @@ void tlb_set_page(CPUState *env, target_ulong vaddr,
            and avoid full address decoding in every device.
            We can't use the high bits of pd for this because
            IO_MEM_ROMD uses these as a ram address.  */
-        iotlb = memory_region_get_ram_addr(section.mr) & ~TARGET_PAGE_MASK;
-        iotlb += section.offset_within_region;
+        iotlb = memory_region_get_ram_addr(section->mr) & ~TARGET_PAGE_MASK;
+        iotlb += section_addr(section, paddr);
     }
 
     code_address = address;
@@ -2261,14 +2258,14 @@ void tlb_set_page(CPUState *env, target_ulong vaddr,
         te->addr_code = -1;
     }
     if (prot & PAGE_WRITE) {
-        if ((memory_region_is_ram(section.mr) && section.readonly)
-            || is_romd(&section)) {
+        if ((memory_region_is_ram(section->mr) && section->readonly)
+            || is_romd(section)) {
             /* Write access calls the I/O callback.  */
             te->addr_write = address | TLB_MMIO;
-        } else if (memory_region_is_ram(section.mr)
+        } else if (memory_region_is_ram(section->mr)
                    && !cpu_physical_memory_is_dirty(
-                           section.mr->ram_addr
-                           + section.offset_within_region)) {
+                           section->mr->ram_addr
+                           + section_addr(section, paddr))) {
             te->addr_write = address | TLB_NOTDIRTY;
         } else {
             te->addr_write = address;
@@ -2631,22 +2628,22 @@ static void register_subpage(MemoryRegionSection *section)
     subpage_t *subpage;
     target_phys_addr_t base = section->offset_within_address_space
         & TARGET_PAGE_MASK;
-    MemoryRegionSection existing = phys_page_find(base >> TARGET_PAGE_BITS);
+    MemoryRegionSection *existing = phys_page_find(base >> TARGET_PAGE_BITS);
     MemoryRegionSection subsection = {
         .offset_within_address_space = base,
         .size = TARGET_PAGE_SIZE,
     };
     target_phys_addr_t start, end;
 
-    assert(existing.mr->subpage || existing.mr == &io_mem_unassigned);
+    assert(existing->mr->subpage || existing->mr == &io_mem_unassigned);
 
-    if (!(existing.mr->subpage)) {
+    if (!(existing->mr->subpage)) {
         subpage = subpage_init(base);
         subsection.mr = &subpage->iomem;
         phys_page_set(base >> TARGET_PAGE_BITS, 1,
                       phys_section_add(&subsection));
     } else {
-        subpage = container_of(existing.mr, subpage_t, iomem);
+        subpage = container_of(existing->mr, subpage_t, iomem);
     }
     start = section->offset_within_address_space & ~TARGET_PAGE_MASK;
     end = start + section->size;
@@ -3830,7 +3827,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
     uint8_t *ptr;
     uint32_t val;
     target_phys_addr_t page;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     while (len > 0) {
         page = addr & TARGET_PAGE_MASK;
@@ -3840,12 +3837,11 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
         section = phys_page_find(page >> TARGET_PAGE_BITS);
 
         if (is_write) {
-            if (!memory_region_is_ram(section.mr)) {
+            if (!memory_region_is_ram(section->mr)) {
                 target_phys_addr_t addr1;
-                io_index = memory_region_get_ram_addr(section.mr)
+                io_index = memory_region_get_ram_addr(section->mr)
                     & (IO_MEM_NB_ENTRIES - 1);
-                addr1 = (addr & ~TARGET_PAGE_MASK)
-                    + section.offset_within_region;
+                addr1 = section_addr(section, addr);
                 /* XXX: could force cpu_single_env to NULL to avoid
                    potential bugs */
                 if (l >= 4 && ((addr1 & 3) == 0)) {
@@ -3864,11 +3860,10 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                     io_mem_write(io_index, addr1, val, 1);
                     l = 1;
                 }
-            } else if (!section.readonly) {
+            } else if (!section->readonly) {
                 ram_addr_t addr1;
-                addr1 = (memory_region_get_ram_addr(section.mr)
-                         + section.offset_within_region)
-                    | (addr & ~TARGET_PAGE_MASK);
+                addr1 = memory_region_get_ram_addr(section->mr)
+                    + section_addr(section, addr);
                 /* RAM case */
                 ptr = qemu_get_ram_ptr(addr1);
                 memcpy(ptr, buf, l);
@@ -3882,13 +3877,12 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                 qemu_put_ram_ptr(ptr);
             }
         } else {
-            if (!is_ram_rom_romd(&section)) {
+            if (!is_ram_rom_romd(section)) {
                 target_phys_addr_t addr1;
                 /* I/O case */
-                io_index = memory_region_get_ram_addr(section.mr)
+                io_index = memory_region_get_ram_addr(section->mr)
                     & (IO_MEM_NB_ENTRIES - 1);
-                addr1 = (addr & ~TARGET_PAGE_MASK)
-                    + section.offset_within_region;
+                addr1 = section_addr(section, addr);
                 if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit read access */
                     val = io_mem_read(io_index, addr1, 4);
@@ -3907,9 +3901,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                 }
             } else {
                 /* RAM case */
-                ptr = qemu_get_ram_ptr(section.mr->ram_addr
-                                       + section.offset_within_region);
-                memcpy(buf, ptr + (addr & ~TARGET_PAGE_MASK), l);
+                ptr = qemu_get_ram_ptr(section->mr->ram_addr)
+                    + section_addr(section, addr);
+                memcpy(buf, ptr, l);
                 qemu_put_ram_ptr(ptr);
             }
         }
@@ -3926,7 +3920,7 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr,
     int l;
     uint8_t *ptr;
     target_phys_addr_t page;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     while (len > 0) {
         page = addr & TARGET_PAGE_MASK;
@@ -3935,13 +3929,12 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr,
             l = len;
         section = phys_page_find(page >> TARGET_PAGE_BITS);
 
-        if (!is_ram_rom_romd(&section)) {
+        if (!is_ram_rom_romd(section)) {
             /* do nothing */
         } else {
             unsigned long addr1;
-            addr1 = (memory_region_get_ram_addr(section.mr)
-                     + section.offset_within_region)
-                + (addr & ~TARGET_PAGE_MASK);
+            addr1 = memory_region_get_ram_addr(section->mr)
+                + section_addr(section, addr);
             /* ROM/RAM case */
             ptr = qemu_get_ram_ptr(addr1);
             memcpy(ptr, buf, l);
@@ -4014,7 +4007,7 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
     target_phys_addr_t todo = 0;
     int l;
     target_phys_addr_t page;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
     ram_addr_t raddr = RAM_ADDR_MAX;
     ram_addr_t rlen;
     void *ret;
@@ -4026,7 +4019,7 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
             l = len;
         section = phys_page_find(page >> TARGET_PAGE_BITS);
 
-        if (!(memory_region_is_ram(section.mr) && !section.readonly)) {
+        if (!(memory_region_is_ram(section->mr) && !section->readonly)) {
             if (todo || bounce.buffer) {
                 break;
             }
@@ -4041,9 +4034,8 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
             return bounce.buffer;
         }
         if (!todo) {
-            raddr = memory_region_get_ram_addr(section.mr)
-                + section.offset_within_region
-                + (addr & ~TARGET_PAGE_MASK);
+            raddr = memory_region_get_ram_addr(section->mr)
+                + section_addr(section, addr);
         }
 
         len -= l;
@@ -4102,15 +4094,15 @@ static inline uint32_t ldl_phys_internal(target_phys_addr_t addr,
     int io_index;
     uint8_t *ptr;
     uint32_t val;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
-    if (!is_ram_rom_romd(&section)) {
+    if (!is_ram_rom_romd(section)) {
         /* I/O case */
-        io_index = memory_region_get_ram_addr(section.mr)
+        io_index = memory_region_get_ram_addr(section->mr)
             & (IO_MEM_NB_ENTRIES - 1);
-        addr = (addr & ~TARGET_PAGE_MASK) + section.offset_within_region;
+        addr = section_addr(section, addr);
         val = io_mem_read(io_index, addr, 4);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
@@ -4123,10 +4115,9 @@ static inline uint32_t ldl_phys_internal(target_phys_addr_t addr,
 #endif
     } else {
         /* RAM case */
-        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section.mr)
+        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
                                 & TARGET_PAGE_MASK)
-                               + section.offset_within_region) +
-            (addr & ~TARGET_PAGE_MASK);
+                               + section_addr(section, addr));
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
             val = ldl_le_p(ptr);
@@ -4164,15 +4155,15 @@ static inline uint64_t ldq_phys_internal(target_phys_addr_t addr,
     int io_index;
     uint8_t *ptr;
     uint64_t val;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
-    if (!is_ram_rom_romd(&section)) {
+    if (!is_ram_rom_romd(section)) {
         /* I/O case */
-        io_index = memory_region_get_ram_addr(section.mr)
+        io_index = memory_region_get_ram_addr(section->mr)
             & (IO_MEM_NB_ENTRIES - 1);
-        addr = (addr & ~TARGET_PAGE_MASK) + section.offset_within_region;
+        addr = section_addr(section, addr);
 
         /* XXX This is broken when device endian != cpu endian.
                Fix and add "endian" variable check */
@@ -4185,10 +4176,9 @@ static inline uint64_t ldq_phys_internal(target_phys_addr_t addr,
 #endif
     } else {
         /* RAM case */
-        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section.mr)
+        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
                                 & TARGET_PAGE_MASK)
-                               + section.offset_within_region)
-            + (addr & ~TARGET_PAGE_MASK);
+                               + section_addr(section, addr));
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
             val = ldq_le_p(ptr);
@@ -4234,15 +4224,15 @@ static inline uint32_t lduw_phys_internal(target_phys_addr_t addr,
     int io_index;
     uint8_t *ptr;
     uint64_t val;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
-    if (!is_ram_rom_romd(&section)) {
+    if (!is_ram_rom_romd(section)) {
         /* I/O case */
-        io_index = memory_region_get_ram_addr(section.mr)
+        io_index = memory_region_get_ram_addr(section->mr)
             & (IO_MEM_NB_ENTRIES - 1);
-        addr = (addr & ~TARGET_PAGE_MASK) + section.offset_within_region;
+        addr = section_addr(section, addr);
         val = io_mem_read(io_index, addr, 2);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
@@ -4255,10 +4245,9 @@ static inline uint32_t lduw_phys_internal(target_phys_addr_t addr,
 #endif
     } else {
         /* RAM case */
-        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section.mr)
+        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
                                 & TARGET_PAGE_MASK)
-                               + section.offset_within_region)
-            + (addr & ~TARGET_PAGE_MASK);
+                               + section_addr(section, addr));
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
             val = lduw_le_p(ptr);
@@ -4296,23 +4285,22 @@ void stl_phys_notdirty(target_phys_addr_t addr, uint32_t val)
 {
     int io_index;
     uint8_t *ptr;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
-    if (!memory_region_is_ram(section.mr) || section.readonly) {
-        if (memory_region_is_ram(section.mr)) {
+    if (!memory_region_is_ram(section->mr) || section->readonly) {
+        if (memory_region_is_ram(section->mr)) {
             io_index = io_mem_rom.ram_addr;
         } else {
-            io_index = memory_region_get_ram_addr(section.mr);
+            io_index = memory_region_get_ram_addr(section->mr);
         }
-        addr = (addr & ~TARGET_PAGE_MASK) + section.offset_within_region;
+        addr = section_addr(section, addr);
         io_mem_write(io_index, addr, val, 4);
     } else {
-        unsigned long addr1 = (memory_region_get_ram_addr(section.mr)
+        unsigned long addr1 = (memory_region_get_ram_addr(section->mr)
                                & TARGET_PAGE_MASK)
-            + section.offset_within_region
-            + (addr & ~TARGET_PAGE_MASK);
+            + section_addr(section, addr);
         ptr = qemu_get_ram_ptr(addr1);
         stl_p(ptr, val);
 
@@ -4332,18 +4320,18 @@ void stq_phys_notdirty(target_phys_addr_t addr, uint64_t val)
 {
     int io_index;
     uint8_t *ptr;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
-    if (!memory_region_is_ram(section.mr) || section.readonly) {
-        if (memory_region_is_ram(section.mr)) {
+    if (!memory_region_is_ram(section->mr) || section->readonly) {
+        if (memory_region_is_ram(section->mr)) {
                 io_index = io_mem_rom.ram_addr;
         } else {
-            io_index = memory_region_get_ram_addr(section.mr)
+            io_index = memory_region_get_ram_addr(section->mr)
                 & (IO_MEM_NB_ENTRIES - 1);
         }
-        addr = (addr & ~TARGET_PAGE_MASK) + section.offset_within_region;
+        addr = section_addr(section, addr);
 #ifdef TARGET_WORDS_BIGENDIAN
         io_mem_write(io_index, addr, val >> 32, 4);
         io_mem_write(io_index, addr + 4, (uint32_t)val, 4);
@@ -4352,10 +4340,9 @@ void stq_phys_notdirty(target_phys_addr_t addr, uint64_t val)
         io_mem_write(io_index, addr + 4, val >> 32, 4);
 #endif
     } else {
-        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section.mr)
+        ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
                                 & TARGET_PAGE_MASK)
-                               + section.offset_within_region)
-            + (addr & ~TARGET_PAGE_MASK);
+                               + section_addr(section, addr));
         stq_p(ptr, val);
     }
 }
@@ -4366,18 +4353,18 @@ static inline void stl_phys_internal(target_phys_addr_t addr, uint32_t val,
 {
     int io_index;
     uint8_t *ptr;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
-    if (!memory_region_is_ram(section.mr) || section.readonly) {
-        if (memory_region_is_ram(section.mr)) {
+    if (!memory_region_is_ram(section->mr) || section->readonly) {
+        if (memory_region_is_ram(section->mr)) {
             io_index = io_mem_rom.ram_addr;
         } else {
-            io_index = memory_region_get_ram_addr(section.mr)
+            io_index = memory_region_get_ram_addr(section->mr)
                 & (IO_MEM_NB_ENTRIES - 1);
         }
-        addr = (addr & ~TARGET_PAGE_MASK) + section.offset_within_region;
+        addr = section_addr(section, addr);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap32(val);
@@ -4390,9 +4377,8 @@ static inline void stl_phys_internal(target_phys_addr_t addr, uint32_t val,
         io_mem_write(io_index, addr, val, 4);
     } else {
         unsigned long addr1;
-        addr1 = (memory_region_get_ram_addr(section.mr) & TARGET_PAGE_MASK)
-            + section.offset_within_region
-            + (addr & ~TARGET_PAGE_MASK);
+        addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
+            + section_addr(section, addr);
         /* RAM case */
         ptr = qemu_get_ram_ptr(addr1);
         switch (endian) {
@@ -4444,18 +4430,18 @@ static inline void stw_phys_internal(target_phys_addr_t addr, uint32_t val,
 {
     int io_index;
     uint8_t *ptr;
-    MemoryRegionSection section;
+    MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
-    if (!memory_region_is_ram(section.mr) || section.readonly) {
-        if (memory_region_is_ram(section.mr)) {
+    if (!memory_region_is_ram(section->mr) || section->readonly) {
+        if (memory_region_is_ram(section->mr)) {
             io_index = io_mem_rom.ram_addr;
         } else {
-            io_index = memory_region_get_ram_addr(section.mr)
+            io_index = memory_region_get_ram_addr(section->mr)
                 & (IO_MEM_NB_ENTRIES - 1);
         }
-        addr = (addr & ~TARGET_PAGE_MASK) + section.offset_within_region;
+        addr = section_addr(section, addr);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap16(val);
@@ -4468,8 +4454,8 @@ static inline void stw_phys_internal(target_phys_addr_t addr, uint32_t val,
         io_mem_write(io_index, addr, val, 2);
     } else {
         unsigned long addr1;
-        addr1 = (memory_region_get_ram_addr(section.mr) & TARGET_PAGE_MASK)
-            + section.offset_within_region + (addr & ~TARGET_PAGE_MASK);
+        addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
+            + section_addr(section, addr);
         /* RAM case */
         ptr = qemu_get_ram_ptr(addr1);
         switch (endian) {
-- 
1.7.9

[Qemu-devel] [PATCH 2/5] memory: store section indices in iotlb instead of io indices

[Avi Kivity]

A step towards eliminating io indices.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec-all.h         |    1 +
 exec.c             |   23 +++++++++++++++++++----
 softmmu_template.h |    8 ++++----
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 51d01f2..3ffe9dd 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -299,6 +299,7 @@ extern void *tci_tb_ptr;
 
 #if !defined(CONFIG_USER_ONLY)
 
+target_phys_addr_t section_to_ioaddr(target_phys_addr_t section_io_addr);
 uint64_t io_mem_read(int index, target_phys_addr_t addr, unsigned size);
 void io_mem_write(int index, target_phys_addr_t addr, uint64_t value,
                   unsigned size);
diff --git a/exec.c b/exec.c
index 1fb0158..a35eb4f 100644
--- a/exec.c
+++ b/exec.c
@@ -191,6 +191,9 @@
 static MemoryRegionSection *phys_sections;
 static unsigned phys_sections_nb, phys_sections_nb_alloc;
 static uint16_t phys_section_unassigned;
+static uint16_t phys_section_notdirty;
+static uint16_t phys_section_rom;
+static uint16_t phys_section_watch;
 
 struct PhysPageEntry {
     uint16_t is_leaf : 1;
@@ -2214,9 +2217,9 @@ void tlb_set_page(CPUState *env, target_ulong vaddr,
         iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
             + section_addr(section, paddr);
         if (!section->readonly)
-            iotlb |= io_mem_notdirty.ram_addr;
+            iotlb |= phys_section_notdirty;
         else
-            iotlb |= io_mem_rom.ram_addr;
+            iotlb |= phys_section_rom;
     } else {
         /* IO handlers are currently passed a physical address.
            It would be nice to pass an offset from the base address
@@ -2224,7 +2227,7 @@ void tlb_set_page(CPUState *env, target_ulong vaddr,
            and avoid full address decoding in every device.
            We can't use the high bits of pd for this because
            IO_MEM_ROMD uses these as a ram address.  */
-        iotlb = memory_region_get_ram_addr(section->mr) & ~TARGET_PAGE_MASK;
+        iotlb = section - phys_sections;
         iotlb += section_addr(section, paddr);
     }
 
@@ -2235,7 +2238,7 @@ void tlb_set_page(CPUState *env, target_ulong vaddr,
         if (vaddr == (wp->vaddr & TARGET_PAGE_MASK)) {
             /* Avoid trapping reads of pages with a write breakpoint. */
             if ((prot & PAGE_WRITE) || (wp->flags & BP_MEM_READ)) {
-                iotlb = io_mem_watch.ram_addr + paddr;
+                iotlb = phys_section_watch + paddr;
                 address |= TLB_MMIO;
                 break;
             }
@@ -3559,6 +3562,15 @@ static uint16_t dummy_section(MemoryRegion *mr)
     return phys_section_add(&section);
 }
 
+target_phys_addr_t section_to_ioaddr(target_phys_addr_t section_io_addr)
+{
+    MemoryRegionSection *section;
+
+    section = &phys_sections[section_io_addr & ~TARGET_PAGE_MASK];
+    return (section_io_addr & TARGET_PAGE_MASK)
+        | (section->mr->ram_addr & ~TARGET_PAGE_MASK);
+}
+
 static void io_mem_init(void)
 {
     int i;
@@ -3586,6 +3598,9 @@ static void core_begin(MemoryListener *listener)
     phys_sections_clear();
     phys_map.ptr = PHYS_MAP_NODE_NIL;
     phys_section_unassigned = dummy_section(&io_mem_unassigned);
+    phys_section_notdirty = dummy_section(&io_mem_notdirty);
+    phys_section_rom = dummy_section(&io_mem_rom);
+    phys_section_watch = dummy_section(&io_mem_watch);
 }
 
 static void core_commit(MemoryListener *listener)
diff --git a/softmmu_template.h b/softmmu_template.h
index 97020f8..7c7e15b 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -110,7 +110,7 @@ DATA_TYPE REGPARM glue(glue(__ld, SUFFIX), MMUSUFFIX)(target_ulong addr,
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
             retaddr = GETPC();
-            ioaddr = env->iotlb[mmu_idx][index];
+            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
             res = glue(io_read, SUFFIX)(ioaddr, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
             /* slow unaligned access (it spans two pages or IO) */
@@ -164,7 +164,7 @@ static DATA_TYPE glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(target_ulong addr,
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            ioaddr = env->iotlb[mmu_idx][index];
+            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
             res = glue(io_read, SUFFIX)(ioaddr, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
@@ -251,7 +251,7 @@ void REGPARM glue(glue(__st, SUFFIX), MMUSUFFIX)(target_ulong addr,
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
             retaddr = GETPC();
-            ioaddr = env->iotlb[mmu_idx][index];
+            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
             glue(io_write, SUFFIX)(ioaddr, val, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
@@ -303,7 +303,7 @@ static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(target_ulong addr,
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            ioaddr = env->iotlb[mmu_idx][index];
+            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
             glue(io_write, SUFFIX)(ioaddr, val, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
-- 
1.7.9

[Qemu-devel] [PATCH 3/5] exec: fix code tlb entry misused as iotlb in get_page_addr_code()

[Avi Kivity]

get_page_addr_code() reads a code tlb entry, but interprets it as an
iotlb entry.  This works by accident since the low bits of a RAM code
tlb entry are clear, and match a RAM iotlb entry.  This accident is
about to unhappen, so fix the code to use an iotlb entry (using the
code entry with TLB_MMIO may fail if the page is a watchpoint).

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/exec.c b/exec.c
index a35eb4f..f26d1b0 100644
--- a/exec.c
+++ b/exec.c
@@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState *env1, target_ulong addr)
                  (addr & TARGET_PAGE_MASK))) {
         ldub_code(addr);
     }
-    pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK;
+    pd = env1->iotlb[mmu_idx][page_index] & ~TARGET_PAGE_MASK;
     if (pd != io_mem_ram.ram_addr && pd != io_mem_rom.ram_addr
         && !io_mem_region[pd]->rom_device) {
 #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
-- 
1.7.9

[Qemu-devel] [PATCH 4/5] memory: dispatch directly via MemoryRegion

[Avi Kivity]

Instead of indirecting via io_mem_region, dispatch directly
through the MemoryRegion obtained from the iotlb or phys_page_find().

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec-all.h         |    9 +++--
 exec.c             |  102 ++++++++++++++++++---------------------------------
 memory.c           |    8 ++--
 softmmu_template.h |   48 ++++++++++++------------
 4 files changed, 69 insertions(+), 98 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 3ffe9dd..4e8c7f5 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -299,10 +299,11 @@ extern void *tci_tb_ptr;
 
 #if !defined(CONFIG_USER_ONLY)
 
-target_phys_addr_t section_to_ioaddr(target_phys_addr_t section_io_addr);
-uint64_t io_mem_read(int index, target_phys_addr_t addr, unsigned size);
-void io_mem_write(int index, target_phys_addr_t addr, uint64_t value,
-                  unsigned size);
+struct MemoryRegion *iotlb_to_region(target_phys_addr_t index);
+uint64_t io_mem_read(struct MemoryRegion *mr, target_phys_addr_t addr,
+                     unsigned size);
+void io_mem_write(struct MemoryRegion *mr, target_phys_addr_t addr,
+                  uint64_t value, unsigned size);
 extern struct MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
 
 void tlb_fill(CPUState *env1, target_ulong addr, int is_write, int mmu_idx,
diff --git a/exec.c b/exec.c
index f26d1b0..6e14048 100644
--- a/exec.c
+++ b/exec.c
@@ -3399,7 +3399,7 @@ static uint64_t subpage_read(void *opaque, target_phys_addr_t addr,
     addr += mmio->base;
     addr -= section->offset_within_address_space;
     addr += section->offset_within_region;
-    return io_mem_read(section->mr->ram_addr, addr, len);
+    return io_mem_read(section->mr, addr, len);
 }
 
 static void subpage_write(void *opaque, target_phys_addr_t addr,
@@ -3418,7 +3418,7 @@ static void subpage_write(void *opaque, target_phys_addr_t addr,
     addr += mmio->base;
     addr -= section->offset_within_address_space;
     addr += section->offset_within_region;
-    io_mem_write(section->mr->ram_addr, addr, value, len);
+    io_mem_write(section->mr, addr, value, len);
 }
 
 static const MemoryRegionOps subpage_ops = {
@@ -3562,13 +3562,9 @@ static uint16_t dummy_section(MemoryRegion *mr)
     return phys_section_add(&section);
 }
 
-target_phys_addr_t section_to_ioaddr(target_phys_addr_t section_io_addr)
+MemoryRegion *iotlb_to_region(target_phys_addr_t index)
 {
-    MemoryRegionSection *section;
-
-    section = &phys_sections[section_io_addr & ~TARGET_PAGE_MASK];
-    return (section_io_addr & TARGET_PAGE_MASK)
-        | (section->mr->ram_addr & ~TARGET_PAGE_MASK);
+    return phys_sections[index & ~TARGET_PAGE_MASK].mr;
 }
 
 static void io_mem_init(void)
@@ -3838,7 +3834,7 @@ int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write)
 {
-    int l, io_index;
+    int l;
     uint8_t *ptr;
     uint32_t val;
     target_phys_addr_t page;
@@ -3854,25 +3850,23 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
         if (is_write) {
             if (!memory_region_is_ram(section->mr)) {
                 target_phys_addr_t addr1;
-                io_index = memory_region_get_ram_addr(section->mr)
-                    & (IO_MEM_NB_ENTRIES - 1);
                 addr1 = section_addr(section, addr);
                 /* XXX: could force cpu_single_env to NULL to avoid
                    potential bugs */
                 if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit write access */
                     val = ldl_p(buf);
-                    io_mem_write(io_index, addr1, val, 4);
+                    io_mem_write(section->mr, addr1, val, 4);
                     l = 4;
                 } else if (l >= 2 && ((addr1 & 1) == 0)) {
                     /* 16 bit write access */
                     val = lduw_p(buf);
-                    io_mem_write(io_index, addr1, val, 2);
+                    io_mem_write(section->mr, addr1, val, 2);
                     l = 2;
                 } else {
                     /* 8 bit write access */
                     val = ldub_p(buf);
-                    io_mem_write(io_index, addr1, val, 1);
+                    io_mem_write(section->mr, addr1, val, 1);
                     l = 1;
                 }
             } else if (!section->readonly) {
@@ -3895,22 +3889,20 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
             if (!is_ram_rom_romd(section)) {
                 target_phys_addr_t addr1;
                 /* I/O case */
-                io_index = memory_region_get_ram_addr(section->mr)
-                    & (IO_MEM_NB_ENTRIES - 1);
                 addr1 = section_addr(section, addr);
                 if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit read access */
-                    val = io_mem_read(io_index, addr1, 4);
+                    val = io_mem_read(section->mr, addr1, 4);
                     stl_p(buf, val);
                     l = 4;
                 } else if (l >= 2 && ((addr1 & 1) == 0)) {
                     /* 16 bit read access */
-                    val = io_mem_read(io_index, addr1, 2);
+                    val = io_mem_read(section->mr, addr1, 2);
                     stw_p(buf, val);
                     l = 2;
                 } else {
                     /* 8 bit read access */
-                    val = io_mem_read(io_index, addr1, 1);
+                    val = io_mem_read(section->mr, addr1, 1);
                     stb_p(buf, val);
                     l = 1;
                 }
@@ -4106,7 +4098,6 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
 static inline uint32_t ldl_phys_internal(target_phys_addr_t addr,
                                          enum device_endian endian)
 {
-    int io_index;
     uint8_t *ptr;
     uint32_t val;
     MemoryRegionSection *section;
@@ -4115,10 +4106,8 @@ static inline uint32_t ldl_phys_internal(target_phys_addr_t addr,
 
     if (!is_ram_rom_romd(section)) {
         /* I/O case */
-        io_index = memory_region_get_ram_addr(section->mr)
-            & (IO_MEM_NB_ENTRIES - 1);
         addr = section_addr(section, addr);
-        val = io_mem_read(io_index, addr, 4);
+        val = io_mem_read(section->mr, addr, 4);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap32(val);
@@ -4167,7 +4156,6 @@ uint32_t ldl_be_phys(target_phys_addr_t addr)
 static inline uint64_t ldq_phys_internal(target_phys_addr_t addr,
                                          enum device_endian endian)
 {
-    int io_index;
     uint8_t *ptr;
     uint64_t val;
     MemoryRegionSection *section;
@@ -4176,18 +4164,16 @@ static inline uint64_t ldq_phys_internal(target_phys_addr_t addr,
 
     if (!is_ram_rom_romd(section)) {
         /* I/O case */
-        io_index = memory_region_get_ram_addr(section->mr)
-            & (IO_MEM_NB_ENTRIES - 1);
         addr = section_addr(section, addr);
 
         /* XXX This is broken when device endian != cpu endian.
                Fix and add "endian" variable check */
 #ifdef TARGET_WORDS_BIGENDIAN
-        val = io_mem_read(io_index, addr, 4) << 32;
-        val |= io_mem_read(io_index, addr + 4, 4);
+        val = io_mem_read(section->mr, addr, 4) << 32;
+        val |= io_mem_read(section->mr, addr + 4, 4);
 #else
-        val = io_mem_read(io_index, addr, 4);
-        val |= io_mem_read(io_index, addr + 4, 4) << 32;
+        val = io_mem_read(section->mr, addr, 4);
+        val |= io_mem_read(section->mr, addr + 4, 4) << 32;
 #endif
     } else {
         /* RAM case */
@@ -4236,7 +4222,6 @@ uint32_t ldub_phys(target_phys_addr_t addr)
 static inline uint32_t lduw_phys_internal(target_phys_addr_t addr,
                                           enum device_endian endian)
 {
-    int io_index;
     uint8_t *ptr;
     uint64_t val;
     MemoryRegionSection *section;
@@ -4245,10 +4230,8 @@ static inline uint32_t lduw_phys_internal(target_phys_addr_t addr,
 
     if (!is_ram_rom_romd(section)) {
         /* I/O case */
-        io_index = memory_region_get_ram_addr(section->mr)
-            & (IO_MEM_NB_ENTRIES - 1);
         addr = section_addr(section, addr);
-        val = io_mem_read(io_index, addr, 2);
+        val = io_mem_read(section->mr, addr, 2);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap16(val);
@@ -4298,20 +4281,17 @@ uint32_t lduw_be_phys(target_phys_addr_t addr)
    bits are used to track modified PTEs */
 void stl_phys_notdirty(target_phys_addr_t addr, uint32_t val)
 {
-    int io_index;
     uint8_t *ptr;
     MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
     if (!memory_region_is_ram(section->mr) || section->readonly) {
+        addr = section_addr(section, addr);
         if (memory_region_is_ram(section->mr)) {
-            io_index = io_mem_rom.ram_addr;
-        } else {
-            io_index = memory_region_get_ram_addr(section->mr);
+            section = &phys_sections[phys_section_rom];
         }
-        addr = section_addr(section, addr);
-        io_mem_write(io_index, addr, val, 4);
+        io_mem_write(section->mr, addr, val, 4);
     } else {
         unsigned long addr1 = (memory_region_get_ram_addr(section->mr)
                                & TARGET_PAGE_MASK)
@@ -4333,26 +4313,22 @@ void stl_phys_notdirty(target_phys_addr_t addr, uint32_t val)
 
 void stq_phys_notdirty(target_phys_addr_t addr, uint64_t val)
 {
-    int io_index;
     uint8_t *ptr;
     MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
     if (!memory_region_is_ram(section->mr) || section->readonly) {
+        addr = section_addr(section, addr);
         if (memory_region_is_ram(section->mr)) {
-                io_index = io_mem_rom.ram_addr;
-        } else {
-            io_index = memory_region_get_ram_addr(section->mr)
-                & (IO_MEM_NB_ENTRIES - 1);
+            section = &phys_sections[phys_section_rom];
         }
-        addr = section_addr(section, addr);
 #ifdef TARGET_WORDS_BIGENDIAN
-        io_mem_write(io_index, addr, val >> 32, 4);
-        io_mem_write(io_index, addr + 4, (uint32_t)val, 4);
+        io_mem_write(section->mr, addr, val >> 32, 4);
+        io_mem_write(section->mr, addr + 4, (uint32_t)val, 4);
 #else
-        io_mem_write(io_index, addr, (uint32_t)val, 4);
-        io_mem_write(io_index, addr + 4, val >> 32, 4);
+        io_mem_write(section->mr, addr, (uint32_t)val, 4);
+        io_mem_write(section->mr, addr + 4, val >> 32, 4);
 #endif
     } else {
         ptr = qemu_get_ram_ptr((memory_region_get_ram_addr(section->mr)
@@ -4366,20 +4342,16 @@ void stq_phys_notdirty(target_phys_addr_t addr, uint64_t val)
 static inline void stl_phys_internal(target_phys_addr_t addr, uint32_t val,
                                      enum device_endian endian)
 {
-    int io_index;
     uint8_t *ptr;
     MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
     if (!memory_region_is_ram(section->mr) || section->readonly) {
+        addr = section_addr(section, addr);
         if (memory_region_is_ram(section->mr)) {
-            io_index = io_mem_rom.ram_addr;
-        } else {
-            io_index = memory_region_get_ram_addr(section->mr)
-                & (IO_MEM_NB_ENTRIES - 1);
+            section = &phys_sections[phys_section_rom];
         }
-        addr = section_addr(section, addr);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap32(val);
@@ -4389,7 +4361,7 @@ static inline void stl_phys_internal(target_phys_addr_t addr, uint32_t val,
             val = bswap32(val);
         }
 #endif
-        io_mem_write(io_index, addr, val, 4);
+        io_mem_write(section->mr, addr, val, 4);
     } else {
         unsigned long addr1;
         addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
@@ -4443,20 +4415,16 @@ void stb_phys(target_phys_addr_t addr, uint32_t val)
 static inline void stw_phys_internal(target_phys_addr_t addr, uint32_t val,
                                      enum device_endian endian)
 {
-    int io_index;
     uint8_t *ptr;
     MemoryRegionSection *section;
 
     section = phys_page_find(addr >> TARGET_PAGE_BITS);
 
     if (!memory_region_is_ram(section->mr) || section->readonly) {
+        addr = section_addr(section, addr);
         if (memory_region_is_ram(section->mr)) {
-            io_index = io_mem_rom.ram_addr;
-        } else {
-            io_index = memory_region_get_ram_addr(section->mr)
-                & (IO_MEM_NB_ENTRIES - 1);
+            section = &phys_sections[phys_section_rom];
         }
-        addr = section_addr(section, addr);
 #if defined(TARGET_WORDS_BIGENDIAN)
         if (endian == DEVICE_LITTLE_ENDIAN) {
             val = bswap16(val);
@@ -4466,7 +4434,7 @@ static inline void stw_phys_internal(target_phys_addr_t addr, uint32_t val,
             val = bswap16(val);
         }
 #endif
-        io_mem_write(io_index, addr, val, 2);
+        io_mem_write(section->mr, addr, val, 2);
     } else {
         unsigned long addr1;
         addr1 = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
@@ -4678,6 +4646,7 @@ tb_page_addr_t get_page_addr_code(CPUState *env1, target_ulong addr)
 {
     int mmu_idx, page_index, pd;
     void *p;
+    MemoryRegion *mr;
 
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = cpu_mmu_index(env1);
@@ -4686,8 +4655,9 @@ tb_page_addr_t get_page_addr_code(CPUState *env1, target_ulong addr)
         ldub_code(addr);
     }
     pd = env1->iotlb[mmu_idx][page_index] & ~TARGET_PAGE_MASK;
-    if (pd != io_mem_ram.ram_addr && pd != io_mem_rom.ram_addr
-        && !io_mem_region[pd]->rom_device) {
+    mr = iotlb_to_region(pd);
+    if (mr != &io_mem_ram && mr != &io_mem_rom
+        && mr != &io_mem_notdirty && !mr->rom_device) {
 #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
         cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
 #else
diff --git a/memory.c b/memory.c
index 4c3dc49..bc76f55 100644
--- a/memory.c
+++ b/memory.c
@@ -1501,15 +1501,15 @@ void set_system_io_map(MemoryRegion *mr)
     memory_region_update_topology(NULL);
 }
 
-uint64_t io_mem_read(int io_index, target_phys_addr_t addr, unsigned size)
+uint64_t io_mem_read(MemoryRegion *mr, target_phys_addr_t addr, unsigned size)
 {
-    return memory_region_dispatch_read(io_mem_region[io_index], addr, size);
+    return memory_region_dispatch_read(mr, addr, size);
 }
 
-void io_mem_write(int io_index, target_phys_addr_t addr,
+void io_mem_write(MemoryRegion *mr, target_phys_addr_t addr,
                   uint64_t val, unsigned size)
 {
-    memory_region_dispatch_write(io_mem_region[io_index], addr, val, size);
+    memory_region_dispatch_write(mr, addr, val, size);
 }
 
 typedef struct MemoryRegionList MemoryRegionList;
diff --git a/softmmu_template.h b/softmmu_template.h
index 7c7e15b..e395020 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -62,27 +62,27 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(target_phys_addr_t physaddr,
                                               void *retaddr)
 {
     DATA_TYPE res;
-    int index;
-    index = physaddr & (IO_MEM_NB_ENTRIES - 1);
+    MemoryRegion *mr = iotlb_to_region(physaddr);
+
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
     env->mem_io_pc = (unsigned long)retaddr;
-    if (index != io_mem_ram.ram_addr && index != io_mem_rom.ram_addr
-        && index != io_mem_unassigned.ram_addr
-        && index != io_mem_notdirty.ram_addr
+    if (mr != &io_mem_ram && mr != &io_mem_rom
+        && mr != &io_mem_unassigned
+        && mr != &io_mem_notdirty
             && !can_do_io(env)) {
         cpu_io_recompile(env, retaddr);
     }
 
     env->mem_io_vaddr = addr;
 #if SHIFT <= 2
-    res = io_mem_read(index, physaddr, 1 << SHIFT);
+    res = io_mem_read(mr, physaddr, 1 << SHIFT);
 #else
 #ifdef TARGET_WORDS_BIGENDIAN
-    res = io_mem_read(index, physaddr, 4) << 32;
-    res |= io_mem_read(index, physaddr + 4, 4);
+    res = io_mem_read(mr, physaddr, 4) << 32;
+    res |= io_mem_read(mr, physaddr + 4, 4);
 #else
-    res = io_mem_read(index, physaddr, 4);
-    res |= io_mem_read(index, physaddr + 4, 4) << 32;
+    res = io_mem_read(mr, physaddr, 4);
+    res |= io_mem_read(mr, physaddr + 4, 4) << 32;
 #endif
 #endif /* SHIFT > 2 */
     return res;
@@ -110,7 +110,7 @@ DATA_TYPE REGPARM glue(glue(__ld, SUFFIX), MMUSUFFIX)(target_ulong addr,
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
             retaddr = GETPC();
-            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
+            ioaddr = env->iotlb[mmu_idx][index];
             res = glue(io_read, SUFFIX)(ioaddr, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
             /* slow unaligned access (it spans two pages or IO) */
@@ -164,7 +164,7 @@ static DATA_TYPE glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(target_ulong addr,
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
+            ioaddr = env->iotlb[mmu_idx][index];
             res = glue(io_read, SUFFIX)(ioaddr, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
@@ -207,12 +207,12 @@ static inline void glue(io_write, SUFFIX)(target_phys_addr_t physaddr,
                                           target_ulong addr,
                                           void *retaddr)
 {
-    int index;
-    index = physaddr & (IO_MEM_NB_ENTRIES - 1);
+    MemoryRegion *mr = iotlb_to_region(physaddr);
+
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
-    if (index != io_mem_ram.ram_addr && index != io_mem_rom.ram_addr
-        && index != io_mem_unassigned.ram_addr
-        && index != io_mem_notdirty.ram_addr
+    if (mr != &io_mem_ram && mr != &io_mem_rom
+        && mr != &io_mem_unassigned
+        && mr != &io_mem_notdirty
             && !can_do_io(env)) {
         cpu_io_recompile(env, retaddr);
     }
@@ -220,14 +220,14 @@ static inline void glue(io_write, SUFFIX)(target_phys_addr_t physaddr,
     env->mem_io_vaddr = addr;
     env->mem_io_pc = (unsigned long)retaddr;
 #if SHIFT <= 2
-    io_mem_write(index, physaddr, val, 1 << SHIFT);
+    io_mem_write(mr, physaddr, val, 1 << SHIFT);
 #else
 #ifdef TARGET_WORDS_BIGENDIAN
-    io_mem_write(index, physaddr, (val >> 32), 4);
-    io_mem_write(index, physaddr + 4, (uint32_t)val, 4);
+    io_mem_write(mr, physaddr, (val >> 32), 4);
+    io_mem_write(mr, physaddr + 4, (uint32_t)val, 4);
 #else
-    io_mem_write(index, physaddr, (uint32_t)val, 4);
-    io_mem_write(index, physaddr + 4, val >> 32, 4);
+    io_mem_write(mr, physaddr, (uint32_t)val, 4);
+    io_mem_write(mr, physaddr + 4, val >> 32, 4);
 #endif
 #endif /* SHIFT > 2 */
 }
@@ -251,7 +251,7 @@ void REGPARM glue(glue(__st, SUFFIX), MMUSUFFIX)(target_ulong addr,
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
             retaddr = GETPC();
-            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
+            ioaddr = env->iotlb[mmu_idx][index];
             glue(io_write, SUFFIX)(ioaddr, val, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
@@ -303,7 +303,7 @@ static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(target_ulong addr,
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            ioaddr = section_to_ioaddr(env->iotlb[mmu_idx][index]);
+            ioaddr = env->iotlb[mmu_idx][index];
             glue(io_write, SUFFIX)(ioaddr, val, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
-- 
1.7.9

[Qemu-devel] [PATCH 5/5] memory: get rid of cpu_register_io_memory()

[Avi Kivity]

The return value of cpu_register_io_memory() is no longer used anywhere, so
we can remove it and all associated data and code.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 cpu-all.h       |    8 -------
 exec-all.h      |    1 -
 exec-obsolete.h |    3 --
 exec.c          |   57 -------------------------------------------------------
 memory.c        |    5 +---
 5 files changed, 1 insertions(+), 73 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 80e6d42..b87f2ce 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -498,14 +498,6 @@ extern RAMList ram_list;
 extern const char *mem_path;
 extern int mem_prealloc;
 
-/* physical memory access */
-
-/* MMIO pages are identified by a combination of an IO device index and
-   3 flags.  The ROMD code stores the page ram offset in iotlb entry, 
-   so only a limited number of ids are avaiable.  */
-
-#define IO_MEM_NB_ENTRIES  (1 << TARGET_PAGE_BITS)
-
 /* Flags stored in the low bits of the TLB virtual address.  These are
    defined so that fast path ram access is all zeros.  */
 /* Zero if TLB entry is valid.  */
diff --git a/exec-all.h b/exec-all.h
index 4e8c7f5..3ec60a2 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -304,7 +304,6 @@ uint64_t io_mem_read(struct MemoryRegion *mr, target_phys_addr_t addr,
                      unsigned size);
 void io_mem_write(struct MemoryRegion *mr, target_phys_addr_t addr,
                   uint64_t value, unsigned size);
-extern struct MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
 
 void tlb_fill(CPUState *env1, target_ulong addr, int is_write, int mmu_idx,
               void *retaddr);
diff --git a/exec-obsolete.h b/exec-obsolete.h
index 4dbe476..792c831 100644
--- a/exec-obsolete.h
+++ b/exec-obsolete.h
@@ -32,9 +32,6 @@ void qemu_ram_free(ram_addr_t addr);
 void qemu_ram_free_from_ptr(ram_addr_t addr);
 
 struct MemoryRegion;
-int cpu_register_io_memory(MemoryRegion *mr);
-void cpu_unregister_io_memory(int table_address);
-
 struct MemoryRegionSection;
 void cpu_register_physical_memory_log(struct MemoryRegionSection *section,
                                       bool readonly);
diff --git a/exec.c b/exec.c
index 6e14048..0c86bce 100644
--- a/exec.c
+++ b/exec.c
@@ -214,9 +214,6 @@ struct PhysPageEntry {
 static void io_mem_init(void);
 static void memory_map_init(void);
 
-/* io memory support */
-MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
-static char io_mem_used[IO_MEM_NB_ENTRIES];
 static MemoryRegion io_mem_watch;
 #endif
 
@@ -3503,53 +3500,6 @@ static subpage_t *subpage_init(target_phys_addr_t base)
     return mmio;
 }
 
-static int get_free_io_mem_idx(void)
-{
-    int i;
-
-    for (i = 0; i<IO_MEM_NB_ENTRIES; i++)
-        if (!io_mem_used[i]) {
-            io_mem_used[i] = 1;
-            return i;
-        }
-    fprintf(stderr, "RAN out out io_mem_idx, max %d !\n", IO_MEM_NB_ENTRIES);
-    return -1;
-}
-
-/* mem_read and mem_write are arrays of functions containing the
-   function to access byte (index 0), word (index 1) and dword (index
-   2). Functions can be omitted with a NULL function pointer.
-   If io_index is non zero, the corresponding io zone is
-   modified. If it is zero, a new io zone is allocated. The return
-   value can be used with cpu_register_physical_memory(). (-1) is
-   returned if error. */
-static int cpu_register_io_memory_fixed(int io_index, MemoryRegion *mr)
-{
-    if (io_index <= 0) {
-        io_index = get_free_io_mem_idx();
-        if (io_index == -1)
-            return io_index;
-    } else {
-        if (io_index >= IO_MEM_NB_ENTRIES)
-            return -1;
-    }
-
-    io_mem_region[io_index] = mr;
-
-    return io_index;
-}
-
-int cpu_register_io_memory(MemoryRegion *mr)
-{
-    return cpu_register_io_memory_fixed(0, mr);
-}
-
-void cpu_unregister_io_memory(int io_index)
-{
-    io_mem_region[io_index] = NULL;
-    io_mem_used[io_index] = 0;
-}
-
 static uint16_t dummy_section(MemoryRegion *mr)
 {
     MemoryRegionSection section = {
@@ -3569,11 +3519,7 @@ MemoryRegion *iotlb_to_region(target_phys_addr_t index)
 
 static void io_mem_init(void)
 {
-    int i;
-
-    /* Must be first: */
     memory_region_init_io(&io_mem_ram, &error_mem_ops, NULL, "ram", UINT64_MAX);
-    assert(io_mem_ram.ram_addr == 0);
     memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
     memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
                           "unassigned", UINT64_MAX);
@@ -3581,9 +3527,6 @@ static void io_mem_init(void)
                           "notdirty", UINT64_MAX);
     memory_region_init_io(&io_mem_subpage_ram, &subpage_ram_ops, NULL,
                           "subpage-ram", UINT64_MAX);
-    for (i=0; i<5; i++)
-        io_mem_used[i] = 1;
-
     memory_region_init_io(&io_mem_watch, &watch_mem_ops, NULL,
                           "watch", UINT64_MAX);
 }
diff --git a/memory.c b/memory.c
index bc76f55..22b0352 100644
--- a/memory.c
+++ b/memory.c
@@ -781,13 +781,11 @@ static void memory_region_destructor_ram_from_ptr(MemoryRegion *mr)
 
 static void memory_region_destructor_iomem(MemoryRegion *mr)
 {
-    cpu_unregister_io_memory(mr->ram_addr);
 }
 
 static void memory_region_destructor_rom_device(MemoryRegion *mr)
 {
     qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
-    cpu_unregister_io_memory(mr->ram_addr & ~TARGET_PAGE_MASK);
 }
 
 static bool memory_region_wrong_endianness(MemoryRegion *mr)
@@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
     mr->opaque = opaque;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_iomem;
-    mr->ram_addr = cpu_register_io_memory(mr);
+    mr->ram_addr = ~(ram_addr_t)0;
 }
 
 void memory_region_init_ram(MemoryRegion *mr,
@@ -992,7 +990,6 @@ void memory_region_init_rom_device(MemoryRegion *mr,
     mr->rom_device = true;
     mr->destructor = memory_region_destructor_rom_device;
     mr->ram_addr = qemu_ram_alloc(size, mr);
-    mr->ram_addr |= cpu_register_io_memory(mr);
 }
 
 static uint64_t invalid_read(void *opaque, target_phys_addr_t addr,
-- 
1.7.9

Re: [Qemu-devel] [PATCH 3/5] exec: fix code tlb entry misused as iotlb in get_page_addr_code()

[jcmvbkbc]

Hi.

> get_page_addr_code() reads a code tlb entry, but interprets it as an
> iotlb entry.  This works by accident since the low bits of a RAM code
> tlb entry are clear, and match a RAM iotlb entry.  This accident is
> about to unhappen, so fix the code to use an iotlb entry (using the
> code entry with TLB_MMIO may fail if the page is a watchpoint).
>
> Signed-off-by: Avi Kivity<avi@redhat.com>
> ---
>   exec.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index a35eb4f..f26d1b0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState *env1, target_ulong addr)
>                    (addr&  TARGET_PAGE_MASK))) {
>           ldub_code(addr);
>       }
> -    pd = env1->tlb_table[mmu_idx][page_index].addr_code&  ~TARGET_PAGE_MASK;
> +    pd = env1->iotlb[mmu_idx][page_index]&  ~TARGET_PAGE_MASK;
>       if (pd != io_mem_ram.ram_addr&&  pd != io_mem_rom.ram_addr
>           &&  !io_mem_region[pd]->rom_device) {
>   #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)

With this patch xtensa debug option unit test causes qemu abort with the message

  qemu: fatal: Trying to execute code outside RAM or ROM at 0xd000088c

This happens immediately after a watchpoint setup for a data breakpoint at the same memory page where the currently
executed code is located. Any idea on how to fix it?

(gdb) bt
#0  cpu_abort (env=0x55555626a310, fmt=0x55555573f540 "Trying to execute code outside RAM or ROM at 0x%08x\n") at 
/home/dumb/ws/m/awt/emu/xtensa/qemu/exec.c:1848
#1  0x00005555556c1657 in get_page_addr_code (env1=0x55555626a310, addr=0xd000088c) at 
/home/dumb/ws/m/awt/emu/xtensa/qemu/exec.c:4694
#2  0x00005555556b3441 in tb_find_slow (env=0x55555626a310, pc=0xd000088c, cs_base=0x0, flags=0x0) at 
/home/dumb/ws/m/awt/emu/xtensa/qemu/cpu-exec.c:95
#3  0x00005555556b36ac in tb_find_fast (env=0x55555626a310) at /home/dumb/ws/m/awt/emu/xtensa/qemu/cpu-exec.c:151
#4  0x00005555556b3a2d in cpu_xtensa_exec (env=0x55555626a310) at /home/dumb/ws/m/awt/emu/xtensa/qemu/cpu-exec.c:534
#5  0x00005555556b6380 in tcg_cpu_exec (env=0x55555626a310) at /home/dumb/ws/m/awt/emu/xtensa/qemu/cpus.c:1022
#6  0x00005555556b6498 in tcg_exec_all () at /home/dumb/ws/m/awt/emu/xtensa/qemu/cpus.c:1054
#7  0x00005555556b5a30 in qemu_tcg_cpu_thread_fn (arg=0x55555626a310) at /home/dumb/ws/m/awt/emu/xtensa/qemu/cpus.c:772
#8  0x00007ffff62e0d90 in start_thread () from /lib64/libpthread.so.0
#9  0x00007ffff6011f5d in clone () from /lib64/libc.so.6

Memory maps are the following:

(qemu) info mtree
memory
00000000-fffffffe (prio 0, RW): system
   00000000-07ffffff (prio 0, RW): xtensa.sram
   fe000000-fe000fff (prio 0, RW): xtensa.rom
(qemu) info tlb
ITLB:
Way 5 (128 MB)
         Vaddr       Paddr       ASID  Attr RWX
         ----------  ----------  ----  ---- ---
         0xd0000000  0x00000000  0x01  0x07 RWX
         0xd8000000  0x00000000  0x01  0x03 RWX
Way 6 (256 MB)
         Vaddr       Paddr       ASID  Attr RWX
         ----------  ----------  ----  ---- ---
         0xe0000000  0xf0000000  0x01  0x07 RWX
         0xf0000000  0xf0000000  0x01  0x03 RWX

DTLB:
Way 5 (128 MB)
         Vaddr       Paddr       ASID  Attr RWX
         ----------  ----------  ----  ---- ---
         0xd0000000  0x00000000  0x01  0x07 RWX
         0xd8000000  0x00000000  0x01  0x03 RWX
Way 6 (256 MB)
         Vaddr       Paddr       ASID  Attr RWX
         ----------  ----------  ----  ---- ---
         0xe0000000  0xf0000000  0x01  0x07 RWX
         0xf0000000  0xf0000000  0x01  0x03 RWX

--
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/5] exec: fix code tlb entry misused as iotlb in get_page_addr_code()

[Avi Kivity]

On 03/15/2012 09:23 PM, jcmvbkbc wrote:
> Hi.
>
>> get_page_addr_code() reads a code tlb entry, but interprets it as an
>> iotlb entry.  This works by accident since the low bits of a RAM code
>> tlb entry are clear, and match a RAM iotlb entry.  This accident is
>> about to unhappen, so fix the code to use an iotlb entry (using the
>> code entry with TLB_MMIO may fail if the page is a watchpoint).
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> ---
>>   exec.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index a35eb4f..f26d1b0 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState
>> *env1, target_ulong addr)
>>                    (addr&  TARGET_PAGE_MASK))) {
>>           ldub_code(addr);
>>       }
>> -    pd = env1->tlb_table[mmu_idx][page_index].addr_code& 
>> ~TARGET_PAGE_MASK;
>> +    pd = env1->iotlb[mmu_idx][page_index]&  ~TARGET_PAGE_MASK;
>>       if (pd != io_mem_ram.ram_addr&&  pd != io_mem_rom.ram_addr
>>           &&  !io_mem_region[pd]->rom_device) {
>>   #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) ||
>> defined(TARGET_SPARC)
>
> With this patch xtensa debug option unit test causes qemu abort with
> the message
>
>  qemu: fatal: Trying to execute code outside RAM or ROM at 0xd000088c
>
> This happens immediately after a watchpoint setup for a data
> breakpoint at the same memory page where the currently
> executed code is located. Any idea on how to fix it?

Can you provide details on how to reproduce this?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/5] exec: fix code tlb entry misused as iotlb in get_page_addr_code()

[Max Filippov]

>>> get_page_addr_code() reads a code tlb entry, but interprets it as an
>>> iotlb entry.  This works by accident since the low bits of a RAM code
>>> tlb entry are clear, and match a RAM iotlb entry.  This accident is
>>> about to unhappen, so fix the code to use an iotlb entry (using the
>>> code entry with TLB_MMIO may fail if the page is a watchpoint).
>>>
>>> Signed-off-by: Avi Kivity<avi@redhat.com>
>>> ---
>>>   exec.c |    2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/exec.c b/exec.c
>>> index a35eb4f..f26d1b0 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState
>>> *env1, target_ulong addr)
>>>                    (addr&  TARGET_PAGE_MASK))) {
>>>           ldub_code(addr);
>>>       }
>>> -    pd = env1->tlb_table[mmu_idx][page_index].addr_code&
>>> ~TARGET_PAGE_MASK;
>>> +    pd = env1->iotlb[mmu_idx][page_index]&  ~TARGET_PAGE_MASK;
>>>       if (pd != io_mem_ram.ram_addr&&  pd != io_mem_rom.ram_addr
>>>           &&  !io_mem_region[pd]->rom_device) {
>>>   #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) ||
>>> defined(TARGET_SPARC)
>>
>> With this patch xtensa debug option unit test causes qemu abort with
>> the message
>>
>>  qemu: fatal: Trying to execute code outside RAM or ROM at 0xd000088c
>>
>> This happens immediately after a watchpoint setup for a data
>> breakpoint at the same memory page where the currently
>> executed code is located. Any idea on how to fix it?
>
> Can you provide details on how to reproduce this?

It may be reproduced by running test_break.tst unit test:

qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting  -kernel
./test_break.tst

Compiled test binary is here:
http://jcmvbkbc.spb.ru/~dumb/ws/osll/qemu-xtensa/20120318/test_break.tst

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/5] exec: fix code tlb entry misused as iotlb in get_page_addr_code()

[Avi Kivity]

[-- Attachment #1: Type: text/plain, Size: 2054 bytes --]

On 03/18/2012 01:07 PM, Max Filippov wrote:
> >>> get_page_addr_code() reads a code tlb entry, but interprets it as an
> >>> iotlb entry.  This works by accident since the low bits of a RAM code
> >>> tlb entry are clear, and match a RAM iotlb entry.  This accident is
> >>> about to unhappen, so fix the code to use an iotlb entry (using the
> >>> code entry with TLB_MMIO may fail if the page is a watchpoint).
> >>>
> >>> Signed-off-by: Avi Kivity<avi@redhat.com>
> >>> ---
> >>>   exec.c |    2 +-
> >>>   1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/exec.c b/exec.c
> >>> index a35eb4f..f26d1b0 100644
> >>> --- a/exec.c
> >>> +++ b/exec.c
> >>> @@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState
> >>> *env1, target_ulong addr)
> >>>                    (addr&  TARGET_PAGE_MASK))) {
> >>>           ldub_code(addr);
> >>>       }
> >>> -    pd = env1->tlb_table[mmu_idx][page_index].addr_code&
> >>> ~TARGET_PAGE_MASK;
> >>> +    pd = env1->iotlb[mmu_idx][page_index]&  ~TARGET_PAGE_MASK;
> >>>       if (pd != io_mem_ram.ram_addr&&  pd != io_mem_rom.ram_addr
> >>>           &&  !io_mem_region[pd]->rom_device) {
> >>>   #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) ||
> >>> defined(TARGET_SPARC)
> >>
> >> With this patch xtensa debug option unit test causes qemu abort with
> >> the message
> >>
> >>  qemu: fatal: Trying to execute code outside RAM or ROM at 0xd000088c
> >>
> >> This happens immediately after a watchpoint setup for a data
> >> breakpoint at the same memory page where the currently
> >> executed code is located. Any idea on how to fix it?
> >
> > Can you provide details on how to reproduce this?
>
> It may be reproduced by running test_break.tst unit test:
>
> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting  -kernel
> ./test_break.tst
>
> Compiled test binary is here:
> http://jcmvbkbc.spb.ru/~dumb/ws/osll/qemu-xtensa/20120318/test_break.tst
>

Thanks.  Please try out the attached patch.


-- 
error compiling committee.c: too many arguments to function


[-- Attachment #2: 0001-memory-check-for-watchpoints-when-getting-code-ram_a.patch --]
[-- Type: text/x-patch, Size: 1159 bytes --]

>From a7b99c89dfe9784a94af86df344d4ef893c15ee4 Mon Sep 17 00:00:00 2001
From: Avi Kivity <avi@redhat.com>
Date: Sun, 18 Mar 2012 18:31:13 +0200
Subject: [PATCH] memory: check for watchpoints when getting code ram_addr

The code to get the ram_addr from a (tlb entry, vaddr) pair
checks that the resulting memory is not MMIO, but neglects to
check whether the region is hidden by a watchpoint page.

Add the missing check.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/exec.c b/exec.c
index d8b089e..405e277 100644
--- a/exec.c
+++ b/exec.c
@@ -4605,7 +4605,8 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
     pd = env1->iotlb[mmu_idx][page_index] & ~TARGET_PAGE_MASK;
     mr = iotlb_to_region(pd);
     if (mr != &io_mem_ram && mr != &io_mem_rom
-        && mr != &io_mem_notdirty && !mr->rom_device) {
+        && mr != &io_mem_notdirty && !mr->rom_device
+        && mr != &io_mem_watch) {
 #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
         cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
 #else
-- 
1.7.9

Re: [Qemu-devel] [PATCH 3/5] exec: fix code tlb entry misused as iotlb in get_page_addr_code()

[Max Filippov]

>> >>> get_page_addr_code() reads a code tlb entry, but interprets it as an
>> >>> iotlb entry.  This works by accident since the low bits of a RAM code
>> >>> tlb entry are clear, and match a RAM iotlb entry.  This accident is
>> >>> about to unhappen, so fix the code to use an iotlb entry (using the
>> >>> code entry with TLB_MMIO may fail if the page is a watchpoint).
>> >>>
>> >>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> >>> ---
>> >>>   exec.c |    2 +-
>> >>>   1 files changed, 1 insertions(+), 1 deletions(-)
>> >>>
>> >>> diff --git a/exec.c b/exec.c
>> >>> index a35eb4f..f26d1b0 100644
>> >>> --- a/exec.c
>> >>> +++ b/exec.c
>> >>> @@ -4685,7 +4685,7 @@ tb_page_addr_t get_page_addr_code(CPUState
>> >>> *env1, target_ulong addr)
>> >>>                    (addr&  TARGET_PAGE_MASK))) {
>> >>>           ldub_code(addr);
>> >>>       }
>> >>> -    pd = env1->tlb_table[mmu_idx][page_index].addr_code&
>> >>> ~TARGET_PAGE_MASK;
>> >>> +    pd = env1->iotlb[mmu_idx][page_index]&  ~TARGET_PAGE_MASK;
>> >>>       if (pd != io_mem_ram.ram_addr&&  pd != io_mem_rom.ram_addr
>> >>>           &&  !io_mem_region[pd]->rom_device) {
>> >>>   #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) ||
>> >>> defined(TARGET_SPARC)
>> >>
>> >> With this patch xtensa debug option unit test causes qemu abort with
>> >> the message
>> >>
>> >>  qemu: fatal: Trying to execute code outside RAM or ROM at 0xd000088c
>> >>
>> >> This happens immediately after a watchpoint setup for a data
>> >> breakpoint at the same memory page where the currently
>> >> executed code is located. Any idea on how to fix it?
>> >
>> > Can you provide details on how to reproduce this?
>>
>> It may be reproduced by running test_break.tst unit test:
>>
>> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting  -kernel
>> ./test_break.tst
>>
>> Compiled test binary is here:
>> http://jcmvbkbc.spb.ru/~dumb/ws/osll/qemu-xtensa/20120318/test_break.tst
>>
>
> Thanks.  Please try out the attached patch.

Thanks, with this patch it works.

-- Max

Re: [Qemu-devel] [PATCH 5/5] memory: get rid of cpu_register_io_memory()

[TeLeMan]

On Fri, Mar 9, 2012 at 01:20, Avi Kivity <avi@redhat.com> wrote:
> The return value of cpu_register_io_memory() is no longer used anywhere, so
> we can remove it and all associated data and code.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  cpu-all.h       |    8 -------
>  exec-all.h      |    1 -
>  exec-obsolete.h |    3 --
>  exec.c          |   57 -------------------------------------------------------
>  memory.c        |    5 +---
>  5 files changed, 1 insertions(+), 73 deletions(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 80e6d42..b87f2ce 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -498,14 +498,6 @@ extern RAMList ram_list;
>  extern const char *mem_path;
>  extern int mem_prealloc;
>
> -/* physical memory access */
> -
> -/* MMIO pages are identified by a combination of an IO device index and
> -   3 flags.  The ROMD code stores the page ram offset in iotlb entry,
> -   so only a limited number of ids are avaiable.  */
> -
> -#define IO_MEM_NB_ENTRIES  (1 << TARGET_PAGE_BITS)
> -
>  /* Flags stored in the low bits of the TLB virtual address.  These are
>    defined so that fast path ram access is all zeros.  */
>  /* Zero if TLB entry is valid.  */
> diff --git a/exec-all.h b/exec-all.h
> index 4e8c7f5..3ec60a2 100644
> --- a/exec-all.h
> +++ b/exec-all.h
> @@ -304,7 +304,6 @@ uint64_t io_mem_read(struct MemoryRegion *mr, target_phys_addr_t addr,
>                      unsigned size);
>  void io_mem_write(struct MemoryRegion *mr, target_phys_addr_t addr,
>                   uint64_t value, unsigned size);
> -extern struct MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
>
>  void tlb_fill(CPUState *env1, target_ulong addr, int is_write, int mmu_idx,
>               void *retaddr);
> diff --git a/exec-obsolete.h b/exec-obsolete.h
> index 4dbe476..792c831 100644
> --- a/exec-obsolete.h
> +++ b/exec-obsolete.h
> @@ -32,9 +32,6 @@ void qemu_ram_free(ram_addr_t addr);
>  void qemu_ram_free_from_ptr(ram_addr_t addr);
>
>  struct MemoryRegion;
> -int cpu_register_io_memory(MemoryRegion *mr);
> -void cpu_unregister_io_memory(int table_address);
> -
>  struct MemoryRegionSection;
>  void cpu_register_physical_memory_log(struct MemoryRegionSection *section,
>                                       bool readonly);
> diff --git a/exec.c b/exec.c
> index 6e14048..0c86bce 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -214,9 +214,6 @@ struct PhysPageEntry {
>  static void io_mem_init(void);
>  static void memory_map_init(void);
>
> -/* io memory support */
> -MemoryRegion *io_mem_region[IO_MEM_NB_ENTRIES];
> -static char io_mem_used[IO_MEM_NB_ENTRIES];
>  static MemoryRegion io_mem_watch;
>  #endif
>
> @@ -3503,53 +3500,6 @@ static subpage_t *subpage_init(target_phys_addr_t base)
>     return mmio;
>  }
>
> -static int get_free_io_mem_idx(void)
> -{
> -    int i;
> -
> -    for (i = 0; i<IO_MEM_NB_ENTRIES; i++)
> -        if (!io_mem_used[i]) {
> -            io_mem_used[i] = 1;
> -            return i;
> -        }
> -    fprintf(stderr, "RAN out out io_mem_idx, max %d !\n", IO_MEM_NB_ENTRIES);
> -    return -1;
> -}
> -
> -/* mem_read and mem_write are arrays of functions containing the
> -   function to access byte (index 0), word (index 1) and dword (index
> -   2). Functions can be omitted with a NULL function pointer.
> -   If io_index is non zero, the corresponding io zone is
> -   modified. If it is zero, a new io zone is allocated. The return
> -   value can be used with cpu_register_physical_memory(). (-1) is
> -   returned if error. */
> -static int cpu_register_io_memory_fixed(int io_index, MemoryRegion *mr)
> -{
> -    if (io_index <= 0) {
> -        io_index = get_free_io_mem_idx();
> -        if (io_index == -1)
> -            return io_index;
> -    } else {
> -        if (io_index >= IO_MEM_NB_ENTRIES)
> -            return -1;
> -    }
> -
> -    io_mem_region[io_index] = mr;
> -
> -    return io_index;
> -}
> -
> -int cpu_register_io_memory(MemoryRegion *mr)
> -{
> -    return cpu_register_io_memory_fixed(0, mr);
> -}
> -
> -void cpu_unregister_io_memory(int io_index)
> -{
> -    io_mem_region[io_index] = NULL;
> -    io_mem_used[io_index] = 0;
> -}
> -
>  static uint16_t dummy_section(MemoryRegion *mr)
>  {
>     MemoryRegionSection section = {
> @@ -3569,11 +3519,7 @@ MemoryRegion *iotlb_to_region(target_phys_addr_t index)
>
>  static void io_mem_init(void)
>  {
> -    int i;
> -
> -    /* Must be first: */
>     memory_region_init_io(&io_mem_ram, &error_mem_ops, NULL, "ram", UINT64_MAX);
> -    assert(io_mem_ram.ram_addr == 0);
>     memory_region_init_io(&io_mem_rom, &rom_mem_ops, NULL, "rom", UINT64_MAX);
>     memory_region_init_io(&io_mem_unassigned, &unassigned_mem_ops, NULL,
>                           "unassigned", UINT64_MAX);
> @@ -3581,9 +3527,6 @@ static void io_mem_init(void)
>                           "notdirty", UINT64_MAX);
>     memory_region_init_io(&io_mem_subpage_ram, &subpage_ram_ops, NULL,
>                           "subpage-ram", UINT64_MAX);
> -    for (i=0; i<5; i++)
> -        io_mem_used[i] = 1;
> -
>     memory_region_init_io(&io_mem_watch, &watch_mem_ops, NULL,
>                           "watch", UINT64_MAX);
>  }
> diff --git a/memory.c b/memory.c
> index bc76f55..22b0352 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -781,13 +781,11 @@ static void memory_region_destructor_ram_from_ptr(MemoryRegion *mr)
>
>  static void memory_region_destructor_iomem(MemoryRegion *mr)
>  {
> -    cpu_unregister_io_memory(mr->ram_addr);
>  }
>
>  static void memory_region_destructor_rom_device(MemoryRegion *mr)
>  {
>     qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
> -    cpu_unregister_io_memory(mr->ram_addr & ~TARGET_PAGE_MASK);
>  }
>
>  static bool memory_region_wrong_endianness(MemoryRegion *mr)
> @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
>     mr->opaque = opaque;
>     mr->terminates = true;
>     mr->destructor = memory_region_destructor_iomem;
> -    mr->ram_addr = cpu_register_io_memory(mr);
> +    mr->ram_addr = ~(ram_addr_t)0;
Why not 0 but -1?

>  }
>
>  void memory_region_init_ram(MemoryRegion *mr,
> @@ -992,7 +990,6 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>     mr->rom_device = true;
>     mr->destructor = memory_region_destructor_rom_device;
>     mr->ram_addr = qemu_ram_alloc(size, mr);
> -    mr->ram_addr |= cpu_register_io_memory(mr);
>  }
>
>  static uint64_t invalid_read(void *opaque, target_phys_addr_t addr,
> --
> 1.7.9
>
>

Re: [Qemu-devel] [PATCH 5/5] memory: get rid of cpu_register_io_memory()

[Avi Kivity]

On 03/19/2012 06:52 AM, TeLeMan wrote:
> >  static bool memory_region_wrong_endianness(MemoryRegion *mr)
> > @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
> >     mr->opaque = opaque;
> >     mr->terminates = true;
> >     mr->destructor = memory_region_destructor_iomem;
> > -    mr->ram_addr = cpu_register_io_memory(mr);
> > +    mr->ram_addr = ~(ram_addr_t)0;
> Why not 0 but -1?
>

To catch bugs.  In fact it triggered bugs (not the ones I wanted though).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 5/5] memory: get rid of cpu_register_io_memory()

[TeLeMan]

On Mon, Mar 19, 2012 at 17:16, Avi Kivity <avi@redhat.com> wrote:
> On 03/19/2012 06:52 AM, TeLeMan wrote:
>> >  static bool memory_region_wrong_endianness(MemoryRegion *mr)
>> > @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
>> >     mr->opaque = opaque;
>> >     mr->terminates = true;
>> >     mr->destructor = memory_region_destructor_iomem;
>> > -    mr->ram_addr = cpu_register_io_memory(mr);
>> > +    mr->ram_addr = ~(ram_addr_t)0;
>> Why not 0 but -1?
>>
>
> To catch bugs.  In fact it triggered bugs (not the ones I wanted though).
There may be BSOD on the guest windows xp after applying this patch.
>
> --
> error compiling committee.c: too many arguments to function
>

Re: [Qemu-devel] [PATCH 5/5] memory: get rid of cpu_register_io_memory()

[Avi Kivity]

On 03/19/2012 12:37 PM, TeLeMan wrote:
> On Mon, Mar 19, 2012 at 17:16, Avi Kivity <avi@redhat.com> wrote:
> > On 03/19/2012 06:52 AM, TeLeMan wrote:
> >> >  static bool memory_region_wrong_endianness(MemoryRegion *mr)
> >> > @@ -942,7 +940,7 @@ void memory_region_init_io(MemoryRegion *mr,
> >> >     mr->opaque = opaque;
> >> >     mr->terminates = true;
> >> >     mr->destructor = memory_region_destructor_iomem;
> >> > -    mr->ram_addr = cpu_register_io_memory(mr);
> >> > +    mr->ram_addr = ~(ram_addr_t)0;
> >> Why not 0 but -1?
> >>
> >
> > To catch bugs.  In fact it triggered bugs (not the ones I wanted though).
> There may be BSOD on the guest windows xp after applying this patch.
>

May? Where?

-- 
error compiling committee.c: too many arguments to function