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

[Anthony Liguori <anthony@codemonkey.ws>]

On 11/30/2011 09:26 AM, Andreas Färber wrote:
> 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 change the previous fix to use a new IO_MEM_SUBPAGE_RAM
> instead of IO_MEM_UNASSIGNED. Suggested by Avi.
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> Cc: Avi Kivity<avi@redhat.com>
> Cc: Gleb Natapov<gleb@redhat.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   cpu-common.h |    1 +
>   exec.c       |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/cpu-common.h b/cpu-common.h
> index c9878ba..3f45428 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -172,6 +172,7 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr,
>   #define IO_MEM_ROM         (1<<  IO_MEM_SHIFT) /* hardcoded offset */
>   #define IO_MEM_UNASSIGNED  (2<<  IO_MEM_SHIFT)
>   #define IO_MEM_NOTDIRTY    (3<<  IO_MEM_SHIFT)
> +#define IO_MEM_SUBPAGE_RAM (4<<  IO_MEM_SHIFT)
>
>   /* Acts like a ROM when read and like a device when written.  */
>   #define IO_MEM_ROMD        (1)
> diff --git a/exec.c b/exec.c
> index 6b92198..6c206ff 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3570,6 +3570,63 @@ static CPUWriteMemoryFunc * const subpage_write[] = {
>       &subpage_writel,
>   };
>
> +static uint32_t subpage_ram_readb(void *opaque, target_phys_addr_t addr)
> +{
> +    ram_addr_t raddr = addr;
> +    void *ptr = qemu_get_ram_ptr(raddr);
> +    return ldub_p(ptr);
> +}
> +
> +static void subpage_ram_writeb(void *opaque, target_phys_addr_t addr,
> +                               uint32_t value)
> +{
> +    ram_addr_t raddr = addr;
> +    void *ptr = qemu_get_ram_ptr(raddr);
> +    stb_p(ptr, value);
> +}
> +
> +static uint32_t subpage_ram_readw(void *opaque, target_phys_addr_t addr)
> +{
> +    ram_addr_t raddr = addr;
> +    void *ptr = qemu_get_ram_ptr(raddr);
> +    return lduw_p(ptr);
> +}
> +
> +static void subpage_ram_writew(void *opaque, target_phys_addr_t addr,
> +                               uint32_t value)
> +{
> +    ram_addr_t raddr = addr;
> +    void *ptr = qemu_get_ram_ptr(raddr);
> +    stw_p(ptr, value);
> +}
> +
> +static uint32_t subpage_ram_readl(void *opaque, target_phys_addr_t addr)
> +{
> +    ram_addr_t raddr = addr;
> +    void *ptr = qemu_get_ram_ptr(raddr);
> +    return ldl_p(ptr);
> +}
> +
> +static void subpage_ram_writel(void *opaque, target_phys_addr_t addr,
> +                               uint32_t value)
> +{
> +    ram_addr_t raddr = addr;
> +    void *ptr = qemu_get_ram_ptr(raddr);
> +    stl_p(ptr, value);
> +}
> +
> +static CPUReadMemoryFunc * const subpage_ram_read[] = {
> +&subpage_ram_readb,
> +&subpage_ram_readw,
> +&subpage_ram_readl,
> +};
> +
> +static CPUWriteMemoryFunc * const subpage_ram_write[] = {
> +&subpage_ram_writeb,
> +&subpage_ram_writew,
> +&subpage_ram_writel,
> +};
> +
>   static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
>                                ram_addr_t memory, ram_addr_t region_offset)
>   {
> @@ -3583,8 +3640,9 @@ 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;
> +    if ((memory&  ~TARGET_PAGE_MASK) == IO_MEM_RAM) {
> +        memory = IO_MEM_SUBPAGE_RAM;
> +    }
>       memory = (memory>>  IO_MEM_SHIFT)&  (IO_MEM_NB_ENTRIES - 1);
>       for (; idx<= eidx; idx++) {
>           mmio->sub_io_index[idx] = memory;
> @@ -3817,6 +3875,9 @@ static void io_mem_init(void)
>       cpu_register_io_memory_fixed(IO_MEM_NOTDIRTY, error_mem_read,
>                                    notdirty_mem_write, NULL,
>                                    DEVICE_NATIVE_ENDIAN);
> +    cpu_register_io_memory_fixed(IO_MEM_SUBPAGE_RAM, subpage_ram_read,
> +                                 subpage_ram_write, NULL,
> +                                 DEVICE_NATIVE_ENDIAN);
>       for (i=0; i<5; i++)
>           io_mem_used[i] = 1;
>