Re: [RFC PATCH 6/6] memory: Use CPU register size as default access_size_max

[Peter Maydell]

On Sun, 31 May 2020 at 18:54, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Do not restrict 64-bit CPU to 32-bit max access by default.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC because this probably require an audit of all devices
> used on 64-bit targets.
> But if we find such problematic devices, they should instead
> enforce their access_size_max = 4 rather than expecting the
> default value to be valid...
> ---
>  memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/memory.c b/memory.c
> index fd6f3d6aca..1d6bb5cdb0 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1370,7 +1370,7 @@ bool memory_region_access_valid(MemoryRegion *mr,
>
>      access_size_max = mr->ops->valid.max_access_size;
>      if (!mr->ops->valid.max_access_size) {
> -        access_size_max = 4;
> +        access_size_max = TARGET_LONG_SIZE;
>      }

This is definitely not the right approach. TARGET_LONG_SIZE
is a property of the CPU, but memory_region_access_valid()
is testing properties of the MemoryRegion (ie the device
being addressed). One can have devices in a system with a
64-bit CPU which can only handle being accessed at 32-bit
width (indeed, it's pretty common). The behaviour of a device
shouldn't change depending on whether we happened to compile
it into a system with TARGET_LONG_SIZE=4 or 8.

(If you want to argue that we should make all our devices
explicit about the valid.max_access_size rather than relying
on "default means 4" then I wouldn't necessarily disagree.)

thanks
-- PMM