[PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Palmer Dabbelt]

Similar to the previous patch, this allows a dt-selected downgrade to
sv48 on systems that support sv57 in case users don't need the extra VA
bits and want to save memory or improve performance.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
This is on top of the patches from Alex's set that I dropped.
---
 arch/riscv/mm/init.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 16a0fa2e8432..33f7f9357ffa 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -740,6 +740,11 @@ static __init void set_satp_mode(uintptr_t dtb_pa)
 			if (!mmu_type)
 				continue;
 
+			if (!strcmp(mmu_type, "riscv,sv48")) {
+				disable_pgtable_l5();
+				return;
+			}
+
 			if (!strcmp(mmu_type, "riscv,sv39")) {
 				disable_pgtable_l5();
 				disable_pgtable_l4();
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Andreas Schwab]

s/used/user/?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Geert Uytterhoeven]

Hi Palmer,.

On Fri, Apr 22, 2022 at 11:42 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
> Similar to the previous patch, this allows a dt-selected downgrade to
> sv48 on systems that support sv57 in case users don't need the extra VA
> bits and want to save memory or improve performance.
>
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> ---
> This is on top of the patches from Alex's set that I dropped.

You mean "[PATCH v3 13/13] riscv: Allow user to downgrade to sv39
when hw supports sv48 if !KASAN"?
20211206104657.433304-14-alexandre.ghiti@canonical.com

For both: "DT describes hardware, not software policy"?

> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -740,6 +740,11 @@ static __init void set_satp_mode(uintptr_t dtb_pa)
>                         if (!mmu_type)
>                                 continue;
>
> +                       if (!strcmp(mmu_type, "riscv,sv48")) {
> +                               disable_pgtable_l5();
> +                               return;
> +                       }
> +
>                         if (!strcmp(mmu_type, "riscv,sv39")) {
>                                 disable_pgtable_l5();
>                                 disable_pgtable_l4();

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Anup Patel]

On Mon, Apr 25, 2022 at 6:12 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Palmer,.
>
> On Fri, Apr 22, 2022 at 11:42 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
> > Similar to the previous patch, this allows a dt-selected downgrade to
> > sv48 on systems that support sv57 in case users don't need the extra VA
> > bits and want to save memory or improve performance.
> >
> > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> > ---
> > This is on top of the patches from Alex's set that I dropped.
>
> You mean "[PATCH v3 13/13] riscv: Allow user to downgrade to sv39
> when hw supports sv48 if !KASAN"?
> 20211206104657.433304-14-alexandre.ghiti@canonical.com
>
> For both: "DT describes hardware, not software policy"?

It is possible that HW is designed to support both Sv48 and Sv39 but
there is some errata due to which Sv48 does not work correctly ?

We should allow users to downgrade the MMU mode, due to
their own reasons. In fact, users can also disable an extension
by not showing it in the DT ISA string.

Regards,
Anup

>
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -740,6 +740,11 @@ static __init void set_satp_mode(uintptr_t dtb_pa)
> >                         if (!mmu_type)
> >                                 continue;
> >
> > +                       if (!strcmp(mmu_type, "riscv,sv48")) {
> > +                               disable_pgtable_l5();
> > +                               return;
> > +                       }
> > +
> >                         if (!strcmp(mmu_type, "riscv,sv39")) {
> >                                 disable_pgtable_l5();
> >                                 disable_pgtable_l4();
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Geert Uytterhoeven]

Hi Anup,

On Mon, Apr 25, 2022 at 3:14 PM Anup Patel <apatel@ventanamicro.com> wrote:
> On Mon, Apr 25, 2022 at 6:12 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Apr 22, 2022 at 11:42 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
> > > Similar to the previous patch, this allows a dt-selected downgrade to
> > > sv48 on systems that support sv57 in case users don't need the extra VA
> > > bits and want to save memory or improve performance.
> > >
> > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> > > ---
> > > This is on top of the patches from Alex's set that I dropped.
> >
> > You mean "[PATCH v3 13/13] riscv: Allow user to downgrade to sv39
> > when hw supports sv48 if !KASAN"?
> > 20211206104657.433304-14-alexandre.ghiti@canonical.com
> >
> > For both: "DT describes hardware, not software policy"?
>
> It is possible that HW is designed to support both Sv48 and Sv39 but
> there is some errata due to which Sv48 does not work correctly ?

In that case, I assume the software has to disable Sv48 on its own?
Fixed hardware should use a different compatible value, so software
will know when the issue is fixed, and the feature can be used.
How else is DTB backwards-compatibility supposed to work?

> We should allow users to downgrade the MMU mode, due to
> their own reasons. In fact, users can also disable an extension
> by not showing it in the DT ISA string.

