LinuxLists.cc - [PATCH v4 0/9] Support Zve32[xf] and Zve64[xfd] Vector subextensions

2024-04-12 06:59:18

Subject: [PATCH v4 0/9] Support Zve32[xf] and Zve64[xfd] Vector subextensions

The series composes of two parts. The first part provides a quick fix for
the issue on a recent thread[1]. The issue happens when a platform has
ununified vector register length across multiple cores. Specifically,
patch 1 adds a comment at a callsite of riscv_setup_vsize to clarify how
vlenb is observed by the system. Patch 2 fixes the issue by failing the
boot process of a secondary core if vlenb mismatches.

The second part of the series provide a finer grain view of the Vector
extension. Patch 3 give the obsolete ISA parser the ability to expand
ISA extensions for sigle letter extensions. Patch 3, 4 introduces Zve32x,
Zve32f, Zve64x, Zve64f, Zve64d for isa parsing and hwprobe. Patch 5
updates all callsites such that Vector subextensions are maximumly
supported by the kernel.

Two parts of the series are sent together to ease the effort of picking
dependency patches. The first part can be merged independent of the
second one if necessary.

The series is tested on a QEMU and verified that booting, Vector
programs context-switch, signal, ptrace, prctl interfaces works when we
only report partial V from the ISA.

Note that the signal test was performed after applying the commit
c27fa53b858b ("riscv: Fix vector state restore in rt_sigreturn()")

This patch should be able to apply on risc-v for-next branch on top of
the commit ba5ea59f768f ("riscv: Do not save the scratch CSR during suspend")

[1]: https://lore.kernel.org/all/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/T/#u

Changes in v4:
- Add a patch to trigger prctl test on ZVE32X (9)
- Add a patch to fix integer promotion bug in hwprobe (8)
- Fix a build fail on !CONFIG_RISCV_ISA_V (7)
- Add more comment in the assembly code change (2)
- Link to v3: https://lore.kernel.org/r/[email protected]

Changelog v3:
- Include correct maintainers and mailing list into CC.
- Cleanup isa string parser code (3)
- Adjust extensions order and name (4, 5)
- Refine commit message (6)

Changelog v2:
- Update comments and commit messages (1, 2, 7)
- Refine isa_exts[] lists for zve extensions (4)
- Add a patch for dt-binding (5)
- Make ZVE* extensions depend on has_vector(ZVE32X) (6, 7)

---
---
Andy Chiu (9):
riscv: vector: add a comment when calling riscv_setup_vsize()
riscv: smp: fail booting up smp if inconsistent vlen is detected
riscv: cpufeature: call match_isa_ext() for single-letter extensions
riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection
dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description
riscv: hwprobe: add zve Vector subextensions into hwprobe interface
riscv: vector: adjust minimum Vector requirement to ZVE32X
hwprobe: fix integer promotion in RISCV_HWPROBE_EXT macro
selftest: run vector prctl test for ZVE32X

Documentation/arch/riscv/hwprobe.rst | 15 ++++++
.../devicetree/bindings/riscv/extensions.yaml | 30 ++++++++++++
arch/riscv/include/asm/hwcap.h | 5 ++
arch/riscv/include/asm/switch_to.h | 2 +-
arch/riscv/include/asm/vector.h | 25 ++++++----
arch/riscv/include/asm/xor.h | 2 +-
arch/riscv/include/uapi/asm/hwprobe.h | 7 ++-
arch/riscv/kernel/cpufeature.c | 56 ++++++++++++++++++----
arch/riscv/kernel/head.S | 19 +++++---
arch/riscv/kernel/kernel_mode_vector.c | 4 +-
arch/riscv/kernel/process.c | 4 +-
arch/riscv/kernel/signal.c | 6 +--
arch/riscv/kernel/smpboot.c | 14 ++++--
arch/riscv/kernel/sys_hwprobe.c | 13 ++++-
arch/riscv/kernel/vector.c | 15 +++---
arch/riscv/lib/uaccess.S | 2 +-
.../testing/selftests/riscv/vector/vstate_prctl.c | 6 +--
17 files changed, 174 insertions(+), 51 deletions(-)
---
base-commit: ba5ea59f768f67d127b319b26ba209ff67e0d9a5
change-id: 20240318-zve-detection-50106d2da527

Best regards,
--
Andy Chiu <[email protected]>

2024-04-12 07:05:33

by Andy Chiu

[permalink] [raw]

Subject: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

(No filename) (2.33 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-09 22:23:18

by Conor Dooley

[permalink] [raw]

Subject: Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

On Thu, May 09, 2024 at 09:25:25AM +0100, Conor Dooley wrote:
> On Thu, May 09, 2024 at 08:48:09AM +0100, Conor Dooley wrote:
> > On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
> > > Hi Conor,
> > >
> > > Should we check if "v" presents for vector crypto extensions in
> > > riscv_isa_extension_check()? We are not checking this for now. So a
> > > kernel compiled with RISCV_ISA_V still has a problem if its isa-string
> > > includes any of vector crypto ("zvbb, zvkg, etc") but not "v".
> >
> >
> > Yeah, one of the things I took away from this discussion is that we need
> > to improve the implementation of both the methods we have at the moment
> > for drivers etc to check if extensions are present and usable.
> > In general, I don't think checks like that are "safe" to do in
> > riscv_isa_extension_check(), because the dependencies may not all have
> > been resolved when we probe an extension (Clement's current Zca etc
> > series improves the situation though by only calling the checks after
> > we probe all extensions).
> >
> > The simple V cases are all fine though - the DT binding and ACPI rules
> > for compatible strings all mandate that single-letter extensions must
> > come before multi-letter ones. For riscv,isa-extensions we control the
> > probe ordering and probe V before any multi-letter stuff. Additionally,
> > we should make it a requirement for V to be present if things that
> > depend on it are.
> >
> > That said, is it permitted by the specs to have any of the extensions
> > you mention without the full V extension, but with one of the cut-down
> > variants you mention here? If not, I'd be more interested in figuring
> > out the non-extension dependencies: whether or not the kernel itself
> > supports vector and if the kernel has opted to disable vector due to
> > detecting that harts have mismatching vector lengths.
> >
> > TL;DR: I think we should add some checks in riscv_isa_extension_check().
>
> Also, unless this only becomes a problem with this series that adds the
> cut-down forms of vector, I think this is a separate problem to solve
> and I can send some patches for it (along with some other cleanup I'd like
> to do as a result of Eric's comments) and you can just submit the v2 you
> were planning to without it. I can't, off the top of my head, think of
> why this particular series would break the vector crypto stuff though,
> the problems with enabling extensions seem underlying.

Here's something buggy that I chucked together as an idea of what I
meant:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-check_vector
Beware, it is entirely untested :)
It's based on both this series and patches 2 & 3 of Charlie's series doing
the T-Head vector stuff. It really needs Clement's extension_check()
rework that I mentioned 2 mails ago to function correctly for any of these
vector subsets. Without Clement's stuff, it'll have "random" behaviour
depending on probe order for riscv,isa and a determinate, but incorrect,
behaviour otherwise.

Cheers,
Conor.

Attachments:

(No filename) (3.05 kB)
signature.asc (235.00 B)
Download all attachments