[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.
|
From: |
Artyom Tarasenko |
|
Subject: |
Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH. |
|
Date: |
Mon, 26 Apr 2010 23:54:20 +0200 |
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 <address@hidden>:
> Thanks, applied.
>
> On 4/23/10, Richard Henderson <address@hidden> 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 <address@hidden>
>> ---
>> 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/
- [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths, Richard Henderson, 2010/04/20
- [Qemu-devel] [PATCH 2/2] io: Add readq/writeq hooks and use them., Richard Henderson, 2010/04/20
- [Qemu-devel] [PATCH 1/2] io: Add CPUIOMemoryOps and use it., Richard Henderson, 2010/04/20
- Re: [Qemu-devel] [PATCH 0/2] [RFC] 64-bit io paths, Blue Swirl, 2010/04/22
- [Qemu-devel] [PATCH] Remove IO_MEM_SUBWIDTH., Richard Henderson, 2010/04/22
- [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH., Blue Swirl, 2010/04/25
- Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH.,
Artyom Tarasenko <=
- Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH., Richard Henderson, 2010/04/27
- Re: [Qemu-devel] Re: [PATCH] Remove IO_MEM_SUBWIDTH., Artyom Tarasenko, 2010/04/28