On Wed, 22 Nov 2023 09:37:33 PST (-0800), jerry.shih@xxxxxxxxxx wrote:
On Nov 21, 2023, at 21:14, Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote:
On Tue, Nov 21, 2023 at 06:55:07PM +0800, Jerry Shih wrote:
Sorry, I just use my `internal` qemu with vector-crypto and rva22 patches.
The public qemu haven't supported rva22 profiles. Here is the qemu patch[1] for
that. But here is the discussion why the qemu doesn't export these
`named extensions`(e.g. Zicclsm).
I try to add Zicclsm in DT in the v2 patch set. Maybe we will have more discussion
about the rva22 profiles in kernel DT.
Please do, that'll be fun! Please take some time to read what the
profiles spec actually defines Zicclsm fore before you send those patches
though. I think you might come to find you have misunderstood what it
means - certainly I did the first time I saw it!
From the rva22 profile:
This requires misaligned support for all regular load and store instructions (including
scalar and ``vector``)
The spec includes the explicit `vector` keyword.
So, I still think we could use Zicclsm checking for these vector-crypto implementations.
My proposed patch is just a simple patch which only update the DT document and
update the isa string parser for Zicclsm. If it's still not recommend to use Zicclsm
checking, I will turn to use `RISCV_HWPROBE_MISALIGNED_*` instead.
IMO that's the way to go: even if these are required to be supported by
Zicclsm, we still need to deal with the performance implications.
[1]
LINK: https://lore.kernel.org/all/d1d6f2dc-55b2-4dce-a48a-4afbbf6df526@xxxxxxxxxxxxxxxx/#t
I don't know whether it's a good practice to check unaligned access using
`Zicclsm`.
Here is another related cpu feature for unaligned access:
RISCV_HWPROBE_MISALIGNED_*
But it looks like it always be initialized with `RISCV_HWPROBE_MISALIGNED_SLOW`[2].
It implies that linux kernel always supports unaligned access. But we have the
actual HW which doesn't support unaligned access for vector unit.
https://docs.kernel.org/arch/riscv/uabi.html#misaligned-accesses
Misaligned accesses are part of the user ABI & the hwprobe stuff for
that allows userspace to figure out whether they're fast (likely
implemented in hardware), slow (likely emulated in firmware) or emulated
in the kernel.
The HWPROBE_MISALIGNED_* checking function is at:
https://github.com/torvalds/linux/blob/c2d5304e6c648ebcf653bace7e51e0e6742e46c8/arch/riscv/kernel/cpufeature.c#L564-L647
The tests are all scalar. No `vector` test inside. So, I'm not sure the
HWPROBE_MISALIGNED_* is related to vector unit or not.
The goal is to check whether `vector` support unaligned access or not
in this crypto patch.
I haven't seen the emulated path for unaligned-vector-access in OpenSBI
and kernel. Is the unaligned-vector-access included in user ABI?
I guess it's kind of a grey area, but I'd agrue that it is: we merged
support for V when the only implementation (ie, QEMU) supported
misaligned accesses, so we're stuck with that being the defacto
behavior. As part of adding support for the K230 we'll need to then add
the kernel-mode vector misaligned access handlers, but that doesn't seem
so hard.
So I'd say we should update the hwprobe docs to say that key only
reflects scalar accesses (or maybe even just integer accesses? that's
all we're testing for) -- essentially just make the documentation match
the implementation, as that'll keep ABI compatibility. Then we can add
a new key for vector misaligned access performance.
Thanks,
Jerry
