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

[Deepak Gupta <debug@rivosinc.com>]

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:
>>>>>> Hi Conor,
>>>>>>
>>>>>> On 31/05/2024 19:31, Conor Dooley wrote:
>>>>>>> On Fri, May 31, 2024 at 12:23:27PM -0400, Jesse Taube wrote:
>>>>>>>> Dectect the Zkr extension and use it to seed the kernel base
>>>>>> address.
>>>>>>>>
>>>>>>>> Detection of the extension can not be done in the typical
>>>>>> fashion, as
>>>>>>>> this is very early in the boot process. Instead, add a trap handler
>>>>>>>> and run it to see if the extension is present.
>>>>>>> You can't rely on the lack of a trap meaning that Zkr is present
>>>>>> unless
>>>>>>> you know that the platform implements Ssstrict. The CSR with that
>>>>>> number
>>>>>>> could do anything if not Ssstrict compliant, so this approach gets a
>>>>>>> nak from me. Unfortunately, Ssstrict doesn't provide a way to detect
>>>>>>> it, so you're stuck with getting that information from firmware.
>>>>>>
>>>>>>
>>>>>> FYI, this patch is my idea, so I'm the one to blame here :)
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> For DT systems, you can actually parse the DT in the pi, we do it
>>>>>> to get
>>>>>>> the kaslr seed if present, so you can actually check for Zkr. With
>>>>>> ACPI
>>>>>>> I have no idea how you can get that information, I amn't an ACPI-ist.
>>>>>>
>>>>>>
>>>>>> I took a look at how to access ACPI tables this early when
>>>>>> implementing the
>>>>>> Zabha/Zacas patches, but it seems not possible.
>>>>>>
>>>>>> But I'll look into this more, this is not the first time we need the
>>>>>> extensions list very early and since we have no way to detect the
>>>>>> presence
>>>>>> of an extension at runtime, something needs to be done.
>>>>>
>>>>> Aye, having remembered that reading CSR_SEED could have side-effects on a
>>>>> system with non-conforming extensions, it'd be good to see if we can
>>>>> actually do this via detection on ACPI - especially for some other
>>>>> extensions that we may need to turn on very early (I forget which ones we
>>>>> talked about this before for). I didn't arm64 do anything with ACPI in
>>>>> the
>>>>> pi code, is the code arch/x86/boot/compressed run at an equivilent-ish
>>>>> point
>>>>> in boot?
>>>>
>>>> cc: +Clement and Atish
>>>>
>>>> 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.

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.

>
>>
>>> Link: https://github.com/riscv/configuration-structure [1]
>>