linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Sean Anderson <seanga2@gmail.com>
To: Atish Patra <atishp@atishpatra.org>
Cc: linux-riscv <linux-riscv@lists.infradead.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Damien Le Moal <Damien.LeMoal@wdc.com>
Subject: Re: [PATCH v2 9/9] riscv: Do not initialize PMP on Kendryte SoC
Date: Thu, 12 Mar 2020 16:09:31 -0400	[thread overview]
Message-ID: <f691a251-1317-1ee8-78dd-8103dd14b339@gmail.com> (raw)
In-Reply-To: <CAOnJCUK5bY8tbPLOCdTzzNMcy2yjK7rHL4jiM=0j=waBo1y-Pw@mail.gmail.com>

On 3/12/20 3:12 PM, Atish Patra wrote:
> On Thu, Mar 12, 2020 at 11:49 AM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> On 3/12/20 2:29 PM, Atish Patra wrote:
>>> On Thu, Mar 12, 2020 at 11:14 AM Sean Anderson <seanga2@gmail.com> wrote:
>>>>
>>>>
>>>>> I don't think supporting old specs in Linux is a good idea. As per the
>>>>> patch guideline
>>>>> for RISC-V Linux, patches for only "frozen" or "ratified"
>>>>> specifications are allowed.
>>>>>
>>>>
>>>> Well this CPU follows the v1.9 spec. It's real hardware, if it is to be
>>>> be supported, then the 1.9 spec needs to be as well.
>>>>
>>>
>>> As RISC-V is an open ISA and it's continuously evolving, there will be
>>> some hardware
>>> that will implement old specifications or non-backward compatible features.
>>> I fear the number of hardware with such features/implementations will
>>> grow in the future.
>>> If Linux is going to support all of them, it may be a maintenance nightmare.
>>
>> I agree. There is also no standard way to communicate the implemented
>> privileged spec level e.g. in the device tree. The base integer
>> instruction set version can be specified in the riscv,isa property, such
>> as
>>
>> riscv,isa = "rv64i2p1..."
>>
>> However, there is no "extension" for the privileged specification.
>> A method to specify this would be helpful, especially since the
>> bootloader may need to enable the MMU before loading Linux since there
>> is no way to enable it from S-mode until v1.10.
>>
>> On the other hand, there is relatively little changed from v1.9 to the
>> current revision. The following list has the differences from the
>> current spec:
>>
>> * The PMP has flipped polarity
>> * The mcounteren CSRs are split
>> * sfence.vma is sfence.vm (though this should be handled by the sbi
>>   anyway)
>> * satp has a different name, and mode no longer exists in the top four
>>   bits. Since these bits used to be part of ASID, it's fine to write the
>>   mode to those bits. If linux never switches from (e.g.) sv39 to
>>   something else, there will be no observed difference either.
>>
>> Everything else is mostly forwards-compatible, as far as I can tell.
>> That is, assuming new behaviour on old hardware won't cause problems.
>>
> Even if the changes are minimal and we can easily hide under macro magic,
> it will create a bad precedent for the future. What if somebody sends
> a patch for
> a non-standard extension and cites kendryte support as an example.

I think there are substantial differences between a non-standard
extension, and what we would need for the K210. First, the changes we
would need are for the official specification. At the time this chip was
designed, this was *the* authritative privileged spec. I think if a
hardware vendor makes the effort to comply with the specification as it
exists at the time, then we should support that.

In addition, the incompatibilities are within the core boot process.
Most non-standard extensions will be optional extras which can be
completely ignored. For example, the GAP8 processor has a non-standard
extension which adds some instructions for complex number arithmetic
(and other operations). These instructions have no effect on the usual
boot process, and (if there was an MMU) Linux could run fine on that
board with no knowledge of these extensions.

Lastly, these non-standard instructions can be documented in a standard
way through the isa version string. This incompatibility in the spec has
no standard way to be documented.

>> A sufficiently smart kernel could even detect the version at runtime by
>> intentionally triggering behaviour which is illegal depending on the
>> privileged version, and then checking for an exception.
>>
> That's the maintenance nightmare I was talking about. For kendryte, it
> is only few incompatible changes
> in privilege specification but what if some hardware implements a
> variation of hypervisor spec or vector extension.

Hopefully that will not happen, but I think given the long development
period of the vector spec, it is inevitable that a chip will be released
with some subtle (or not-so-subtle) incompatibilities. As far as I can
tell, the restriction on non-ratified extensions is to prevent work
towards experimental specifications which may never have real hardware
which uses them. However, I think if real hardware is incompatible small
but fundamental way, then we should make the effort to support it,
especially when the patches already exist.

--Sean



  reply	other threads:[~2020-03-12 20:09 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-12  5:10 [PATCH v2 0/9] Kendryte k210 SoC boards support Damien Le Moal
2020-03-12  5:10 ` [PATCH v2 1/9] riscv: Unaligned load/store handling for M_MODE Damien Le Moal
2020-03-12  5:11 ` [PATCH v2 2/9] riscv: Add BUILTIN_DTB support Damien Le Moal
2020-03-12 20:03   ` Atish Patra
2020-03-12  5:11 ` [PATCH v2 3/9] riscv: Add SOC early init support Damien Le Moal
2020-03-12 18:21   ` Sean Anderson
2020-03-13  5:52     ` Damien Le Moal
2020-03-12 19:50   ` Atish Patra
2020-03-12 20:53   ` Atish Patra
2020-03-13  6:42     ` Damien Le Moal
2020-03-13 21:10       ` Atish Patra
2020-03-16  0:34         ` Damien Le Moal
2020-03-12  5:11 ` [PATCH v2 4/9] riscv: Add Kendryte K210 SoC support Damien Le Moal
2020-03-12 18:19   ` Sean Anderson
2020-03-13  5:43     ` Damien Le Moal
2020-03-12  5:11 ` [PATCH v2 5/9] riscv: Select required drivers for Kendryte SOC Damien Le Moal
2020-03-12  5:11 ` [PATCH v2 6/9] riscv: Add Kendryte K210 device tree Damien Le Moal
2020-03-12 18:16   ` Sean Anderson
2020-03-13  5:29     ` Damien Le Moal
2020-03-12  5:11 ` [PATCH v2 7/9] riscv: Kendryte K210 default config Damien Le Moal
2020-03-12 19:03   ` Atish Patra
2020-03-13  5:52     ` Damien Le Moal
2020-03-12  5:11 ` [PATCH v2 8/9] riscv: create a loader.bin boot image for Kendryte SoC Damien Le Moal
2020-03-12  5:11 ` [PATCH v2 9/9] riscv: Do not initialize PMP on " Damien Le Moal
2020-03-12 15:20   ` Sean Anderson
2020-03-12 18:10     ` Atish Patra
2020-03-12 18:14       ` Sean Anderson
2020-03-12 18:29         ` Atish Patra
2020-03-12 18:49           ` Sean Anderson
2020-03-12 19:12             ` Atish Patra
2020-03-12 20:09               ` Sean Anderson [this message]
2020-03-13  6:03               ` Damien Le Moal
2020-03-13  5:40             ` Anup Patel
2020-03-13  5:25       ` Damien Le Moal
2020-03-13  5:47         ` Atish Patra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f691a251-1317-1ee8-78dd-8103dd14b339@gmail.com \
    --to=seanga2@gmail.com \
    --cc=Damien.LeMoal@wdc.com \
    --cc=atishp@atishpatra.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).