Re: [EXT] Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte

[Alistair Francis]

On Fri, Apr 22, 2022 at 10:10 PM Ralf Ramsauer
<ralf.ramsauer@oth-regensburg.de> wrote:
>
>
>
> On 22/04/2022 04:54, Bin Meng wrote:
> > On Fri, Apr 22, 2022 at 10:53 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >>
> >> On Tue, Apr 5, 2022 at 1:34 AM Ralf Ramsauer
> >> <ralf.ramsauer@oth-regensburg.de> wrote:
> >>>
> >>> Two non-subsequent PTEs can be mapped to subsequent paddrs. In this
> >>> case, walk_pte will erroneously merge them.
> >>>
> >>> Enforce the split up, by tracking the virtual base address.
> >>>
> >>> Let's say we have the mapping:
> >>> 0x81200000 -> 0x89623000 (4K)
> >>> 0x8120f000 -> 0x89624000 (4K)
> >>>
> >>> Before, walk_pte would have shown:
> >>>
> >>> vaddr            paddr            size             attr
> >>> ---------------- ---------------- ---------------- -------
> >>> 0000000081200000 0000000089623000 0000000000002000 rwxu-ad
> >>>
> >>> as it only checks for subsequent paddrs. With this patch, it becomes:
> >>>
> >>> vaddr            paddr            size             attr
> >>> ---------------- ---------------- ---------------- -------
> >>> 0000000081200000 0000000089623000 0000000000001000 rwxu-ad
> >>> 000000008120f000 0000000089624000 0000000000001000 rwxu-ad
> >>>
> >>> Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de>
> >>> ---
> >>>   target/riscv/monitor.c | 5 ++++-
> >>>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> >>> index 7efb4b62c1..9dc4cb1156 100644
> >>> --- a/target/riscv/monitor.c
> >>> +++ b/target/riscv/monitor.c
> >>> @@ -84,6 +84,7 @@ static void walk_pte(Monitor *mon, hwaddr base, 
> >>> target_ulong start,
> >>>   {
> >>>       hwaddr pte_addr;
> >>>       hwaddr paddr;
> >>> +    target_ulong last_start = -1;
> >>>       target_ulong pgsize;
> >>>       target_ulong pte;
> >>>       int ptshift;
> >>> @@ -116,7 +117,8 @@ static void walk_pte(Monitor *mon, hwaddr base, 
> >>> target_ulong start,
> >>>                    * contiguous mapped block details.
> >>>                    */
> >>
> >> Please also update the comments above to mention the new case you added 
> >> here.
>
> Shall I provide a v3? No problem, if that makes your life easier.
> Otherwise, you could also squash attached comment on integration.

Yes, please submit a v3

Alistair

>
> Thanks
>    Ralf
>
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index 02512ed48f..1cb0932e03 100644
> --- a/target/riscv/monitor.c
> +++ b/target/riscv/monitor.c
> @@ -143,9 +143,9 @@ static void walk_pte(Monitor *mon, hwaddr base,
> target_ulong start,
>                    * A leaf PTE has been found
>                    *
>                    * If current PTE's permission bits differ from the
> last one,
> -                 * or current PTE's ppn does not make a contiguous physical
> -                 * address block together with the last one, print out
> the last
> -                 * contiguous mapped block details.
> +                * or the current PTE breaks up a contiguous virtual or
> +                * physical mapping, address block together with the
> last one,
> +                * print out the last contiguous mapped block details.
>                    */
>                   if ((*last_attr != attr) ||
>                       (*last_paddr + *last_size != paddr) ||
>