* [PATCH] RISC-V: Add support for the zicbom extension @ 2022-08-11 3:31 Palmer Dabbelt 2022-08-12 8:25 ` Conor Dooley 2023-12-18 13:58 ` Luc Van Oostenryck 0 siblings, 2 replies; 7+ messages in thread From: Palmer Dabbelt @ 2022-08-11 3:31 UTC (permalink / raw) To: luc.vanoostenryck, linux-sparse; +Cc: Palmer Dabbelt This was recently added to binutils and with any luck will soon be in Linux, without it sparse will fail when trying to build new kernels on systems with new toolchains. Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> --- target-riscv.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target-riscv.c b/target-riscv.c index 217ab7e8..db0f7e57 100644 --- a/target-riscv.c +++ b/target-riscv.c @@ -19,6 +19,7 @@ #define RISCV_GENERIC (RISCV_MUL|RISCV_DIV|RISCV_ATOMIC|RISCV_FPU) #define RISCV_ZICSR (1 << 10) #define RISCV_ZIFENCEI (1 << 11) +#define RISCV_ZICBOM (1 << 12) static unsigned int riscv_flags; @@ -41,6 +42,7 @@ static void parse_march_riscv(const char *arg) { "c", RISCV_COMP }, { "_zicsr", RISCV_ZICSR }, { "_zifencei", RISCV_ZIFENCEI }, + { "_zicbom", RISCV_ZICBOM }, }; int i; @@ -131,6 +133,8 @@ static void predefine_riscv(const struct target *self) predefine("__riscv_zicsr", 1, "1"); if (riscv_flags & RISCV_ZIFENCEI) predefine("__riscv_zifencei", 1, "1"); + if (riscv_flags & RISCV_ZICBOM) + predefine("__riscv_zicbom", 1, "1"); if (cmodel) predefine_strong("__riscv_cmodel_%s", cmodel); -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Add support for the zicbom extension 2022-08-11 3:31 [PATCH] RISC-V: Add support for the zicbom extension Palmer Dabbelt @ 2022-08-12 8:25 ` Conor Dooley 2022-10-12 21:11 ` Conor Dooley 2023-12-15 12:37 ` Geert Uytterhoeven 2023-12-18 13:58 ` Luc Van Oostenryck 1 sibling, 2 replies; 7+ messages in thread From: Conor Dooley @ 2022-08-12 8:25 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: luc.vanoostenryck, linux-sparse On Wed, Aug 10, 2022 at 08:31:38PM -0700, Palmer Dabbelt wrote: > This was recently added to binutils and with any luck will soon be in > Linux, without it sparse will fail when trying to build new kernels on > systems with new toolchains. > In passing while testing the zihintpause one: Tested-by: Conor Dooley <conor.dooley@microchip.com> > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > --- > target-riscv.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/target-riscv.c b/target-riscv.c > index 217ab7e8..db0f7e57 100644 > --- a/target-riscv.c > +++ b/target-riscv.c > @@ -19,6 +19,7 @@ > #define RISCV_GENERIC (RISCV_MUL|RISCV_DIV|RISCV_ATOMIC|RISCV_FPU) > #define RISCV_ZICSR (1 << 10) > #define RISCV_ZIFENCEI (1 << 11) > +#define RISCV_ZICBOM (1 << 12) > > static unsigned int riscv_flags; > > @@ -41,6 +42,7 @@ static void parse_march_riscv(const char *arg) > { "c", RISCV_COMP }, > { "_zicsr", RISCV_ZICSR }, > { "_zifencei", RISCV_ZIFENCEI }, > + { "_zicbom", RISCV_ZICBOM }, > }; > int i; > > @@ -131,6 +133,8 @@ static void predefine_riscv(const struct target *self) > predefine("__riscv_zicsr", 1, "1"); > if (riscv_flags & RISCV_ZIFENCEI) > predefine("__riscv_zifencei", 1, "1"); > + if (riscv_flags & RISCV_ZICBOM) > + predefine("__riscv_zicbom", 1, "1"); > > if (cmodel) > predefine_strong("__riscv_cmodel_%s", cmodel); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Add support for the zicbom extension 2022-08-12 8:25 ` Conor Dooley @ 2022-10-12 21:11 ` Conor Dooley 2022-10-12 23:34 ` Palmer Dabbelt 2023-12-15 12:37 ` Geert Uytterhoeven 1 sibling, 1 reply; 7+ messages in thread From: Conor Dooley @ 2022-10-12 21:11 UTC (permalink / raw) To: Conor Dooley, luc.vanoostenryck Cc: Palmer Dabbelt, luc.vanoostenryck, linux-sparse On Fri, Aug 12, 2022 at 09:25:23AM +0100, Conor Dooley wrote: > On Wed, Aug 10, 2022 at 08:31:38PM -0700, Palmer Dabbelt wrote: > > This was recently added to binutils and with any luck will soon be in > > Linux, without it sparse will fail when trying to build new kernels on > > systems with new toolchains. > > > > In passing while testing the zihintpause one: > Tested-by: Conor Dooley <conor.dooley@microchip.com> Hey Luc, Would you be able to take a look at this patch and at https://lore.kernel.org/linux-sparse/YvYQSdQBuZGSit2s@wendy/T/#t please? They're causing sparse to fail for recent kernels when the extensions are used. Thanks, Conor. > > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > > --- > > target-riscv.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/target-riscv.c b/target-riscv.c > > index 217ab7e8..db0f7e57 100644 > > --- a/target-riscv.c > > +++ b/target-riscv.c > > @@ -19,6 +19,7 @@ > > #define RISCV_GENERIC (RISCV_MUL|RISCV_DIV|RISCV_ATOMIC|RISCV_FPU) > > #define RISCV_ZICSR (1 << 10) > > #define RISCV_ZIFENCEI (1 << 11) > > +#define RISCV_ZICBOM (1 << 12) > > > > static unsigned int riscv_flags; > > > > @@ -41,6 +42,7 @@ static void parse_march_riscv(const char *arg) > > { "c", RISCV_COMP }, > > { "_zicsr", RISCV_ZICSR }, > > { "_zifencei", RISCV_ZIFENCEI }, > > + { "_zicbom", RISCV_ZICBOM }, > > }; > > int i; > > > > @@ -131,6 +133,8 @@ static void predefine_riscv(const struct target *self) > > predefine("__riscv_zicsr", 1, "1"); > > if (riscv_flags & RISCV_ZIFENCEI) > > predefine("__riscv_zifencei", 1, "1"); > > + if (riscv_flags & RISCV_ZICBOM) > > + predefine("__riscv_zicbom", 1, "1"); > > > > if (cmodel) > > predefine_strong("__riscv_cmodel_%s", cmodel); > > -- > > 2.34.1 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Add support for the zicbom extension 2022-10-12 21:11 ` Conor Dooley @ 2022-10-12 23:34 ` Palmer Dabbelt 2022-10-13 11:34 ` Conor Dooley 0 siblings, 1 reply; 7+ messages in thread From: Palmer Dabbelt @ 2022-10-12 23:34 UTC (permalink / raw) To: Conor Dooley; +Cc: conor.dooley, luc.vanoostenryck, linux-sparse On Wed, 12 Oct 2022 14:11:58 PDT (-0700), Conor Dooley wrote: > On Fri, Aug 12, 2022 at 09:25:23AM +0100, Conor Dooley wrote: >> On Wed, Aug 10, 2022 at 08:31:38PM -0700, Palmer Dabbelt wrote: >> > This was recently added to binutils and with any luck will soon be in >> > Linux, without it sparse will fail when trying to build new kernels on >> > systems with new toolchains. >> > >> >> In passing while testing the zihintpause one: >> Tested-by: Conor Dooley <conor.dooley@microchip.com> > > Hey Luc, > Would you be able to take a look at this patch and at > https://lore.kernel.org/linux-sparse/YvYQSdQBuZGSit2s@wendy/T/#t > please? They're causing sparse to fail for recent kernels when the > extensions are used. Just kind of thinking out loud here, but: Another option would be to just convert the kernel over to Kconfig-based ifdefs and ignore the -march stuff in sparse. As per the discussion over here <https://github.com/riscv/riscv-isa-manual/issues/869> it looks like we're going to end up with different string->behavior mappings for user-mode vs privileged software and compilers will be expected to follow the user-mode mappings, so we'll probably have to do this at some point anyway. That would mean sparse only works right for Linux, I'm not sure if that's the design point today or not. If that's an issue we could still convert Linux over and then just have some sort of "--sparse-ignore-march-on-riscv" argument so we don't keep coupling kernel builds to sparse updates. There's going to be a ton of new extensions so this kind of thing is just going to keep happening. > > Thanks, > Conor. > >> >> > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> >> > --- >> > target-riscv.c | 4 ++++ >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/target-riscv.c b/target-riscv.c >> > index 217ab7e8..db0f7e57 100644 >> > --- a/target-riscv.c >> > +++ b/target-riscv.c >> > @@ -19,6 +19,7 @@ >> > #define RISCV_GENERIC (RISCV_MUL|RISCV_DIV|RISCV_ATOMIC|RISCV_FPU) >> > #define RISCV_ZICSR (1 << 10) >> > #define RISCV_ZIFENCEI (1 << 11) >> > +#define RISCV_ZICBOM (1 << 12) >> > >> > static unsigned int riscv_flags; >> > >> > @@ -41,6 +42,7 @@ static void parse_march_riscv(const char *arg) >> > { "c", RISCV_COMP }, >> > { "_zicsr", RISCV_ZICSR }, >> > { "_zifencei", RISCV_ZIFENCEI }, >> > + { "_zicbom", RISCV_ZICBOM }, >> > }; >> > int i; >> > >> > @@ -131,6 +133,8 @@ static void predefine_riscv(const struct target *self) >> > predefine("__riscv_zicsr", 1, "1"); >> > if (riscv_flags & RISCV_ZIFENCEI) >> > predefine("__riscv_zifencei", 1, "1"); >> > + if (riscv_flags & RISCV_ZICBOM) >> > + predefine("__riscv_zicbom", 1, "1"); >> > >> > if (cmodel) >> > predefine_strong("__riscv_cmodel_%s", cmodel); >> > -- >> > 2.34.1 >> > >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Add support for the zicbom extension 2022-10-12 23:34 ` Palmer Dabbelt @ 2022-10-13 11:34 ` Conor Dooley 0 siblings, 0 replies; 7+ messages in thread From: Conor Dooley @ 2022-10-13 11:34 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: Conor Dooley, luc.vanoostenryck, linux-sparse On Wed, Oct 12, 2022 at 04:34:00PM -0700, Palmer Dabbelt wrote: > On Wed, 12 Oct 2022 14:11:58 PDT (-0700), Conor Dooley wrote: > > On Fri, Aug 12, 2022 at 09:25:23AM +0100, Conor Dooley wrote: > > > On Wed, Aug 10, 2022 at 08:31:38PM -0700, Palmer Dabbelt wrote: > > > > This was recently added to binutils and with any luck will soon be in > > > > Linux, without it sparse will fail when trying to build new kernels on > > > > systems with new toolchains. > > > > > > > > > > In passing while testing the zihintpause one: > > > Tested-by: Conor Dooley <conor.dooley@microchip.com> > > > > Hey Luc, > > Would you be able to take a look at this patch and at > > https://lore.kernel.org/linux-sparse/YvYQSdQBuZGSit2s@wendy/T/#t > > please? They're causing sparse to fail for recent kernels when the > > extensions are used. > > Just kind of thinking out loud here, but: Wee brain dump in response, dunno if I am missing your point & if so please correct me... > > Another option would be to just convert the kernel over to Kconfig-based > ifdefs and ignore the -march stuff in sparse. As per the discussion over > here <https://github.com/riscv/riscv-isa-manual/issues/869> it looks like hmm, gnu tools meetings - are bystanders like myself allowed to attend? I doubt I'd have anything interesting to say, but given the names it seems like a good thing to keep an eye on. To be blunt, if the toolchain people are not clear on what the craic is with how extensions interact how are the rest of us supposed to get it? In my naivety I'd expect that sort of distinction to be clear from the spec docs... By kconfig based, do you mean w/ the sort of thing I was messing with in this patchset: https://lore.kernel.org/linux-riscv/20221006173520.1785507-2-conor@kernel.org/ I really don't like having ifdef-ery, seems to just result in missed conditions as the recent Zicbom stuff has shown. Since it's thinking- out-loud o clock - while its not my place to make decisions, I think for future extension patchsets I am going to try and push people towards as few conditionally defined variables etc as possible. I think you said the other day that things like __riscv_zmmul are conditionally defined based on the toolchain supporting them right? That'd make sense since older toolchains wouldn't know they existed... Anyways, what I meant by all that is that instead of ifdef-ery, I'd rather IS_ENABLED() with unconditionally {defined,defined} variables so that we don't end up with little pockets of code that end up missed because of some overlapping extension ifdef hell. Upstream seems to have a lot of people that run bells & whistles toolchains with the new & shiny toys available or the standard riscv-gnu-toolchain builds of something significantly older. I'd just be a little worried about potentially having poor coverage of some odd combinations of things with #ifdefs. > we're going to end up with different string->behavior mappings for user-mode > vs privileged software and compilers will be expected to follow the > user-mode mappings, so we'll probably have to do this at some point anyway. Could you explain this one a little more? Even after reading that issue I am a little unsure what you mean by different string->behaviour mappings. Something along the lines of some Zfoo extension could deny user-mode software from using certain instructions but in kernel-land we would want to (or need to?) use those instructions? > That would mean sparse only works right for Linux, I'm not sure if that's > the design point today or not. If that's an issue we could still convert > Linux over and then just have some sort of "--sparse-ignore-march-on-riscv" Do we ignore march, or could we just chop the string after the single letter extensions? I am not up on how much of (if any) of the march string is/would ever be used. I had a quick look at target-riscv.c and it seems we just use it to predefine __riscv_foo & set xlen. Does that mean if we used ifdef-ery or IS_ENABLED to gate features we could avoid having to use new __riscv_foo symbols and therefore not have to predefine them in sparse? Maybe I am misunderstanding - something, something out of my depth... > argument so we don't keep coupling kernel builds to sparse updates. There's > going to be a ton of new extensions so this kind of thing is just going to > keep happening. Yeah, it'd be nice to avoid having to deal with new march updates constantly, especially since we cannot expect everyone to run ToT sparse either. The current situation makes me wonder who outside the pair of us is even running sparse in the first place? Thanks, Conor. > > > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > > > > --- > > > > target-riscv.c | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/target-riscv.c b/target-riscv.c > > > > index 217ab7e8..db0f7e57 100644 > > > > --- a/target-riscv.c > > > > +++ b/target-riscv.c > > > > @@ -19,6 +19,7 @@ > > > > #define RISCV_GENERIC (RISCV_MUL|RISCV_DIV|RISCV_ATOMIC|RISCV_FPU) > > > > #define RISCV_ZICSR (1 << 10) > > > > #define RISCV_ZIFENCEI (1 << 11) > > > > +#define RISCV_ZICBOM (1 << 12) > > > > > > > > static unsigned int riscv_flags; > > > > > > > > @@ -41,6 +42,7 @@ static void parse_march_riscv(const char *arg) > > > > { "c", RISCV_COMP }, > > > > { "_zicsr", RISCV_ZICSR }, > > > > { "_zifencei", RISCV_ZIFENCEI }, > > > > + { "_zicbom", RISCV_ZICBOM }, > > > > }; > > > > int i; > > > > > > > > @@ -131,6 +133,8 @@ static void predefine_riscv(const struct target *self) > > > > predefine("__riscv_zicsr", 1, "1"); > > > > if (riscv_flags & RISCV_ZIFENCEI) > > > > predefine("__riscv_zifencei", 1, "1"); > > > > + if (riscv_flags & RISCV_ZICBOM) > > > > + predefine("__riscv_zicbom", 1, "1"); > > > > > > > > if (cmodel) > > > > predefine_strong("__riscv_cmodel_%s", cmodel); > > > > -- > > > > 2.34.1 > > > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Add support for the zicbom extension 2022-08-12 8:25 ` Conor Dooley 2022-10-12 21:11 ` Conor Dooley @ 2023-12-15 12:37 ` Geert Uytterhoeven 1 sibling, 0 replies; 7+ messages in thread From: Geert Uytterhoeven @ 2023-12-15 12:37 UTC (permalink / raw) To: Conor Dooley; +Cc: Palmer Dabbelt, luc.vanoostenryck, linux-sparse On Fri, 12 Aug 2022, Conor Dooley wrote: > On Wed, Aug 10, 2022 at 08:31:38PM -0700, Palmer Dabbelt wrote: >> This was recently added to binutils and with any luck will soon be in >> Linux, without it sparse will fail when trying to build new kernels on >> systems with new toolchains. >> > > In passing while testing the zihintpause one: > Tested-by: Conor Dooley <conor.dooley@microchip.com> Likewise: Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Add support for the zicbom extension 2022-08-11 3:31 [PATCH] RISC-V: Add support for the zicbom extension Palmer Dabbelt 2022-08-12 8:25 ` Conor Dooley @ 2023-12-18 13:58 ` Luc Van Oostenryck 1 sibling, 0 replies; 7+ messages in thread From: Luc Van Oostenryck @ 2023-12-18 13:58 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: luc.vanoostenryck, linux-sparse On Wed, Aug 10, 2022 at 08:31:38PM -0700, Palmer Dabbelt wrote: > This was recently added to binutils and with any luck will soon be in > Linux, without it sparse will fail when trying to build new kernels on > systems with new toolchains. > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> Pushed to mainline now. Sorry for this very long delay. -- Luc ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-18 13:58 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-11 3:31 [PATCH] RISC-V: Add support for the zicbom extension Palmer Dabbelt 2022-08-12 8:25 ` Conor Dooley 2022-10-12 21:11 ` Conor Dooley 2022-10-12 23:34 ` Palmer Dabbelt 2022-10-13 11:34 ` Conor Dooley 2023-12-15 12:37 ` Geert Uytterhoeven 2023-12-18 13:58 ` Luc Van Oostenryck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).