| Message ID | 20220412033335.1384230-1-apatel@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: mm: Fix set_satp_mode() for platform not having Sv57 | expand |
Hi Anup,
>
> When Sv57 is not available the satp.MODE test in set_satp_mode() will
> fail and lead to pgdir re-programming for Sv48. The pgdir re-programming
> will fail as well due to pre-existing pgdir entry used for Sv57 and as
> a result kernel fails to boot on RISC-V platform not having Sv57.
>
> To fix above issue, we should clear the pgdir memory in set_satp_mode()
> before re-programming.
>
> Fixes: 011f09d12052 ("riscv: mm: Set sv57 on defaultly")
> Reported-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> arch/riscv/mm/init.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 9535bea8688c..b0793dc0c291 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -718,6 +718,7 @@ static __init void set_satp_mode(void)
> if (!check_l4) {
> disable_pgtable_l5();
> check_l4 = true;
> + memset(early_pg_dir, 0, PAGE_SIZE);
> goto retry;
> }
> disable_pgtable_l4();
> --
I find it that the set_satp_mode function is in .init.text section which begins at 0x80800000.
And its pgd_index in both Sv48 and Sv57 will be 0. So it may not be necessary to clear the
early_pg_dir when the kernel find Sv57 is not supported? And may I get the steps of reproduction
from you?
Yours,
Qinglin
</apatel@ventanamicro.com></mchitale@ventanamicro.com>
On Fri, Apr 15, 2022 at 10:18 AM 潘庆霖 <panqinglin2020@iscas.ac.cn> wrote: > > Hi Anup, > > > > > When Sv57 is not available the satp.MODE test in set_satp_mode() will > > fail and lead to pgdir re-programming for Sv48. The pgdir re-programming > > will fail as well due to pre-existing pgdir entry used for Sv57 and as > > a result kernel fails to boot on RISC-V platform not having Sv57. > > > > To fix above issue, we should clear the pgdir memory in set_satp_mode() > > before re-programming. > > > > Fixes: 011f09d12052 ("riscv: mm: Set sv57 on defaultly") > > Reported-by: Mayuresh Chitale <mchitale@ventanamicro.com> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > arch/riscv/mm/init.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > > index 9535bea8688c..b0793dc0c291 100644 > > --- a/arch/riscv/mm/init.c > > +++ b/arch/riscv/mm/init.c > > @@ -718,6 +718,7 @@ static __init void set_satp_mode(void) > > if (!check_l4) { > > disable_pgtable_l5(); > > check_l4 = true; > > + memset(early_pg_dir, 0, PAGE_SIZE); > > goto retry; > > } > > disable_pgtable_l4(); > > -- > > > I find it that the set_satp_mode function is in .init.text section which begins at 0x80800000. > And its pgd_index in both Sv48 and Sv57 will be 0. So it may not be necessary to clear the > early_pg_dir when the kernel find Sv57 is not supported? And may I get the steps of reproduction > from you? We can't assume that it will be the same pgd_index for Sv48 and Sv57. For example, some hypothetical SoC might have RAM starting after 1TB space. We should ensure that early_pg_dir is cleaned entirely for detecting the next mode. Regards, Anup > > Yours, > Qinglin > </apatel@ventanamicro.com></mchitale@ventanamicro.com>
Hi Anup, > > We can't assume that it will be the same pgd_index for Sv48 and Sv57. > > For example, some hypothetical SoC might have RAM starting after 1TB space. > > We should ensure that early_pg_dir is cleaned entirely for detecting the next > mode. > Got it. Your idea is really more stable. Thank you for pointing out that! Yours, Qinglin
On Mon, Apr 11, 2022 at 8:34 PM Anup Patel <apatel@ventanamicro.com> wrote: > > When Sv57 is not available the satp.MODE test in set_satp_mode() will > fail and lead to pgdir re-programming for Sv48. The pgdir re-programming > will fail as well due to pre-existing pgdir entry used for Sv57 and as > a result kernel fails to boot on RISC-V platform not having Sv57. > > To fix above issue, we should clear the pgdir memory in set_satp_mode() > before re-programming. > > Fixes: 011f09d12052 ("riscv: mm: Set sv57 on defaultly") > Reported-by: Mayuresh Chitale <mchitale@ventanamicro.com> > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > arch/riscv/mm/init.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index 9535bea8688c..b0793dc0c291 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -718,6 +718,7 @@ static __init void set_satp_mode(void) > if (!check_l4) { > disable_pgtable_l5(); > check_l4 = true; > + memset(early_pg_dir, 0, PAGE_SIZE); > goto retry; > } > disable_pgtable_l4(); > -- > 2.25.1 > Reviewed-by: Atish Patra <atishp@rivosinc.com>
Hi Palmer, On Tue, Apr 12, 2022 at 9:04 AM Anup Patel <apatel@ventanamicro.com> wrote: > > When Sv57 is not available the satp.MODE test in set_satp_mode() will > fail and lead to pgdir re-programming for Sv48. The pgdir re-programming > will fail as well due to pre-existing pgdir entry used for Sv57 and as > a result kernel fails to boot on RISC-V platform not having Sv57. > > To fix above issue, we should clear the pgdir memory in set_satp_mode() > before re-programming. > > Fixes: 011f09d12052 ("riscv: mm: Set sv57 on defaultly") > Reported-by: Mayuresh Chitale <mchitale@ventanamicro.com> > Signed-off-by: Anup Patel <apatel@ventanamicro.com> Can this be considered for 5.18-rcX ? Regards, Anup > --- > arch/riscv/mm/init.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index 9535bea8688c..b0793dc0c291 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -718,6 +718,7 @@ static __init void set_satp_mode(void) > if (!check_l4) { > disable_pgtable_l5(); > check_l4 = true; > + memset(early_pg_dir, 0, PAGE_SIZE); > goto retry; > } > disable_pgtable_l4(); > -- > 2.25.1 >
On Thu, 21 Apr 2022 02:30:05 PDT (-0700), apatel@ventanamicro.com wrote: > Hi Palmer, > > On Tue, Apr 12, 2022 at 9:04 AM Anup Patel <apatel@ventanamicro.com> wrote: >> >> When Sv57 is not available the satp.MODE test in set_satp_mode() will >> fail and lead to pgdir re-programming for Sv48. The pgdir re-programming >> will fail as well due to pre-existing pgdir entry used for Sv57 and as >> a result kernel fails to boot on RISC-V platform not having Sv57. >> >> To fix above issue, we should clear the pgdir memory in set_satp_mode() >> before re-programming. >> >> Fixes: 011f09d12052 ("riscv: mm: Set sv57 on defaultly") >> Reported-by: Mayuresh Chitale <mchitale@ventanamicro.com> >> Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > Can this be considered for 5.18-rcX ? Sorry, there's a queue but I just say this one at the top and it's super simple so I'm going to take it now -- IRC is good to ping this sort of stuff. It's in fixes. Thanks! > > Regards, > Anup > >> --- >> arch/riscv/mm/init.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c >> index 9535bea8688c..b0793dc0c291 100644 >> --- a/arch/riscv/mm/init.c >> +++ b/arch/riscv/mm/init.c >> @@ -718,6 +718,7 @@ static __init void set_satp_mode(void) >> if (!check_l4) { >> disable_pgtable_l5(); >> check_l4 = true; >> + memset(early_pg_dir, 0, PAGE_SIZE); >> goto retry; >> } >> disable_pgtable_l4(); >> -- >> 2.25.1 >>
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c index 9535bea8688c..b0793dc0c291 100644 --- a/arch/riscv/mm/init.c +++ b/arch/riscv/mm/init.c @@ -718,6 +718,7 @@ static __init void set_satp_mode(void) if (!check_l4) { disable_pgtable_l5(); check_l4 = true; + memset(early_pg_dir, 0, PAGE_SIZE); goto retry; } disable_pgtable_l4();
When Sv57 is not available the satp.MODE test in set_satp_mode() will fail and lead to pgdir re-programming for Sv48. The pgdir re-programming will fail as well due to pre-existing pgdir entry used for Sv57 and as a result kernel fails to boot on RISC-V platform not having Sv57. To fix above issue, we should clear the pgdir memory in set_satp_mode() before re-programming. Fixes: 011f09d12052 ("riscv: mm: Set sv57 on defaultly") Reported-by: Mayuresh Chitale <mchitale@ventanamicro.com> Signed-off-by: Anup Patel <apatel@ventanamicro.com> --- arch/riscv/mm/init.c | 1 + 1 file changed, 1 insertion(+)