Re: [PATCH] system/physmem: Fix cpu_memory_rw_debug for armv7m MPU

[Richard Henderson <richard.henderson@linaro.org>]

On 11/20/24 09:15, Xiong Nandi wrote:
> The actual page size (region size for MPU) of armv7m may
> smaller than TARGET_PAGE_SIZE (2^5 vs 2^10). So we should
> use the actual virtual address to get the phys page address.
> 
> Signed-off-by: Xiong Nandi <xndchn@gmail.com>
> ---
>   system/physmem.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/system/physmem.c b/system/physmem.c
> index dc1db3a384..a76b305130 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -3564,11 +3564,12 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>           MemTxResult res;
>   
>           page = addr & TARGET_PAGE_MASK;
> -        phys_addr = cpu_get_phys_page_attrs_debug(cpu, page, &attrs);
> +        phys_addr = cpu_get_phys_page_attrs_debug(cpu, addr, &attrs);
>           asidx = cpu_asidx_from_attrs(cpu, attrs);
>           /* if no physical page mapped, return an error */
>           if (phys_addr == -1)
>               return -1;
> +        phys_addr &= TARGET_PAGE_MASK;
>           l = (page + TARGET_PAGE_SIZE) - addr;
>           if (l > len)
>               l = len;

So... I guess this might accidentally work, but L is definitely incorrect under the 
circumstances.  So we could easily be exchanging one set of bugs for another.

We really need to be returning the range of addresses under which the address translation 
is valid.  One solution could be passing in 'l = len, &l' to be modified so that (addr, l) 
translates to (phys_addr, l) after the call; iterate for sum l < len as we're currently doing.


r~