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 <[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
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 Mon, Apr 11, 2022 at 8:34 PM Anup Patel <[email protected]> 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 <[email protected]>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> 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 <[email protected]>
--
Regards,
Atish
On Fri, Apr 15, 2022 at 10:18 AM 潘庆霖 <[email protected]> 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 <[email protected]>
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > 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
> </[email protected]></[email protected]>
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 Thu, 21 Apr 2022 02:30:05 PDT (-0700), [email protected] wrote:
> Hi Palmer,
>
> On Tue, Apr 12, 2022 at 9:04 AM Anup Patel <[email protected]> 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 <[email protected]>
>> Signed-off-by: Anup Patel <[email protected]>
>
> 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
>>
Hi Palmer,
On Tue, Apr 12, 2022 at 9:04 AM Anup Patel <[email protected]> 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 <[email protected]>
> Signed-off-by: Anup Patel <[email protected]>
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
>