All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC] 64 bit device I/O
@ 2009-02-21 15:05 Robert Reif
  2009-02-24 12:50 ` Robert Reif
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Reif @ 2009-02-21 15:05 UTC (permalink / raw)
  To: qemu-devel

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

Here is my second proof of concept attempt at supporting
64 bit device I/O.

This attempt doesn't require changing all devices to add a forth
function for 64 bit I/O unless actually needed.  Unfortunately it
does require an ugly cast when 64 bit is required.

This has been tested only on sparc32 and only the slavio_timer
hw device has been converted to support 64 bit I/O.

All other architectures should work as before except when
64 bit I/O is performed.  They will now get an unassigned
access error because there is no 64 bit callback defined by
default.  Adding a 64 bit callback and using
cpu_register_io_memory64 to register the new callback
will fix it.  See slavio_timer.c to to see how the new callback
is registered.

Comments welcome.

[-- Attachment #2: io64.diff.txt --]
[-- Type: text/plain, Size: 20914 bytes --]

Index: softmmu_template.h
===================================================================
--- softmmu_template.h	(revision 6636)
+++ softmmu_template.h	(working copy)
@@ -68,13 +68,7 @@
 #if SHIFT <= 2
     res = io_mem_read[index][SHIFT](io_mem_opaque[index], physaddr);
 #else
-#ifdef TARGET_WORDS_BIGENDIAN
-    res = (uint64_t)io_mem_read[index][2](io_mem_opaque[index], physaddr) << 32;
-    res |= io_mem_read[index][2](io_mem_opaque[index], physaddr + 4);
-#else
-    res = io_mem_read[index][2](io_mem_opaque[index], physaddr);
-    res |= (uint64_t)io_mem_read[index][2](io_mem_opaque[index], physaddr + 4) << 32;
-#endif
+    res = ((CPUReadMemoryFunc64*)(io_mem_read[index][3]))(io_mem_opaque[index], physaddr);
 #endif /* SHIFT > 2 */
 #ifdef USE_KQEMU
     env->last_io_time = cpu_get_time_fast();
@@ -213,13 +207,7 @@
 #if SHIFT <= 2
     io_mem_write[index][SHIFT](io_mem_opaque[index], physaddr, val);
 #else
-#ifdef TARGET_WORDS_BIGENDIAN
-    io_mem_write[index][2](io_mem_opaque[index], physaddr, val >> 32);
-    io_mem_write[index][2](io_mem_opaque[index], physaddr + 4, val);
-#else
-    io_mem_write[index][2](io_mem_opaque[index], physaddr, val);
-    io_mem_write[index][2](io_mem_opaque[index], physaddr + 4, val >> 32);
-#endif
+    ((CPUWriteMemoryFunc64*)(io_mem_write[index][3]))(io_mem_opaque[index], physaddr, val);
 #endif /* SHIFT > 2 */
 #ifdef USE_KQEMU
     env->last_io_time = cpu_get_time_fast();
Index: exec.c
===================================================================
--- exec.c	(revision 6636)
+++ exec.c	(working copy)
@@ -47,7 +47,7 @@
 //#define DEBUG_TB_INVALIDATE
 //#define DEBUG_FLUSH
 //#define DEBUG_TLB
-//#define DEBUG_UNASSIGNED
+#define DEBUG_UNASSIGNED
 
 /* make various TB consistency checks */
 //#define DEBUG_TB_CHECK
@@ -197,8 +197,8 @@
 #define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
 typedef struct subpage_t {
     target_phys_addr_t base;
-    CPUReadMemoryFunc **mem_read[TARGET_PAGE_SIZE][4];
-    CPUWriteMemoryFunc **mem_write[TARGET_PAGE_SIZE][4];
+    CPUReadMemoryFunc *mem_read[TARGET_PAGE_SIZE][4];
+    CPUWriteMemoryFunc *mem_write[TARGET_PAGE_SIZE][4];
     void *opaque[TARGET_PAGE_SIZE][2][4];
     ram_addr_t region_offset[TARGET_PAGE_SIZE][2][4];
 } subpage_t;
@@ -2419,6 +2419,17 @@
     return 0;
 }
 
+static uint64_t unassigned_mem_readq(void *opaque, target_phys_addr_t addr)
+{
+#ifdef DEBUG_UNASSIGNED
+    printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
+#endif
+#if defined(TARGET_SPARC)
+    do_unassigned_access(addr, 0, 0, 0, 8);
+#endif
+    return 0;
+}
+
 static void unassigned_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
 #ifdef DEBUG_UNASSIGNED
@@ -2449,16 +2460,28 @@
 #endif
 }
 
-static CPUReadMemoryFunc *unassigned_mem_read[3] = {
+static void unassigned_mem_writeq(void *opaque, target_phys_addr_t addr, uint64_t val)
+{
+#ifdef DEBUG_UNASSIGNED
+    printf("Unassigned mem write " TARGET_FMT_plx " = 0x%llx\n", addr, val);
+#endif
+#if defined(TARGET_SPARC)
+    do_unassigned_access(addr, 1, 0, 0, 8);
+#endif
+}
+
+static CPUReadMemoryFunc *unassigned_mem_read[4] = {
     unassigned_mem_readb,
     unassigned_mem_readw,
     unassigned_mem_readl,
+    (CPUReadMemoryFunc*)unassigned_mem_readq,
 };
 
-static CPUWriteMemoryFunc *unassigned_mem_write[3] = {
+static CPUWriteMemoryFunc *unassigned_mem_write[4] = {
     unassigned_mem_writeb,
     unassigned_mem_writew,
     unassigned_mem_writel,
+    (CPUWriteMemoryFunc*)unassigned_mem_writeq,
 };
 
 static void notdirty_mem_writeb(void *opaque, target_phys_addr_t ram_addr,
@@ -2536,16 +2559,43 @@
         tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
 }
 
-static CPUReadMemoryFunc *error_mem_read[3] = {
+static void notdirty_mem_writeq(void *opaque, target_phys_addr_t ram_addr,
+                                uint64_t val)
+{
+    int dirty_flags;
+    dirty_flags = phys_ram_dirty[ram_addr >> TARGET_PAGE_BITS];
+    if (!(dirty_flags & CODE_DIRTY_FLAG)) {
+#if !defined(CONFIG_USER_ONLY)
+        tb_invalidate_phys_page_fast(ram_addr, 8);
+        dirty_flags = phys_ram_dirty[ram_addr >> TARGET_PAGE_BITS];
+#endif
+    }
+    stq_p(phys_ram_base + ram_addr, val);
+#ifdef USE_KQEMU
+    if (cpu_single_env->kqemu_enabled &&
+        (dirty_flags & KQEMU_MODIFY_PAGE_MASK) != KQEMU_MODIFY_PAGE_MASK)
+        kqemu_modify_page(cpu_single_env, ram_addr);
+#endif
+    dirty_flags |= (0xff & ~CODE_DIRTY_FLAG);
+    phys_ram_dirty[ram_addr >> TARGET_PAGE_BITS] = dirty_flags;
+    /* we remove the notdirty callback only if the code has been
+       flushed */
+    if (dirty_flags == 0xff)
+        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
+}
+
+static CPUReadMemoryFunc *error_mem_read[4] = {
     NULL, /* never used */
     NULL, /* never used */
     NULL, /* never used */
+    NULL, /* never used */
 };
 
-static CPUWriteMemoryFunc *notdirty_mem_write[3] = {
+static CPUWriteMemoryFunc *notdirty_mem_write[4] = {
     notdirty_mem_writeb,
     notdirty_mem_writew,
     notdirty_mem_writel,
+    (CPUWriteMemoryFunc*)notdirty_mem_writeq,
 };
 
 /* Generate a debug exception if a watchpoint has been hit.  */
@@ -2614,6 +2664,12 @@
     return ldl_phys(addr);
 }
 
+static uint64_t watch_mem_readq(void *opaque, target_phys_addr_t addr)
+{
+    check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x7, BP_MEM_READ);
+    return ldq_phys(addr);
+}
+
 static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
                              uint32_t val)
 {
@@ -2635,16 +2691,25 @@
     stl_phys(addr, val);
 }
 
-static CPUReadMemoryFunc *watch_mem_read[3] = {
+static void watch_mem_writeq(void *opaque, target_phys_addr_t addr,
+                             uint64_t val)
+{
+    check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x7, BP_MEM_WRITE);
+    stq_phys(addr, val);
+}
+
+static CPUReadMemoryFunc *watch_mem_read[4] = {
     watch_mem_readb,
     watch_mem_readw,
     watch_mem_readl,
+    (CPUReadMemoryFunc*)watch_mem_readq,
 };
 
-static CPUWriteMemoryFunc *watch_mem_write[3] = {
+static CPUWriteMemoryFunc *watch_mem_write[4] = {
     watch_mem_writeb,
     watch_mem_writew,
     watch_mem_writel,
+    (CPUWriteMemoryFunc*)watch_mem_writeq,
 };
 
 static inline uint32_t subpage_readlen (subpage_t *mmio, target_phys_addr_t addr,
@@ -2658,12 +2723,28 @@
     printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d\n", __func__,
            mmio, len, addr, idx);
 #endif
-    ret = (**mmio->mem_read[idx][len])(mmio->opaque[idx][0][len],
+    ret = mmio->mem_read[idx][len](mmio->opaque[idx][0][len],
                                        addr + mmio->region_offset[idx][0][len]);
 
     return ret;
 }
 
+static inline uint64_t subpage_readlen64 (subpage_t *mmio, target_phys_addr_t addr)
+{
+    uint32_t ret;
+    unsigned int idx;
+
+    idx = SUBPAGE_IDX(addr);
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n", __func__,
+           mmio, addr, idx);
+#endif
+    ret = ((CPUReadMemoryFunc64*)mmio->mem_read[idx][3])(mmio->opaque[idx][0][3],
+                                       addr + mmio->region_offset[idx][0][3]);
+
+    return ret;
+}
+
 static inline void subpage_writelen (subpage_t *mmio, target_phys_addr_t addr,
                               uint32_t value, unsigned int len)
 {
@@ -2674,11 +2755,26 @@
     printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d value %08x\n", __func__,
            mmio, len, addr, idx, value);
 #endif
-    (**mmio->mem_write[idx][len])(mmio->opaque[idx][1][len],
+    mmio->mem_write[idx][len](mmio->opaque[idx][1][len],
                                   addr + mmio->region_offset[idx][1][len],
                                   value);
 }
 
+static inline void subpage_writelen64 (subpage_t *mmio, target_phys_addr_t addr,
+                              uint64_t value)
+{
+    unsigned int idx;
+
+    idx = SUBPAGE_IDX(addr);
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d value %016llx\n", __func__,
+           mmio, addr, idx, value);
+#endif
+    ((CPUWriteMemoryFunc64*)mmio->mem_write[idx][3])(mmio->opaque[idx][1][3],
+                                  addr + mmio->region_offset[idx][1][3],
+                                  value);
+}
+
 static uint32_t subpage_readb (void *opaque, target_phys_addr_t addr)
 {
 #if defined(DEBUG_SUBPAGE)
@@ -2733,16 +2829,36 @@
     subpage_writelen(opaque, addr, value, 2);
 }
 
-static CPUReadMemoryFunc *subpage_read[] = {
-    &subpage_readb,
-    &subpage_readw,
-    &subpage_readl,
+static uint64_t subpage_readq (void *opaque, target_phys_addr_t addr)
+{
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+#endif
+
+    return subpage_readlen64(opaque, addr);
+}
+
+static void subpage_writeq (void *opaque,
+                         target_phys_addr_t addr, uint64_t value)
+{
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: addr " TARGET_FMT_plx " val %016llx\n", __func__, addr, value);
+#endif
+    subpage_writelen64(opaque, addr, value);
+}
+
+static CPUReadMemoryFunc *subpage_read[4] = {
+    subpage_readb,
+    subpage_readw,
+    subpage_readl,
+    (CPUReadMemoryFunc*)subpage_readq,
 };
 
-static CPUWriteMemoryFunc *subpage_write[] = {
-    &subpage_writeb,
-    &subpage_writew,
-    &subpage_writel,
+static CPUWriteMemoryFunc *subpage_write[4] = {
+    subpage_writeb,
+    subpage_writew,
+    subpage_writel,
+    (CPUWriteMemoryFunc*)subpage_writeq,
 };
 
 static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
@@ -2763,12 +2879,12 @@
     for (; idx <= eidx; idx++) {
         for (i = 0; i < 4; i++) {
             if (io_mem_read[memory][i]) {
-                mmio->mem_read[idx][i] = &io_mem_read[memory][i];
+                mmio->mem_read[idx][i] = io_mem_read[memory][i];
                 mmio->opaque[idx][0][i] = io_mem_opaque[memory];
                 mmio->region_offset[idx][0][i] = region_offset;
             }
             if (io_mem_write[memory][i]) {
-                mmio->mem_write[idx][i] = &io_mem_write[memory][i];
+                mmio->mem_write[idx][i] = io_mem_write[memory][i];
                 mmio->opaque[idx][1][i] = io_mem_opaque[memory];
                 mmio->region_offset[idx][1][i] = region_offset;
             }
@@ -2787,7 +2903,7 @@
     mmio = qemu_mallocz(sizeof(subpage_t));
 
     mmio->base = base;
-    subpage_memory = cpu_register_io_memory(0, subpage_read, subpage_write, mmio);
+    subpage_memory = cpu_register_io_memory64(0, subpage_read, subpage_write, mmio);
 #if defined(DEBUG_SUBPAGE)
     printf("%s: %p base " TARGET_FMT_plx " len %08x %d\n", __func__,
            mmio, base, TARGET_PAGE_SIZE, subpage_memory);
@@ -2816,14 +2932,14 @@
 {
     int i;
 
-    cpu_register_io_memory(IO_MEM_ROM >> IO_MEM_SHIFT, error_mem_read, unassigned_mem_write, NULL);
-    cpu_register_io_memory(IO_MEM_UNASSIGNED >> IO_MEM_SHIFT, unassigned_mem_read, unassigned_mem_write, NULL);
-    cpu_register_io_memory(IO_MEM_NOTDIRTY >> IO_MEM_SHIFT, error_mem_read, notdirty_mem_write, NULL);
+    cpu_register_io_memory64(IO_MEM_ROM >> IO_MEM_SHIFT, error_mem_read, unassigned_mem_write, NULL);
+    cpu_register_io_memory64(IO_MEM_UNASSIGNED >> IO_MEM_SHIFT, unassigned_mem_read, unassigned_mem_write, NULL);
+    cpu_register_io_memory64(IO_MEM_NOTDIRTY >> IO_MEM_SHIFT, error_mem_read, notdirty_mem_write, NULL);
     for (i=0; i<5; i++)
         io_mem_used[i] = 1;
 
-    io_mem_watch = cpu_register_io_memory(0, watch_mem_read,
-                                          watch_mem_write, NULL);
+    io_mem_watch = cpu_register_io_memory64(0, watch_mem_read,
+                                            watch_mem_write, NULL);
     /* alloc dirty bits array */
     phys_ram_dirty = qemu_vmalloc(phys_ram_size >> TARGET_PAGE_BITS);
     memset(phys_ram_dirty, 0xff, phys_ram_size >> TARGET_PAGE_BITS);
@@ -2859,10 +2975,38 @@
         io_mem_read[io_index][i] = mem_read[i];
         io_mem_write[io_index][i] = mem_write[i];
     }
+    io_mem_read[io_index][3] = 0;
+    io_mem_write[io_index][3] = 0;
     io_mem_opaque[io_index] = opaque;
     return (io_index << IO_MEM_SHIFT) | subwidth;
 }
 
+int cpu_register_io_memory64(int io_index,
+                             CPUReadMemoryFunc **mem_read,
+                             CPUWriteMemoryFunc **mem_write,
+                             void *opaque)
+{
+    int i, subwidth = 0;
+
+    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;
+    }
+
+    for(i = 0;i < 4; i++) {
+        if (!mem_read[i] || !mem_write[i])
+            subwidth = IO_MEM_SUBWIDTH;
+        io_mem_read[io_index][i] = mem_read[i];
+        io_mem_write[io_index][i] = mem_write[i];
+    }
+    io_mem_opaque[io_index] = opaque;
+    return (io_index << IO_MEM_SHIFT) | subwidth;
+}
+
 void cpu_unregister_io_memory(int io_table_address)
 {
     int i;
@@ -2876,6 +3020,19 @@
     io_mem_used[io_index] = 0;
 }
 
+void cpu_unregister_io_memory64(int io_table_address)
+{
+    int i;
+    int io_index = io_table_address >> IO_MEM_SHIFT;
+
+    for (i=0;i < 4; i++) {
+        io_mem_read[io_index][i] = unassigned_mem_read[i];
+        io_mem_write[io_index][i] = unassigned_mem_write[i];
+    }
+    io_mem_opaque[io_index] = NULL;
+    io_mem_used[io_index] = 0;
+}
+
 CPUWriteMemoryFunc **cpu_get_io_memory_write(int io_index)
 {
     return io_mem_write[io_index >> IO_MEM_SHIFT];
@@ -2937,6 +3094,7 @@
     int l, io_index;
     uint8_t *ptr;
     uint32_t val;
+    uint64_t val64;
     target_phys_addr_t page;
     unsigned long pd;
     PhysPageDesc *p;
@@ -2961,7 +3119,12 @@
                     addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
                 /* XXX: could force cpu_single_env to NULL to avoid
                    potential bugs */
-                if (l >= 4 && ((addr1 & 3) == 0)) {
+                if (l >= 8 && ((addr1 & 7) == 0)) {
+                    /* 64 bit write access */
+                    val64 = ldq_p(buf);
+                    ((CPUWriteMemoryFunc64*)(io_mem_write[io_index][3]))(io_mem_opaque[io_index], addr1, val64);
+                    l = 8;
+                } else if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit write access */
                     val = ldl_p(buf);
                     io_mem_write[io_index][2](io_mem_opaque[io_index], addr1, val);
@@ -2999,7 +3162,12 @@
                 io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
                 if (p)
                     addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-                if (l >= 4 && ((addr1 & 3) == 0)) {
+                if (l >= 8 && ((addr1 & 7) == 0)) {
+                    /* 64 bit read access */
+                    val64 = ((CPUReadMemoryFunc64*)(io_mem_read[io_index][3]))(io_mem_opaque[io_index], addr1);
+                    stq_p(buf, val64);
+                    l = 8;
+                } else if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit read access */
                     val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr1);
                     stl_p(buf, val);
@@ -3264,13 +3432,7 @@
         io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
         if (p)
             addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-#ifdef TARGET_WORDS_BIGENDIAN
-        val = (uint64_t)io_mem_read[io_index][2](io_mem_opaque[io_index], addr) << 32;
-        val |= io_mem_read[io_index][2](io_mem_opaque[io_index], addr + 4);
-#else
-        val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
-        val |= (uint64_t)io_mem_read[io_index][2](io_mem_opaque[io_index], addr + 4) << 32;
-#endif
+        val = ((CPUReadMemoryFunc64*)(io_mem_read[io_index][3]))(io_mem_opaque[io_index], addr);
     } else {
         /* RAM case */
         ptr = phys_ram_base + (pd & TARGET_PAGE_MASK) +
@@ -3353,13 +3515,7 @@
         io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
         if (p)
             addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-#ifdef TARGET_WORDS_BIGENDIAN
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val >> 32);
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr + 4, val);
-#else
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr + 4, val >> 32);
-#endif
+        ((CPUWriteMemoryFunc64*)(io_mem_write[io_index][3]))(io_mem_opaque[io_index], addr, val);
     } else {
         ptr = phys_ram_base + (pd & TARGET_PAGE_MASK) +
             (addr & ~TARGET_PAGE_MASK);
Index: hw/slavio_timer.c
===================================================================
--- hw/slavio_timer.c	(revision 6636)
+++ hw/slavio_timer.c	(working copy)
@@ -177,6 +177,26 @@
     return ret;
 }
 
+static uint64_t slavio_timer_mem_readq(void *opaque, target_phys_addr_t addr)
+{
+    SLAVIO_TIMERState *s = opaque;
+    uint32_t saddr;
+    uint64_t ret = 0;
+
+    saddr = addr >> 2;
+    switch (saddr) {
+    case TIMER_LIMIT:
+        if (slavio_timer_is_user(s)) {
+            slavio_timer_get_out(s);
+            ret = (uint64_t)(s->counthigh | s->reached) << 32 | s->count;
+        }
+        break;
+    }
+    DPRINTF("read " TARGET_FMT_plx " = %016llx\n", addr, ret);
+
+    return ret;
+}
+
 static void slavio_timer_mem_writel(void *opaque, target_phys_addr_t addr,
                                     uint32_t val)
 {
@@ -303,16 +323,45 @@
     }
 }
 
-static CPUReadMemoryFunc *slavio_timer_mem_read[3] = {
+static void slavio_timer_mem_writeq(void *opaque, target_phys_addr_t addr,
+                                    uint64_t val)
+{
+    SLAVIO_TIMERState *s = opaque;
+    uint32_t saddr;
+
+    DPRINTF("write " TARGET_FMT_plx " %016llx\n", addr, val);
+    saddr = addr >> 2;
+    switch (saddr) {
+    case TIMER_LIMIT:
+        if (slavio_timer_is_user(s)) {
+            uint64_t count;
+
+            s->limit = TIMER_MAX_COUNT64;
+            s->count = val;
+            s->counthigh = (val & TIMER_MAX_COUNT64) >> 32;
+            s->reached = 0;
+            count = ((uint64_t)s->counthigh << 32) | s->count;
+            DPRINTF("processor %d user timer set to %016llx\n", s->slave_index,
+                    count);
+            if (s->timer)
+                ptimer_set_count(s->timer, LIMIT_TO_PERIODS(s->limit - count));
+        }
+        break;
+    }
+}
+
+static CPUReadMemoryFunc *slavio_timer_mem_read[4] = {
     NULL,
     NULL,
     slavio_timer_mem_readl,
+    (CPUReadMemoryFunc*)slavio_timer_mem_readq,
 };
 
-static CPUWriteMemoryFunc *slavio_timer_mem_write[3] = {
+static CPUWriteMemoryFunc *slavio_timer_mem_write[4] = {
     NULL,
     NULL,
     slavio_timer_mem_writel,
+    (CPUWriteMemoryFunc*)slavio_timer_mem_writeq,
 };
 
 static void slavio_timer_save(QEMUFile *f, void *opaque)
@@ -381,8 +430,8 @@
         ptimer_set_period(s->timer, TIMER_PERIOD);
     }
 
-    slavio_timer_io_memory = cpu_register_io_memory(0, slavio_timer_mem_read,
-                                                    slavio_timer_mem_write, s);
+    slavio_timer_io_memory = cpu_register_io_memory64(0, slavio_timer_mem_read,
+                                                      slavio_timer_mem_write, s);
     if (master)
         cpu_register_physical_memory(addr, CPU_TIMER_SIZE,
                                      slavio_timer_io_memory);
Index: cpu-all.h
===================================================================
--- cpu-all.h	(revision 6636)
+++ cpu-all.h	(working copy)
@@ -891,6 +891,9 @@
 typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
 typedef uint32_t CPUReadMemoryFunc(void *opaque, target_phys_addr_t addr);
 
+typedef void CPUWriteMemoryFunc64(void *opaque, target_phys_addr_t addr, uint64_t value);
+typedef uint64_t CPUReadMemoryFunc64(void *opaque, target_phys_addr_t addr);
+
 void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
                                          ram_addr_t size,
                                          ram_addr_t phys_offset,
@@ -909,7 +912,12 @@
                            CPUReadMemoryFunc **mem_read,
                            CPUWriteMemoryFunc **mem_write,
                            void *opaque);
+int cpu_register_io_memory64(int io_index,
+                             CPUReadMemoryFunc **mem_read,
+                             CPUWriteMemoryFunc **mem_write,
+                             void *opaque);
 void cpu_unregister_io_memory(int table_address);
+void cpu_unregister_io_memory64(int table_address);
 CPUWriteMemoryFunc **cpu_get_io_memory_write(int io_index);
 CPUReadMemoryFunc **cpu_get_io_memory_read(int io_index);
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-21 15:05 [Qemu-devel] [RFC] 64 bit device I/O Robert Reif
@ 2009-02-24 12:50 ` Robert Reif
  2009-02-24 20:16   ` Blue Swirl
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Reif @ 2009-02-24 12:50 UTC (permalink / raw)
  To: qemu-devel

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

Robert Reif wrote:
> Here is my second proof of concept attempt at supporting
> 64 bit device I/O.
>
> This attempt doesn't require changing all devices to add a forth
> function for 64 bit I/O unless actually needed.  Unfortunately it
> does require an ugly cast when 64 bit is required.
>
> This has been tested only on sparc32 and only the slavio_timer
> hw device has been converted to support 64 bit I/O.
>
> All other architectures should work as before except when
> 64 bit I/O is performed.  They will now get an unassigned
> access error because there is no 64 bit callback defined by
> default.  Adding a 64 bit callback and using
> cpu_register_io_memory64 to register the new callback
> will fix it.  See slavio_timer.c to to see how the new callback
> is registered.
>
> Comments welcome.
This is a bug fix version of the previous patch.
  exec.c  subpage_readlen64 had a truncation bug
  slavio_timer.c had a timer limit bug

With this patch, a sparc32 SS5-170 openboot image
passes the 64bit user timer self test.  I have also tested it on
sparc32 with an unreleased hw device that does extensive
64 bit i/o.

Please let me know what other ports and hardware configurations
perform 64 bit i/o so I can make the necessary changes for
inclusion into QEMU.

[-- Attachment #2: io64.diff.txt --]
[-- Type: text/plain, Size: 23724 bytes --]

Index: softmmu_template.h
===================================================================
--- softmmu_template.h	(revision 6642)
+++ softmmu_template.h	(working copy)
@@ -68,13 +68,7 @@
 #if SHIFT <= 2
     res = io_mem_read[index][SHIFT](io_mem_opaque[index], physaddr);
 #else
-#ifdef TARGET_WORDS_BIGENDIAN
-    res = (uint64_t)io_mem_read[index][2](io_mem_opaque[index], physaddr) << 32;
-    res |= io_mem_read[index][2](io_mem_opaque[index], physaddr + 4);
-#else
-    res = io_mem_read[index][2](io_mem_opaque[index], physaddr);
-    res |= (uint64_t)io_mem_read[index][2](io_mem_opaque[index], physaddr + 4) << 32;
-#endif
+    res = ((CPUReadMemoryFunc64*)(io_mem_read[index][3]))(io_mem_opaque[index], physaddr);
 #endif /* SHIFT > 2 */
 #ifdef USE_KQEMU
     env->last_io_time = cpu_get_time_fast();
@@ -213,13 +207,7 @@
 #if SHIFT <= 2
     io_mem_write[index][SHIFT](io_mem_opaque[index], physaddr, val);
 #else
-#ifdef TARGET_WORDS_BIGENDIAN
-    io_mem_write[index][2](io_mem_opaque[index], physaddr, val >> 32);
-    io_mem_write[index][2](io_mem_opaque[index], physaddr + 4, val);
-#else
-    io_mem_write[index][2](io_mem_opaque[index], physaddr, val);
-    io_mem_write[index][2](io_mem_opaque[index], physaddr + 4, val >> 32);
-#endif
+    ((CPUWriteMemoryFunc64*)(io_mem_write[index][3]))(io_mem_opaque[index], physaddr, val);
 #endif /* SHIFT > 2 */
 #ifdef USE_KQEMU
     env->last_io_time = cpu_get_time_fast();
Index: exec.c
===================================================================
--- exec.c	(revision 6642)
+++ exec.c	(working copy)
@@ -197,8 +197,8 @@
 #define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
 typedef struct subpage_t {
     target_phys_addr_t base;
-    CPUReadMemoryFunc **mem_read[TARGET_PAGE_SIZE][4];
-    CPUWriteMemoryFunc **mem_write[TARGET_PAGE_SIZE][4];
+    CPUReadMemoryFunc *mem_read[TARGET_PAGE_SIZE][4];
+    CPUWriteMemoryFunc *mem_write[TARGET_PAGE_SIZE][4];
     void *opaque[TARGET_PAGE_SIZE][2][4];
     ram_addr_t region_offset[TARGET_PAGE_SIZE][2][4];
 } subpage_t;
@@ -2394,7 +2394,7 @@
 static uint32_t unassigned_mem_readb(void *opaque, target_phys_addr_t addr)
 {
 #ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
+    printf("Unassigned mem read byte " TARGET_FMT_plx "\n", addr);
 #endif
 #if defined(TARGET_SPARC)
     do_unassigned_access(addr, 0, 0, 0, 1);
@@ -2405,7 +2405,7 @@
 static uint32_t unassigned_mem_readw(void *opaque, target_phys_addr_t addr)
 {
 #ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
+    printf("Unassigned mem read word " TARGET_FMT_plx "\n", addr);
 #endif
 #if defined(TARGET_SPARC)
     do_unassigned_access(addr, 0, 0, 0, 2);
@@ -2416,7 +2416,7 @@
 static uint32_t unassigned_mem_readl(void *opaque, target_phys_addr_t addr)
 {
 #ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
+    printf("Unassigned mem read long " TARGET_FMT_plx "\n", addr);
 #endif
 #if defined(TARGET_SPARC)
     do_unassigned_access(addr, 0, 0, 0, 4);
@@ -2424,10 +2424,21 @@
     return 0;
 }
 
+static uint64_t unassigned_mem_readq(void *opaque, target_phys_addr_t addr)
+{
+#ifdef DEBUG_UNASSIGNED
+    printf("Unassigned mem read quad " TARGET_FMT_plx "\n", addr);
+#endif
+#if defined(TARGET_SPARC)
+    do_unassigned_access(addr, 0, 0, 0, 8);
+#endif
+    return 0;
+}
+
 static void unassigned_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
 #ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
+    printf("Unassigned mem write byte " TARGET_FMT_plx " = 0x%02x\n", addr, val);
 #endif
 #if defined(TARGET_SPARC)
     do_unassigned_access(addr, 1, 0, 0, 1);
@@ -2437,7 +2448,7 @@
 static void unassigned_mem_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
 #ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
+    printf("Unassigned mem write word " TARGET_FMT_plx " = 0x%04x\n", addr, val);
 #endif
 #if defined(TARGET_SPARC)
     do_unassigned_access(addr, 1, 0, 0, 2);
@@ -2447,23 +2458,35 @@
 static void unassigned_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
 {
 #ifdef DEBUG_UNASSIGNED
-    printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
+    printf("Unassigned mem write long " TARGET_FMT_plx " = 0x%08x\n", addr, val);
 #endif
 #if defined(TARGET_SPARC)
     do_unassigned_access(addr, 1, 0, 0, 4);
 #endif
 }
 
-static CPUReadMemoryFunc *unassigned_mem_read[3] = {
+static void unassigned_mem_writeq(void *opaque, target_phys_addr_t addr, uint64_t val)
+{
+#ifdef DEBUG_UNASSIGNED
+    printf("Unassigned mem write quad " TARGET_FMT_plx " = 0x%016llx\n", addr, val);
+#endif
+#if defined(TARGET_SPARC)
+    do_unassigned_access(addr, 1, 0, 0, 8);
+#endif
+}
+
+static CPUReadMemoryFunc *unassigned_mem_read[4] = {
     unassigned_mem_readb,
     unassigned_mem_readw,
     unassigned_mem_readl,
+    (CPUReadMemoryFunc*)unassigned_mem_readq,
 };
 
-static CPUWriteMemoryFunc *unassigned_mem_write[3] = {
+static CPUWriteMemoryFunc *unassigned_mem_write[4] = {
     unassigned_mem_writeb,
     unassigned_mem_writew,
     unassigned_mem_writel,
+    (CPUWriteMemoryFunc*)unassigned_mem_writeq,
 };
 
 static void notdirty_mem_writeb(void *opaque, target_phys_addr_t ram_addr,
@@ -2541,16 +2564,43 @@
         tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
 }
 
-static CPUReadMemoryFunc *error_mem_read[3] = {
+static void notdirty_mem_writeq(void *opaque, target_phys_addr_t ram_addr,
+                                uint64_t val)
+{
+    int dirty_flags;
+    dirty_flags = phys_ram_dirty[ram_addr >> TARGET_PAGE_BITS];
+    if (!(dirty_flags & CODE_DIRTY_FLAG)) {
+#if !defined(CONFIG_USER_ONLY)
+        tb_invalidate_phys_page_fast(ram_addr, 8);
+        dirty_flags = phys_ram_dirty[ram_addr >> TARGET_PAGE_BITS];
+#endif
+    }
+    stq_p(phys_ram_base + ram_addr, val);
+#ifdef USE_KQEMU
+    if (cpu_single_env->kqemu_enabled &&
+        (dirty_flags & KQEMU_MODIFY_PAGE_MASK) != KQEMU_MODIFY_PAGE_MASK)
+        kqemu_modify_page(cpu_single_env, ram_addr);
+#endif
+    dirty_flags |= (0xff & ~CODE_DIRTY_FLAG);
+    phys_ram_dirty[ram_addr >> TARGET_PAGE_BITS] = dirty_flags;
+    /* we remove the notdirty callback only if the code has been
+       flushed */
+    if (dirty_flags == 0xff)
+        tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
+}
+
+static CPUReadMemoryFunc *error_mem_read[4] = {
     NULL, /* never used */
     NULL, /* never used */
     NULL, /* never used */
+    NULL, /* never used */
 };
 
-static CPUWriteMemoryFunc *notdirty_mem_write[3] = {
+static CPUWriteMemoryFunc *notdirty_mem_write[4] = {
     notdirty_mem_writeb,
     notdirty_mem_writew,
     notdirty_mem_writel,
+    (CPUWriteMemoryFunc*)notdirty_mem_writeq,
 };
 
 /* Generate a debug exception if a watchpoint has been hit.  */
@@ -2619,6 +2669,12 @@
     return ldl_phys(addr);
 }
 
+static uint64_t watch_mem_readq(void *opaque, target_phys_addr_t addr)
+{
+    check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x7, BP_MEM_READ);
+    return ldq_phys(addr);
+}
+
 static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
                              uint32_t val)
 {
@@ -2640,16 +2696,25 @@
     stl_phys(addr, val);
 }
 
-static CPUReadMemoryFunc *watch_mem_read[3] = {
+static void watch_mem_writeq(void *opaque, target_phys_addr_t addr,
+                             uint64_t val)
+{
+    check_watchpoint(addr & ~TARGET_PAGE_MASK, ~0x7, BP_MEM_WRITE);
+    stq_phys(addr, val);
+}
+
+static CPUReadMemoryFunc *watch_mem_read[4] = {
     watch_mem_readb,
     watch_mem_readw,
     watch_mem_readl,
+    (CPUReadMemoryFunc*)watch_mem_readq,
 };
 
-static CPUWriteMemoryFunc *watch_mem_write[3] = {
+static CPUWriteMemoryFunc *watch_mem_write[4] = {
     watch_mem_writeb,
     watch_mem_writew,
     watch_mem_writel,
+    (CPUWriteMemoryFunc*)watch_mem_writeq,
 };
 
 static inline uint32_t subpage_readlen (subpage_t *mmio, target_phys_addr_t addr,
@@ -2663,12 +2728,28 @@
     printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d\n", __func__,
            mmio, len, addr, idx);
 #endif
-    ret = (**mmio->mem_read[idx][len])(mmio->opaque[idx][0][len],
+    ret = mmio->mem_read[idx][len](mmio->opaque[idx][0][len],
                                        addr + mmio->region_offset[idx][0][len]);
 
     return ret;
 }
 
+static inline uint64_t subpage_readlen64 (subpage_t *mmio, target_phys_addr_t addr)
+{
+    uint64_t ret;
+    unsigned int idx;
+
+    idx = SUBPAGE_IDX(addr);
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n", __func__,
+           mmio, addr, idx);
+#endif
+    ret = ((CPUReadMemoryFunc64*)mmio->mem_read[idx][3])(mmio->opaque[idx][0][3],
+                                       addr + mmio->region_offset[idx][0][3]);
+
+    return ret;
+}
+
 static inline void subpage_writelen (subpage_t *mmio, target_phys_addr_t addr,
                               uint32_t value, unsigned int len)
 {
@@ -2679,11 +2760,26 @@
     printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d value %08x\n", __func__,
            mmio, len, addr, idx, value);
 #endif
-    (**mmio->mem_write[idx][len])(mmio->opaque[idx][1][len],
+    mmio->mem_write[idx][len](mmio->opaque[idx][1][len],
                                   addr + mmio->region_offset[idx][1][len],
                                   value);
 }
 
+static inline void subpage_writelen64 (subpage_t *mmio, target_phys_addr_t addr,
+                              uint64_t value)
+{
+    unsigned int idx;
+
+    idx = SUBPAGE_IDX(addr);
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d value %016llx\n", __func__,
+           mmio, addr, idx, value);
+#endif
+    ((CPUWriteMemoryFunc64*)mmio->mem_write[idx][3])(mmio->opaque[idx][1][3],
+                                  addr + mmio->region_offset[idx][1][3],
+                                  value);
+}
+
 static uint32_t subpage_readb (void *opaque, target_phys_addr_t addr)
 {
 #if defined(DEBUG_SUBPAGE)
@@ -2738,16 +2834,36 @@
     subpage_writelen(opaque, addr, value, 2);
 }
 
-static CPUReadMemoryFunc *subpage_read[] = {
-    &subpage_readb,
-    &subpage_readw,
-    &subpage_readl,
+static uint64_t subpage_readq (void *opaque, target_phys_addr_t addr)
+{
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+#endif
+
+    return subpage_readlen64(opaque, addr);
+}
+
+static void subpage_writeq (void *opaque,
+                         target_phys_addr_t addr, uint64_t value)
+{
+#if defined(DEBUG_SUBPAGE)
+    printf("%s: addr " TARGET_FMT_plx " val %016llx\n", __func__, addr, value);
+#endif
+    subpage_writelen64(opaque, addr, value);
+}
+
+static CPUReadMemoryFunc *subpage_read[4] = {
+    subpage_readb,
+    subpage_readw,
+    subpage_readl,
+    (CPUReadMemoryFunc*)subpage_readq,
 };
 
-static CPUWriteMemoryFunc *subpage_write[] = {
-    &subpage_writeb,
-    &subpage_writew,
-    &subpage_writel,
+static CPUWriteMemoryFunc *subpage_write[4] = {
+    subpage_writeb,
+    subpage_writew,
+    subpage_writel,
+    (CPUWriteMemoryFunc*)subpage_writeq,
 };
 
 static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
@@ -2768,12 +2884,12 @@
     for (; idx <= eidx; idx++) {
         for (i = 0; i < 4; i++) {
             if (io_mem_read[memory][i]) {
-                mmio->mem_read[idx][i] = &io_mem_read[memory][i];
+                mmio->mem_read[idx][i] = io_mem_read[memory][i];
                 mmio->opaque[idx][0][i] = io_mem_opaque[memory];
                 mmio->region_offset[idx][0][i] = region_offset;
             }
             if (io_mem_write[memory][i]) {
-                mmio->mem_write[idx][i] = &io_mem_write[memory][i];
+                mmio->mem_write[idx][i] = io_mem_write[memory][i];
                 mmio->opaque[idx][1][i] = io_mem_opaque[memory];
                 mmio->region_offset[idx][1][i] = region_offset;
             }
@@ -2792,7 +2908,7 @@
     mmio = qemu_mallocz(sizeof(subpage_t));
 
     mmio->base = base;
-    subpage_memory = cpu_register_io_memory(0, subpage_read, subpage_write, mmio);
+    subpage_memory = cpu_register_io_memory64(0, subpage_read, subpage_write, mmio);
 #if defined(DEBUG_SUBPAGE)
     printf("%s: %p base " TARGET_FMT_plx " len %08x %d\n", __func__,
            mmio, base, TARGET_PAGE_SIZE, subpage_memory);
@@ -2821,14 +2937,14 @@
 {
     int i;
 
-    cpu_register_io_memory(IO_MEM_ROM >> IO_MEM_SHIFT, error_mem_read, unassigned_mem_write, NULL);
-    cpu_register_io_memory(IO_MEM_UNASSIGNED >> IO_MEM_SHIFT, unassigned_mem_read, unassigned_mem_write, NULL);
-    cpu_register_io_memory(IO_MEM_NOTDIRTY >> IO_MEM_SHIFT, error_mem_read, notdirty_mem_write, NULL);
+    cpu_register_io_memory64(IO_MEM_ROM >> IO_MEM_SHIFT, error_mem_read, unassigned_mem_write, NULL);
+    cpu_register_io_memory64(IO_MEM_UNASSIGNED >> IO_MEM_SHIFT, unassigned_mem_read, unassigned_mem_write, NULL);
+    cpu_register_io_memory64(IO_MEM_NOTDIRTY >> IO_MEM_SHIFT, error_mem_read, notdirty_mem_write, NULL);
     for (i=0; i<5; i++)
         io_mem_used[i] = 1;
 
-    io_mem_watch = cpu_register_io_memory(0, watch_mem_read,
-                                          watch_mem_write, NULL);
+    io_mem_watch = cpu_register_io_memory64(0, watch_mem_read,
+                                            watch_mem_write, NULL);
     /* alloc dirty bits array */
     phys_ram_dirty = qemu_vmalloc(phys_ram_size >> TARGET_PAGE_BITS);
     memset(phys_ram_dirty, 0xff, phys_ram_size >> TARGET_PAGE_BITS);
@@ -2864,10 +2980,38 @@
         io_mem_read[io_index][i] = mem_read[i];
         io_mem_write[io_index][i] = mem_write[i];
     }
+    io_mem_read[io_index][3] = 0;
+    io_mem_write[io_index][3] = 0;
     io_mem_opaque[io_index] = opaque;
     return (io_index << IO_MEM_SHIFT) | subwidth;
 }
 
+int cpu_register_io_memory64(int io_index,
+                             CPUReadMemoryFunc **mem_read,
+                             CPUWriteMemoryFunc **mem_write,
+                             void *opaque)
+{
+    int i, subwidth = 0;
+
+    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;
+    }
+
+    for(i = 0;i < 4; i++) {
+        if (!mem_read[i] || !mem_write[i])
+            subwidth = IO_MEM_SUBWIDTH;
+        io_mem_read[io_index][i] = mem_read[i];
+        io_mem_write[io_index][i] = mem_write[i];
+    }
+    io_mem_opaque[io_index] = opaque;
+    return (io_index << IO_MEM_SHIFT) | subwidth;
+}
+
 void cpu_unregister_io_memory(int io_table_address)
 {
     int i;
@@ -2881,6 +3025,19 @@
     io_mem_used[io_index] = 0;
 }
 
+void cpu_unregister_io_memory64(int io_table_address)
+{
+    int i;
+    int io_index = io_table_address >> IO_MEM_SHIFT;
+
+    for (i=0;i < 4; i++) {
+        io_mem_read[io_index][i] = unassigned_mem_read[i];
+        io_mem_write[io_index][i] = unassigned_mem_write[i];
+    }
+    io_mem_opaque[io_index] = NULL;
+    io_mem_used[io_index] = 0;
+}
+
 CPUWriteMemoryFunc **cpu_get_io_memory_write(int io_index)
 {
     return io_mem_write[io_index >> IO_MEM_SHIFT];
@@ -2942,6 +3099,7 @@
     int l, io_index;
     uint8_t *ptr;
     uint32_t val;
+    uint64_t val64;
     target_phys_addr_t page;
     unsigned long pd;
     PhysPageDesc *p;
@@ -2966,7 +3124,12 @@
                     addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
                 /* XXX: could force cpu_single_env to NULL to avoid
                    potential bugs */
-                if (l >= 4 && ((addr1 & 3) == 0)) {
+                if (l >= 8 && ((addr1 & 7) == 0)) {
+                    /* 64 bit write access */
+                    val64 = ldq_p(buf);
+                    ((CPUWriteMemoryFunc64*)(io_mem_write[io_index][3]))(io_mem_opaque[io_index], addr1, val64);
+                    l = 8;
+                } else if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit write access */
                     val = ldl_p(buf);
                     io_mem_write[io_index][2](io_mem_opaque[io_index], addr1, val);
@@ -3004,7 +3167,12 @@
                 io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
                 if (p)
                     addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-                if (l >= 4 && ((addr1 & 3) == 0)) {
+                if (l >= 8 && ((addr1 & 7) == 0)) {
+                    /* 64 bit read access */
+                    val64 = ((CPUReadMemoryFunc64*)(io_mem_read[io_index][3]))(io_mem_opaque[io_index], addr1);
+                    stq_p(buf, val64);
+                    l = 8;
+                } else if (l >= 4 && ((addr1 & 3) == 0)) {
                     /* 32 bit read access */
                     val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr1);
                     stl_p(buf, val);
@@ -3269,13 +3437,7 @@
         io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
         if (p)
             addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-#ifdef TARGET_WORDS_BIGENDIAN
-        val = (uint64_t)io_mem_read[io_index][2](io_mem_opaque[io_index], addr) << 32;
-        val |= io_mem_read[io_index][2](io_mem_opaque[io_index], addr + 4);
-#else
-        val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
-        val |= (uint64_t)io_mem_read[io_index][2](io_mem_opaque[io_index], addr + 4) << 32;
-#endif
+        val = ((CPUReadMemoryFunc64*)(io_mem_read[io_index][3]))(io_mem_opaque[io_index], addr);
     } else {
         /* RAM case */
         ptr = phys_ram_base + (pd & TARGET_PAGE_MASK) +
@@ -3358,13 +3520,7 @@
         io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
         if (p)
             addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-#ifdef TARGET_WORDS_BIGENDIAN
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val >> 32);
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr + 4, val);
-#else
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
-        io_mem_write[io_index][2](io_mem_opaque[io_index], addr + 4, val >> 32);
-#endif
+        ((CPUWriteMemoryFunc64*)(io_mem_write[io_index][3]))(io_mem_opaque[io_index], addr, val);
     } else {
         ptr = phys_ram_base + (pd & TARGET_PAGE_MASK) +
             (addr & ~TARGET_PAGE_MASK);
Index: hw/slavio_timer.c
===================================================================
--- hw/slavio_timer.c	(revision 6642)
+++ hw/slavio_timer.c	(working copy)
@@ -137,9 +137,17 @@
         // read limit (system counter mode) or read most signifying
         // part of counter (user mode)
         if (slavio_timer_is_user(s)) {
+            uint64_t last_count = (uint64_t)(s->counthigh) << 32 | s->count;
             // read user timer MSW
             slavio_timer_get_out(s);
             ret = s->counthigh | s->reached;
+            if (last_count == TIMER_MAX_COUNT64) { 
+                uint64_t new_count = (uint64_t)ret << 32 | s->count;
+                if (new_count != last_count) {
+                    s->reached = TIMER_REACHED;
+                    ret |= TIMER_REACHED;
+                }
+            }  
         } else {
             // read limit
             // clear irq
@@ -177,6 +185,31 @@
     return ret;
 }
 
+static uint64_t slavio_timer_mem_readq(void *opaque, target_phys_addr_t addr)
+{
+    SLAVIO_TIMERState *s = opaque;
+    uint32_t saddr;
+    uint64_t ret = 0;
+
+    saddr = addr >> 2;
+    switch (saddr) {
+    case TIMER_LIMIT:
+        if (slavio_timer_is_user(s)) {
+            uint64_t last_count = (uint64_t)(s->counthigh) << 32 | s->count;
+            slavio_timer_get_out(s);
+            ret = (uint64_t)(s->counthigh | s->reached) << 32 | s->count;
+            if (last_count == TIMER_MAX_COUNT64 && ret != last_count) {
+                s->reached = TIMER_REACHED;
+                ret |= ((uint64_t)TIMER_REACHED << 32);
+            }  
+        }
+        break;
+    }
+    DPRINTF("read " TARGET_FMT_plx " = %016llx\n", addr, ret);
+
+    return ret;
+}
+
 static void slavio_timer_mem_writel(void *opaque, target_phys_addr_t addr,
                                     uint32_t val)
 {
@@ -303,16 +336,45 @@
     }
 }
 
-static CPUReadMemoryFunc *slavio_timer_mem_read[3] = {
+static void slavio_timer_mem_writeq(void *opaque, target_phys_addr_t addr,
+                                    uint64_t val)
+{
+    SLAVIO_TIMERState *s = opaque;
+    uint32_t saddr;
+
+    DPRINTF("write " TARGET_FMT_plx " %016llx\n", addr, val);
+    saddr = addr >> 2;
+    switch (saddr) {
+    case TIMER_LIMIT:
+        if (slavio_timer_is_user(s)) {
+            uint64_t count;
+
+            s->limit = TIMER_MAX_COUNT64;
+            s->count = val & TIMER_MAX_COUNT64;
+            s->counthigh = (val & TIMER_MAX_COUNT64) >> 32;
+            s->reached = 0;
+            count = ((uint64_t)s->counthigh << 32) | s->count;
+            DPRINTF("processor %d user timer set to %016llx\n", s->slave_index,
+                    count);
+            if (s->timer)
+                ptimer_set_count(s->timer, LIMIT_TO_PERIODS(s->limit - count));
+        }
+        break;
+    }
+}
+
+static CPUReadMemoryFunc *slavio_timer_mem_read[4] = {
     NULL,
     NULL,
     slavio_timer_mem_readl,
+    (CPUReadMemoryFunc*)slavio_timer_mem_readq,
 };
 
-static CPUWriteMemoryFunc *slavio_timer_mem_write[3] = {
+static CPUWriteMemoryFunc *slavio_timer_mem_write[4] = {
     NULL,
     NULL,
     slavio_timer_mem_writel,
+    (CPUWriteMemoryFunc*)slavio_timer_mem_writeq,
 };
 
 static void slavio_timer_save(QEMUFile *f, void *opaque)
@@ -381,8 +443,8 @@
         ptimer_set_period(s->timer, TIMER_PERIOD);
     }
 
-    slavio_timer_io_memory = cpu_register_io_memory(0, slavio_timer_mem_read,
-                                                    slavio_timer_mem_write, s);
+    slavio_timer_io_memory = cpu_register_io_memory64(0, slavio_timer_mem_read,
+                                                      slavio_timer_mem_write, s);
     if (master)
         cpu_register_physical_memory(addr, CPU_TIMER_SIZE,
                                      slavio_timer_io_memory);
Index: cpu-all.h
===================================================================
--- cpu-all.h	(revision 6642)
+++ cpu-all.h	(working copy)
@@ -891,6 +891,9 @@
 typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
 typedef uint32_t CPUReadMemoryFunc(void *opaque, target_phys_addr_t addr);
 
+typedef void CPUWriteMemoryFunc64(void *opaque, target_phys_addr_t addr, uint64_t value);
+typedef uint64_t CPUReadMemoryFunc64(void *opaque, target_phys_addr_t addr);
+
 void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
                                          ram_addr_t size,
                                          ram_addr_t phys_offset,
@@ -909,7 +912,12 @@
                            CPUReadMemoryFunc **mem_read,
                            CPUWriteMemoryFunc **mem_write,
                            void *opaque);
+int cpu_register_io_memory64(int io_index,
+                             CPUReadMemoryFunc **mem_read,
+                             CPUWriteMemoryFunc **mem_write,
+                             void *opaque);
 void cpu_unregister_io_memory(int table_address);
+void cpu_unregister_io_memory64(int table_address);
 CPUWriteMemoryFunc **cpu_get_io_memory_write(int io_index);
 CPUReadMemoryFunc **cpu_get_io_memory_read(int io_index);
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-24 12:50 ` Robert Reif
@ 2009-02-24 20:16   ` Blue Swirl
  2009-02-24 23:46     ` Robert Reif
  0 siblings, 1 reply; 9+ messages in thread
