Received-SPF: pass (google.com: domain of linux-kernel+bounces-211100-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240531162327.2436962-1-jesse@rivosinc.com> <20240531-uselessly-spied-262ecf44e694@spud>
 <b199fde6-c24e-4c18-9c38-fdc923294551@ghiti.fr> <20240603-stinking-roster-cfad46696ae5@spud>
 <ZmNWmxak9Rc80Kpb@debug.ba.rivosinc.com> <c288f739-2a1a-4fd7-884b-12e360a33142@rivosinc.com>
 <20240610-qualm-chalice-72d0cc743658@wendy> <01547275-8c8c-43cf-9da0-64825c234707@rivosinc.com>
 <Zmdq6kszPBxAvLdD@debug.ba.rivosinc.com> <20240610-earplugs-anybody-ebd04a5fa777@spud>
 <Zmht+h75GgxP3ocz@debug.ba.rivosinc.com> <c1f5a958-1130-4bdb-a154-0c0eeb06c8f9@rivosinc.com>
In-Reply-To: <c1f5a958-1130-4bdb-a154-0c0eeb06c8f9@rivosinc.com>
From: Atish Kumar Patra <atishp@rivosinc.com>
Date: Wed, 12 Jun 2024 00:48:23 -0700
Message-ID: <CAHBxVyEpfdpnazufL-oC=r1BoMzyXGdUhSbHPMOZfOj=cb6XkA@mail.gmail.com>
Subject: Re: [PATCH v0] RISC-V: Use Zkr to seed KASLR base address
To: =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= <cleger@rivosinc.com>
Cc: Deepak Gupta <debug@rivosinc.com>, Conor Dooley <conor@kernel.org>, 
	Conor Dooley <conor.dooley@microchip.com>, Alexandre Ghiti <alex@ghiti.fr>, 
	Jesse Taube <jesse@rivosinc.com>, linux-riscv@lists.infradead.org, 
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev, 
	Alexandre Ghiti <alexghiti@rivosinc.com>, Palmer Dabbelt <palmer@dabbelt.com>, 
	Albert Ou <aou@eecs.berkeley.edu>, =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@rivosinc.com>, 
	Paul Walmsley <paul.walmsley@sifive.com>, Nathan Chancellor <nathan@kernel.org>, 
	Nick Desaulniers <ndesaulniers@google.com>, Masahiro Yamada <masahiroy@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 12, 2024 at 12:15=E2=80=AFAM Cl=C3=A9ment L=C3=A9ger <cleger@ri=
vosinc.com> wrote:
>
>
>
> On 11/06/2024 17:32, Deepak Gupta wrote:
> > On Mon, Jun 10, 2024 at 10:56:35PM +0100, Conor Dooley wrote:
> >> On Mon, Jun 10, 2024 at 02:06:50PM -0700, Deepak Gupta wrote:
> >>> On Mon, Jun 10, 2024 at 11:16:42AM +0200, Cl=C3=A9ment L=C3=A9ger wro=
te:
> >>> > On 10/06/2024 11:02, Conor Dooley wrote:
> >>> > > On Mon, Jun 10, 2024 at 10:33:34AM +0200, Cl=C3=A9ment L=C3=A9ger=
 wrote:
> >>> > > > On 07/06/2024 20:51, Deepak Gupta wrote:
> >>> > > > > On Mon, Jun 03, 2024 at 01:47:40PM +0100, Conor Dooley wrote:
> >>> > > > > > On Mon, Jun 03, 2024 at 11:14:49AM +0200, Alexandre Ghiti
> >>> wrote:
> >>> > > > > I don't know all the details but on first glance it seems
> >>> like instead
> >>> > > > > of ACPI,
> >>> > > > > may be FWFT is a better place for discovery ?
> >>> > > > >
> >>> https://lists.riscv.org/g/tech-prs/topic/patch_v12_add_firmware/10647=
9571
> >>> > > >
> >>> > > > IMHO, doing discovery in FWFT is not the goal of this
> >>> extension. I think
> >>> > > > the "real" solution would be to wait for the unified discovery
> >>> task
> >>> > > > group to come up with something for that (which is their goal I
> >>> think) [1]
> >>>
> >>> Yeah I understand the conundrum here.
> >>>
> >>> > >
> >>> > > I'm curious to see how that works out. The proposal documents an
> >>> m-mode
> >>> > > csr, so we'd have to smuggle the information to s-mode somehow...
> >>> >
> >>> > Ahem, yeah, I spoke a bit too fast. Looked at the proposal and the
> >>> > mconfigptr CSR will be accessible by M-mode only so I guess we will
> >>> have
> >>> > to find another way...
> >>>
> >>> That's not the only problem. Even if you get mconfigptr access,
> >>> parsing the format
> >>> is another thing that has to be done. This is early in boot. Although
> >>> its strictly (pun
> >>> intended) not a firmware feature extension, I think it's much easier
> >>> to ask underlying
> >>> firmware if platform is `Sstrict`. or may be expose something like be=
low
> >>>
> >>> `ENABLE_SSTRICT`.
> >>> Platforms which support `Sstrict` can return success for this while
> >>> platforms which don't
> >>> have `Sstrict` can return error code (not supported or not implemente=
d).
> >>> This way its not feature discovery.
> >>
> >> I mean, it's feature discovery in all but name. You're calling it
> >> enable, but the behaviour you describe is feature discovery - not that=
 I
> >> am against this being feature discovery since it gets us out of an
> >> annoying bind.
> >
> > Yes I know it's cheating but at least for this case, it seems like easy
> > solution which
> > doesn't break anything. Neither I see it creating any future problems
> > (except FWFT becoming
> > to look like discovery mechanism :-) and Clement/Atish hating me for th=
at)
>
> Ahah no worries;) Thinking a bit more about it, if we need only a few
> extensions to be discoverable, it seems manageable (ie add a "locked"
> feature that report the extension availability itself, get only will
> work, set will return SBI_EDENIED). But if need more of them, then a
> dedicated mechanism should probably be designed.
>
Retrying again as gmail switched my default to html again :(. Sorry
for the spam.

Yes. Once it is allowed, there will be many more in the future :).
Reading this thread, it seems we need early detection for these 3 now.

Zabha/Zacas/Zkr

Is there any use case for obtaining additional information related to
an ISA or just extension presence is enough ?
+Sunil (in case he has some tricks in ACPI land for early parsing).

Otherwise, we may have to come up with a separate mechanism for early detec=
tion.

> Cl=C3=A9ment
>
> >
> > Another solution to this could be introducing a riscv config
> > `CONFIG_RISCV_SSTRICT`.
> > By default always select `CONFIG_RISCV_SSTRICT` and any platform owner
> > who are not
> > sstrict, they can build their own.
> > I expect distro (ubuntu, red hat, etc) would want by default
> > `CONFIG_RISCV_SSTRICT`.
> >
> >>
> >> I forget which extension Alex and I discussed previously, but there's
> >> some mm-related things that you're gonna have to probe super early and
> >> we need to figure out if we can get that info from ACPI or not. I know
> >> we discussed it w.r.t. one of the T-Head vendor extensions, but I thin=
k
> >> another standard one got mentioned too.
> >>
> >>> It seems like arm64 parses fdt and it reads certain CSRs too
> >>> (`arch/arm64/kernel/pi/kaslr_early.c`). Although it doesn't look like
> >>> it has to do any
> >>> discovery for them.
> >>
> >> A decree from the Palmer that we don't support things that do not conf=
orm
> >> in this manner would allow us to do what arm64 does. I brought this up
> >> originally because it's been discussed before that we cannot rely on
> >> conformance because we want to support people's platforms, whether the=
y
> >> comply or not. I'd be wary of making this an exception now, and then
> >> later on someone makes a platform we want to support that doesn't
> >> conform and hey presto, we regress KASLR support - even if I think it =
is
> >> pretty unlikely that someone is going to repurpose the Zkr CSRs.
> >>
> >> One of the problems with only supporting conforming platforms is that
> >> the definition of conforming changes over time! This has happened with
> >> the Andes PMU for example, which I believe uses an interrupt number th=
at
> >> was later co-opted by AIA spec. That conformed at the time, but doesn'=
t
> >> anymore - do they get to mark themselves as Sstrict?
> >>
> >> Maybe we can do this on a case-by-case basis, but it's up to Palmer
> >> whether or not we can do that.