* [Qemu-devel] [PATCH 1/2] io: Add CPUIOMemoryOps and use it.
2010-04-20 20:26 [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Richard Henderson
@ 2010-04-20 19:54 ` Richard Henderson
2010-04-20 20:22 ` [Qemu-devel] [PATCH 2/2] io: Add readq/writeq hooks and use them Richard Henderson
2010-04-22 19:38 ` [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Blue Swirl
2 siblings, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2010-04-20 19:54 UTC (permalink / raw)
To: qemu-devel; +Cc: paul, agraf
Transition the core i/o bits away from a couple of flat arrays
to use a structure naming the read/write callbacks. For now,
retain the flat array interface for the drivers.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
cpu-common.h | 14 ++-
exec-all.h | 3 +-
exec.c | 321 ++++++++++++++++++++++++++-------------------------
softmmu_template.h | 36 ++++--
4 files changed, 203 insertions(+), 171 deletions(-)
diff --git a/cpu-common.h b/cpu-common.h
index b730ca0..0566654 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -25,8 +25,17 @@ typedef unsigned long ram_addr_t;
/* memory API */
-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 CPUWriteMemoryFunc(void *opaque, target_phys_addr_t, uint32_t);
+typedef uint32_t CPUReadMemoryFunc(void *opaque, target_phys_addr_t);
+
+typedef struct CPUIOMemoryOps {
+ CPUReadMemoryFunc *readb;
+ CPUReadMemoryFunc *readw;
+ CPUReadMemoryFunc *readl;
+ CPUWriteMemoryFunc *writeb;
+ CPUWriteMemoryFunc *writew;
+ CPUWriteMemoryFunc *writel;
+} CPUIOMemoryOps;
void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
ram_addr_t size,
@@ -47,6 +56,7 @@ void *qemu_get_ram_ptr(ram_addr_t addr);
/* This should not be used by devices. */
ram_addr_t qemu_ram_addr_from_host(void *ptr);
+int cpu_register_io_memory_st(const CPUIOMemoryOps *mem_ops, void *opaque);
int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
CPUWriteMemoryFunc * const *mem_write,
void *opaque);
diff --git a/exec-all.h b/exec-all.h
index 4bae1e2..5007d59 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -267,8 +267,7 @@ extern int tb_invalidated_flag;
#if !defined(CONFIG_USER_ONLY)
-extern CPUWriteMemoryFunc *io_mem_write[IO_MEM_NB_ENTRIES][4];
-extern CPUReadMemoryFunc *io_mem_read[IO_MEM_NB_ENTRIES][4];
+extern CPUIOMemoryOps io_mem_ops[IO_MEM_NB_ENTRIES];
extern void *io_mem_opaque[IO_MEM_NB_ENTRIES];
void tlb_fill(target_ulong addr, int is_write, int mmu_idx,
diff --git a/exec.c b/exec.c
index 43366ac..d8691c9 100644
--- a/exec.c
+++ b/exec.c
@@ -215,8 +215,7 @@ static void *l1_phys_map[P_L1_SIZE];
static void io_mem_init(void);
/* io memory support */
-CPUWriteMemoryFunc *io_mem_write[IO_MEM_NB_ENTRIES][4];
-CPUReadMemoryFunc *io_mem_read[IO_MEM_NB_ENTRIES][4];
+CPUIOMemoryOps io_mem_ops[IO_MEM_NB_ENTRIES];
void *io_mem_opaque[IO_MEM_NB_ENTRIES];
static char io_mem_used[IO_MEM_NB_ENTRIES];
static int io_mem_watch;
@@ -2549,10 +2548,9 @@ static inline void tlb_set_dirty(CPUState *env,
#define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
typedef struct subpage_t {
target_phys_addr_t base;
- CPUReadMemoryFunc * const *mem_read[TARGET_PAGE_SIZE][4];
- CPUWriteMemoryFunc * const *mem_write[TARGET_PAGE_SIZE][4];
- void *opaque[TARGET_PAGE_SIZE][2][4];
- ram_addr_t region_offset[TARGET_PAGE_SIZE][2][4];
+ CPUIOMemoryOps mem_ops[TARGET_PAGE_SIZE];
+ void *opaque[TARGET_PAGE_SIZE];
+ ram_addr_t region_offset[TARGET_PAGE_SIZE];
} subpage_t;
static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
@@ -2962,16 +2960,20 @@ static void unassigned_mem_writel(void *opaque, target_phys_addr_t addr, uint32_
#endif
}
-static CPUReadMemoryFunc * const unassigned_mem_read[3] = {
- unassigned_mem_readb,
- unassigned_mem_readw,
- unassigned_mem_readl,
+static const CPUIOMemoryOps unassigned_mem_ops = {
+ .readb = unassigned_mem_readb,
+ .readw = unassigned_mem_readw,
+ .readl = unassigned_mem_readl,
+ .writeb = unassigned_mem_writeb,
+ .writew = unassigned_mem_writew,
+ .writel = unassigned_mem_writel,
};
-static CPUWriteMemoryFunc * const unassigned_mem_write[3] = {
- unassigned_mem_writeb,
- unassigned_mem_writew,
- unassigned_mem_writel,
+static const CPUIOMemoryOps rom_mem_ops = {
+ /* Read functions never used. */
+ .writeb = unassigned_mem_writeb,
+ .writew = unassigned_mem_writew,
+ .writel = unassigned_mem_writel,
};
static void notdirty_mem_writeb(void *opaque, target_phys_addr_t ram_addr,
@@ -3034,16 +3036,11 @@ static void notdirty_mem_writel(void *opaque, target_phys_addr_t ram_addr,
tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
}
-static CPUReadMemoryFunc * const error_mem_read[3] = {
- NULL, /* never used */
- NULL, /* never used */
- NULL, /* never used */
-};
-
-static CPUWriteMemoryFunc * const notdirty_mem_write[3] = {
- notdirty_mem_writeb,
- notdirty_mem_writew,
- notdirty_mem_writel,
+static const CPUIOMemoryOps notdirty_mem_ops = {
+ /* Read functions never used. */
+ .writeb = notdirty_mem_writeb,
+ .writew = notdirty_mem_writew,
+ .writel = notdirty_mem_writel,
};
/* Generate a debug exception if a watchpoint has been hit. */
@@ -3133,121 +3130,115 @@ static void watch_mem_writel(void *opaque, target_phys_addr_t addr,
stl_phys(addr, val);
}
-static CPUReadMemoryFunc * const watch_mem_read[3] = {
- watch_mem_readb,
- watch_mem_readw,
- watch_mem_readl,
-};
-
-static CPUWriteMemoryFunc * const watch_mem_write[3] = {
- watch_mem_writeb,
- watch_mem_writew,
- watch_mem_writel,
+static const CPUIOMemoryOps watch_mem_ops = {
+ .readb = watch_mem_readb,
+ .readw = watch_mem_readw,
+ .readl = watch_mem_readl,
+ .writeb = watch_mem_writeb,
+ .writew = watch_mem_writew,
+ .writel = watch_mem_writel,
};
-static inline uint32_t subpage_readlen (subpage_t *mmio, target_phys_addr_t addr,
- unsigned int len)
-{
- uint32_t ret;
- unsigned int idx;
-
- idx = SUBPAGE_IDX(addr);
-#if defined(DEBUG_SUBPAGE)
- 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],
- addr + mmio->region_offset[idx][0][len]);
-
- return ret;
-}
-
-static inline void subpage_writelen (subpage_t *mmio, target_phys_addr_t addr,
- uint32_t value, unsigned int len)
-{
- unsigned int idx;
-
- idx = SUBPAGE_IDX(addr);
-#if defined(DEBUG_SUBPAGE)
- 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],
- addr + mmio->region_offset[idx][1][len],
- value);
-}
-
static uint32_t subpage_readb (void *opaque, target_phys_addr_t addr)
{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
#endif
-
- return subpage_readlen(opaque, addr, 0);
+
+ return mmio->mem_ops[idx].readb(mmio->opaque[idx],
+ addr + mmio->region_offset[idx]);
}
static void subpage_writeb (void *opaque, target_phys_addr_t addr,
uint32_t value)
{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
#endif
- subpage_writelen(opaque, addr, value, 0);
+
+ return mmio->mem_ops[idx].writeb(mmio->opaque[idx],
+ addr + mmio->region_offset[idx], value);
}
static uint32_t subpage_readw (void *opaque, target_phys_addr_t addr)
{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
#endif
-
- return subpage_readlen(opaque, addr, 1);
+
+ return mmio->mem_ops[idx].readw(mmio->opaque[idx],
+ addr + mmio->region_offset[idx]);
}
static void subpage_writew (void *opaque, target_phys_addr_t addr,
uint32_t value)
{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
#endif
- subpage_writelen(opaque, addr, value, 1);
+
+ return mmio->mem_ops[idx].writew(mmio->opaque[idx],
+ addr + mmio->region_offset[idx], value);
}
static uint32_t subpage_readl (void *opaque, target_phys_addr_t addr)
{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
#endif
-
- return subpage_readlen(opaque, addr, 2);
+
+ return mmio->mem_ops[idx].readl(mmio->opaque[idx],
+ addr + mmio->region_offset[idx]);
}
static void subpage_writel (void *opaque,
target_phys_addr_t addr, uint32_t value)
{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
#endif
- subpage_writelen(opaque, addr, value, 2);
-}
-static CPUReadMemoryFunc * const subpage_read[] = {
- &subpage_readb,
- &subpage_readw,
- &subpage_readl,
-};
+ return mmio->mem_ops[idx].writel(mmio->opaque[idx],
+ addr + mmio->region_offset[idx], value);
+}
-static CPUWriteMemoryFunc * const subpage_write[] = {
- &subpage_writeb,
- &subpage_writew,
- &subpage_writel,
+static const CPUIOMemoryOps subpage_ops = {
+ .readb = subpage_readb,
+ .readw = subpage_readw,
+ .readl = subpage_readl,
+ .writeb = subpage_writeb,
+ .writew = subpage_writew,
+ .writel = subpage_writel,
};
static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
ram_addr_t memory, ram_addr_t region_offset)
{
int idx, eidx;
- unsigned int i;
if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE)
return -1;
@@ -3258,19 +3249,11 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
mmio, start, end, idx, eidx, memory);
#endif
memory >>= IO_MEM_SHIFT;
+
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->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->opaque[idx][1][i] = io_mem_opaque[memory];
- mmio->region_offset[idx][1][i] = region_offset;
- }
- }
+ mmio->mem_ops[idx] = io_mem_ops[memory];
+ mmio->opaque[idx] = io_mem_opaque[memory];
+ mmio->region_offset[idx] = region_offset;
}
return 0;
@@ -3285,7 +3268,7 @@ static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
mmio = qemu_mallocz(sizeof(subpage_t));
mmio->base = base;
- subpage_memory = cpu_register_io_memory(subpage_read, subpage_write, mmio);
+ subpage_memory = cpu_register_io_memory_st(&subpage_ops, mmio);
#if defined(DEBUG_SUBPAGE)
printf("%s: %p base " TARGET_FMT_plx " len %08x %d\n", __func__,
mmio, base, TARGET_PAGE_SIZE, subpage_memory);
@@ -3310,19 +3293,17 @@ static int get_free_io_mem_idx(void)
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. */
+/* MEM_OPS contains pointers to the functions giving sized acceses to
+ the I/O. Access functions may 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,
- CPUReadMemoryFunc * const *mem_read,
- CPUWriteMemoryFunc * const *mem_write,
+ const CPUIOMemoryOps *mem_ops,
void *opaque)
{
- int i, subwidth = 0;
+ int subwidth = 0;
if (io_index <= 0) {
io_index = get_free_io_mem_idx();
@@ -3334,32 +3315,45 @@ static int cpu_register_io_memory_fixed(int io_index,
return -1;
}
- for(i = 0;i < 3; 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_ops[io_index] = *mem_ops;
io_mem_opaque[io_index] = opaque;
+
+ if (!mem_ops->readl || !mem_ops->writel
+ || !mem_ops->readw || !mem_ops->writew
+ || !mem_ops->readb || !mem_ops->writeb) {
+ subwidth = IO_MEM_SUBWIDTH;
+ }
+
return (io_index << IO_MEM_SHIFT) | subwidth;
}
+int cpu_register_io_memory_st(const CPUIOMemoryOps *mem_ops, void *opaque)
+{
+ return cpu_register_io_memory_fixed(0, mem_ops, opaque);
+}
+
+/* Temporarily leave the existing array-based interface in place while
+ drivers are updated. */
int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
CPUWriteMemoryFunc * const *mem_write,
void *opaque)
{
- return cpu_register_io_memory_fixed(0, mem_read, mem_write, opaque);
+ CPUIOMemoryOps ops;
+ ops.readb = mem_read[0];
+ ops.readw = mem_read[1];
+ ops.readl = mem_read[2];
+ ops.writeb = mem_write[0];
+ ops.writew = mem_write[1];
+ ops.writel = mem_write[2];
+
+ return cpu_register_io_memory_fixed(0, &ops, opaque);
}
void cpu_unregister_io_memory(int io_table_address)
{
- int i;
int io_index = io_table_address >> IO_MEM_SHIFT;
- for (i=0;i < 3; i++) {
- io_mem_read[io_index][i] = unassigned_mem_read[i];
- io_mem_write[io_index][i] = unassigned_mem_write[i];
- }
+ io_mem_ops[io_index] = unassigned_mem_ops;
io_mem_opaque[io_index] = NULL;
io_mem_used[io_index] = 0;
}
@@ -3368,14 +3362,14 @@ static void io_mem_init(void)
{
int i;
- cpu_register_io_memory_fixed(IO_MEM_ROM, error_mem_read, unassigned_mem_write, NULL);
- cpu_register_io_memory_fixed(IO_MEM_UNASSIGNED, unassigned_mem_read, unassigned_mem_write, NULL);
- cpu_register_io_memory_fixed(IO_MEM_NOTDIRTY, error_mem_read, notdirty_mem_write, NULL);
- for (i=0; i<5; i++)
+ cpu_register_io_memory_fixed(IO_MEM_ROM, &rom_mem_ops, NULL);
+ cpu_register_io_memory_fixed(IO_MEM_UNASSIGNED, &unassigned_mem_ops, NULL);
+ cpu_register_io_memory_fixed(IO_MEM_NOTDIRTY, ¬dirty_mem_ops, NULL);
+ for (i=0; i<5; i++) {
io_mem_used[i] = 1;
+ }
- io_mem_watch = cpu_register_io_memory(watch_mem_read,
- watch_mem_write, NULL);
+ io_mem_watch = cpu_register_io_memory_st(&watch_mem_ops, NULL);
}
#endif /* !defined(CONFIG_USER_ONLY) */
@@ -3425,7 +3419,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;
@@ -3447,7 +3441,8 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
if (is_write) {
if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
target_phys_addr_t addr1 = addr;
- io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ int io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ void *opaque = io_mem_opaque[io_index];
if (p)
addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
/* XXX: could force cpu_single_env to NULL to avoid
@@ -3455,17 +3450,17 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
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);
+ io_mem_ops[io_index].writel(opaque, addr1, val);
l = 4;
} else if (l >= 2 && ((addr1 & 1) == 0)) {
/* 16 bit write access */
val = lduw_p(buf);
- io_mem_write[io_index][1](io_mem_opaque[io_index], addr1, val);
+ io_mem_ops[io_index].writew(opaque, addr1, val);
l = 2;
} else {
/* 8 bit write access */
val = ldub_p(buf);
- io_mem_write[io_index][0](io_mem_opaque[io_index], addr1, val);
+ io_mem_ops[io_index].writeb(opaque, addr1, val);
l = 1;
}
} else {
@@ -3487,22 +3482,23 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
!(pd & IO_MEM_ROMD)) {
target_phys_addr_t addr1 = addr;
/* I/O case */
- io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ int io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ void *opaque = io_mem_opaque[io_index];
if (p)
addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
if (l >= 4 && ((addr1 & 3) == 0)) {
/* 32 bit read access */
- val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr1);
+ val = io_mem_ops[io_index].readl(opaque, addr1);
stl_p(buf, val);
l = 4;
} else if (l >= 2 && ((addr1 & 1) == 0)) {
/* 16 bit read access */
- val = io_mem_read[io_index][1](io_mem_opaque[io_index], addr1);
+ val = io_mem_ops[io_index].readw(opaque, addr1);
stw_p(buf, val);
l = 2;
} else {
/* 8 bit read access */
- val = io_mem_read[io_index][0](io_mem_opaque[io_index], addr1);
+ val = io_mem_ops[io_index].readb(opaque, addr1);
stb_p(buf, val);
l = 1;
}
@@ -3724,7 +3720,7 @@ uint32_t ldl_phys(target_phys_addr_t addr)
io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
if (p)
addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
- val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
+ val = io_mem_ops[io_index].readl(io_mem_opaque[io_index], addr);
} else {
/* RAM case */
ptr = qemu_get_ram_ptr(pd & TARGET_PAGE_MASK) +
@@ -3737,7 +3733,6 @@ uint32_t ldl_phys(target_phys_addr_t addr)
/* warning: addr must be aligned */
uint64_t ldq_phys(target_phys_addr_t addr)
{
- int io_index;
uint8_t *ptr;
uint64_t val;
unsigned long pd;
@@ -3753,15 +3748,22 @@ uint64_t ldq_phys(target_phys_addr_t addr)
if ((pd & ~TARGET_PAGE_MASK) > IO_MEM_ROM &&
!(pd & IO_MEM_ROMD)) {
/* I/O case */
- io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ int io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ void *opaque = io_mem_opaque[io_index];
+ CPUReadMemoryFunc *readl;
+ uint32_t v1, v2;
+
if (p)
addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
+
+ readl = io_mem_ops[io_index].readl;
+ v1 = readl(opaque, addr);
+ v2 = readl(opaque, addr + 4);
+
#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);
+ val = ((uint64_t)v1 << 32) | v2;
#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;
+ val = ((uint64_t)v2 << 32) | v1;
#endif
} else {
/* RAM case */
@@ -3809,7 +3811,7 @@ void stl_phys_notdirty(target_phys_addr_t addr, uint32_t val)
io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
if (p)
addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
- io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
+ io_mem_ops[io_index].writel(io_mem_opaque[io_index], addr, val);
} else {
unsigned long addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
ptr = qemu_get_ram_ptr(addr1);
@@ -3829,7 +3831,6 @@ 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;
unsigned long pd;
PhysPageDesc *p;
@@ -3842,16 +3843,22 @@ void stq_phys_notdirty(target_phys_addr_t addr, uint64_t val)
}
if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
- io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ int io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+ void *opaque = io_mem_opaque[io_index];
+ CPUWriteMemoryFunc *writel;
+ uint32_t v1, v2;
+
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);
+ v1 = val >> 32, v2 = 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);
+ v1 = val, v2 = val >> 32;
#endif
+ writel = io_mem_ops[io_index].writel;
+
+ writel(opaque, addr, v1);
+ writel(opaque, addr + 4, v2);
} else {
ptr = qemu_get_ram_ptr(pd & TARGET_PAGE_MASK) +
(addr & ~TARGET_PAGE_MASK);
@@ -3878,7 +3885,7 @@ void stl_phys(target_phys_addr_t addr, uint32_t val)
io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
if (p)
addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
- io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
+ io_mem_ops[io_index].writel(io_mem_opaque[io_index], addr, val);
} else {
unsigned long addr1;
addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
diff --git a/softmmu_template.h b/softmmu_template.h
index c2df9ec..27e23c9 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -57,6 +57,8 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(target_phys_addr_t physaddr,
{
DATA_TYPE res;
int index;
+ void *opaque;
+
index = (physaddr >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
env->mem_io_pc = (unsigned long)retaddr;
@@ -66,16 +68,21 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(target_phys_addr_t physaddr,
}
env->mem_io_vaddr = addr;
+ opaque = io_mem_opaque[index];
#if SHIFT <= 2
- res = io_mem_read[index][SHIFT](io_mem_opaque[index], physaddr);
+ res = io_mem_ops[index].glue(read,SUFFIX)(opaque, physaddr);
#else
+ {
+ CPUReadMemoryFunc *readl = io_mem_ops[index].readl;
+ uint32_t v1 = readl(opaque, physaddr);
+ uint32_t v2 = readl(opaque, physaddr + 4);
+
#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);
+ res = ((uint64_t)v1 << 32) | v2;
#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;
+ res = ((uint64_t)v2 << 32) | v1;
#endif
+ }
#endif /* SHIFT > 2 */
return res;
}
@@ -200,6 +207,8 @@ static inline void glue(io_write, SUFFIX)(target_phys_addr_t physaddr,
void *retaddr)
{
int index;
+ void *opaque;
+
index = (physaddr >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
if (index > (IO_MEM_NOTDIRTY >> IO_MEM_SHIFT)
@@ -209,16 +218,23 @@ static inline void glue(io_write, SUFFIX)(target_phys_addr_t physaddr,
env->mem_io_vaddr = addr;
env->mem_io_pc = (unsigned long)retaddr;
+ opaque = io_mem_opaque[index];
#if SHIFT <= 2
- io_mem_write[index][SHIFT](io_mem_opaque[index], physaddr, val);
+ io_mem_ops[index].glue(write,SUFFIX)(opaque, physaddr, val);
#else
+ {
+ CPUWriteMemoryFunc *writel = io_mem_ops[index].writel;
+ uint32_t v1, v2;
+
#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);
+ v1 = val >> 32, v2 = 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);
+ v1 = val, v2 = val >> 32;
#endif
+
+ writel(opaque, physaddr, v1);
+ writel(opaque, physaddr + 4, v2);
+ }
#endif /* SHIFT > 2 */
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 2/2] io: Add readq/writeq hooks and use them.
2010-04-20 20:26 [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Richard Henderson
2010-04-20 19:54 ` [Qemu-devel] [PATCH 1/2] io: Add CPUIOMemoryOps and use it Richard Henderson
@ 2010-04-20 20:22 ` Richard Henderson
2010-04-22 19:38 ` [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Blue Swirl
2 siblings, 0 replies; 18+ messages in thread
From: Richard Henderson @ 2010-04-20 20:22 UTC (permalink / raw)
To: qemu-devel; +Cc: paul, agraf
If the device provides full 64-bit i/o routines, use them instead
of forcing the i/o to be split into two 32-bit i/o calls.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
cpu-common.h | 5 ++
exec.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++------
softmmu_template.h | 8 ++-
3 files changed, 139 insertions(+), 18 deletions(-)
diff --git a/cpu-common.h b/cpu-common.h
index 0566654..df5ad32 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -28,13 +28,18 @@ typedef unsigned long ram_addr_t;
typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t, uint32_t);
typedef uint32_t CPUReadMemoryFunc(void *opaque, target_phys_addr_t);
+typedef void CPUWriteMemory64Func(void *opaque, target_phys_addr_t, uint64_t);
+typedef uint64_t CPUReadMemory64Func(void *opaque, target_phys_addr_t);
+
typedef struct CPUIOMemoryOps {
CPUReadMemoryFunc *readb;
CPUReadMemoryFunc *readw;
CPUReadMemoryFunc *readl;
+ CPUReadMemory64Func *readq;
CPUWriteMemoryFunc *writeb;
CPUWriteMemoryFunc *writew;
CPUWriteMemoryFunc *writel;
+ CPUWriteMemory64Func *writeq;
} CPUIOMemoryOps;
void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
diff --git a/exec.c b/exec.c
index d8691c9..596e2e4 100644
--- a/exec.c
+++ b/exec.c
@@ -2930,6 +2930,17 @@ static uint32_t unassigned_mem_readl(void *opaque, target_phys_addr_t addr)
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) || defined(TARGET_MICROBLAZE)
+ 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
@@ -2960,13 +2971,26 @@ static void unassigned_mem_writel(void *opaque, target_phys_addr_t addr, uint32_
#endif
}
+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" PRIx64 "\n",
+ addr, val);
+#endif
+#if defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE)
+ do_unassigned_access(addr, 1, 0, 0, 8);
+#endif
+}
+
static const CPUIOMemoryOps unassigned_mem_ops = {
.readb = unassigned_mem_readb,
.readw = unassigned_mem_readw,
.readl = unassigned_mem_readl,
+ .readq = unassigned_mem_readq,
.writeb = unassigned_mem_writeb,
.writew = unassigned_mem_writew,
.writel = unassigned_mem_writel,
+ .writeq = unassigned_mem_writeq,
};
static const CPUIOMemoryOps rom_mem_ops = {
@@ -2974,6 +2998,7 @@ static const CPUIOMemoryOps rom_mem_ops = {
.writeb = unassigned_mem_writeb,
.writew = unassigned_mem_writew,
.writel = unassigned_mem_writel,
+ .writeq = unassigned_mem_writeq,
};
static void notdirty_mem_writeb(void *opaque, target_phys_addr_t ram_addr,
@@ -3036,11 +3061,32 @@ static void notdirty_mem_writel(void *opaque, target_phys_addr_t ram_addr,
tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr);
}
+static void notdirty_mem_writeq(void *opaque, target_phys_addr_t ram_addr,
+ uint64_t val)
+{
+ int dirty_flags;
+ dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
+ if (!(dirty_flags & CODE_DIRTY_FLAG)) {
+#if !defined(CONFIG_USER_ONLY)
+ tb_invalidate_phys_page_fast(ram_addr, 8);
+ dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
+#endif
+ }
+ stq_p(qemu_get_ram_ptr(ram_addr), val);
+ dirty_flags |= (0xff & ~CODE_DIRTY_FLAG);
+ cpu_physical_memory_set_dirty_flags(ram_addr, 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 const CPUIOMemoryOps notdirty_mem_ops = {
/* Read functions never used. */
.writeb = notdirty_mem_writeb,
.writew = notdirty_mem_writew,
.writel = notdirty_mem_writel,
+ .writeq = notdirty_mem_writeq,
};
/* Generate a debug exception if a watchpoint has been hit. */
@@ -3109,6 +3155,12 @@ static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr)
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)
{
@@ -3130,13 +3182,22 @@ static void watch_mem_writel(void *opaque, target_phys_addr_t addr,
stl_phys(addr, val);
}
+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 const CPUIOMemoryOps watch_mem_ops = {
.readb = watch_mem_readb,
.readw = watch_mem_readw,
.readl = watch_mem_readl,
+ .readq = watch_mem_readq,
.writeb = watch_mem_writeb,
.writew = watch_mem_writew,
.writel = watch_mem_writel,
+ .writeq = watch_mem_writeq,
};
static uint32_t subpage_readb (void *opaque, target_phys_addr_t addr)
@@ -3226,13 +3287,44 @@ static void subpage_writel (void *opaque,
addr + mmio->region_offset[idx], value);
}
+static uint64_t subpage_readq (void *opaque, target_phys_addr_t addr)
+{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
+#if defined(DEBUG_SUBPAGE)
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
+#endif
+
+ return mmio->mem_ops[idx].readq(mmio->opaque[idx],
+ addr + mmio->region_offset[idx]);
+}
+
+static void subpage_writeq (void *opaque,
+ target_phys_addr_t addr, uint64_t value)
+{
+ subpage_t *mmio = opaque;
+ unsigned int idx = SUBPAGE_IDX(addr);
+
+#if defined(DEBUG_SUBPAGE)
+ printf("%s: subpage %p addr " TARGET_FMT_plx " idx %d\n",
+ __func__, mmio, addr, idx);
+#endif
+
+ return mmio->mem_ops[idx].writeq(mmio->opaque[idx],
+ addr + mmio->region_offset[idx], value);
+}
+
static const CPUIOMemoryOps subpage_ops = {
.readb = subpage_readb,
.readw = subpage_readw,
.readl = subpage_readl,
+ .readq = subpage_readq,
.writeb = subpage_writeb,
.writew = subpage_writew,
.writel = subpage_writel,
+ .writeq = subpage_writeq,
};
static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
@@ -3342,9 +3434,11 @@ int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
ops.readb = mem_read[0];
ops.readw = mem_read[1];
ops.readl = mem_read[2];
+ ops.readq = NULL;
ops.writeb = mem_write[0];
ops.writew = mem_write[1];
ops.writel = mem_write[2];
+ ops.writeq = NULL;
return cpu_register_io_memory_fixed(0, &ops, opaque);
}
@@ -3447,7 +3541,11 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
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 && io_mem_ops[io_index].writeq) {
+ uint64_t v64 = ldq_p(buf);
+ io_mem_ops[io_index].writeq(opaque, addr1, v64);
+ l = 8;
+ } else if (l >= 4 && ((addr1 & 3) == 0)) {
/* 32 bit write access */
val = ldl_p(buf);
io_mem_ops[io_index].writel(opaque, addr1, val);
@@ -3486,7 +3584,12 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
void *opaque = io_mem_opaque[io_index];
if (p)
addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
- if (l >= 4 && ((addr1 & 3) == 0)) {
+ if (l >= 8 && (addr1 & 7) == 0 && io_mem_ops[io_index].readq) {
+ uint64_t v64;
+ v64 = io_mem_ops[io_index].readq(opaque, addr1);
+ stq_p(buf, val);
+ l = 8;
+ } else if (l >= 4 && ((addr1 & 3) == 0)) {
/* 32 bit read access */
val = io_mem_ops[io_index].readl(opaque, addr1);
stl_p(buf, val);
@@ -3750,21 +3853,25 @@ uint64_t ldq_phys(target_phys_addr_t addr)
/* I/O case */
int io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
void *opaque = io_mem_opaque[io_index];
- CPUReadMemoryFunc *readl;
- uint32_t v1, v2;
if (p)
addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
- readl = io_mem_ops[io_index].readl;
- v1 = readl(opaque, addr);
- v2 = readl(opaque, addr + 4);
+ if (io_mem_ops[io_index].readq) {
+ val = io_mem_ops[io_index].readq(opaque, addr);
+ } else {
+ CPUReadMemoryFunc *readl = io_mem_ops[io_index].readl;
+ uint32_t v1, v2;
+
+ v1 = readl(opaque, addr);
+ v2 = readl(opaque, addr + 4);
#ifdef TARGET_WORDS_BIGENDIAN
- val = ((uint64_t)v1 << 32) | v2;
+ val = ((uint64_t)v1 << 32) | v2;
#else
- val = ((uint64_t)v2 << 32) | v1;
+ val = ((uint64_t)v2 << 32) | v1;
#endif
+ }
} else {
/* RAM case */
ptr = qemu_get_ram_ptr(pd & TARGET_PAGE_MASK) +
@@ -3845,20 +3952,25 @@ void stq_phys_notdirty(target_phys_addr_t addr, uint64_t val)
if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
int io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
void *opaque = io_mem_opaque[io_index];
- CPUWriteMemoryFunc *writel;
- uint32_t v1, v2;
if (p)
addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
+
+ if (io_mem_ops[io_index].writeq) {
+ io_mem_ops[io_index].writeq(opaque, addr, val);
+ } else {
+ CPUWriteMemoryFunc *writel = io_mem_ops[io_index].writel;
+ uint32_t v1, v2;
+
#ifdef TARGET_WORDS_BIGENDIAN
- v1 = val >> 32, v2 = val;
+ v1 = val >> 32, v2 = val;
#else
- v1 = val, v2 = val >> 32;
+ v1 = val, v2 = val >> 32;
#endif
- writel = io_mem_ops[io_index].writel;
- writel(opaque, addr, v1);
- writel(opaque, addr + 4, v2);
+ writel(opaque, addr, v1);
+ writel(opaque, addr + 4, v2);
+ }
} else {
ptr = qemu_get_ram_ptr(pd & TARGET_PAGE_MASK) +
(addr & ~TARGET_PAGE_MASK);
diff --git a/softmmu_template.h b/softmmu_template.h
index 27e23c9..37992e0 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -72,7 +72,9 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(target_phys_addr_t physaddr,
#if SHIFT <= 2
res = io_mem_ops[index].glue(read,SUFFIX)(opaque, physaddr);
#else
- {
+ if (io_mem_ops[index].readq) {
+ res = io_mem_ops[index].readq(opaque, physaddr);
+ } else {
CPUReadMemoryFunc *readl = io_mem_ops[index].readl;
uint32_t v1 = readl(opaque, physaddr);
uint32_t v2 = readl(opaque, physaddr + 4);
@@ -222,7 +224,9 @@ static inline void glue(io_write, SUFFIX)(target_phys_addr_t physaddr,
#if SHIFT <= 2
io_mem_ops[index].glue(write,SUFFIX)(opaque, physaddr, val);
#else
- {
+ if (io_mem_ops[index].writeq) {
+ io_mem_ops[index].writeq(opaque, physaddr, val);
+ } else {
CPUWriteMemoryFunc *writel = io_mem_ops[index].writel;
uint32_t v1, v2;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths
@ 2010-04-20 20:26 Richard Henderson
2010-04-20 19:54 ` [Qemu-devel] [PATCH 1/2] io: Add CPUIOMemoryOps and use it Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Richard Henderson @ 2010-04-20 20:26 UTC (permalink / raw)
To: qemu-devel; +Cc: paul, agraf
Step 1 to implementing alpha-softmmu is to properly handle 64-bit
I/O operations. Tristan Gingold managed a hack where he buffered
half of the I/O operation in the host bridge; I think that's not
something we want to encourage.
I'm a bit confused about IO_MEM_SUBWIDTH and subpage_register, and
why things are done the way that they are. The loop in subpage_register
appears to be written to support different devices at the same I/O
address so long as they use different access widths. Is this really
a consideration to how this code is written? Are there really systems
that need to support this?
>From what I know about PCI this isn't possible with that bus, so this
isn't something that's going to appear on PCs. So if true, it's got
to be some strange embedded weirdness.
I've reviewed all of the devices in hw/. Almost all of them have
non-zero values for all entries. The ones that do have zeros are:
axis_dev88.c; gpio
eccmemctl.c; ecc, ecc_diag
escc.c; escc
esp.c; esp
etraxfs_eth.c; eth
etraxfs_pic.c; pic
etraxfs_ser.c; ser
etraxfs_timer.c; timer
fdc.c; fdctrl_mem_read_strict
fw_cfg.c; fw_cfg_ctl_mem, fw_cfg_data_mem
hpet.c; hpet_ram
lance.c; lance_mem
mac_dbdma.c; dbdma
mips_jazz.c; dma_dummy_read
r2d.c; r2d_fpga
sbi.c; sbi_mem
sh_pci.c; sh_pci
slavio_intctl.c; slavio_intctl, slavio_intctlm
slabio_misc.c; slavio_cfg_mem, etc.
sm501.c; sm501_system_config, sm501_disp_ctrl
sparc32_dma.c; dma_mem
sun4c_intctl.c; sun4c_intctl
sun4m_iommu.c; iommu_mem
tcx.c; tcx_dac, tcx_dummy
xilinx_ethlite.c; eth, pic
xilinx_timer.c; timer
Some of the ones that are obviously to be used together (e.g. etraxfs*
and xilinx*) exclusively use one access method (e.g. readb or readl),
which indicates that they cannot be using overlapping addresses.
Some of the others (e.g. hpet.c) it is obvious via surrounding context
that the driver author assumed that the "functions may be NULL" comment
above cpu_register_io_memory meant that accesses that are undefined
need not be implemented. (This is what I assumed when I first read
that comment as well.)
So... What's supposed to be going on here?
For what it's worth, this patch series has been tested with arm-test,
sparc-test, coldfire-test from the qemu downloads page, as well as
with a full Fedora and WinNT system boots. The second patch has not
been properly tested beyond compile, obviously, since there are not
yet any drivers that implement the hook.
r~
Richard Henderson (2):
io: Add CPUIOMemoryOps and use it.
io: Add readq/writeq hooks and use them.
cpu-common.h | 19 ++-
exec-all.h | 3 +-
exec.c | 425 +++++++++++++++++++++++++++++++++-------------------
softmmu_template.h | 40 ++++--
4 files changed, 320 insertions(+), 167 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths
2010-04-20 20:26 [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Richard Henderson
2010-04-20 19:54 ` [Qemu-devel] [PATCH 1/2] io: Add CPUIOMemoryOps and use it Richard Henderson
2010-04-20 20:22 ` [Qemu-devel] [PATCH 2/2] io: Add readq/writeq hooks and use them Richard Henderson
@ 2010-04-22 19:38 ` Blue Swirl
2010-04-22 19:55 ` Richard Henderson
2 siblings, 1 reply; 18+ messages in thread
From: Blue Swirl @ 2010-04-22 19:38 UTC (permalink / raw)
To: Richard Henderson; +Cc: agraf, qemu-devel, paul
On 4/20/10, Richard Henderson <rth@twiddle.net> wrote:
> Step 1 to implementing alpha-softmmu is to properly handle 64-bit
> I/O operations. Tristan Gingold managed a hack where he buffered
> half of the I/O operation in the host bridge; I think that's not
> something we want to encourage.
>
> I'm a bit confused about IO_MEM_SUBWIDTH and subpage_register, and
> why things are done the way that they are. The loop in subpage_register
> appears to be written to support different devices at the same I/O
> address so long as they use different access widths. Is this really
> a consideration to how this code is written? Are there really systems
> that need to support this?
Subpages are used when there are several devices on the same page.
It's needed for at least Sparc32.
Subwidth (with NULL) is used mainly to indicate that the device does
not accept accesses in some access widths. Sparc32 and Sparc64 need
this (or some other way to signal bus errors for bad access widths).
In fact, many system devices on Sparc64 should only accept 64 bit
accesses, but currently we can't enforce this.
> From what I know about PCI this isn't possible with that bus, so this
> isn't something that's going to appear on PCs. So if true, it's got
> to be some strange embedded weirdness.
I would not call for example Enterprise M9000 an embedded system.
> I've reviewed all of the devices in hw/. Almost all of them have
> non-zero values for all entries. The ones that do have zeros are:
>
> axis_dev88.c; gpio
> eccmemctl.c; ecc, ecc_diag
> escc.c; escc
> esp.c; esp
> etraxfs_eth.c; eth
> etraxfs_pic.c; pic
> etraxfs_ser.c; ser
> etraxfs_timer.c; timer
> fdc.c; fdctrl_mem_read_strict
> fw_cfg.c; fw_cfg_ctl_mem, fw_cfg_data_mem
> hpet.c; hpet_ram
> lance.c; lance_mem
> mac_dbdma.c; dbdma
> mips_jazz.c; dma_dummy_read
> r2d.c; r2d_fpga
> sbi.c; sbi_mem
> sh_pci.c; sh_pci
> slavio_intctl.c; slavio_intctl, slavio_intctlm
> slabio_misc.c; slavio_cfg_mem, etc.
> sm501.c; sm501_system_config, sm501_disp_ctrl
> sparc32_dma.c; dma_mem
> sun4c_intctl.c; sun4c_intctl
> sun4m_iommu.c; iommu_mem
> tcx.c; tcx_dac, tcx_dummy
> xilinx_ethlite.c; eth, pic
> xilinx_timer.c; timer
>
> Some of the ones that are obviously to be used together (e.g. etraxfs*
> and xilinx*) exclusively use one access method (e.g. readb or readl),
> which indicates that they cannot be using overlapping addresses.
>
> Some of the others (e.g. hpet.c) it is obvious via surrounding context
> that the driver author assumed that the "functions may be NULL" comment
> above cpu_register_io_memory meant that accesses that are undefined
> need not be implemented. (This is what I assumed when I first read
> that comment as well.)
>
> So... What's supposed to be going on here?
>
> For what it's worth, this patch series has been tested with arm-test,
> sparc-test, coldfire-test from the qemu downloads page, as well as
> with a full Fedora and WinNT system boots. The second patch has not
> been properly tested beyond compile, obviously, since there are not
> yet any drivers that implement the hook.
>
>
> r~
>
>
> Richard Henderson (2):
> io: Add CPUIOMemoryOps and use it.
> io: Add readq/writeq hooks and use them.
>
> cpu-common.h | 19 ++-
> exec-all.h | 3 +-
> exec.c | 425 +++++++++++++++++++++++++++++++++-------------------
> softmmu_template.h | 40 ++++--
> 4 files changed, 320 insertions(+), 167 deletions(-)
>
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths
2010-04-22 19:38 ` [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Blue Swirl
@ 2010-04-22 19:55 ` Richard Henderson
2010-04-22 20:01 ` Blue Swirl
2010-04-22 23:47 ` [Qemu-devel] [PATCH] Remove IO_MEM_SUBWIDTH Richard Henderson
0 siblings, 2 replies; 18+ messages in thread
From: Richard Henderson @ 2010-04-22 19:55 UTC (permalink / raw)
To: Blue Swirl; +Cc: agraf, qemu-devel, paul
On 04/22/2010 12:38 PM, Blue Swirl wrote:
> Subpages are used when there are several devices on the same page.
> It's needed for at least Sparc32.
That's fine.
> Subwidth (with NULL) is used mainly to indicate that the device does
> not accept accesses in some access widths. Sparc32 and Sparc64 need
> this (or some other way to signal bus errors for bad access widths).
This is also fine. Although by using NULL all you'd get is a qemu
null pointer dereference; I suppose this might get caught and
translated to an cpu exception, but I think it would be preferable
long-term to be more explicit about this and fill in the entries
with a function that would explicitly raise the exception.
What this *does* confirm for me is that we don't need to support
multiple devices at the same address, differentiated by the size
of the reference. Which is something that the current subpage
implementation actually supports.
I'll submit a patch to clean this up.
> In fact, many system devices on Sparc64 should only accept 64 bit
> accesses, but currently we can't enforce this.
Cool. This is something that should be fixed by the end result
of the driver interface re-design that pbrook and I have been
discussing on IRC.
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths
2010-04-22 19:55 ` Richard Henderson
@ 2010-04-22 20:01 ` Blue Swirl
2010-04-22 21:19 ` Richard Henderson
2010-04-22 23:47 ` [Qemu-devel] [PATCH] Remove IO_MEM_SUBWIDTH Richard Henderson
1 sibling, 1 reply; 18+ messages in thread
From: Blue Swirl @ 2010-04-22 20:01 UTC (permalink / raw)
To: Richard Henderson; +Cc: agraf, qemu-devel, paul
On 4/22/10, Richard Henderson <rth@twiddle.net> wrote:
> On 04/22/2010 12:38 PM, Blue Swirl wrote:
> > Subpages are used when there are several devices on the same page.
> > It's needed for at least Sparc32.
>
>
> That's fine.
>
>
> > Subwidth (with NULL) is used mainly to indicate that the device does
> > not accept accesses in some access widths. Sparc32 and Sparc64 need
> > this (or some other way to signal bus errors for bad access widths).
>
>
> This is also fine. Although by using NULL all you'd get is a qemu
> null pointer dereference; I suppose this might get caught and
> translated to an cpu exception, but I think it would be preferable
> long-term to be more explicit about this and fill in the entries
> with a function that would explicitly raise the exception.
Perhaps also the bus layer could do something here.
> What this *does* confirm for me is that we don't need to support
> multiple devices at the same address, differentiated by the size
> of the reference. Which is something that the current subpage
> implementation actually supports.
Yes, I also agree it's a bit overkill.
> I'll submit a patch to clean this up.
>
>
> > In fact, many system devices on Sparc64 should only accept 64 bit
> > accesses, but currently we can't enforce this.
>
>
> Cool. This is something that should be fixed by the end result
> of the driver interface re-design that pbrook and I have been
> discussing on IRC.
Interesting. Could you make a summary of the design for the benefit of the list?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths
2010-04-22 20:01 ` Blue Swirl
@ 2010-04-22 21:19 ` Richard Henderson
2010-04-23 18:30 ` Blue Swirl
2010-05-28 20:45 ` Paul Brook
0 siblings, 2 replies; 18+ messages in thread
From: Richard Henderson @ 2010-04-22 21:19 UTC (permalink / raw)
To: Blue Swirl; +Cc: paul, agraf, qemu-devel
On 04/22/2010 01:01 PM, Blue Swirl wrote:
>> This is also fine. Although by using NULL all you'd get is a qemu
>> null pointer dereference; I suppose this might get caught and
>> translated to an cpu exception, but I think it would be preferable
>> long-term to be more explicit about this and fill in the entries
>> with a function that would explicitly raise the exception.
>
> Perhaps also the bus layer could do something here.
What do you mean?
> Interesting. Could you make a summary of the design for the benefit of the list?
The basic device interface looks like
+/* Object based memory region registration. */
+
+typedef struct MemoryRegion MemoryRegion;
+
+typedef uint32_t MemoryCallbackRead(MemoryRegion *, target_phys_addr_t ofs);
+typedef uint64_t MemoryCallbackRead64(MemoryRegion *, target_phys_addr_t ofs);
+typedef void MemoryCallbackWrite(MemoryRegion *, target_phys_addr_t ofs,
+ uint32_t val);
+typedef void MemoryCallbackWrite64(MemoryRegion *, target_phys_addr_t ofs,
+ uint64_t val);
+
+typedef struct MemoryCallbackInfo {
+ MemoryCallbackRead *read8;
+ MemoryCallbackRead *read16;
+ MemoryCallbackRead *read32;
+ MemoryCallbackRead64 *read64;
+
+ MemoryCallbackWrite *write8;
+ MemoryCallbackWrite *write16;
+ MemoryCallbackWrite *write32;
+ MemoryCallbackWrite64 *write64;
+
+ /* This describes RAM. The callbacks above need not be used, and
+ the host memory backing the RAM begins at qemu_get_ram_ptr_r. */
+ _Bool ram;
+
+ /* Legacy shim: propagate the IO_MEM_ROMD bit. */
+ _Bool romd;
+} MemoryCallbackInfo;
+
+/* This structure describes the region. It is logically opaque -- drivers
+ should not be peeking inside. But in the interest of efficiency we want
+ to directly embed this structure in the driver's private strucure. In
+ this way we can avoid having to have an extra table of opaque pointers
+ for consumption by the driver. The intended use is
+
+ struct FooDeviceState {
+ DeviceState qdev;
+ MemoryRegion mem_region;
+ ...
+ };
+
+ static uint32_t foo_read8(MemoryRegion *region, target_phys_addr_t ofs)
+ {
+ FooDeviceState *s = container_of(region, FooDeviceState, mem_region);
+ ...
+ }
+*/
+
+struct MemoryRegion {
+ const MemoryCallbackInfo *cb;
+ target_phys_addr_t start;
+ target_phys_addr_t size;
+
+ /* If this value is not -1, this memory region is registred in
+ the io_mem_region array, used by softmmu_header.h to resolve
+ memory-mapped operations in the guest address space. */
+ int io_index;
+};
+
+/* Register a memory region at START_ADDR/SIZE. The REGION structure will
+ be initialized appropriately for DEV using CB as the operation set. */
+extern void cpu_register_memory_region(MemoryRegion *region,
+ const MemoryCallbackInfo *cb,
+ target_phys_addr_t start_addr,
+ target_phys_addr_t size);
+
+/* Unregister a memory region. */
+extern void cpu_unregister_memory_region(MemoryRegion *);
+
+/* Allocate ram for use with cpu_register_memory_region. */
+extern const MemoryCallbackInfo *qemu_ram_alloc_r(ram_addr_t);
+extern void qemu_ram_free_r(const MemoryCallbackInfo *);
The Basic Idea is that we have a MemoryRegion object that describes
a contiguous mapping within the guest address space. This object
needs to handle RAM, ROM and devices. The desire to handle memory
and devices the same comes from the wish to have PCI device BARs
show up as plain memory in the TLB as plain memory, and to be able
to handle all PCI device regions identically within sysbus.
There are a number of restrictions on the interface above what we
currently support:
* Objects may not be mapped over the top of existing objects.
This is most abused by pflash devices with their IO_MEM_ROMD thing.
* Objects must be unmapped explicitly, rather than mapping
IO_MEM_UNASSIGNED over the top of an object. This is most
abused by the PCI subsystem.
The use of the MemoryRegion pointer doubling as the device's opaque
pointer means that it's easy to implement generic helpers such as
uint64_t composite_read64(MemoryRegion *region, target_phys_addr_t ofs)
{
uint32_t v1 = region->cb->read32(region, ofs);
uint32_t v2 = region->cb->read32(region, ofs + 4);
return combine_for_target_endian(v1, v2);
}
or even
uin32_t invalid_read8(MemoryRegion *region, target_phys_addr_t ofs)
{
do_unassigned_access(region->start + ofs, 0, 0, 0, 1);
return 0;
}
without having to have supplementary opaque pointer caches, as we
currently do for e.g. subpage_t.
These can be entered into the device's MemoryCallbackInfo structure
so that we're sure that there's defined semantics for every access,
and the device need not invent its own invalid_read functions as
quite a few do now.
I will admit that I do not yet have a good replacement for IO_MEM_ROMD,
or toggling the read-only bit on a RAM region. Or even for the way
that pc.c allocates RAM around the 640k-1M hole.
r~
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH] Remove IO_MEM_SUBWIDTH.
2010-04-22 19:55 ` Richard Henderson
2010-04-22 20:01 ` Blue Swirl
@ 2010-04-22 23:47 ` Richard Henderson
2010-04-25 15:06 ` [Qemu-devel] " Blue Swirl
1 sibling, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2010-04-22 23:47 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel
Greatly simplify the subpage implementation by not supporting
multiple devices at the same address at different widths. We
don't need full copies of mem_read/mem_write/opaque for each
address, only a single index back into the main io_mem_* arrays.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
cpu-common.h | 1 -
exec.c | 113 ++++++++++++++++++---------------------------------------
2 files changed, 36 insertions(+), 78 deletions(-)
diff --git a/cpu-common.h b/cpu-common.h
index b730ca0..b24cecc 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -125,7 +125,6 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr,
/* Acts like a ROM when read and like a device when written. */
#define IO_MEM_ROMD (1)
#define IO_MEM_SUBPAGE (2)
-#define IO_MEM_SUBWIDTH (4)
#endif
diff --git a/exec.c b/exec.c
index 43366ac..14d1fd7 100644
--- a/exec.c
+++ b/exec.c
@@ -2549,16 +2549,15 @@ static inline void tlb_set_dirty(CPUState *env,
#define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
typedef struct subpage_t {
target_phys_addr_t base;
- CPUReadMemoryFunc * const *mem_read[TARGET_PAGE_SIZE][4];
- CPUWriteMemoryFunc * const *mem_write[TARGET_PAGE_SIZE][4];
- void *opaque[TARGET_PAGE_SIZE][2][4];
- ram_addr_t region_offset[TARGET_PAGE_SIZE][2][4];
+ ram_addr_t sub_io_index[TARGET_PAGE_SIZE];
+ ram_addr_t region_offset[TARGET_PAGE_SIZE];
} subpage_t;
static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
ram_addr_t memory, ram_addr_t region_offset);
-static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
- ram_addr_t orig_memory, ram_addr_t region_offset);
+static subpage_t *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
+ ram_addr_t orig_memory,
+ ram_addr_t region_offset);
#define CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr, end_addr2, \
need_subpage) \
do { \
@@ -2596,7 +2595,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
PhysPageDesc *p;
CPUState *env;
ram_addr_t orig_size = size;
- void *subpage;
+ subpage_t *subpage;
cpu_notify_set_memory(start_addr, size, phys_offset);
@@ -2615,7 +2614,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr, end_addr2,
need_subpage);
- if (need_subpage || phys_offset & IO_MEM_SUBWIDTH) {
+ if (need_subpage) {
if (!(orig_memory & IO_MEM_SUBPAGE)) {
subpage = subpage_init((addr & TARGET_PAGE_MASK),
&p->phys_offset, orig_memory,
@@ -2647,7 +2646,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr,
end_addr2, need_subpage);
- if (need_subpage || phys_offset & IO_MEM_SUBWIDTH) {
+ if (need_subpage) {
subpage = subpage_init((addr & TARGET_PAGE_MASK),
&p->phys_offset, IO_MEM_UNASSIGNED,
addr & TARGET_PAGE_MASK);
@@ -3145,89 +3144,65 @@ static CPUWriteMemoryFunc * const watch_mem_write[3] = {
watch_mem_writel,
};
-static inline uint32_t subpage_readlen (subpage_t *mmio, target_phys_addr_t addr,
- unsigned int len)
+static inline uint32_t subpage_readlen (subpage_t *mmio,
+ target_phys_addr_t addr,
+ unsigned int len)
{
- uint32_t ret;
- unsigned int idx;
-
- idx = SUBPAGE_IDX(addr);
+ unsigned int idx = SUBPAGE_IDX(addr);
#if defined(DEBUG_SUBPAGE)
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],
- addr + mmio->region_offset[idx][0][len]);
- return ret;
+ addr += mmio->region_offset[idx];
+ idx = mmio->sub_io_index[idx];
+ return io_mem_read[idx][len](io_mem_opaque[idx], addr);
}
static inline void subpage_writelen (subpage_t *mmio, target_phys_addr_t addr,
- uint32_t value, unsigned int len)
+ uint32_t value, unsigned int len)
{
- unsigned int idx;
-
- idx = SUBPAGE_IDX(addr);
+ unsigned int idx = SUBPAGE_IDX(addr);
#if defined(DEBUG_SUBPAGE)
- printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d value %08x\n", __func__,
- mmio, len, addr, idx, value);
+ 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],
- addr + mmio->region_offset[idx][1][len],
- value);
+
+ addr += mmio->region_offset[idx];
+ idx = mmio->sub_io_index[idx];
+ io_mem_write[idx][len](io_mem_opaque[idx], addr, value);
}
static uint32_t subpage_readb (void *opaque, target_phys_addr_t addr)
{
-#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-
return subpage_readlen(opaque, addr, 0);
}
static void subpage_writeb (void *opaque, target_phys_addr_t addr,
uint32_t value)
{
-#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
-#endif
subpage_writelen(opaque, addr, value, 0);
}
static uint32_t subpage_readw (void *opaque, target_phys_addr_t addr)
{
-#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-
return subpage_readlen(opaque, addr, 1);
}
static void subpage_writew (void *opaque, target_phys_addr_t addr,
uint32_t value)
{
-#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
-#endif
subpage_writelen(opaque, addr, value, 1);
}
static uint32_t subpage_readl (void *opaque, target_phys_addr_t addr)
{
-#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
-#endif
-
return subpage_readlen(opaque, addr, 2);
}
-static void subpage_writel (void *opaque,
- target_phys_addr_t addr, uint32_t value)
+static void subpage_writel (void *opaque, target_phys_addr_t addr,
+ uint32_t value)
{
-#if defined(DEBUG_SUBPAGE)
- printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
-#endif
subpage_writelen(opaque, addr, value, 2);
}
@@ -3247,7 +3222,6 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
ram_addr_t memory, ram_addr_t region_offset)
{
int idx, eidx;
- unsigned int i;
if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE)
return -1;
@@ -3257,27 +3231,18 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
printf("%s: %p start %08x end %08x idx %08x eidx %08x mem %ld\n", __func__,
mmio, start, end, idx, eidx, memory);
#endif
- memory >>= IO_MEM_SHIFT;
+ memory = (memory >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
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->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->opaque[idx][1][i] = io_mem_opaque[memory];
- mmio->region_offset[idx][1][i] = region_offset;
- }
- }
+ mmio->sub_io_index[idx] = memory;
+ mmio->region_offset[idx] = region_offset;
}
return 0;
}
-static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
- ram_addr_t orig_memory, ram_addr_t region_offset)
+static subpage_t *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
+ ram_addr_t orig_memory,
+ ram_addr_t region_offset)
{
subpage_t *mmio;
int subpage_memory;
@@ -3291,8 +3256,7 @@ static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
mmio, base, TARGET_PAGE_SIZE, subpage_memory);
#endif
*phys = subpage_memory | IO_MEM_SUBPAGE;
- subpage_register(mmio, 0, TARGET_PAGE_SIZE - 1, orig_memory,
- region_offset);
+ subpage_register(mmio, 0, TARGET_PAGE_SIZE-1, orig_memory, region_offset);
return mmio;
}
@@ -3322,8 +3286,6 @@ static int cpu_register_io_memory_fixed(int io_index,
CPUWriteMemoryFunc * const *mem_write,
void *opaque)
{
- int i, subwidth = 0;
-
if (io_index <= 0) {
io_index = get_free_io_mem_idx();
if (io_index == -1)
@@ -3334,14 +3296,11 @@ static int cpu_register_io_memory_fixed(int io_index,
return -1;
}
- for(i = 0;i < 3; 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];
- }
+ memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
+ memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
io_mem_opaque[io_index] = opaque;
- return (io_index << IO_MEM_SHIFT) | subwidth;
+
+ return (io_index << IO_MEM_SHIFT);
}
int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
--
1.6.6.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths
2010-04-22 21:19 ` Richard Henderson
@ 2010-04-23 18:30 ` Blue Swirl
2010-05-28 20:45 ` Paul Brook
1 sibling, 0 replies; 18+ messages in thread
From: Blue Swirl @ 2010-04-23 18:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: paul, agraf, qemu-devel
On 4/23/10, Richard Henderson <rth@twiddle.net> wrote:
> On 04/22/2010 01:01 PM, Blue Swirl wrote:
> >> This is also fine. Although by using NULL all you'd get is a qemu
> >> null pointer dereference; I suppose this might get caught and
> >> translated to an cpu exception, but I think it would be preferable
> >> long-term to be more explicit about this and fill in the entries
> >> with a function that would explicitly raise the exception.
> >
> > Perhaps also the bus layer could do something here.
>
>
> What do you mean?
If we had a bus layer that handled conversion from bus width to device
width (discussed earlier), it could as well handle the cases where
instead of conversion, a bus error should be reported.
> > Interesting. Could you make a summary of the design for the benefit of the list?
>
>
> The basic device interface looks like
>
> +/* Object based memory region registration. */
> +
> +typedef struct MemoryRegion MemoryRegion;
> +
> +typedef uint32_t MemoryCallbackRead(MemoryRegion *, target_phys_addr_t ofs);
> +typedef uint64_t MemoryCallbackRead64(MemoryRegion *, target_phys_addr_t ofs);
> +typedef void MemoryCallbackWrite(MemoryRegion *, target_phys_addr_t ofs,
> + uint32_t val);
> +typedef void MemoryCallbackWrite64(MemoryRegion *, target_phys_addr_t ofs,
> + uint64_t val);
> +
> +typedef struct MemoryCallbackInfo {
> + MemoryCallbackRead *read8;
> + MemoryCallbackRead *read16;
> + MemoryCallbackRead *read32;
> + MemoryCallbackRead64 *read64;
> +
> + MemoryCallbackWrite *write8;
> + MemoryCallbackWrite *write16;
> + MemoryCallbackWrite *write32;
> + MemoryCallbackWrite64 *write64;
> +
> + /* This describes RAM. The callbacks above need not be used, and
> + the host memory backing the RAM begins at qemu_get_ram_ptr_r. */
> + _Bool ram;
> +
> + /* Legacy shim: propagate the IO_MEM_ROMD bit. */
> + _Bool romd;
> +} MemoryCallbackInfo;
> +
> +/* This structure describes the region. It is logically opaque -- drivers
> + should not be peeking inside. But in the interest of efficiency we want
> + to directly embed this structure in the driver's private strucure. In
> + this way we can avoid having to have an extra table of opaque pointers
> + for consumption by the driver. The intended use is
> +
> + struct FooDeviceState {
> + DeviceState qdev;
> + MemoryRegion mem_region;
> + ...
> + };
> +
> + static uint32_t foo_read8(MemoryRegion *region, target_phys_addr_t ofs)
> + {
> + FooDeviceState *s = container_of(region, FooDeviceState, mem_region);
> + ...
> + }
> +*/
> +
> +struct MemoryRegion {
> + const MemoryCallbackInfo *cb;
> + target_phys_addr_t start;
> + target_phys_addr_t size;
> +
> + /* If this value is not -1, this memory region is registred in
> + the io_mem_region array, used by softmmu_header.h to resolve
> + memory-mapped operations in the guest address space. */
> + int io_index;
> +};
> +
> +/* Register a memory region at START_ADDR/SIZE. The REGION structure will
> + be initialized appropriately for DEV using CB as the operation set. */
> +extern void cpu_register_memory_region(MemoryRegion *region,
> + const MemoryCallbackInfo *cb,
> + target_phys_addr_t start_addr,
> + target_phys_addr_t size);
> +
> +/* Unregister a memory region. */
> +extern void cpu_unregister_memory_region(MemoryRegion *);
> +
> +/* Allocate ram for use with cpu_register_memory_region. */
> +extern const MemoryCallbackInfo *qemu_ram_alloc_r(ram_addr_t);
> +extern void qemu_ram_free_r(const MemoryCallbackInfo *);
>
> The Basic Idea is that we have a MemoryRegion object that describes
> a contiguous mapping within the guest address space. This object
> needs to handle RAM, ROM and devices. The desire to handle memory
> and devices the same comes from the wish to have PCI device BARs
> show up as plain memory in the TLB as plain memory, and to be able
> to handle all PCI device regions identically within sysbus.
>
> There are a number of restrictions on the interface above what we
> currently support:
>
> * Objects may not be mapped over the top of existing objects.
> This is most abused by pflash devices with their IO_MEM_ROMD thing.
Perhaps with small changes we could make this design stackable, so
that a device providing a bus inside another bus could use the same
registration functions, only the bus handle would change. Like:
upa = bus_register_memory_region(cpu_bus,,);
pci = bus_register_memory_region(upa,,);
ebus = bus_register_memory_region(pci,,):
ide = bus_register_memory_region(ebus,,):
Currently "cpu_bus" is always implicit and pci has separate
registration functions.
I'd suppose that ROMD, subpages and unassigned memory handlers could
be implemented by stacking instead of special casing them.
> * Objects must be unmapped explicitly, rather than mapping
> IO_MEM_UNASSIGNED over the top of an object. This is most
> abused by the PCI subsystem.
>
> The use of the MemoryRegion pointer doubling as the device's opaque
> pointer means that it's easy to implement generic helpers such as
>
> uint64_t composite_read64(MemoryRegion *region, target_phys_addr_t ofs)
> {
> uint32_t v1 = region->cb->read32(region, ofs);
> uint32_t v2 = region->cb->read32(region, ofs + 4);
>
> return combine_for_target_endian(v1, v2);
> }
>
> or even
>
> uin32_t invalid_read8(MemoryRegion *region, target_phys_addr_t ofs)
> {
> do_unassigned_access(region->start + ofs, 0, 0, 0, 1);
> return 0;
> }
>
> without having to have supplementary opaque pointer caches, as we
> currently do for e.g. subpage_t.
>
> These can be entered into the device's MemoryCallbackInfo structure
> so that we're sure that there's defined semantics for every access,
> and the device need not invent its own invalid_read functions as
> quite a few do now.
>
> I will admit that I do not yet have a good replacement for IO_MEM_ROMD,
> or toggling the read-only bit on a RAM region. Or even for the way
> that pc.c allocates RAM around the 640k-1M hole.
Read-only bit could be implemented in the stackable design by creating
a simple bus which can suppress writes when configured so, The device
behind would be just RAM.
Likewise for PC RAM mapping, the memory controller provides a bus
where the addresses may be twisted as needed before passing to
underlying RAM.
However, the outcome of the Generic DMA discussions (last year?) was
that instead of read/write functions, a mapping API would be better
because then we have the possibility to do zero copy DMA. Perhaps the
same applies here, too. This would be a bigger change obviously but if
there will be widespread changes to devices, it would be nice to get
this right.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.
2010-04-22 23:47 ` [Qemu-devel] [PATCH] Remove IO_MEM_SUBWIDTH Richard Henderson
@ 2010-04-25 15:06 ` Blue Swirl
2010-04-26 21:54 ` Artyom Tarasenko
0 siblings, 1 reply; 18+ messages in thread
From: Blue Swirl @ 2010-04-25 15:06 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Thanks, applied.
On 4/23/10, Richard Henderson <rth@twiddle.net> wrote:
> Greatly simplify the subpage implementation by not supporting
> multiple devices at the same address at different widths. We
> don't need full copies of mem_read/mem_write/opaque for each
> address, only a single index back into the main io_mem_* arrays.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> cpu-common.h | 1 -
> exec.c | 113 ++++++++++++++++++---------------------------------------
> 2 files changed, 36 insertions(+), 78 deletions(-)
>
> diff --git a/cpu-common.h b/cpu-common.h
> index b730ca0..b24cecc 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -125,7 +125,6 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr,
> /* Acts like a ROM when read and like a device when written. */
> #define IO_MEM_ROMD (1)
> #define IO_MEM_SUBPAGE (2)
> -#define IO_MEM_SUBWIDTH (4)
>
> #endif
>
> diff --git a/exec.c b/exec.c
> index 43366ac..14d1fd7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2549,16 +2549,15 @@ static inline void tlb_set_dirty(CPUState *env,
> #define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
> typedef struct subpage_t {
> target_phys_addr_t base;
> - CPUReadMemoryFunc * const *mem_read[TARGET_PAGE_SIZE][4];
> - CPUWriteMemoryFunc * const *mem_write[TARGET_PAGE_SIZE][4];
> - void *opaque[TARGET_PAGE_SIZE][2][4];
> - ram_addr_t region_offset[TARGET_PAGE_SIZE][2][4];
> + ram_addr_t sub_io_index[TARGET_PAGE_SIZE];
> + ram_addr_t region_offset[TARGET_PAGE_SIZE];
> } subpage_t;
>
> static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
> ram_addr_t memory, ram_addr_t region_offset);
> -static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
> - ram_addr_t orig_memory, ram_addr_t region_offset);
> +static subpage_t *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
> + ram_addr_t orig_memory,
> + ram_addr_t region_offset);
> #define CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr, end_addr2, \
> need_subpage) \
> do { \
> @@ -2596,7 +2595,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
> PhysPageDesc *p;
> CPUState *env;
> ram_addr_t orig_size = size;
> - void *subpage;
> + subpage_t *subpage;
>
> cpu_notify_set_memory(start_addr, size, phys_offset);
>
> @@ -2615,7 +2614,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
>
> CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr, end_addr2,
> need_subpage);
> - if (need_subpage || phys_offset & IO_MEM_SUBWIDTH) {
> + if (need_subpage) {
> if (!(orig_memory & IO_MEM_SUBPAGE)) {
> subpage = subpage_init((addr & TARGET_PAGE_MASK),
> &p->phys_offset, orig_memory,
> @@ -2647,7 +2646,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
> CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr,
> end_addr2, need_subpage);
>
> - if (need_subpage || phys_offset & IO_MEM_SUBWIDTH) {
> + if (need_subpage) {
> subpage = subpage_init((addr & TARGET_PAGE_MASK),
> &p->phys_offset, IO_MEM_UNASSIGNED,
> addr & TARGET_PAGE_MASK);
> @@ -3145,89 +3144,65 @@ static CPUWriteMemoryFunc * const watch_mem_write[3] = {
> watch_mem_writel,
> };
>
> -static inline uint32_t subpage_readlen (subpage_t *mmio, target_phys_addr_t addr,
> - unsigned int len)
> +static inline uint32_t subpage_readlen (subpage_t *mmio,
> + target_phys_addr_t addr,
> + unsigned int len)
> {
> - uint32_t ret;
> - unsigned int idx;
> -
> - idx = SUBPAGE_IDX(addr);
> + unsigned int idx = SUBPAGE_IDX(addr);
> #if defined(DEBUG_SUBPAGE)
> 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],
> - addr + mmio->region_offset[idx][0][len]);
>
> - return ret;
> + addr += mmio->region_offset[idx];
> + idx = mmio->sub_io_index[idx];
> + return io_mem_read[idx][len](io_mem_opaque[idx], addr);
> }
>
> static inline void subpage_writelen (subpage_t *mmio, target_phys_addr_t addr,
> - uint32_t value, unsigned int len)
> + uint32_t value, unsigned int len)
> {
> - unsigned int idx;
> -
> - idx = SUBPAGE_IDX(addr);
> + unsigned int idx = SUBPAGE_IDX(addr);
> #if defined(DEBUG_SUBPAGE)
> - printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d value %08x\n", __func__,
> - mmio, len, addr, idx, value);
> + 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],
> - addr + mmio->region_offset[idx][1][len],
> - value);
> +
> + addr += mmio->region_offset[idx];
> + idx = mmio->sub_io_index[idx];
> + io_mem_write[idx][len](io_mem_opaque[idx], addr, value);
> }
>
> static uint32_t subpage_readb (void *opaque, target_phys_addr_t addr)
> {
> -#if defined(DEBUG_SUBPAGE)
> - printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
> -#endif
> -
> return subpage_readlen(opaque, addr, 0);
> }
>
> static void subpage_writeb (void *opaque, target_phys_addr_t addr,
> uint32_t value)
> {
> -#if defined(DEBUG_SUBPAGE)
> - printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
> -#endif
> subpage_writelen(opaque, addr, value, 0);
> }
>
> static uint32_t subpage_readw (void *opaque, target_phys_addr_t addr)
> {
> -#if defined(DEBUG_SUBPAGE)
> - printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
> -#endif
> -
> return subpage_readlen(opaque, addr, 1);
> }
>
> static void subpage_writew (void *opaque, target_phys_addr_t addr,
> uint32_t value)
> {
> -#if defined(DEBUG_SUBPAGE)
> - printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
> -#endif
> subpage_writelen(opaque, addr, value, 1);
> }
>
> static uint32_t subpage_readl (void *opaque, target_phys_addr_t addr)
> {
> -#if defined(DEBUG_SUBPAGE)
> - printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
> -#endif
> -
> return subpage_readlen(opaque, addr, 2);
> }
>
> -static void subpage_writel (void *opaque,
> - target_phys_addr_t addr, uint32_t value)
> +static void subpage_writel (void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> {
> -#if defined(DEBUG_SUBPAGE)
> - printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
> -#endif
> subpage_writelen(opaque, addr, value, 2);
> }
>
> @@ -3247,7 +3222,6 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
> ram_addr_t memory, ram_addr_t region_offset)
> {
> int idx, eidx;
> - unsigned int i;
>
> if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE)
> return -1;
> @@ -3257,27 +3231,18 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
> printf("%s: %p start %08x end %08x idx %08x eidx %08x mem %ld\n", __func__,
> mmio, start, end, idx, eidx, memory);
> #endif
> - memory >>= IO_MEM_SHIFT;
> + memory = (memory >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
> 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->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->opaque[idx][1][i] = io_mem_opaque[memory];
> - mmio->region_offset[idx][1][i] = region_offset;
> - }
> - }
> + mmio->sub_io_index[idx] = memory;
> + mmio->region_offset[idx] = region_offset;
> }
>
> return 0;
> }
>
> -static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
> - ram_addr_t orig_memory, ram_addr_t region_offset)
> +static subpage_t *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
> + ram_addr_t orig_memory,
> + ram_addr_t region_offset)
> {
> subpage_t *mmio;
> int subpage_memory;
> @@ -3291,8 +3256,7 @@ static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
> mmio, base, TARGET_PAGE_SIZE, subpage_memory);
> #endif
> *phys = subpage_memory | IO_MEM_SUBPAGE;
> - subpage_register(mmio, 0, TARGET_PAGE_SIZE - 1, orig_memory,
> - region_offset);
> + subpage_register(mmio, 0, TARGET_PAGE_SIZE-1, orig_memory, region_offset);
>
> return mmio;
> }
> @@ -3322,8 +3286,6 @@ static int cpu_register_io_memory_fixed(int io_index,
> CPUWriteMemoryFunc * const *mem_write,
> void *opaque)
> {
> - int i, subwidth = 0;
> -
> if (io_index <= 0) {
> io_index = get_free_io_mem_idx();
> if (io_index == -1)
> @@ -3334,14 +3296,11 @@ static int cpu_register_io_memory_fixed(int io_index,
> return -1;
> }
>
> - for(i = 0;i < 3; 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];
> - }
> + memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
> + memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
> io_mem_opaque[io_index] = opaque;
> - return (io_index << IO_MEM_SHIFT) | subwidth;
> +
> + return (io_index << IO_MEM_SHIFT);
> }
>
> int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
>
> --
> 1.6.6.1
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.
2010-04-25 15:06 ` [Qemu-devel] " Blue Swirl
@ 2010-04-26 21:54 ` Artyom Tarasenko
2010-04-27 18:30 ` Richard Henderson
0 siblings, 1 reply; 18+ messages in thread
From: Artyom Tarasenko @ 2010-04-26 21:54 UTC (permalink / raw)
To: Blue Swirl, Richard Henderson; +Cc: qemu-devel
This patch introduces a regression. qemu crashes on lance test:
6.2.1 i/o lance getid Pass
6.2.2 i/o lance csr
Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0 0x0000000000000000 in ?? ()
#1 0x00000000004aaa3c in cpu_physical_memory_rw (addr=2017460240,
buf=0x7fffffffda67 "", len=1, is_write=1)
at /mnt/terra/projects/vanilla/qemu/exec.c:3427
#2 0x00000000004aaead in cpu_physical_memory_write (len=<value
optimized out>, buf=<value optimized out>,
addr=<value optimized out>) at
/mnt/terra/projects/vanilla/qemu/cpu-common.h:65
#3 stb_phys (len=<value optimized out>, buf=<value optimized out>,
addr=<value optimized out>)
at /mnt/terra/projects/vanilla/qemu/exec.c:3861
#4 0x0000000040010802 in ?? ()
#5 0x0000000000005000 in ?? ()
#6 0xa8f9b203db7724d3 in ?? ()
#7 0x0000000000bef270 in ?? ()
#8 0x0000000000005000 in ?? ()
#9 0x00007ffff5880b60 in ?? ()
#10 0x0000000000000100 in ?? ()
#11 0x0000000000005a7c in ?? ()
#12 0x00000000004abcb1 in tb_gen_code (env=0x78400010, pc=<value
optimized out>, cs_base=<value optimized out>,
flags=<value optimized out>, cflags=<value optimized out>) at
/mnt/terra/projects/vanilla/qemu/exec.c:997
#13 0x00000000004ad69d in cpu_sparc_exec (env1=<value optimized out>)
at /mnt/terra/projects/vanilla/qemu/cpu-exec.c:620
#14 0x0000000000406fc0 in qemu_cpu_exec (env=<value optimized out>) at
/mnt/terra/projects/vanilla/qemu/cpus.c:716
#15 tcg_cpu_exec (env=<value optimized out>) at
/mnt/terra/projects/vanilla/qemu/cpus.c:746
#16 0x00000000004ed715 in main_loop () at
/mnt/terra/projects/vanilla/qemu/vl.c:1961
#17 main () at /mnt/terra/projects/vanilla/qemu/vl.c:3828
2010/4/25 Blue Swirl <blauwirbel@gmail.com>:
> Thanks, applied.
>
> On 4/23/10, Richard Henderson <rth@twiddle.net> wrote:
>> Greatly simplify the subpage implementation by not supporting
>> multiple devices at the same address at different widths. We
>> don't need full copies of mem_read/mem_write/opaque for each
>> address, only a single index back into the main io_mem_* arrays.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>> cpu-common.h | 1 -
>> exec.c | 113 ++++++++++++++++++---------------------------------------
>> 2 files changed, 36 insertions(+), 78 deletions(-)
>>
>> diff --git a/cpu-common.h b/cpu-common.h
>> index b730ca0..b24cecc 100644
>> --- a/cpu-common.h
>> +++ b/cpu-common.h
>> @@ -125,7 +125,6 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr,
>> /* Acts like a ROM when read and like a device when written. */
>> #define IO_MEM_ROMD (1)
>> #define IO_MEM_SUBPAGE (2)
>> -#define IO_MEM_SUBWIDTH (4)
>>
>> #endif
>>
>> diff --git a/exec.c b/exec.c
>> index 43366ac..14d1fd7 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -2549,16 +2549,15 @@ static inline void tlb_set_dirty(CPUState *env,
>> #define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
>> typedef struct subpage_t {
>> target_phys_addr_t base;
>> - CPUReadMemoryFunc * const *mem_read[TARGET_PAGE_SIZE][4];
>> - CPUWriteMemoryFunc * const *mem_write[TARGET_PAGE_SIZE][4];
>> - void *opaque[TARGET_PAGE_SIZE][2][4];
>> - ram_addr_t region_offset[TARGET_PAGE_SIZE][2][4];
>> + ram_addr_t sub_io_index[TARGET_PAGE_SIZE];
>> + ram_addr_t region_offset[TARGET_PAGE_SIZE];
>> } subpage_t;
>>
>> static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
>> ram_addr_t memory, ram_addr_t region_offset);
>> -static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
>> - ram_addr_t orig_memory, ram_addr_t region_offset);
>> +static subpage_t *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
>> + ram_addr_t orig_memory,
>> + ram_addr_t region_offset);
>> #define CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr, end_addr2, \
>> need_subpage) \
>> do { \
>> @@ -2596,7 +2595,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
>> PhysPageDesc *p;
>> CPUState *env;
>> ram_addr_t orig_size = size;
>> - void *subpage;
>> + subpage_t *subpage;
>>
>> cpu_notify_set_memory(start_addr, size, phys_offset);
>>
>> @@ -2615,7 +2614,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
>>
>> CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr, end_addr2,
>> need_subpage);
>> - if (need_subpage || phys_offset & IO_MEM_SUBWIDTH) {
>> + if (need_subpage) {
>> if (!(orig_memory & IO_MEM_SUBPAGE)) {
>> subpage = subpage_init((addr & TARGET_PAGE_MASK),
>> &p->phys_offset, orig_memory,
>> @@ -2647,7 +2646,7 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
>> CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr,
>> end_addr2, need_subpage);
>>
>> - if (need_subpage || phys_offset & IO_MEM_SUBWIDTH) {
>> + if (need_subpage) {
>> subpage = subpage_init((addr & TARGET_PAGE_MASK),
>> &p->phys_offset, IO_MEM_UNASSIGNED,
>> addr & TARGET_PAGE_MASK);
>> @@ -3145,89 +3144,65 @@ static CPUWriteMemoryFunc * const watch_mem_write[3] = {
>> watch_mem_writel,
>> };
>>
>> -static inline uint32_t subpage_readlen (subpage_t *mmio, target_phys_addr_t addr,
>> - unsigned int len)
>> +static inline uint32_t subpage_readlen (subpage_t *mmio,
>> + target_phys_addr_t addr,
>> + unsigned int len)
>> {
>> - uint32_t ret;
>> - unsigned int idx;
>> -
>> - idx = SUBPAGE_IDX(addr);
>> + unsigned int idx = SUBPAGE_IDX(addr);
>> #if defined(DEBUG_SUBPAGE)
>> 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],
>> - addr + mmio->region_offset[idx][0][len]);
>>
>> - return ret;
>> + addr += mmio->region_offset[idx];
>> + idx = mmio->sub_io_index[idx];
>> + return io_mem_read[idx][len](io_mem_opaque[idx], addr);
>> }
>>
>> static inline void subpage_writelen (subpage_t *mmio, target_phys_addr_t addr,
>> - uint32_t value, unsigned int len)
>> + uint32_t value, unsigned int len)
>> {
>> - unsigned int idx;
>> -
>> - idx = SUBPAGE_IDX(addr);
>> + unsigned int idx = SUBPAGE_IDX(addr);
>> #if defined(DEBUG_SUBPAGE)
>> - printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d value %08x\n", __func__,
>> - mmio, len, addr, idx, value);
>> + 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],
>> - addr + mmio->region_offset[idx][1][len],
>> - value);
>> +
>> + addr += mmio->region_offset[idx];
>> + idx = mmio->sub_io_index[idx];
>> + io_mem_write[idx][len](io_mem_opaque[idx], addr, value);
>> }
>>
>> static uint32_t subpage_readb (void *opaque, target_phys_addr_t addr)
>> {
>> -#if defined(DEBUG_SUBPAGE)
>> - printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
>> -#endif
>> -
>> return subpage_readlen(opaque, addr, 0);
>> }
>>
>> static void subpage_writeb (void *opaque, target_phys_addr_t addr,
>> uint32_t value)
>> {
>> -#if defined(DEBUG_SUBPAGE)
>> - printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
>> -#endif
>> subpage_writelen(opaque, addr, value, 0);
>> }
>>
>> static uint32_t subpage_readw (void *opaque, target_phys_addr_t addr)
>> {
>> -#if defined(DEBUG_SUBPAGE)
>> - printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
>> -#endif
>> -
>> return subpage_readlen(opaque, addr, 1);
>> }
>>
>> static void subpage_writew (void *opaque, target_phys_addr_t addr,
>> uint32_t value)
>> {
>> -#if defined(DEBUG_SUBPAGE)
>> - printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
>> -#endif
>> subpage_writelen(opaque, addr, value, 1);
>> }
>>
>> static uint32_t subpage_readl (void *opaque, target_phys_addr_t addr)
>> {
>> -#if defined(DEBUG_SUBPAGE)
>> - printf("%s: addr " TARGET_FMT_plx "\n", __func__, addr);
>> -#endif
>> -
>> return subpage_readlen(opaque, addr, 2);
>> }
>>
>> -static void subpage_writel (void *opaque,
>> - target_phys_addr_t addr, uint32_t value)
>> +static void subpage_writel (void *opaque, target_phys_addr_t addr,
>> + uint32_t value)
>> {
>> -#if defined(DEBUG_SUBPAGE)
>> - printf("%s: addr " TARGET_FMT_plx " val %08x\n", __func__, addr, value);
>> -#endif
>> subpage_writelen(opaque, addr, value, 2);
>> }
>>
>> @@ -3247,7 +3222,6 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
>> ram_addr_t memory, ram_addr_t region_offset)
>> {
>> int idx, eidx;
>> - unsigned int i;
>>
>> if (start >= TARGET_PAGE_SIZE || end >= TARGET_PAGE_SIZE)
>> return -1;
>> @@ -3257,27 +3231,18 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
>> printf("%s: %p start %08x end %08x idx %08x eidx %08x mem %ld\n", __func__,
>> mmio, start, end, idx, eidx, memory);
>> #endif
>> - memory >>= IO_MEM_SHIFT;
>> + memory = (memory >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
>> 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->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->opaque[idx][1][i] = io_mem_opaque[memory];
>> - mmio->region_offset[idx][1][i] = region_offset;
>> - }
>> - }
>> + mmio->sub_io_index[idx] = memory;
>> + mmio->region_offset[idx] = region_offset;
>> }
>>
>> return 0;
>> }
>>
>> -static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
>> - ram_addr_t orig_memory, ram_addr_t region_offset)
>> +static subpage_t *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
>> + ram_addr_t orig_memory,
>> + ram_addr_t region_offset)
>> {
>> subpage_t *mmio;
>> int subpage_memory;
>> @@ -3291,8 +3256,7 @@ static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
>> mmio, base, TARGET_PAGE_SIZE, subpage_memory);
>> #endif
>> *phys = subpage_memory | IO_MEM_SUBPAGE;
>> - subpage_register(mmio, 0, TARGET_PAGE_SIZE - 1, orig_memory,
>> - region_offset);
>> + subpage_register(mmio, 0, TARGET_PAGE_SIZE-1, orig_memory, region_offset);
>>
>> return mmio;
>> }
>> @@ -3322,8 +3286,6 @@ static int cpu_register_io_memory_fixed(int io_index,
>> CPUWriteMemoryFunc * const *mem_write,
>> void *opaque)
>> {
>> - int i, subwidth = 0;
>> -
>> if (io_index <= 0) {
>> io_index = get_free_io_mem_idx();
>> if (io_index == -1)
>> @@ -3334,14 +3296,11 @@ static int cpu_register_io_memory_fixed(int io_index,
>> return -1;
>> }
>>
>> - for(i = 0;i < 3; 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];
>> - }
>> + memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
>> + memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
>> io_mem_opaque[io_index] = opaque;
>> - return (io_index << IO_MEM_SHIFT) | subwidth;
>> +
>> + return (io_index << IO_MEM_SHIFT);
>> }
>>
>> int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
>>
>> --
>> 1.6.6.1
>>
>>
>
>
>
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.
2010-04-26 21:54 ` Artyom Tarasenko
@ 2010-04-27 18:30 ` Richard Henderson
2010-04-28 19:29 ` Artyom Tarasenko
0 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2010-04-27 18:30 UTC (permalink / raw)
To: Artyom Tarasenko; +Cc: Blue Swirl, qemu-devel
On 04/26/2010 02:54 PM, Artyom Tarasenko wrote:
> This patch introduces a regression. qemu crashes on lance test:
I'm not sure how to get to this, since the sparc-test images don't
include ifconfig, and I havn't been able to find a sparc install
image that works (doesn't support sparc32 or sparc64 fails to load).
That said, try this and see if it works.
r~
---
diff --git a/exec.c b/exec.c
index 14d1fd7..572d3fd 100644
--- a/exec.c
+++ b/exec.c
@@ -3286,6 +3286,8 @@ static int cpu_register_io_memory_fixed(int io_index,
CPUWriteMemoryFunc * const *mem_write,
void *opaque)
{
+ int i;
+
if (io_index <= 0) {
io_index = get_free_io_mem_idx();
if (io_index == -1)
@@ -3296,8 +3298,14 @@ static int cpu_register_io_memory_fixed(int io_index,
return -1;
}
- memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
- memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
+ for (i = 0; i < 3; ++i) {
+ io_mem_read[io_index][i]
+ = (mem_read[i] ? mem_read[i] : unassigned_mem_read[i]);
+ }
+ for (i = 0; i < 3; ++i) {
+ io_mem_write[io_index][i]
+ = (mem_write[i] ? mem_write[i] : unassigned_mem_write[i]);
+ }
io_mem_opaque[io_index] = opaque;
return (io_index << IO_MEM_SHIFT);
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.
2010-04-27 18:30 ` Richard Henderson
@ 2010-04-28 19:29 ` Artyom Tarasenko
2010-05-06 20:25 ` Artyom Tarasenko
0 siblings, 1 reply; 18+ messages in thread
From: Artyom Tarasenko @ 2010-04-28 19:29 UTC (permalink / raw)
To: Richard Henderson; +Cc: Blue Swirl, qemu-devel
2010/4/27 Richard Henderson <rth@twiddle.net>:
> On 04/26/2010 02:54 PM, Artyom Tarasenko wrote:
>> This patch introduces a regression. qemu crashes on lance test:
>
> I'm not sure how to get to this, since the sparc-test images don't
> include ifconfig, and I havn't been able to find a sparc install
> image that works (doesn't support sparc32 or sparc64 fails to load).
>
> That said, try this and see if it works.
>
>
> r~
>
> ---
> diff --git a/exec.c b/exec.c
> index 14d1fd7..572d3fd 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3286,6 +3286,8 @@ static int cpu_register_io_memory_fixed(int io_index,
> CPUWriteMemoryFunc * const *mem_write,
> void *opaque)
> {
> + int i;
> +
> if (io_index <= 0) {
> io_index = get_free_io_mem_idx();
> if (io_index == -1)
> @@ -3296,8 +3298,14 @@ static int cpu_register_io_memory_fixed(int io_index,
> return -1;
> }
>
> - memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
> - memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
> + for (i = 0; i < 3; ++i) {
> + io_mem_read[io_index][i]
> + = (mem_read[i] ? mem_read[i] : unassigned_mem_read[i]);
> + }
> + for (i = 0; i < 3; ++i) {
> + io_mem_write[io_index][i]
> + = (mem_write[i] ? mem_write[i] : unassigned_mem_write[i]);
> + }
> io_mem_opaque[io_index] = opaque;
>
> return (io_index << IO_MEM_SHIFT);
>
Looks good, thanks.
Acked-by: Artyom Tarasenko <atar4qemu@gmail.com>
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.
2010-04-28 19:29 ` Artyom Tarasenko
@ 2010-05-06 20:25 ` Artyom Tarasenko
2010-05-07 15:28 ` Blue Swirl
0 siblings, 1 reply; 18+ messages in thread
From: Artyom Tarasenko @ 2010-05-06 20:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: Blue Swirl, qemu-devel
2010/4/28 Artyom Tarasenko <atar4qemu@googlemail.com>:
> 2010/4/27 Richard Henderson <rth@twiddle.net>:
>> On 04/26/2010 02:54 PM, Artyom Tarasenko wrote:
>>> This patch introduces a regression. qemu crashes on lance test:
>>
>> I'm not sure how to get to this, since the sparc-test images don't
>> include ifconfig, and I havn't been able to find a sparc install
>> image that works (doesn't support sparc32 or sparc64 fails to load).
>>
>> That said, try this and see if it works.
>>
>>
>> r~
>>
>> ---
>> diff --git a/exec.c b/exec.c
>> index 14d1fd7..572d3fd 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3286,6 +3286,8 @@ static int cpu_register_io_memory_fixed(int io_index,
>> CPUWriteMemoryFunc * const *mem_write,
>> void *opaque)
>> {
>> + int i;
>> +
>> if (io_index <= 0) {
>> io_index = get_free_io_mem_idx();
>> if (io_index == -1)
>> @@ -3296,8 +3298,14 @@ static int cpu_register_io_memory_fixed(int io_index,
>> return -1;
>> }
>>
>> - memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
>> - memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
>> + for (i = 0; i < 3; ++i) {
>> + io_mem_read[io_index][i]
>> + = (mem_read[i] ? mem_read[i] : unassigned_mem_read[i]);
>> + }
>> + for (i = 0; i < 3; ++i) {
>> + io_mem_write[io_index][i]
>> + = (mem_write[i] ? mem_write[i] : unassigned_mem_write[i]);
>> + }
>> io_mem_opaque[io_index] = opaque;
>>
>> return (io_index << IO_MEM_SHIFT);
>>
Why the fix didn't make it into the git?
Does it introduce other problems?
> Looks good, thanks.
>
> Acked-by: Artyom Tarasenko <atar4qemu@gmail.com>
>
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.
2010-05-06 20:25 ` Artyom Tarasenko
@ 2010-05-07 15:28 ` Blue Swirl
2010-05-07 16:52 ` [Qemu-devel] [PATCH] Fill in unassigned mem read/write callbacks Richard Henderson
0 siblings, 1 reply; 18+ messages in thread
From: Blue Swirl @ 2010-05-07 15:28 UTC (permalink / raw)
To: Artyom Tarasenko; +Cc: qemu-devel, Richard Henderson
On 5/6/10, Artyom Tarasenko <atar4qemu@googlemail.com> wrote:
> 2010/4/28 Artyom Tarasenko <atar4qemu@googlemail.com>:
>
> > 2010/4/27 Richard Henderson <rth@twiddle.net>:
> >> On 04/26/2010 02:54 PM, Artyom Tarasenko wrote:
> >>> This patch introduces a regression. qemu crashes on lance test:
> >>
> >> I'm not sure how to get to this, since the sparc-test images don't
> >> include ifconfig, and I havn't been able to find a sparc install
> >> image that works (doesn't support sparc32 or sparc64 fails to load).
> >>
> >> That said, try this and see if it works.
> >>
> >>
> >> r~
> >>
> >> ---
> >> diff --git a/exec.c b/exec.c
> >> index 14d1fd7..572d3fd 100644
> >> --- a/exec.c
> >> +++ b/exec.c
> >> @@ -3286,6 +3286,8 @@ static int cpu_register_io_memory_fixed(int io_index,
> >> CPUWriteMemoryFunc * const *mem_write,
> >> void *opaque)
> >> {
> >> + int i;
> >> +
> >> if (io_index <= 0) {
> >> io_index = get_free_io_mem_idx();
> >> if (io_index == -1)
> >> @@ -3296,8 +3298,14 @@ static int cpu_register_io_memory_fixed(int io_index,
> >> return -1;
> >> }
> >>
> >> - memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
> >> - memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
> >> + for (i = 0; i < 3; ++i) {
> >> + io_mem_read[io_index][i]
> >> + = (mem_read[i] ? mem_read[i] : unassigned_mem_read[i]);
> >> + }
> >> + for (i = 0; i < 3; ++i) {
> >> + io_mem_write[io_index][i]
> >> + = (mem_write[i] ? mem_write[i] : unassigned_mem_write[i]);
> >> + }
> >> io_mem_opaque[io_index] = opaque;
> >>
> >> return (io_index << IO_MEM_SHIFT);
> >>
>
>
> Why the fix didn't make it into the git?
> Does it introduce other problems?
A diff is not a patch, there is no commit description or SoB.
> > Looks good, thanks.
> >
> > Acked-by: Artyom Tarasenko <atar4qemu@gmail.com>
> >
>
> --
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH] Fill in unassigned mem read/write callbacks.
2010-05-07 15:28 ` Blue Swirl
@ 2010-05-07 16:52 ` Richard Henderson
2010-05-07 17:02 ` [Qemu-devel] " Blue Swirl
0 siblings, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2010-05-07 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, atar4qemu
Implement the "functions may be omitted with NULL pointer"
interface mentioned in the function block comment by transforming
NULL entries in the read/write arrays into calls to the
unassigned_mem family of functions.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
exec.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/exec.c b/exec.c
index e980788..3416aed 100644
--- a/exec.c
+++ b/exec.c
@@ -3262,6 +3262,8 @@ static int cpu_register_io_memory_fixed(int io_index,
CPUWriteMemoryFunc * const *mem_write,
void *opaque)
{
+ int i;
+
if (io_index <= 0) {
io_index = get_free_io_mem_idx();
if (io_index == -1)
@@ -3272,8 +3274,14 @@ static int cpu_register_io_memory_fixed(int io_index,
return -1;
}
- memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
- memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
+ for (i = 0; i < 3; ++i) {
+ io_mem_read[io_index][i]
+ = (mem_read[i] ? mem_read[i] : unassigned_mem_read[i]);
+ }
+ for (i = 0; i < 3; ++i) {
+ io_mem_write[io_index][i]
+ = (mem_write[i] ? mem_write[i] : unassigned_mem_write[i]);
+ }
io_mem_opaque[io_index] = opaque;
return (io_index << IO_MEM_SHIFT);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] Re: [PATCH] Fill in unassigned mem read/write callbacks.
2010-05-07 16:52 ` [Qemu-devel] [PATCH] Fill in unassigned mem read/write callbacks Richard Henderson
@ 2010-05-07 17:02 ` Blue Swirl
0 siblings, 0 replies; 18+ messages in thread
From: Blue Swirl @ 2010-05-07 17:02 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, atar4qemu
Thanks, applied.
On 5/7/10, Richard Henderson <rth@twiddle.net> wrote:
> Implement the "functions may be omitted with NULL pointer"
> interface mentioned in the function block comment by transforming
> NULL entries in the read/write arrays into calls to the
> unassigned_mem family of functions.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> exec.c | 12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index e980788..3416aed 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3262,6 +3262,8 @@ static int cpu_register_io_memory_fixed(int io_index,
> CPUWriteMemoryFunc * const *mem_write,
> void *opaque)
> {
> + int i;
> +
> if (io_index <= 0) {
> io_index = get_free_io_mem_idx();
> if (io_index == -1)
> @@ -3272,8 +3274,14 @@ static int cpu_register_io_memory_fixed(int io_index,
> return -1;
> }
>
> - memcpy(io_mem_read[io_index], mem_read, 3 * sizeof(CPUReadMemoryFunc*));
> - memcpy(io_mem_write[io_index], mem_write, 3 * sizeof(CPUWriteMemoryFunc*));
> + for (i = 0; i < 3; ++i) {
> + io_mem_read[io_index][i]
> + = (mem_read[i] ? mem_read[i] : unassigned_mem_read[i]);
> + }
> + for (i = 0; i < 3; ++i) {
> + io_mem_write[io_index][i]
> + = (mem_write[i] ? mem_write[i] : unassigned_mem_write[i]);
> + }
> io_mem_opaque[io_index] = opaque;
>
> return (io_index << IO_MEM_SHIFT);
>
> --
> 1.6.6.1
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths
2010-04-22 21:19 ` Richard Henderson
2010-04-23 18:30 ` Blue Swirl
@ 2010-05-28 20:45 ` Paul Brook
1 sibling, 0 replies; 18+ messages in thread
From: Paul Brook @ 2010-05-28 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, agraf, Richard Henderson
> The basic device interface looks like
> ...
> +
> +/* Register a memory region at START_ADDR/SIZE. The REGION structure will
> + be initialized appropriately for DEV using CB as the operation set. */
> +extern void cpu_register_memory_region(MemoryRegion *region,
> + const MemoryCallbackInfo *cb,
> + target_phys_addr_t start_addr,
> + target_phys_addr_t size);
> +
> +/* Unregister a memory region. */
> +extern void cpu_unregister_memory_region(MemoryRegion *);
> +
> +/* Allocate ram for use with cpu_register_memory_region. */
> +extern const MemoryCallbackInfo *qemu_ram_alloc_r(ram_addr_t);
> +extern void qemu_ram_free_r(const MemoryCallbackInfo *);
>
> The Basic Idea is that we have a MemoryRegion object that describes
> a contiguous mapping within the guest address space. This object
> needs to handle RAM, ROM and devices. The desire to handle memory
> and devices the same comes from the wish to have PCI device BARs
> show up as plain memory in the TLB as plain memory, and to be able
> to handle all PCI device regions identically within sysbus.
Looks reasonable to me.
I'm tempted to add a DeviceState* argument to cpu_register_memory_region.
This might be informative for debugging, and allow future disjoint bus
support. OTOH it may be more trouble than it's worth.
> I will admit that I do not yet have a good replacement for IO_MEM_ROMD,
> or toggling the read-only bit on a RAM region.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2010-05-28 20:46 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-20 20:26 [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Richard Henderson
2010-04-20 19:54 ` [Qemu-devel] [PATCH 1/2] io: Add CPUIOMemoryOps and use it Richard Henderson
2010-04-20 20:22 ` [Qemu-devel] [PATCH 2/2] io: Add readq/writeq hooks and use them Richard Henderson
2010-04-22 19:38 ` [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths Blue Swirl
2010-04-22 19:55 ` Richard Henderson
2010-04-22 20:01 ` Blue Swirl
2010-04-22 21:19 ` Richard Henderson
2010-04-23 18:30 ` Blue Swirl
2010-05-28 20:45 ` Paul Brook
2010-04-22 23:47 ` [Qemu-devel] [PATCH] Remove IO_MEM_SUBWIDTH Richard Henderson
2010-04-25 15:06 ` [Qemu-devel] " Blue Swirl
2010-04-26 21:54 ` Artyom Tarasenko
2010-04-27 18:30 ` Richard Henderson
2010-04-28 19:29 ` Artyom Tarasenko
2010-05-06 20:25 ` Artyom Tarasenko
2010-05-07 15:28 ` Blue Swirl
2010-05-07 16:52 ` [Qemu-devel] [PATCH] Fill in unassigned mem read/write callbacks Richard Henderson
2010-05-07 17:02 ` [Qemu-devel] " Blue Swirl
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.