All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-01 12:22 ` Ralf Ramsauer
  0 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-01 12:22 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, qemu-riscv, qemu-devel, Stefan Huber
  Cc: Bin Meng, Konrad Schwarz, Ralf Ramsauer

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..60e3edd0ad 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,13 +117,15 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
                  * contiguous mapped block details.
                  */
                 if ((*last_attr != attr) ||
-                    (*last_paddr + *last_size != paddr)) {
+                    (*last_paddr + *last_size != paddr) ||
+                    (last_start + *last_size != start)) {
                     print_pte(mon, va_bits, *vbase, *pbase,
                               *last_paddr + *last_size - *pbase, *last_attr);
 
                     *vbase = start;
                     *pbase = paddr;
                     *last_attr = attr;
+                    last_start = start;
                 }
 
                 *last_paddr = paddr;
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-01 12:22 ` Ralf Ramsauer
  0 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-01 12:22 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, qemu-riscv, qemu-devel, Stefan Huber
  Cc: Ralf Ramsauer, Bin Meng, Konrad Schwarz

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..60e3edd0ad 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,13 +117,15 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
                  * contiguous mapped block details.
                  */
                 if ((*last_attr != attr) ||
-                    (*last_paddr + *last_size != paddr)) {
+                    (*last_paddr + *last_size != paddr) ||
+                    (last_start + *last_size != start)) {
                     print_pte(mon, va_bits, *vbase, *pbase,
                               *last_paddr + *last_size - *pbase, *last_attr);
 
                     *vbase = start;
                     *pbase = paddr;
                     *last_attr = attr;
+                    last_start = start;
                 }
 
                 *last_paddr = paddr;
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [PATCH] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-01 12:22 ` Ralf Ramsauer
  (?)
@ 2022-04-01 13:14 ` Richard Henderson
  -1 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2022-04-01 13:14 UTC (permalink / raw)
  To: Ralf Ramsauer, Palmer Dabbelt, Alistair Francis, qemu-riscv,
	qemu-devel, Stefan Huber
  Cc: Bin Meng, Konrad Schwarz