From: Blue Swirl @ 2009-02-24 20:16 UTC (permalink / raw)
  To: qemu-devel

On 2/24/09, Robert Reif <reif@earthlink.net> wrote:
> Robert Reif wrote:
>
> > Here is my second proof of concept attempt at supporting
> > 64 bit device I/O.
> >
> > This attempt doesn't require changing all devices to add a forth
> > function for 64 bit I/O unless actually needed.  Unfortunately it
> > does require an ugly cast when 64 bit is required.
> >
> > This has been tested only on sparc32 and only the slavio_timer
> > hw device has been converted to support 64 bit I/O.
> >
> > All other architectures should work as before except when
> > 64 bit I/O is performed.  They will now get an unassigned
> > access error because there is no 64 bit callback defined by
> > default.  Adding a 64 bit callback and using
> > cpu_register_io_memory64 to register the new callback
> > will fix it.  See slavio_timer.c to to see how the new callback
> > is registered.
> >
> > Comments welcome.
> >
>  This is a bug fix version of the previous patch.
>   exec.c  subpage_readlen64 had a truncation bug
>   slavio_timer.c had a timer limit bug
>
>  With this patch, a sparc32 SS5-170 openboot image
>  passes the 64bit user timer self test.  I have also tested it on
>  sparc32 with an unreleased hw device that does extensive
>  64 bit i/o.
>
>  Please let me know what other ports and hardware configurations
>  perform 64 bit i/o so I can make the necessary changes for
>  inclusion into QEMU.

