Re: [Qemu-devel] [PATCH 05/21] exec: Implement subpage_read/write via address_space_rw

2013-05-31 Thread Richard Henderson
On 05/30/2013 02:16 PM, Paolo Bonzini wrote:
 From: Jan Kiszka jan.kis...@siemens.com
 
 This will allow to add support for unaligned memory regions: the subpage
 container region can activate unaligned support unconditionally because
 the read/write handler will now ensure that accesses are split as
 required by calling address_space_rw. We can furthermore drop the
 special handling of RAM subpages, address_space_rw takes care of this
 already.
 
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  exec.c | 125 
 +
  1 file changed, 47 insertions(+), 78 deletions(-)

I take it this is the subsequent patch that I queried from 4/.
In which case both can have

Reviewed-by: Richard Henderson r...@twiddle.net


r~



[Qemu-devel] [PATCH 05/21] exec: Implement subpage_read/write via address_space_rw

2013-05-30 Thread Paolo Bonzini
From: Jan Kiszka jan.kis...@siemens.com

This will allow to add support for unaligned memory regions: the subpage
container region can activate unaligned support unconditionally because
the read/write handler will now ensure that accesses are split as
required by calling address_space_rw. We can furthermore drop the
special handling of RAM subpages, address_space_rw takes care of this
already.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 exec.c | 125 +
 1 file changed, 47 insertions(+), 78 deletions(-)

diff --git a/exec.c b/exec.c
index ddc51a6..bf374e4 100644
--- a/exec.c
+++ b/exec.c
@@ -66,7 +66,7 @@ AddressSpace address_space_memory;
 DMAContext dma_context_memory;
 
 MemoryRegion io_mem_rom, io_mem_notdirty;
-static MemoryRegion io_mem_unassigned, io_mem_subpage_ram;
+static MemoryRegion io_mem_unassigned;
 
 #endif
 
@@ -95,11 +95,13 @@ struct AddressSpaceDispatch {
  */
 PhysPageEntry phys_map;
 MemoryListener listener;
+AddressSpace *as;
 };
 
 #define SUBPAGE_IDX(addr) ((addr)  ~TARGET_PAGE_MASK)
 typedef struct subpage_t {
 MemoryRegion iomem;
+AddressSpace *as;
 hwaddr base;
 uint16_t sub_section[TARGET_PAGE_SIZE];
 } subpage_t;
@@ -729,7 +731,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
 
 static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
  uint16_t section);
-static subpage_t *subpage_init(hwaddr base);
+static subpage_t *subpage_init(AddressSpace *as, hwaddr base);
 static void destroy_page_desc(uint16_t section_index)
 {
 MemoryRegionSection *section = phys_sections[section_index];
@@ -806,7 +808,7 @@ static void register_subpage(AddressSpaceDispatch *d, 
MemoryRegionSection *secti
 assert(existing-mr-subpage || existing-mr == io_mem_unassigned);
 
 if (!(existing-mr-subpage)) {
-subpage = subpage_init(base);
+subpage = subpage_init(d-as, base);
 subsection.mr = subpage-iomem;
 phys_page_set(d, base  TARGET_PAGE_BITS, 1,
   phys_section_add(subsection));
@@ -1569,60 +1571,64 @@ static const MemoryRegionOps watch_mem_ops = {
 static uint64_t subpage_read(void *opaque, hwaddr addr,
  unsigned len)
 {
-subpage_t *mmio = opaque;
-unsigned int idx = SUBPAGE_IDX(addr);
-uint64_t val;
+subpage_t *subpage = opaque;
+uint8_t buf[4];
 
-MemoryRegionSection *section;
 #if defined(DEBUG_SUBPAGE)
-printf(%s: subpage %p len %d addr  TARGET_FMT_plx  idx %d\n, __func__,
-   mmio, len, addr, idx);
+printf(%s: subpage %p len %d addr  TARGET_FMT_plx \n, __func__,
+   subpage, len, addr);
 #endif
-
-section = phys_sections[mmio-sub_section[idx]];
-addr += mmio-base;
-addr -= section-offset_within_address_space;
-addr += section-offset_within_region;
-io_mem_read(section-mr, addr, val, len);
-return val;
+address_space_read(subpage-as, addr + subpage-base, buf, len);
+switch (len) {
+case 1:
+return ldub_p(buf);
+case 2:
+return lduw_p(buf);
+case 4:
+return ldl_p(buf);
+default:
+abort();
+}
 }
 
 static void subpage_write(void *opaque, hwaddr addr,
   uint64_t value, unsigned len)
 {
-subpage_t *mmio = opaque;
-unsigned int idx = SUBPAGE_IDX(addr);
-MemoryRegionSection *section;
+subpage_t *subpage = opaque;
+uint8_t buf[4];
+
 #if defined(DEBUG_SUBPAGE)
 printf(%s: subpage %p len %d addr  TARGET_FMT_plx
-idx %d value %PRIx64\n,
-   __func__, mmio, len, addr, idx, value);
+value %PRIx64\n,
+   __func__, subpage, len, addr, value);
 #endif
-
-section = phys_sections[mmio-sub_section[idx]];
-addr += mmio-base;
-addr -= section-offset_within_address_space;
-addr += section-offset_within_region;
-io_mem_write(section-mr, addr, value, len);
+switch (len) {
+case 1:
+stb_p(buf, value);
+break;
+case 2:
+stw_p(buf, value);
+break;
+case 4:
+stl_p(buf, value);
+break;
+default:
+abort();
+}
+address_space_write(subpage-as, addr + subpage-base, buf, len);
 }
 
 static bool subpage_accepts(void *opaque, hwaddr addr,
 unsigned size, bool is_write)
 {
-subpage_t *mmio = opaque;
-unsigned int idx = SUBPAGE_IDX(addr);
-MemoryRegionSection *section;
+subpage_t *subpage = opaque;
 #if defined(DEBUG_SUBPAGE)
-printf(%s: subpage %p %c len %d addr  TARGET_FMT_plx
-idx %d\n, __func__, mmio,
-   is_write ? 'w' : 'r', len, addr, idx);
+printf(%s: subpage %p %c len %d addr  TARGET_FMT_plx \n,
+   __func__, subpage, is_write ? 'w' : 'r', len, addr);
 #endif
 
-section = phys_sections[mmio-sub_section[idx]];
-addr