On 4/1/22 06:22, Ralf Ramsauer 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(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-01 12:22 ` Ralf Ramsauer
  (?)
  (?)
@ 2022-04-04 17:22 ` Ralf Ramsauer
  -1 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-04 17:22 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, qemu-riscv, qemu-devel, Stefan Huber
  Cc: Bin Meng, Konrad Schwarz



On 01/04/2022 14:22, Ralf Ramsauer 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..60e3edd0ad 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,13 +117,15 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                    * contiguous mapped block details.
>                    */
>                   if ((*last_attr != attr) ||
> -                    (*last_paddr + *last_size != paddr)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                       print_pte(mon, va_bits, *vbase, *pbase,
>                                 *last_paddr + *last_size - *pbase, *last_attr);
>   
>                       *vbase = start;
>                       *pbase = paddr;
>                       *last_attr = attr;
> +                    last_start = start;
>                   }

Yikes, there's a small bug in my patch that I failed to see:
last_addr = start should be outside the curly brackets, otherwise it 
will rip up too much regions.

I'll return with a V2.

Thanks
   Ralf

>   
>                   *last_paddr = paddr;


^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-01 12:22 ` Ralf Ramsauer
@ 2022-04-04 17:34   ` Ralf Ramsauer
  -1 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-04 17:34 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, qemu-riscv, qemu-devel,
	Stefan Huber, Richard Henderson
  Cc: Bin Meng, Konrad Schwarz, Ralf Ramsauer

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.
                  */
                 if ((*last_attr != attr) ||
-                    (*last_paddr + *last_size != paddr)) {
+                    (*last_paddr + *last_size != paddr) ||
+                    (last_start + *last_size != start)) {
                     print_pte(mon, va_bits, *vbase, *pbase,
                               *last_paddr + *last_size - *pbase, *last_attr);
 
@@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
                     *last_attr = attr;
                 }
 
+                last_start = start;
                 *last_paddr = paddr;
                 *last_size = pgsize;
             } else {
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-04 17:34   ` Ralf Ramsauer
  0 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-04 17:34 UTC (permalink / raw)
  To: Palmer Dabbelt, Alistair Francis, qemu-riscv, qemu-devel,
	Stefan Huber, Richard Henderson
  Cc: Ralf Ramsauer, Bin Meng, Konrad Schwarz

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.
                  */
                 if ((*last_attr != attr) ||
-                    (*last_paddr + *last_size != paddr)) {
+                    (*last_paddr + *last_size != paddr) ||
+                    (last_start + *last_size != start)) {
                     print_pte(mon, va_bits, *vbase, *pbase,
                               *last_paddr + *last_size - *pbase, *last_attr);
 
@@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
                     *last_attr = attr;
                 }
 
+                last_start = start;
                 *last_paddr = paddr;
                 *last_size = pgsize;
             } else {
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-04 17:34   ` Ralf Ramsauer
@ 2022-04-12  4:53     ` Alistair Francis
  -1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-12  4:53 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: open list:RISC-V, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis, Stefan Huber,
	Konrad Schwarz, Palmer Dabbelt

On Tue, Apr 5, 2022 at 3: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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  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.
>                   */
>                  if ((*last_attr != attr) ||
> -                    (*last_paddr + *last_size != paddr)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.35.1
>
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-12  4:53     ` Alistair Francis
  0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-12  4:53 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Palmer Dabbelt, Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Stefan Huber,
	Richard Henderson, Bin Meng, Konrad Schwarz

On Tue, Apr 5, 2022 at 3: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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  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.
>                   */
>                  if ((*last_attr != attr) ||
> -                    (*last_paddr + *last_size != paddr)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.35.1
>
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-04 17:34   ` Ralf Ramsauer
@ 2022-04-22  2:53     ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2022-04-22  2:53 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: open list:RISC-V, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis, Stefan Huber,
	Konrad Schwarz, Palmer Dabbelt

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.

>                  if ((*last_attr != attr) ||
> -                    (*last_paddr + *last_size != paddr)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-22  2:53     ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2022-04-22  2:53 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Palmer Dabbelt, Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Stefan Huber,
	Richard Henderson, Bin Meng, Konrad Schwarz

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.

>                  if ((*last_attr != attr) ||
> -                    (*last_paddr + *last_size != paddr)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-22  2:53     ` Bin Meng
@ 2022-04-22  2:54       ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2022-04-22  2:54 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: open list:RISC-V, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis, Stefan Huber,
	Konrad Schwarz, Palmer Dabbelt

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.
>

Otherwise,

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-22  2:54       ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2022-04-22  2:54 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Palmer Dabbelt, Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Stefan Huber,
	Richard Henderson, Bin Meng, Konrad Schwarz

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.
>

Otherwise,

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [EXT] Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-22  2:54       ` Bin Meng
@ 2022-04-22 11:37         ` Ralf Ramsauer
  -1 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-22 11:37 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis, Stefan Huber,
	Konrad Schwarz, Palmer Dabbelt



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.

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) ||


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [EXT] Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-22 11:37         ` Ralf Ramsauer
  0 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-22 11:37 UTC (permalink / raw)
  To: Bin Meng
  Cc: Palmer Dabbelt, Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Stefan Huber,
	Richard Henderson, Bin Meng, Konrad Schwarz



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.

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) ||


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [EXT] Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-22 11:37         ` Ralf Ramsauer
@ 2022-04-22 23:08           ` Alistair Francis
  -1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-22 23:08 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: open list:RISC-V, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis, Stefan Huber,
	Konrad Schwarz, Bin Meng, Palmer Dabbelt

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) ||
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [EXT] Re: [PATCH v2] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-22 23:08           ` Alistair Francis
  0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-22 23:08 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Bin Meng, open list:RISC-V, Bin Meng, Richard Henderson,
	qemu-devel@nongnu.org Developers, Alistair Francis, Stefan Huber,
	Konrad Schwarz, Palmer Dabbelt

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) ||
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-22 23:08           ` Alistair Francis
@ 2022-04-23 21:59             ` Ralf Ramsauer
  -1 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-23 21:59 UTC (permalink / raw)
  To: Bin Meng, Alistair Francis, qemu-riscv, qemu-devel, Stefan Huber
  Cc: Konrad Schwarz, Richard Henderson, Ralf Ramsauer

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>
---
[since v2: Adjust comment, rebased to latest master]

 target/riscv/monitor.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
