On Nov 23, 2023, at 02:20, Conor Dooley <conor@xxxxxxxxxx> wrote: > On Thu, Nov 23, 2023 at 01:37:33AM +0800, Jerry Shih 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: > > "rva22" is not a profile. As I pointed out to Eric, this is defined in > the RVA22U64 profile (and the RVA20U64 one, but that is effectively a > moot point). The profile descriptions for these only specify "the ISA > features available to user-mode execution environments", so it is not > suitable for use in any other context. I missed that important part: it's for user space. Thx. >> 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. > > In userspace, if Zicclsm was exported somewhere, that would be a valid > argument. Even for userspace, the hwprobe flags probably provide more > information though, since the firmware emulation is insanely slow. I agree. It will be more useful to have the flag like `VECTOR_MISALIGNED_FAST` instead. >> My proposed patch is just a simple patch which only update the DT document and >> update the isa string parser for Zicclsm. > > Zicclsm has no meaning outside of user mode, so it's not suitable for > use in that context. Other "features" defined in the profiles spec might > be suitable for inclusion, but it'll be a case-by-case basis. I will skip the Zicclsm part in my v2 patch. >> If it's still not recommend to use Zicclsm >> checking, I will turn to use `RISCV_HWPROBE_MISALIGNED_*` instead. > > Palmer has commented on the rest, so no need for me :) All crypto algorithms will assume that the vector supports misaligned access in next v2 patch. And the algorithms will also not check for `RISCV_HWPROBE_MISALIGNED_*` since it's related to scalar accesses. Once we have the vector performance related flag, we could go back here to use it. -Jerry