That sounds like a software policy, too.
What is wrong with a kernel command line option?
If you want it in your DTB, you can add it to chosen/bootargs.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Anup Patel]

On Mon, Apr 25, 2022 at 7:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Anup,
>
> On Mon, Apr 25, 2022 at 3:14 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > On Mon, Apr 25, 2022 at 6:12 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Fri, Apr 22, 2022 at 11:42 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
> > > > Similar to the previous patch, this allows a dt-selected downgrade to
> > > > sv48 on systems that support sv57 in case users don't need the extra VA
> > > > bits and want to save memory or improve performance.
> > > >
> > > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> > > > ---
> > > > This is on top of the patches from Alex's set that I dropped.
> > >
> > > You mean "[PATCH v3 13/13] riscv: Allow user to downgrade to sv39
> > > when hw supports sv48 if !KASAN"?
> > > 20211206104657.433304-14-alexandre.ghiti@canonical.com
> > >
> > > For both: "DT describes hardware, not software policy"?
> >
> > It is possible that HW is designed to support both Sv48 and Sv39 but
> > there is some errata due to which Sv48 does not work correctly ?
>
> In that case, I assume the software has to disable Sv48 on its own?
> Fixed hardware should use a different compatible value, so software
> will know when the issue is fixed, and the feature can be used.
> How else is DTB backwards-compatibility supposed to work?

Usually, HW vendors will use different names for incrementally
improving implementations so they will tend to create separate
dts/dtsi files for newer implementations with some sharing via
common dtsi files.

>
> > We should allow users to downgrade the MMU mode, due to
> > their own reasons. In fact, users can also disable an extension
> > by not showing it in the DT ISA string.
>
> That sounds like a software policy, too.
> What is wrong with a kernel command line option?

The MMU modes are detected very early and even before the kernel
command-line is parsed.

> If you want it in your DTB, you can add it to chosen/bootargs.

If HW vendor describe complete details in DT and disables few
things in chosen/bootargs then it means there is some issue with
those things disabled via chosen/bootargs.

A HW vendor might never want to advertise broken extensions in
their implementation so the ISA string and various HART features
in DT will be set based on working functionality on real hardware.

Regards,
Anup

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Geert Uytterhoeven]

Hi Anup,

On Mon, Apr 25, 2022 at 4:14 PM Anup Patel <apatel@ventanamicro.com> wrote:
> On Mon, Apr 25, 2022 at 7:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Apr 25, 2022 at 3:14 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > > On Mon, Apr 25, 2022 at 6:12 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Fri, Apr 22, 2022 at 11:42 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
> > > > > Similar to the previous patch, this allows a dt-selected downgrade to
> > > > > sv48 on systems that support sv57 in case users don't need the extra VA
> > > > > bits and want to save memory or improve performance.
> > > > >
> > > > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> > > > > ---
> > > > > This is on top of the patches from Alex's set that I dropped.
> > > >
> > > > You mean "[PATCH v3 13/13] riscv: Allow user to downgrade to sv39
> > > > when hw supports sv48 if !KASAN"?
> > > > 20211206104657.433304-14-alexandre.ghiti@canonical.com
> > > >
> > > > For both: "DT describes hardware, not software policy"?
> > >
> > > It is possible that HW is designed to support both Sv48 and Sv39 but
> > > there is some errata due to which Sv48 does not work correctly ?
> >
> > In that case, I assume the software has to disable Sv48 on its own?
> > Fixed hardware should use a different compatible value, so software
> > will know when the issue is fixed, and the feature can be used.
> > How else is DTB backwards-compatibility supposed to work?
>
> Usually, HW vendors will use different names for incrementally
> improving implementations so they will tend to create separate
> dts/dtsi files for newer implementations with some sharing via
> common dtsi files.

Even for a minor update of the SoC to fix some errata, where newer
boards just contain a newer revision of the silicon?

> > > We should allow users to downgrade the MMU mode, due to
> > > their own reasons. In fact, users can also disable an extension
> > > by not showing it in the DT ISA string.
> >
> > That sounds like a software policy, too.
> > What is wrong with a kernel command line option?
>
> The MMU modes are detected very early and even before the kernel
> command-line is parsed.

That can be fixed.

> > If you want it in your DTB, you can add it to chosen/bootargs.
>
> If HW vendor describe complete details in DT and disables few
> things in chosen/bootargs then it means there is some issue with
> those things disabled via chosen/bootargs.
>
> A HW vendor might never want to advertise broken extensions in
> their implementation so the ISA string and various HART features
> in DT will be set based on working functionality on real hardware.