index 7efb4b62c1..17e63fab00 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;
@@ -111,12 +112,13 @@ 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)) {
+                    (*last_paddr + *last_size != paddr) ||
+                    (last_start + *last_size != start)) {
                     print_pte(mon, va_bits, *vbase, *pbase,
                               *last_paddr + *last_size - *pbase, *last_attr);
 
@@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
                     *last_attr = attr;
                 }
 
+                last_start = start;
                 *last_paddr = paddr;
                 *last_size = pgsize;
             } else {
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-23 21:59             ` Ralf Ramsauer
  0 siblings, 0 replies; 26+ messages in thread
From: Ralf Ramsauer @ 2022-04-23 21:59 UTC (permalink / raw)
  To: Bin Meng, Alistair Francis, qemu-riscv, qemu-devel, Stefan Huber
  Cc: Ralf Ramsauer, Richard Henderson, Konrad Schwarz

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>
---
[since v2: Adjust comment, rebased to latest master]

 target/riscv/monitor.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
index 7efb4b62c1..17e63fab00 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;
@@ -111,12 +112,13 @@ 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)) {
+                    (*last_paddr + *last_size != paddr) ||
+                    (last_start + *last_size != start)) {
                     print_pte(mon, va_bits, *vbase, *pbase,
                               *last_paddr + *last_size - *pbase, *last_attr);
 
@@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
                     *last_attr = attr;
                 }
 
+                last_start = start;
                 *last_paddr = paddr;
                 *last_size = pgsize;
             } else {
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-23 21:59             ` Ralf Ramsauer
@ 2022-04-24  1:20               ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2022-04-24  1:20 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Konrad Schwarz, open list:RISC-V, Richard Henderson,
	qemu-devel@nongnu.org Developers, Stefan Huber, Alistair Francis

On Sun, Apr 24, 2022 at 5:59 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>
> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-24  1:20               ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2022-04-24  1:20 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Stefan Huber,
	Richard Henderson, Konrad Schwarz

On Sun, Apr 24, 2022 at 5:59 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>
> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-23 21:59             ` Ralf Ramsauer
@ 2022-04-26  0:16               ` Alistair Francis
  -1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-26  0:16 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Konrad Schwarz, Richard Henderson,
	qemu-devel@nongnu.org Developers, Stefan Huber, open list:RISC-V,
	Bin Meng

On Sun, Apr 24, 2022 at 7:59 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index 7efb4b62c1..17e63fab00 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;
> @@ -111,12 +112,13 @@ 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)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.36.0
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-26  0:16               ` Alistair Francis
  0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-26  0:16 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Bin Meng, open list:RISC-V, qemu-devel@nongnu.org Developers,
	Stefan Huber, Richard Henderson, Konrad Schwarz

On Sun, Apr 24, 2022 at 7:59 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index 7efb4b62c1..17e63fab00 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;
> @@ -111,12 +112,13 @@ 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)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.36.0
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-23 21:59             ` Ralf Ramsauer
@ 2022-04-27 23:52               ` Alistair Francis
  -1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-27 23:52 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Konrad Schwarz, Richard Henderson,
	qemu-devel@nongnu.org Developers, Stefan Huber, open list:RISC-V,
	Bin Meng

On Sun, Apr 24, 2022 at 7:59 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>

Thanks for the patch. It doesn't seem to have made it to the QEMU
mailing list though. Do you mind re-sending it and checking to make
sure it is sent to the mailing list?

Alistair

> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index 7efb4b62c1..17e63fab00 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;
> @@ -111,12 +112,13 @@ 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)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.36.0
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-27 23:52               ` Alistair Francis
  0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-27 23:52 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Bin Meng, open list:RISC-V, qemu-devel@nongnu.org Developers,
	Stefan Huber, Richard Henderson, Konrad Schwarz