Sparc64 would use 64 bit IO extensively for most native device
registers, but so far there is only the PCI host bridge and even that
is not fully implemented. Accessing the registers with 8/16/32 bit
operations should trigger a fault.

>  +static CPUReadMemoryFunc *unassigned_mem_read[4] = {
>      unassigned_mem_readb,
>      unassigned_mem_readw,
>      unassigned_mem_readl,
>  +    (CPUReadMemoryFunc*)unassigned_mem_readq,
>   };

Would it be better to use a structure with four elements with correct
types (including uint8/16_t)  instead of the cast? This could also be
limited to only cpu_register_io_memory64.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-24 20:16   ` Blue Swirl
@ 2009-02-24 23:46     ` Robert Reif
  2009-02-25  7:24       ` Avi Kivity
                         ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Robert Reif @ 2009-02-24 23:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Blue Swirl

Blue Swirl wrote:
> Would it be better to use a structure with four elements with correct
> types (including uint8/16_t)  instead of the cast? This could also be
> limited to only cpu_register_io_memory64.
>
>
>   
This is what I would prefer and is what the first version of this patch
that I submitted a year ago did:   http://landley.net/qemu/2008-01-01.html

The problem is that every hardware driver would need to be changed and
some of them would need to be changed drastically because they use the
same functions for all three data sizes.  No one seemed interested in this
approach so I abandoned it.

This approach while uglier requires no changes to the hardware drivers
unless they need 64 bit support so I hoped it would be better received.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-24 23:46     ` Robert Reif
@ 2009-02-25  7:24       ` Avi Kivity
  2009-02-25  7:25       ` Avi Kivity
  2009-02-25 11:55       ` Paul Brook
  2 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2009-02-25  7:24 UTC (permalink / raw)
  To: qemu-devel, reif; +Cc: Blue Swirl

Robert Reif wrote:
> Blue Swirl wrote:
>> Would it be better to use a structure with four elements with correct
>> types (including uint8/16_t)  instead of the cast? This could also be
>> limited to only cpu_register_io_memory64.
>>
>>
>>   
> This is what I would prefer and is what the first version of this patch
> that I submitted a year ago did:   
> http://landley.net/qemu/2008-01-01.html
>
> The problem is that every hardware driver would need to be changed and
> some of them would need to be changed drastically because they use the
> same functions for all three data sizes.  No one seemed interested in 
> this
> approach so I abandoned it.
>
> This approach while uglier requires no changes to the hardware drivers
> unless they need 64 bit support so I hoped it would be better received.

You might try adding a struct-based 4-function interface, and 
implementing the array-based 3-function interface on top of that.  This 
way, no immediate changes would be needed for devices, but we could 
change them incrementally from the old interface to the new interface.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-24 23:46     ` Robert Reif
  2009-02-25  7:24       ` Avi Kivity
@ 2009-02-25  7:25       ` Avi Kivity
  2009-02-25 11:55         ` Robert Reif
  2009-02-25 11:55       ` Paul Brook
  2 siblings, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2009-02-25  7:25 UTC (permalink / raw)
  To: qemu-devel, reif; +Cc: Blue Swirl

