Re: [RFC PATCH 1/9] memory: Initialize MemoryRegionOps for RAM memory regions

[Philippe Mathieu-Daudé]

On 8/18/20 11:52 AM, P J P wrote:
> +-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
> | Fix by initializing the MemoryRegionOps to ram_device_mem_ops, this way the 
> | memory accesses are properly dispatched using 
> | memory_region_ram_device_read() / memory_region_ram_device_write().
> | 
> | Fixes: 4a2e242bbb ("memory: Don't use memcpy for ram_device regions")
> | Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> | ---
> | Since v1: Corrected description (PJP)
> | Cc: P J P <ppandit@redhat.com>
> | ---
> |  softmmu/memory.c | 10 ++++++++++
> |  1 file changed, 10 insertions(+)
> | 
> | diff --git a/softmmu/memory.c b/softmmu/memory.c
> | index 651705b7d1..8139da1a58 100644
> | --- a/softmmu/memory.c
> | +++ b/softmmu/memory.c
> | @@ -1517,6 +1517,8 @@ void 
> memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
> |      Error *err = NULL;
> |      memory_region_init(mr, owner, name, size);
> |      mr->ram = true;
> | +    mr->ops = &ram_device_mem_ops;
> | +    mr->opaque = mr;
> |      mr->terminates = true;
> 
> 
> I wonder if 'mr->ops = &ram_device_mem_ops' could be done in 
> memory_region_init() instead?

I don't think so, because to use &ram_device_mem_ops handlers use
mr->opaque and mr->ram_block, so alias/io/iommu/rom types would
directly crash accessing these NULL fields.

> 
> Otherwise looks okay.

Thanks!

> 
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
>