On Sun, Apr 24, 2022 at 7:59 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>

Thanks for the patch. It doesn't seem to have made it to the QEMU
mailing list though. Do you mind re-sending it and checking to make
sure it is sent to the mailing list?

Alistair

> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index 7efb4b62c1..17e63fab00 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;
> @@ -111,12 +112,13 @@ 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)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.36.0
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
  2022-04-23 21:59             ` Ralf Ramsauer
@ 2022-04-28  0:13               ` Alistair Francis
  -1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-28  0:13 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Konrad Schwarz, Richard Henderson,
	qemu-devel@nongnu.org Developers, Stefan Huber, open list:RISC-V,
	Bin Meng

On Sun, Apr 24, 2022 at 7:59 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>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index 7efb4b62c1..17e63fab00 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;
> @@ -111,12 +112,13 @@ 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)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.36.0
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH v3] target/riscv: Fix incorrect PTE merge in walk_pte
@ 2022-04-28  0:13               ` Alistair Francis
  0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2022-04-28  0:13 UTC (permalink / raw)
  To: Ralf Ramsauer
  Cc: Bin Meng, open list:RISC-V, qemu-devel@nongnu.org Developers,
	Stefan Huber, Richard Henderson, Konrad Schwarz

On Sun, Apr 24, 2022 at 7:59 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>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> [since v2: Adjust comment, rebased to latest master]
>
>  target/riscv/monitor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index 7efb4b62c1..17e63fab00 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;
> @@ -111,12 +112,13 @@ 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)) {
> +                    (*last_paddr + *last_size != paddr) ||
> +                    (last_start + *last_size != start)) {
>                      print_pte(mon, va_bits, *vbase, *pbase,
>                                *last_paddr + *last_size - *pbase, *last_attr);
>
> @@ -125,6 +127,7 @@ static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
>                      *last_attr = attr;
>                  }
>
> +                last_start = start;
>                  *last_paddr = paddr;
>                  *last_size = pgsize;
>              } else {
> --
> 2.36.0
>


^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2022-04-28  0:15 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01 12:22 [PATCH] target/riscv: Fix incorrect PTE merge in walk_pte Ralf Ramsauer
2022-04-01 12:22 ` Ralf Ramsauer
2022-04-01 13:14 ` Richard Henderson
2022-04-04 17:22 ` Ralf Ramsauer
2022-04-04 17:34 ` [PATCH v2] " Ralf Ramsauer
2022-04-04 17:34   ` Ralf Ramsauer
2022-04-12  4:53   ` Alistair Francis
2022-04-12  4:53     ` Alistair Francis
2022-04-22  2:53   ` Bin Meng
2022-04-22  2:53     ` Bin Meng
2022-04-22  2:54     ` Bin Meng
2022-04-22  2:54       ` Bin Meng
2022-04-22 11:37       ` [EXT] " Ralf Ramsauer
2022-04-22 11:37         ` Ralf Ramsauer
2022-04-22 23:08         ` Alistair Francis
2022-04-22 23:08           ` Alistair Francis
2022-04-23 21:59           ` [PATCH v3] " Ralf Ramsauer
2022-04-23 21:59             ` Ralf Ramsauer
2022-04-24  1:20             ` Bin Meng
2022-04-24  1:20               ` Bin Meng
2022-04-26  0:16             ` Alistair Francis
2022-04-26  0:16               ` Alistair Francis
2022-04-27 23:52             ` Alistair Francis
2022-04-27 23:52               ` Alistair Francis
2022-04-28  0:13             ` Alistair Francis
2022-04-28  0:13               ` Alistair Francis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.