| Message ID | 20220401122248.2792180-1-ralf.ramsauer@oth-regensburg.de |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Fix incorrect PTE merge in walk_pte | expand |
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~
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;
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;
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(-)