That may be true in a perfect world.
Depending on the level of brokenness, the issue may not be detected
before devices are released into the world.  So you're better prepared
for such cases.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: Allow the used to downgrade to sv48 when HW supports sv57

[Palmer Dabbelt]

On Mon, 25 Apr 2022 07:30:52 PDT (-0700), geert@linux-m68k.org wrote:
> Hi Anup,
>
> On Mon, Apr 25, 2022 at 4:14 PM Anup Patel <apatel@ventanamicro.com> wrote:
>> On Mon, Apr 25, 2022 at 7:06 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> > On Mon, Apr 25, 2022 at 3:14 PM Anup Patel <apatel@ventanamicro.com> wrote:
>> > > On Mon, Apr 25, 2022 at 6:12 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> > > > On Fri, Apr 22, 2022 at 11:42 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>> > > > > Similar to the previous patch, this allows a dt-selected downgrade to
>> > > > > sv48 on systems that support sv57 in case users don't need the extra VA
>> > > > > bits and want to save memory or improve performance.
>> > > > >
>> > > > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
>> > > > > ---
>> > > > > This is on top of the patches from Alex's set that I dropped.
>> > > >
>> > > > You mean "[PATCH v3 13/13] riscv: Allow user to downgrade to sv39
>> > > > when hw supports sv48 if !KASAN"?
>> > > > 20211206104657.433304-14-alexandre.ghiti@canonical.com
>> > > >
>> > > > For both: "DT describes hardware, not software policy"?

Ya, sorry I missed that -- luckily neither of these actually boot, so 
there wasn't much of a rush...

>> > >
>> > > It is possible that HW is designed to support both Sv48 and Sv39 but
>> > > there is some errata due to which Sv48 does not work correctly ?
>> >
>> > In that case, I assume the software has to disable Sv48 on its own?
>> > Fixed hardware should use a different compatible value, so software
>> > will know when the issue is fixed, and the feature can be used.
>> > How else is DTB backwards-compatibility supposed to work?
>>
>> Usually, HW vendors will use different names for incrementally
>> improving implementations so they will tend to create separate
>> dts/dtsi files for newer implementations with some sharing via
>> common dtsi files.
>
> Even for a minor update of the SoC to fix some errata, where newer
> boards just contain a newer revision of the silicon?
>
>> > > We should allow users to downgrade the MMU mode, due to
>> > > their own reasons. In fact, users can also disable an extension
>> > > by not showing it in the DT ISA string.
>> >
>> > That sounds like a software policy, too.
>> > What is wrong with a kernel command line option?
>>
>> The MMU modes are detected very early and even before the kernel
>> command-line is parsed.
>
> That can be fixed.
>
>> > If you want it in your DTB, you can add it to chosen/bootargs.
>>
>> If HW vendor describe complete details in DT and disables few
>> things in chosen/bootargs then it means there is some issue with
>> those things disabled via chosen/bootargs.
>>
>> A HW vendor might never want to advertise broken extensions in
>> their implementation so the ISA string and various HART features
>> in DT will be set based on working functionality on real hardware.
>
> That may be true in a perfect world.
> Depending on the level of brokenness, the issue may not be detected
> before devices are released into the world.  So you're better prepared
> for such cases.

IMO there's really two use cases here, and they've got different ways 
they should be triggered:

* Users who have systems with larger VA spaces, but know they aren't 
  going to actually use that much VA and don't want to pay for.  That 
  seems like a kernel command-line option is the best bet, as it's a 
  user configuration option and command-line is a pretty standard way to 
  do that.  Certainly forcing a DT modification would be odd.
* Users who have systems that attempted to have larger VA spaces, but 
  are actually broken.  That's just an errata and we already have errata 
  mechanisms, so let's just stick with those.

There is a bit of an elephant in the room here WRT our errata mechanism 
where we're relying on vendors to be sufficiently well behaved that they 
leave something we can detect at runtime to select those errata, but I'm 
not really sure there's any way around that.  We'll just have to add 
complexity there over time as HW vendors find new and exciting ways to 
break things.  Wouldn't be surprised if some scheme ends up passing info 
via DT at some point, but given that we're already relying on SBI to get 
m{vendor,imp,arch}id maybe there's another way to do it.

IIRC someone sent by an errata of this flavor already, so that's 
probably the right place to start.  That one we should be able to do 
without breaking medlow support, which is kind of nice.  A command-line 
option also seems reasonable, but I don't think it's a super pressing 
matter for now so I don't intend on working on it (though happy to take 
patches if someone else wants to).

Regardless of exactly where we go here, sounds like this isn't the right 
answer so I'm not going to take it (or the removal of medlow, at least 
until something needs that).

Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv