lkml.org 
[lkml]   [2023]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 12/12] RISC-V: crypto: add Zvkb accelerated ChaCha20 implementation
    From
    On Wed, 22 Nov 2023 09:37:33 PST (-0800), jerry.shih@sifive.com wrote:
    > On Nov 21, 2023, at 21:14, Conor Dooley <conor.dooley@microchip.com> 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@ventanamicro.com/#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

    \
     
     \ /
      Last update: 2023-11-22 19:07    [W:3.009 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site