| Message ID | 20230711105202.842408-1-suagrfillet@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | riscv: correct pt_level name via pgtable_l5/4_enabled | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be fixes at HEAD 06c2afb862f9 |
| conchuod/fixes_present | success | Fixes tag present in non-next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 11 this patch: 11 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 23 this patch: 23 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 11 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | Fixes tag looks correct |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Hi Song, On 11/07/2023 12:52, Song Shuai wrote: > The pt_level uses CONFIG_PGTABLE_LEVELS to display page table names. > But if downgrading page mode from kernel cmdline in 64BIT, it will > give a wrong name. > > Like, using no4lvl for sv39, ptdump named the 1G-mapping as "PUD" > that should be "PGD": > > 0xffffffd840000000-0xffffffd900000000 0x00000000c0000000 3G PUD D A G . . W R V > > So select "P4D/PUD" or "PGD" via pgtable_l5/4_enabled to correct it. > > Fixes: 26e7aacb83df ("riscv: Allow to downgrade paging mode from the command line") > Signed-off-by: Song Shuai <suagrfillet@gmail.com> > --- > arch/riscv/mm/ptdump.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c > index 20a9f991a6d7..cfdd327981ee 100644 > --- a/arch/riscv/mm/ptdump.c > +++ b/arch/riscv/mm/ptdump.c > @@ -384,6 +384,11 @@ static int __init ptdump_init(void) > > kernel_ptd_info.base_addr = KERN_VIRT_START; > > +#ifdef CONFIG_64BIT > + pg_level[1].name = pgtable_l5_enabled ? "P4D" : "PGD"; > + pg_level[2].name = pgtable_l4_enabled ? "PUD" : "PGD"; > +#endif You don't need the #ifdef here as pgtable_lX_enabled are always declared. > + > for (i = 0; i < ARRAY_SIZE(pg_level); i++) > for (j = 0; j < ARRAY_SIZE(pte_bits); j++) > pg_level[i].mask |= pte_bits[j].mask; The Fixes tag is wrong to me, if satp mode is restricted by the hardware itself (not from the command line), the same problem happens. Maybe that should be the sv48 introduction patch? Or the sv57? It will be more cumbersome to backport with the sv48 introduction though as this patch won't apply as-is. Otherwise, you can add: Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks, Alex
On Tue, 11 Jul 2023 04:14:10 PDT (-0700), alex@ghiti.fr wrote: > Hi Song, > > > On 11/07/2023 12:52, Song Shuai wrote: >> The pt_level uses CONFIG_PGTABLE_LEVELS to display page table names. >> But if downgrading page mode from kernel cmdline in 64BIT, it will >> give a wrong name. >> >> Like, using no4lvl for sv39, ptdump named the 1G-mapping as "PUD" >> that should be "PGD": >> >> 0xffffffd840000000-0xffffffd900000000 0x00000000c0000000 3G PUD D A G . . W R V >> >> So select "P4D/PUD" or "PGD" via pgtable_l5/4_enabled to correct it. >> >> Fixes: 26e7aacb83df ("riscv: Allow to downgrade paging mode from the command line") >> Signed-off-by: Song Shuai <suagrfillet@gmail.com> >> --- >> arch/riscv/mm/ptdump.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c >> index 20a9f991a6d7..cfdd327981ee 100644 >> --- a/arch/riscv/mm/ptdump.c >> +++ b/arch/riscv/mm/ptdump.c >> @@ -384,6 +384,11 @@ static int __init ptdump_init(void) >> >> kernel_ptd_info.base_addr = KERN_VIRT_START; >> >> +#ifdef CONFIG_64BIT >> + pg_level[1].name = pgtable_l5_enabled ? "P4D" : "PGD"; >> + pg_level[2].name = pgtable_l4_enabled ? "PUD" : "PGD"; >> +#endif > > > You don't need the #ifdef here as pgtable_lX_enabled are always declared. > > >> + >> for (i = 0; i < ARRAY_SIZE(pg_level); i++) >> for (j = 0; j < ARRAY_SIZE(pte_bits); j++) >> pg_level[i].mask |= pte_bits[j].mask; > > > The Fixes tag is wrong to me, if satp mode is restricted by the hardware > itself (not from the command line), the same problem happens. Maybe that > should be the sv48 introduction patch? Or the sv57? It will be more > cumbersome to backport with the sv48 introduction though as this patch > won't apply as-is. Sometimes it's easier to base the fix on the original offending commit and then forward-port it via a merge, I haven't looked at this one to know for sure though. > Otherwise, you can add: > > Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks. Song: do you have a v2 that addresses the comments? > > Thanks, > > Alex
在 2023/7/12 21:58, Palmer Dabbelt 写道: > On Tue, 11 Jul 2023 04:14:10 PDT (-0700), alex@ghiti.fr wrote: >> Hi Song, >> >> >> On 11/07/2023 12:52, Song Shuai wrote: >>> The pt_level uses CONFIG_PGTABLE_LEVELS to display page table names. >>> But if downgrading page mode from kernel cmdline in 64BIT, it will >>> give a wrong name. >>> >>> Like, using no4lvl for sv39, ptdump named the 1G-mapping as "PUD" >>> that should be "PGD": >>> >>> 0xffffffd840000000-0xffffffd900000000 0x00000000c0000000 >>> 3G PUD D A G . . W R V >>> >>> So select "P4D/PUD" or "PGD" via pgtable_l5/4_enabled to correct it. >>> >>> Fixes: 26e7aacb83df ("riscv: Allow to downgrade paging mode from the >>> command line") >>> Signed-off-by: Song Shuai <suagrfillet@gmail.com> >>> --- >>> arch/riscv/mm/ptdump.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c >>> index 20a9f991a6d7..cfdd327981ee 100644 >>> --- a/arch/riscv/mm/ptdump.c >>> +++ b/arch/riscv/mm/ptdump.c >>> @@ -384,6 +384,11 @@ static int __init ptdump_init(void) >>> >>> kernel_ptd_info.base_addr = KERN_VIRT_START; >>> >>> +#ifdef CONFIG_64BIT >>> + pg_level[1].name = pgtable_l5_enabled ? "P4D" : "PGD"; >>> + pg_level[2].name = pgtable_l4_enabled ? "PUD" : "PGD"; >>> +#endif >> >> >> You don't need the #ifdef here as pgtable_lX_enabled are always declared. >> >> >>> + >>> for (i = 0; i < ARRAY_SIZE(pg_level); i++) >>> for (j = 0; j < ARRAY_SIZE(pte_bits); j++) >>> pg_level[i].mask |= pte_bits[j].mask; >> >> >> The Fixes tag is wrong to me, if satp mode is restricted by the hardware >> itself (not from the command line), the same problem happens. Maybe that >> should be the sv48 introduction patch? Or the sv57? It will be more >> cumbersome to backport with the sv48 introduction though as this patch >> won't apply as-is. > > Sometimes it's easier to base the fix on the original offending commit > and then forward-port it via a merge, I haven't looked at this one to > know for sure though. > >> Otherwise, you can add: >> >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > Thanks. > > Song: do you have a v2 that addresses the comments? > Here is V2 : https://lore.kernel.org/linux-riscv/20230712115740.943324-1-suagrfillet@gmail.com/ >> >> Thanks, >> >> Alex
diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c index 20a9f991a6d7..cfdd327981ee 100644 --- a/arch/riscv/mm/ptdump.c +++ b/arch/riscv/mm/ptdump.c @@ -384,6 +384,11 @@ static int __init ptdump_init(void) kernel_ptd_info.base_addr = KERN_VIRT_START; +#ifdef CONFIG_64BIT + pg_level[1].name = pgtable_l5_enabled ? "P4D" : "PGD"; + pg_level[2].name = pgtable_l4_enabled ? "PUD" : "PGD"; +#endif + for (i = 0; i < ARRAY_SIZE(pg_level); i++) for (j = 0; j < ARRAY_SIZE(pte_bits); j++) pg_level[i].mask |= pte_bits[j].mask;
The pt_level uses CONFIG_PGTABLE_LEVELS to display page table names. But if downgrading page mode from kernel cmdline in 64BIT, it will give a wrong name. Like, using no4lvl for sv39, ptdump named the 1G-mapping as "PUD" that should be "PGD": 0xffffffd840000000-0xffffffd900000000 0x00000000c0000000 3G PUD D A G . . W R V So select "P4D/PUD" or "PGD" via pgtable_l5/4_enabled to correct it. Fixes: 26e7aacb83df ("riscv: Allow to downgrade paging mode from the command line") Signed-off-by: Song Shuai <suagrfillet@gmail.com> --- arch/riscv/mm/ptdump.c | 5 +++++ 1 file changed, 5 insertions(+)