Am Dienstag, 15. Februar 2022, 10:48:16 CET schrieb Heiko Stübner:

Am Dienstag, 15. Februar 2022, 10:12:53 CET schrieb Atish Kumar Patra:

quoted

On Mon, Feb 14, 2022 at 3:22 PM Atish Kumar Patra [off-list ref] wrote:

quoted

On Mon, Feb 14, 2022 at 2:22 PM Heiko Stübner [off-list ref] wrote:

quoted

Am Montag, 14. Februar 2022, 21:42:32 CET schrieb Atish Patra:

quoted

On Mon, Feb 14, 2022 at 12:24 PM Heiko Stübner [off-list ref] wrote:

quoted

Am Montag, 14. Februar 2022, 21:14:13 CET schrieb Atish Patra:

quoted

On Mon, Feb 14, 2022 at 12:06 PM Heiko Stübner [off-list ref] wrote:

quoted

Am Donnerstag, 10. Februar 2022, 22:40:16 CET schrieb Atish Patra:

quoted

Multi-letter extensions can be probed using exising
riscv_isa_extension_available API now. It doesn't support versioning
right now as there is no use case for it.
Individual extension specific implementation will be added during
each extension support.

Signed-off-by: Atish Patra <redacted>

Tested-by: Heiko Stuebner <heiko@sntech.de>


By the way, does a similar parsing exist for opensbi as well?
Things like svpbmt as well as zicbom have CSR bits controlling how
these functions should behave (enabling them, etc), so I guess
opensbi also needs to parse the extensions from the ISA string?

No. Currently, OpenSBI relies on the CSR read/write & trap method to
identify the extensions [1].

https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/sbi_hart.c#L404

I guess my question is more, who is supposed to set CBIE, CBCFE bits in the
ENVCFG CSR. I.e. at it's default settings CMO instructions will cause
illegal instructions until the level above does allow them.

When the kernel wants to call a cache-invalidate, from my reading menvcfg
needs to be modified accordingly - which would fall in SBI's court?

I think so. I had the same question for the SSTC extension as well.
This is what I currently do:

1. Detect menvcfg first, detect stimecmp
2. Enable SSTC feature only if both are available
3. Set the STCE bit in menvcfg if SSTC is available

Here is the patch
https://github.com/atishp04/opensbi/commit/e6b185821e8302bffdceb4633b413252e0de4889

Hmm, the CBO fields are defined as WARL (write any, read legal),
so I guess some sort of trap won't work here.

Correct. Traps for extensions that introduce new CSRs.
I was suggesting setting the corresponding bits in MENVCFG and reading
it again to check if it sticks.

quoted

The priv-spec only points to the cmo-spec for these bits and the cmo-spec
does not specifiy what the value should be when cmo is not present.

quoted

quoted

quoted

In the future, zicbom can be detected in the same manner. However,
svpbmt is a bit tricky as it doesn't
define any new CSR. Do you think OpenSBI needs to detect svpbmt for any reason ?

There is the PBMTE bit in MENVCFG, which I found while looking through the
zicbom-parts, which is supposed to "control wheter svpbmt is available for
use". So I guess the question is the same as above :-)

PBMTE bit in MENVCFG says if PBMTE bit is available or not. OpenSBI
needs other way to
detect if PBMTE is available.

That's why, I think MENVCFG should be set correctly by the hardware
upon reset. What do you think
about that ? I couldn't find anything related to the reset state for menvcfg.

me neither. Both the priv-spec as well as the cmobase spec do not
specifiy any reset-values it seems.

I have raised an issue in the ISA spec.
https://github.com/riscv/riscv-isa-manual/issues/820

quoted

So I guess in the Qemu case, Qemu needs to set that bit when
its svpbmt extension is enabled?

We can do that if the priv spec is modified to allow that.

As per Greg's response, hardware is not expected to do that.
So we have to dynamically detect the extensions in OpenSBI and write to menvcfg.

Doesn't SBI also get the devicetree and could therefore parse
the ISA string for extensions? Might be less volatile and would
have both Kernel and SBI do the same thing for detection.

quoted

I am not sure what needs to be done for CBIE bits as it both flush(01)
or invalidate(11) are valid values

From looking at the security remark in the cmo-spec, I guess flush would be
the appropriate thing to do?

"Until a modified cache block has updated memory, a CBO.INVAL instruction may expose stale data values
in memory if the CSRs are programmed to perform an invalidate operation. This behavior may result in a
security hole if lower privileged level software performs an invalidate operation and accesses sensitive
information in memory."

But also do we actually _want_ to enable cmo always ... Greg was talking
about backwards compatiblity in his response as well.