Robert Reif wrote:
> Blue Swirl wrote:
>> Would it be better to use a structure with four elements with correct
>> types (including uint8/16_t)  instead of the cast? This could also be
>> limited to only cpu_register_io_memory64.
>>
>>
>>   
> This is what I would prefer and is what the first version of this patch
> that I submitted a year ago did:   
> http://landley.net/qemu/2008-01-01.html
>
> The problem is that every hardware driver would need to be changed and
> some of them would need to be changed drastically because they use the
> same functions for all three data sizes.  No one seemed interested in 
> this
> approach so I abandoned it.
>
> This approach while uglier requires no changes to the hardware drivers
> unless they need 64 bit support so I hoped it would be better received.

You might try adding a struct-based 4-function interface, and 
implementing the array-based 3-function interface on top of that.  This 
way, no immediate changes would be needed for devices, but we could 
change them incrementally from the old interface to the new interface.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-25  7:25       ` Avi Kivity
@ 2009-02-25 11:55         ` Robert Reif
  2009-02-25 20:02           ` Blue Swirl
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Reif @ 2009-02-25 11:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: Blue Swirl

Avi Kivity wrote:
> Robert Reif wrote:
>> Blue Swirl wrote:
>>> Would it be better to use a structure with four elements with correct
>>> types (including uint8/16_t)  instead of the cast? This could also be
>>> limited to only cpu_register_io_memory64.
>>>
>>>
>>>   
>> This is what I would prefer and is what the first version of this patch
>> that I submitted a year ago did:   
>> http://landley.net/qemu/2008-01-01.html
>>
>> The problem is that every hardware driver would need to be changed and
>> some of them would need to be changed drastically because they use the
>> same functions for all three data sizes.  No one seemed interested in 
>> this
>> approach so I abandoned it.
>>
>> This approach while uglier requires no changes to the hardware drivers
>> unless they need 64 bit support so I hoped it would be better received.
>
> You might try adding a struct-based 4-function interface, and 
> implementing the array-based 3-function interface on top of that.  
> This way, no immediate changes would be needed for devices, but we 
> could change them incrementally from the old interface to the new 
> interface.
>
>
I just tried this by changing exec.c to use a structure internally but 
still provide an array interface through the old interface and it's a 
real nice cleanup until you have to deal with subpages.  Changing the 
subpage code over to a structure doesn't really improve it any.

Why is the subpage code now saving so much redundant stuff compared to 
what it initially started out as.  Is it because that one VGA driver is 
messing with the internals of QEMU?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-24 23:46     ` Robert Reif
  2009-02-25  7:24       ` Avi Kivity
  2009-02-25  7:25       ` Avi Kivity
