Re: [Qemu-devel] [PATCH] exec.c: Fix subpage memory access to RAM MemoryRegion

["Andreas Färber" <afaerber@suse.de>]

Am 28.11.2011 16:06, schrieb Andreas Färber:
> Commit 95c318f5e1f88d7e5bcc6deac17330fd4806a2d3 (Fix segfault in mmio subpage
> handling code.) prevented a segfault by making all subpage registrations
> over an existing memory page perform an unassigned access. Symptoms were
> writes not taking effect and reads returning zero.
> 
> Very small page sizes are not currently supported either, so subpage memory
> areas cannot fully be avoided.
> 
> Therefore revert the previous fix and defer recognition of IO_MEM_RAM to
> subpage_{read,write}len() and translate any access there.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Avi Kivity <avi@redhat.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> ---
>  exec.c |   33 +++++++++++++++++++++++++++++++--
>  1 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 6b92198..fba5ba1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3508,6 +3508,21 @@ static inline uint32_t subpage_readlen (subpage_t *mmio,
>  
>      addr += mmio->region_offset[idx];
>      idx = mmio->sub_io_index[idx];
> +    if (unlikely(idx == IO_MEM_RAM)) {
> +        ram_addr_t raddr = /*mmio->base |*/ addr;
> +        void *ptr = qemu_get_ram_ptr(raddr);

This...

> +        switch (len) {
> +        default:
> +        case 0:
> +            return ldub_p(ptr);
> +        case 1:
> +            return lduw_p(ptr);
> +        case 2:
> +            return ldl_p(ptr);
> +        case 3:
> +            return ldq_p(ptr);
> +        }
> +    }
>      return io_mem_read[idx][len](io_mem_opaque[idx], addr);
>  }
>  
> @@ -3522,6 +3537,22 @@ static inline void subpage_writelen (subpage_t *mmio, target_phys_addr_t addr,
>  
>      addr += mmio->region_offset[idx];
>      idx = mmio->sub_io_index[idx];
> +    if (unlikely(idx == IO_MEM_RAM)) {
> +        ram_addr_t raddr = /*mmio->base |*/ addr;
> +        void *ptr = qemu_get_ram_ptr(raddr);

...and/or this seems to lead to "Bad RAM pointer" (or so) when there's
ELF code loaded into the subpage at that address despite being IO_MEM_RAM?
(Seen, e.g., if for RL78 I increase the page size from 12 to 16.)

> +        switch (len) {
> +        default:
> +        case 0:
> +            stb_p(ptr, value);

break;

> +        case 1:
> +            stw_p(ptr, value);

break;

> +        case 2:
> +            stl_p(ptr, value);

break;

> +        case 3:
> +            stq_p(ptr, value);

break;

Andreas

> +        }
> +        return;
> +    }
>      io_mem_write[idx][len](io_mem_opaque[idx], addr, value);
>  }
>  
> @@ -3583,8 +3614,6 @@ 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
> -    if ((memory & ~TARGET_PAGE_MASK) == IO_MEM_RAM)
> -        memory = IO_MEM_UNASSIGNED;
>      memory = (memory >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
>      for (; idx <= eidx; idx++) {
>          mmio->sub_io_index[idx] = memory;


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg