Re: [PATCH v0] RISC-V: Use Zkr to seed KASLR base address

Conor Dooley
2024-06-10T21:56:41Z

[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]

On Mon, Jun 10, 2024 at 02:06:50PM -0700, Deepak Gupta wrote:
> On Mon, Jun 10, 2024 at 11:16:42AM +0200, Clément Léger wrote:
> > On 10/06/2024 11:02, Conor Dooley wrote:
> > > On Mon, Jun 10, 2024 at 10:33:34AM +0200, Clément Léger 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/106479571
> > > >
> > > > 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 below
>
> `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 implemented).
> 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.

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 think
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 conform
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 they
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 that
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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]