@ 2009-02-25 11:55       ` Paul Brook
  2 siblings, 0 replies; 9+ messages in thread
From: Paul Brook @ 2009-02-25 11:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: Blue Swirl, Robert Reif

> > Would it be better to use a structure with four elements with correct
> > types (including uint8/16_t)  instead of the cast? This could also be
> > limited to only cpu_register_io_memory64.
>
> This is what I would prefer and is what the first version of this patch
> that I submitted a year ago did:   http://landley.net/qemu/2008-01-01.html
>
> The problem is that every hardware driver would need to be changed and
> some of them would need to be changed drastically because they use the
> same functions for all three data sizes.

You can always use the same prototype for the first 3 functions. In practice 
there's very little benefit for using types smaller than 32-bit on machines 
capable of runnit qemu.

Paul

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC] 64 bit device I/O
  2009-02-25 11:55         ` Robert Reif
@ 2009-02-25 20:02           ` Blue Swirl
  0 siblings, 0 replies; 9+ messages in thread
From: Blue Swirl @ 2009-02-25 20:02 UTC (permalink / raw)
  To: Robert Reif; +Cc: qemu-devel

On 2/25/09, Robert Reif <reif@earthlink.net> wrote:
> Avi Kivity wrote:
>
> > Robert Reif wrote:
> >
> > > Blue Swirl wrote:
> > >
> > > > Would it be better to use a structure with four elements with correct
> > > > types (including uint8/16_t)  instead of the cast? This could also be
> > > > limited to only cpu_register_io_memory64.
> > > >
> > > >
> > > >
> > > >
> > > This is what I would prefer and is what the first version of this patch
> > > that I submitted a year ago did:
> http://landley.net/qemu/2008-01-01.html
> > >
> > > The problem is that every hardware driver would need to be changed and
> > > some of them would need to be changed drastically because they use the
> > > same functions for all three data sizes.  No one seemed interested in
> this
> > > approach so I abandoned it.
> > >
> > > This approach while uglier requires no changes to the hardware drivers
> > > unless they need 64 bit support so I hoped it would be better received.
> > >
> >
> > You might try adding a struct-based 4-function interface, and implementing
> the array-based 3-function interface on top of that.  This way, no immediate
> changes would be needed for devices, but we could change them incrementally
> from the old interface to the new interface.
> >
> >
> >
>  I just tried this by changing exec.c to use a structure internally but
> still provide an array interface through the old interface and it's a real
> nice cleanup until you have to deal with subpages.  Changing the subpage
> code over to a structure doesn't really improve it any.
>
>  Why is the subpage code now saving so much redundant stuff compared to what
> it initially started out as.  Is it because that one VGA driver is messing
> with the internals of QEMU?

Also CPURead/WriteMemoryFunc structure could be read only, except for
these VGA hacks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2009-02-25 20:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-21 15:05 [Qemu-devel] [RFC] 64 bit device I/O Robert Reif
2009-02-24 12:50 ` Robert Reif
2009-02-24 20:16   ` Blue Swirl
2009-02-24 23:46     ` Robert Reif
2009-02-25  7:24       ` Avi Kivity
2009-02-25  7:25       ` Avi Kivity
2009-02-25 11:55         ` Robert Reif
2009-02-25 20:02           ` Blue Swirl
2009-02-25 11:55       ` Paul Brook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.