Il mer 24 dic 2025, 16:24 Philippe Mathieu-Daudé <philmd@linaro.org> ha
scritto:
> Replace the ldn_he_p/stn_he_p() calls by their explicit endianness
> variants. Duplicate the MemoryRegionOps, using one entry for BIG
> and another for LITTLE endianness. Select the proper MemoryRegionOps
> in memory_region_init_ram_device_ptr().
>
This extra complication makes no sense to me. You're introducing dead code
and dead data, because only one of the MemoryRegionOps will be ever used on
a given host.
Given this example and the ATI one, I would really prefer to keep _he_
functions around and use them whenever you're reading from host memory as
in this case.
Paolo
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> system/memory.c | 68 ++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 48 insertions(+), 20 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 8b84661ae36..5bcdeaf0bee 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -1361,41 +1361,69 @@ const MemoryRegionOps unassigned_mem_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> -static uint64_t memory_region_ram_device_read(void *opaque,
> - hwaddr addr, unsigned size)
> +static uint64_t memory_region_ram_device_read_le(void *opaque,
> + hwaddr addr, unsigned
> size)
> {
> MemoryRegion *mr = opaque;
> - uint64_t data = ldn_he_p(mr->ram_block->host + addr, size);
> + uint64_t data = ldn_le_p(mr->ram_block->host + addr, size);
>
> trace_memory_region_ram_device_read(get_cpu_index(), mr, addr, data,
> size);
>
> return data;
> }
>
> -static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> - uint64_t data, unsigned size)
> +static uint64_t memory_region_ram_device_read_be(void *opaque,
> + hwaddr addr, unsigned
> size)
> +{
> + MemoryRegion *mr = opaque;
> + uint64_t data = ldn_be_p(mr->ram_block->host + addr, size);
> +
> + trace_memory_region_ram_device_read(get_cpu_index(), mr, addr, data,
> size);
> +
> + return data;
> +}
> +
> +static void memory_region_ram_device_write_le(void *opaque, hwaddr addr,
> + uint64_t data, unsigned
> size)
> {
> MemoryRegion *mr = opaque;
>
> trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data,
> size);
>
> - stn_he_p(mr->ram_block->host + addr, size, data);
> + stn_le_p(mr->ram_block->host + addr, size, data);
> }
>
> -static const MemoryRegionOps ram_device_mem_ops = {
> - .read = memory_region_ram_device_read,
> - .write = memory_region_ram_device_write,
> - .endianness = HOST_BIG_ENDIAN ? DEVICE_BIG_ENDIAN :
> DEVICE_LITTLE_ENDIAN,
> - .valid = {
> - .min_access_size = 1,
> - .max_access_size = 8,
> - .unaligned = true,
> - },
> - .impl = {
> - .min_access_size = 1,
> - .max_access_size = 8,
> - .unaligned = true,
> +static void memory_region_ram_device_write_be(void *opaque, hwaddr addr,
> + uint64_t data, unsigned
> size)
> +{
> + MemoryRegion *mr = opaque;
> +
> + trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data,
> size);
> +
> + stn_be_p(mr->ram_block->host + addr, size, data);
> +}
> +
> +static const MemoryRegionOps ram_device_mem_ops[2] = {
> + [0 ... 1] = {
> + .valid = {
> + .min_access_size = 1,
> + .max_access_size = 8,
> + .unaligned = true,
> + },
> + .impl = {
> + .min_access_size = 1,
> + .max_access_size = 8,
> + .unaligned = true,
> + },
> },
> +
> + [0].endianness = DEVICE_LITTLE_ENDIAN,
> + [0].read = memory_region_ram_device_read_le,
> + [0].write = memory_region_ram_device_write_le,
> +
> + [1].endianness = DEVICE_BIG_ENDIAN,
> + [1].read = memory_region_ram_device_read_be,
> + [1].write = memory_region_ram_device_write_be,
> };
>
> bool memory_region_access_valid(MemoryRegion *mr,
> @@ -1712,7 +1740,7 @@ void memory_region_init_ram_device_ptr(MemoryRegion
> *mr,
> mr->ram = true;
> mr->terminates = true;
> mr->ram_device = true;
> - mr->ops = &ram_device_mem_ops;
> + mr->ops = &ram_device_mem_ops[HOST_BIG_ENDIAN];
> mr->opaque = mr;
> mr->destructor = memory_region_destructor_ram;
>
> --
> 2.52.0
>
>