On Mon, 25 Apr 2022 07:30:52 PDT (-0700), geert@xxxxxxxxxxxxxx wrote:
> Hi Anup,
>
> On Mon, Apr 25, 2022 at 4:14 PM Anup Patel <apatel@xxxxxxxxxxxxxxxx> wrote:
>> On Mon, Apr 25, 2022 at 7:06 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>> > On Mon, Apr 25, 2022 at 3:14 PM Anup Patel <apatel@xxxxxxxxxxxxxxxx> wrote:
>> > > On Mon, Apr 25, 2022 at 6:12 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>> > > > On Fri, Apr 22, 2022 at 11:42 PM Palmer Dabbelt <palmer@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>> > > > > ---
>> > > > > 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@xxxxxxxxxxxxx
>> > > >
>> > > > 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!
