target/riscv/monitor.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
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
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;
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~
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.1On 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
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>
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) ||
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) ||
>
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.0On 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
>
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
>
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
>
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>
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
>
>
© 2016 - 2026 Red Hat, Inc.