Thanks. This patch at least guarantees normal read/write access to
addresses with r/w flags, although there is still a risk of
misidentifying accessible regions within continuous address spaces.

Actually, initially I did write a patch with a modified page size as
an argument, but I soon found that the current implementation of
armv7a (pmsav7) will return the page size (lg_page_size) as 0 in many
situations (such as overlapping regions).
Maybe we can simply make the page size as 2^5 (minimal page size) when
the returned lg_page_size == 0?

On Sat, Nov 23, 2024 at 4:32 AM Richard Henderson
<[email protected]> wrote:
>
> 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 <[email protected]>
> > ---
> >   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~

Reply via email to