Message ID | YqY0i22SdbHjB/MX@Sun (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | RISC-V: Add Bitmanip/Scalar Crypto HWCAP | expand |
On Mon, Jun 13, 2022 at 02:46:35AM +0800, Hongren (Zenithal) Zheng wrote: > diff --git a/arch/riscv/include/uapi/asm/hwcap.h b/arch/riscv/include/uapi/asm/hwcap.h > index 46dc3f5ee99f..bfed3e5c338c 100644 > --- a/arch/riscv/include/uapi/asm/hwcap.h > +++ b/arch/riscv/include/uapi/asm/hwcap.h > @@ -22,4 +22,26 @@ > #define COMPAT_HWCAP_ISA_D (1 << ('D' - 'A')) > #define COMPAT_HWCAP_ISA_C (1 << ('C' - 'A')) > > +/* > + * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2 > + * > + * As only 32 bits of elf_hwcap (in kernel) could be used > + * and RISC-V has reserved 26 bits of it, other caps like > + * bitmanip and crypto can not be placed in AT_HWCAP > + */ Have we agreed that multi-letter ISA extensions would use hwcap to be exposed to userspace? With so many potential extensions, we could quickly run out of space on AT_HWCAP2 as well. Cheers, Samuel.
On Thu, Nov 24, 2022 at 10:30:21AM +0100, Samuel Ortiz wrote: > On Mon, Jun 13, 2022 at 02:46:35AM +0800, Hongren (Zenithal) Zheng wrote: > > diff --git a/arch/riscv/include/uapi/asm/hwcap.h b/arch/riscv/include/uapi/asm/hwcap.h > > index 46dc3f5ee99f..bfed3e5c338c 100644 > > --- a/arch/riscv/include/uapi/asm/hwcap.h > > +++ b/arch/riscv/include/uapi/asm/hwcap.h > > @@ -22,4 +22,26 @@ > > #define COMPAT_HWCAP_ISA_D (1 << ('D' - 'A')) > > #define COMPAT_HWCAP_ISA_C (1 << ('C' - 'A')) > > > > +/* > > + * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2 > > + * > > + * As only 32 bits of elf_hwcap (in kernel) could be used > > + * and RISC-V has reserved 26 bits of it, other caps like > > + * bitmanip and crypto can not be placed in AT_HWCAP > > + */ > > Have we agreed that multi-letter ISA extensions would use hwcap to be > exposed to userspace? With so many potential extensions, we could > quickly run out of space on AT_HWCAP2 as well. Palmer whipped up a PoC hwprobe interface (during Plumbers I think) that Heiko is currently looking into - I think his motivation is misaligned access performance. There's a branch but I have no idea if it even compiles... I'm mostly waiting for whatever Heiko comes up with ;) https://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git/log/?h=riscv-hwprobe-v1 This patchset seems to need a rebase anyway per your other reply, but I guess that the new proposed interface would be preferable? Thanks, Conor.
On Thu, Nov 24, 2022 at 09:58:53AM +0000, Conor Dooley wrote: > On Thu, Nov 24, 2022 at 10:30:21AM +0100, Samuel Ortiz wrote: > > On Mon, Jun 13, 2022 at 02:46:35AM +0800, Hongren (Zenithal) Zheng wrote: > > > diff --git a/arch/riscv/include/uapi/asm/hwcap.h b/arch/riscv/include/uapi/asm/hwcap.h > > > index 46dc3f5ee99f..bfed3e5c338c 100644 > > > --- a/arch/riscv/include/uapi/asm/hwcap.h > > > +++ b/arch/riscv/include/uapi/asm/hwcap.h > > > @@ -22,4 +22,26 @@ > > > #define COMPAT_HWCAP_ISA_D (1 << ('D' - 'A')) > > > #define COMPAT_HWCAP_ISA_C (1 << ('C' - 'A')) > > > > > > +/* > > > + * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2 > > > + * > > > + * As only 32 bits of elf_hwcap (in kernel) could be used > > > + * and RISC-V has reserved 26 bits of it, other caps like > > > + * bitmanip and crypto can not be placed in AT_HWCAP > > > + */ > > > > Have we agreed that multi-letter ISA extensions would use hwcap to be > > exposed to userspace? With so many potential extensions, we could > > quickly run out of space on AT_HWCAP2 as well. > > Palmer whipped up a PoC hwprobe interface (during Plumbers I think) that > Heiko is currently looking into - I think his motivation is misaligned > access performance. There's a branch but I have no idea if it even > compiles... I'm mostly waiting for whatever Heiko comes up with ;) > > https://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git/log/?h=riscv-hwprobe-v1 > > This patchset seems to need a rebase anyway per your other reply, but I > guess that the new proposed interface would be preferable? I think so, yes. Patch #1 is definitely needed regardless of which interface we pick for exposing the ISA strings to userspace. Cheers, Samuel.
On Thu, Nov 24, 2022 at 11:47:30AM +0100, Samuel Ortiz wrote: > Patch #1 is definitely needed regardless of which interface we pick for > exposing the ISA strings to userspace. I took another look at #1, and I feel more confused about what constitutes canonical order than I did before! If you know better than I, and you probably do since you're interested in these 6 month old patches, some insight would be appreciated! Thanks, Conor.
On Thu, Nov 24, 2022 at 11:55:01AM +0000, Conor Dooley wrote: > On Thu, Nov 24, 2022 at 11:47:30AM +0100, Samuel Ortiz wrote: > > > Patch #1 is definitely needed regardless of which interface we pick for > > exposing the ISA strings to userspace. > > I took another look at #1, and I feel more confused about what > constitutes canonical order than I did before! If you know better than > I, and you probably do since you're interested in these 6 month old > patches, some insight would be appreciated! Assuming we don't go with hwcap, I dont think the order of the riscv_isa_ext_id enum matters that much? iiuc we're building the cpuinfo string from the riscv_isa_ext_data array, and I think the current code is incorrect: static struct riscv_isa_ext_data isa_ext_arr[] = { __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), }; zicbom and zihintpause should come before supervisor level extensions. I'm going to send a patch for that. And the Zb/Zk ones should come after the Zi ones, and before the supervisor level ones (The I category comes before the B or the K one). So we should check that when patch #1 is rebased. Cheers, Samuel.
On 24/11/2022 17:12, Samuel Ortiz wrote: > [You don't often get email from sameo@rivosinc.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Thu, Nov 24, 2022 at 11:55:01AM +0000, Conor Dooley wrote: >> On Thu, Nov 24, 2022 at 11:47:30AM +0100, Samuel Ortiz wrote: >> >>> Patch #1 is definitely needed regardless of which interface we pick for >>> exposing the ISA strings to userspace. >> >> I took another look at #1, and I feel more confused about what >> constitutes canonical order than I did before! If you know better than >> I, and you probably do since you're interested in these 6 month old >> patches, some insight would be appreciated! > > Assuming we don't go with hwcap, I dont think the order of the > riscv_isa_ext_id enum matters that much? The chief put it in canonical order so that's good enough for me! > > iiuc we're building the cpuinfo string from the riscv_isa_ext_data > array, and I think the current code is incorrect: > > static struct riscv_isa_ext_data isa_ext_arr[] = { > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), > __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), > __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), > __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), > __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), > }; > > zicbom and zihintpause should come before supervisor level extensions. > I'm going to send a patch for that. idk, Palmer explicitly re-ordered this: https://lore.kernel.org/linux-riscv/20220920204518.10988-1-palmer@rivosinc.com/ By my reading of the isa manual, what Palmer did is correct as those are not "Additional Standard Extensions". /shrug > > And the Zb/Zk ones should come after the Zi ones, and before the > supervisor level ones (The I category comes before the B or the K one). This I agree with though.
On Thu, Nov 24, 2022 at 05:20:37PM +0000, Conor Dooley wrote: > On 24/11/2022 17:12, Samuel Ortiz wrote: > > [You don't often get email from sameo@rivosinc.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On Thu, Nov 24, 2022 at 11:55:01AM +0000, Conor Dooley wrote: > >> On Thu, Nov 24, 2022 at 11:47:30AM +0100, Samuel Ortiz wrote: > >> > >>> Patch #1 is definitely needed regardless of which interface we pick for > >>> exposing the ISA strings to userspace. > >> > >> I took another look at #1, and I feel more confused about what > >> constitutes canonical order than I did before! If you know better than > >> I, and you probably do since you're interested in these 6 month old > >> patches, some insight would be appreciated! > > > > Assuming we don't go with hwcap, I dont think the order of the > > riscv_isa_ext_id enum matters that much? > > The chief put it in canonical order so that's good enough for me! > > > > > iiuc we're building the cpuinfo string from the riscv_isa_ext_data > > array, and I think the current code is incorrect: > > > > static struct riscv_isa_ext_data isa_ext_arr[] = { > > __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), > > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), > > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), > > __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), > > __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), > > __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), > > __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), > > }; > > > > zicbom and zihintpause should come before supervisor level extensions. > > I'm going to send a patch for that. > > idk, Palmer explicitly re-ordered this: > https://lore.kernel.org/linux-riscv/20220920204518.10988-1-palmer@rivosinc.com/ > > By my reading of the isa manual, what Palmer did is correct as > those are not "Additional Standard Extensions". /shrug Hmm, by their name (Z[a-b]+) they are Additional Standard Extensions. What am I missing? Cheers, Samuel. >
On 24/11/2022 17:34, Samuel Ortiz wrote: > On Thu, Nov 24, 2022 at 05:20:37PM +0000, Conor Dooley wrote: >> On 24/11/2022 17:12, Samuel Ortiz wrote: >>> [You don't often get email from sameo@rivosinc.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] >>> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On Thu, Nov 24, 2022 at 11:55:01AM +0000, Conor Dooley wrote: >>>> On Thu, Nov 24, 2022 at 11:47:30AM +0100, Samuel Ortiz wrote: >>>> >>>>> Patch #1 is definitely needed regardless of which interface we pick for >>>>> exposing the ISA strings to userspace. >>>> >>>> I took another look at #1, and I feel more confused about what >>>> constitutes canonical order than I did before! If you know better than >>>> I, and you probably do since you're interested in these 6 month old >>>> patches, some insight would be appreciated! >>> >>> Assuming we don't go with hwcap, I dont think the order of the >>> riscv_isa_ext_id enum matters that much? >> >> The chief put it in canonical order so that's good enough for me! >> >>> >>> iiuc we're building the cpuinfo string from the riscv_isa_ext_data >>> array, and I think the current code is incorrect: >>> >>> static struct riscv_isa_ext_data isa_ext_arr[] = { >>> __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), >>> __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), >>> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), >>> __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), >>> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), >>> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), >>> __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), >>> }; >>> >>> zicbom and zihintpause should come before supervisor level extensions. >>> I'm going to send a patch for that. >> >> idk, Palmer explicitly re-ordered this: >> https://lore.kernel.org/linux-riscv/20220920204518.10988-1-palmer@rivosinc.com/ >> >> By my reading of the isa manual, what Palmer did is correct as >> those are not "Additional Standard Extensions". /shrug > > Hmm, by their name (Z[a-b]+) they are Additional Standard Extensions. > What am I missing? Right, and this is where I get confused. Zam and Ztso *are* Additional Standard Extensions, I think we can agree on that one? For those extensions: \chapter{``Ztso'' Standard Extension for Total Store Ordering, v0.1} \chapter{``Zam'' Standard Extension for Misaligned Atomics, v0.1} They're also called out specifically in the table: https://github.com/riscv/riscv-isa-manual/blob/master/src/naming.tex#L147 For Zihintpause however: \chapter{``Zihintpause'' Pause Hint, Version 2.0} See what I mean? I looked at the specs for the bitmanip stuff and for crypto, which both never mention being standard. That table has the caption: > The table also defines the canonical order in which extension names > must appear in the name string, with top-to-bottom in table > indicating first-to-last in the name string. It only calls out Zicsr, Zifencei, Zam and Ztso are being permitted before Sdef, but as I said I am not a specs person, so perhaps some of the extensions in question are intended to go there but have not yet been merged into the isa manual doc. Zihintpause *is* in the isa manual though but not specifically called out. Anyways, hopefully that at least helps with my line of thinking! Conor.
On Thu, Nov 24, 2022 at 05:54:00PM +0000, Conor Dooley wrote: > On 24/11/2022 17:34, Samuel Ortiz wrote: > > On Thu, Nov 24, 2022 at 05:20:37PM +0000, Conor Dooley wrote: > >> On 24/11/2022 17:12, Samuel Ortiz wrote: > >>> [You don't often get email from sameo@rivosinc.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > >>> > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>> > >>> On Thu, Nov 24, 2022 at 11:55:01AM +0000, Conor Dooley wrote: > >>>> On Thu, Nov 24, 2022 at 11:47:30AM +0100, Samuel Ortiz wrote: > >>>> > >>>>> Patch #1 is definitely needed regardless of which interface we pick for > >>>>> exposing the ISA strings to userspace. > >>>> > >>>> I took another look at #1, and I feel more confused about what > >>>> constitutes canonical order than I did before! If you know better than > >>>> I, and you probably do since you're interested in these 6 month old > >>>> patches, some insight would be appreciated! > >>> > >>> Assuming we don't go with hwcap, I dont think the order of the > >>> riscv_isa_ext_id enum matters that much? > >> > >> The chief put it in canonical order so that's good enough for me! > >> > >>> > >>> iiuc we're building the cpuinfo string from the riscv_isa_ext_data > >>> array, and I think the current code is incorrect: > >>> > >>> static struct riscv_isa_ext_data isa_ext_arr[] = { > >>> __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), > >>> __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), > >>> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), > >>> __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), > >>> __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), > >>> __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), > >>> __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), > >>> }; > >>> > >>> zicbom and zihintpause should come before supervisor level extensions. > >>> I'm going to send a patch for that. > >> > >> idk, Palmer explicitly re-ordered this: > >> https://lore.kernel.org/linux-riscv/20220920204518.10988-1-palmer@rivosinc.com/ > >> > >> By my reading of the isa manual, what Palmer did is correct as > >> those are not "Additional Standard Extensions". /shrug > > > > Hmm, by their name (Z[a-b]+) they are Additional Standard Extensions. > > What am I missing? > > Right, and this is where I get confused. Zam and Ztso *are* Additional > Standard Extensions, I think we can agree on that one? For those > extensions: > \chapter{``Ztso'' Standard Extension for Total Store Ordering, v0.1} > \chapter{``Zam'' Standard Extension for Misaligned Atomics, v0.1} > > They're also called out specifically in the table: > https://github.com/riscv/riscv-isa-manual/blob/master/src/naming.tex#L147 > > For Zihintpause however: > \chapter{``Zihintpause'' Pause Hint, Version 2.0} > > See what I mean? I looked at the specs for the bitmanip stuff and for > crypto, which both never mention being standard. I *think* this is because Zihintpause, bitmap and crypto are ratified but not yet part of an official spec (non-draft) release? > That table has the caption: > > The table also defines the canonical order in which extension names > > must appear in the name string, with top-to-bottom in table > > indicating first-to-last in the name string. > > It only calls out Zicsr, Zifencei, Zam and Ztso are being permitted > before Sdef, but as I said I am not a specs person, so perhaps some > of the extensions in question are intended to go there but have not > yet been merged into the isa manual doc. Zihintpause *is* in the > isa manual though but not specifically called out. > > Anyways, hopefully that at least helps with my line of thinking! It does, thanks. It's a little confusing, I agree. Cheers, Samuel.
diff --git a/arch/riscv/include/uapi/asm/hwcap.h b/arch/riscv/include/uapi/asm/hwcap.h index 46dc3f5ee99f..bfed3e5c338c 100644 --- a/arch/riscv/include/uapi/asm/hwcap.h +++ b/arch/riscv/include/uapi/asm/hwcap.h @@ -22,4 +22,26 @@ #define COMPAT_HWCAP_ISA_D (1 << ('D' - 'A')) #define COMPAT_HWCAP_ISA_C (1 << ('C' - 'A')) +/* + * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2 + * + * As only 32 bits of elf_hwcap (in kernel) could be used + * and RISC-V has reserved 26 bits of it, other caps like + * bitmanip and crypto can not be placed in AT_HWCAP + */ +#define COMPAT_HWCAP2_ISA_ZBA (1 << 0) +#define COMPAT_HWCAP2_ISA_ZBB (1 << 1) +#define COMPAT_HWCAP2_ISA_ZBC (1 << 2) +#define COMPAT_HWCAP2_ISA_ZBS (1 << 3) +#define COMPAT_HWCAP2_ISA_ZBKB (1 << 4) +#define COMPAT_HWCAP2_ISA_ZBKC (1 << 5) +#define COMPAT_HWCAP2_ISA_ZBKX (1 << 6) +#define COMPAT_HWCAP2_ISA_ZKND (1 << 7) +#define COMPAT_HWCAP2_ISA_ZKNE (1 << 8) +#define COMPAT_HWCAP2_ISA_ZKNH (1 << 9) +#define COMPAT_HWCAP2_ISA_ZKSED (1 << 10) +#define COMPAT_HWCAP2_ISA_ZKSH (1 << 11) +#define COMPAT_HWCAP2_ISA_ZKR (1 << 12) +#define COMPAT_HWCAP2_ISA_ZKT (1 << 13) + #endif /* _UAPI_ASM_RISCV_HWCAP_H */