| Message ID | 20220309052842.247031-1-michael@michaelkloos.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Work to remove kernel dependence on the M-extension | expand |
On Wed, Mar 9, 2022 at 6:28 AM Michael T. Kloos <michael@michaelkloos.com> wrote: > > Added a new config symbol RISCV_ISA_M to enable the usage of the > multiplication, division, and remainder (modulus) instructions > from the M-extension. This configures the march build flag to > either include or omit it. > > I didn't find any assembly using any of the instructions from > the M-extension. However, the BPF JIT is a complicating factor. > Currently, it emits M-extension instructions to implement various > BPF operations. For now, I have made HAVE_EBPF_JIT depend on > CONFIG_RISCV_ISA_M. > > I have added the supplementary integer arithmetic functions in > the file "arch/riscv/lib/ext_m_supplement.c". All the code > contained in this file is wrapped in an ifndef contingent on the > presence of CONFIG_RISCV_ISA_M. > > Signed-off-by: Michael T. Kloos <michael@michaelkloos.com> The patch looks fine to me, but I increasingly get the feeling that the entire platform feature selection in Kconfig should be guarded with a global flag that switches between "fully generic" and "fully custom" builds, where the generic kernel assumes that all the standard features (64-bit, C, M, FPU, MMU, UEFI, ...) are present, the incompatible options (XIP, PHYS_RAM_BASE_FIXED, CMDLINE_FORCE, BUILTIN_DTB, ...) are force-disabled, and all optional features (V/B/P/H extensions, custom instructions, platform specific device drivers, ...) are runtime detected. At the moment, those three types are listed at the same level, which gives the impression that they can be freely mixed. Arnd
Thank you for your feedback. I don't really have much of an opinion about that right now aside from that I know where things are in the current structure and am comfortable. My goal with this contribution was to keep it in-line with the current config structure. Hence, I put it right next to the menuconfig option to control CONFIG_RISCV_ISA_C under Platform Type. I wouldn't necessarily be opposed to rethinking the way platform feature selection is presented in menuconfig. If people feel that most users will be looking for an rv64gc config and that it should be made for clear, perhaps it could be done. I would need to do more thinking about how exactly that would look. I do think that it is outside the scope of this patch. Were you working on something like that and worried about a merge conflict? Michael On 3/9/22 05:02, Arnd Bergmann wrote: > On Wed, Mar 9, 2022 at 6:28 AM Michael T. Kloos > <michael@michaelkloos.com> wrote: >> Added a new config symbol RISCV_ISA_M to enable the usage of the >> multiplication, division, and remainder (modulus) instructions >> from the M-extension. This configures the march build flag to >> either include or omit it. >> >> I didn't find any assembly using any of the instructions from >> the M-extension. However, the BPF JIT is a complicating factor. >> Currently, it emits M-extension instructions to implement various >> BPF operations. For now, I have made HAVE_EBPF_JIT depend on >> CONFIG_RISCV_ISA_M. >> >> I have added the supplementary integer arithmetic functions in >> the file "arch/riscv/lib/ext_m_supplement.c". All the code >> contained in this file is wrapped in an ifndef contingent on the >> presence of CONFIG_RISCV_ISA_M. >> >> Signed-off-by: Michael T. Kloos <michael@michaelkloos.com> > The patch looks fine to me, but I increasingly get the feeling that the > entire platform feature selection in Kconfig should be guarded with > a global flag that switches between "fully generic" and "fully custom" > builds, where the generic kernel assumes that all the standard > features (64-bit, C, M, FPU, MMU, UEFI, ...) are present, the > incompatible options (XIP, PHYS_RAM_BASE_FIXED, > CMDLINE_FORCE, BUILTIN_DTB, ...) are force-disabled, > and all optional features (V/B/P/H extensions, custom instructions, > platform specific device drivers, ...) are runtime detected. > > At the moment, those three types are listed at the same level, > which gives the impression that they can be freely mixed. > > Arnd
On Wed, Mar 9, 2022 at 12:43 PM Michael T. Kloos <michael@michaelkloos.com> wrote: > > Thank you for your feedback. I don't really have much of an > opinion about that right now aside from that I know where things > are in the current structure and am comfortable. My goal with this > contribution was to keep it in-line with the current config > structure. Hence, I put it right next to the menuconfig option > to control CONFIG_RISCV_ISA_C under Platform Type. > > I wouldn't necessarily be opposed to rethinking the way platform > feature selection is presented in menuconfig. If people feel that > most users will be looking for an rv64gc config and that it should > be made for clear, perhaps it could be done. I would need to do > more thinking about how exactly that would look. > > I do think that it is outside the scope of this patch. Were you > working on something like that and worried about a merge conflict? As I said, I think your patch is ok, and I have no objections to it being merged. It's just one more option among others that can cause problems if users get it wrong. Arnd
Why?
On Wed, 09 Mar 2022 23:06:22 PST (-0800), Christoph Hellwig wrote:
> Why?
I have no idea, but this has come up a few times before.
IIRC the original port had a no-M flavor (don't remember if it even made
it to the original patch submission, but it was around for a bit). We
decided to drop this under the theory that nobody would use it:
essentially, if you can afford the handful of MiB of memory required to
run Linux then you've probably got a multiplier.
If someone has hardware that lacks M and users care about running Linux
on that then I'm happy to support it. I'll still point out the
silliness of that decision, but if it's too late to change things then
I'd rather support the hardware. If it's one of these "fill out every
possible allowed ISA flavor, even if nobody has one that runs Linux"
then I don't see a reason to bother -- there's an infinite amount of
allowable RISC-V implementations, we'll all go crazy chasing them
around.
FWIW: to a first order, that applies to the no-A stuff as well (though
that got dropped because the Arm folks pointed out a way to support that
was way better than ours).
On Wed, 09 Mar 2022 02:02:27 PST (-0800), Arnd Bergmann wrote: > On Wed, Mar 9, 2022 at 6:28 AM Michael T. Kloos > <michael@michaelkloos.com> wrote: >> >> Added a new config symbol RISCV_ISA_M to enable the usage of the >> multiplication, division, and remainder (modulus) instructions >> from the M-extension. This configures the march build flag to >> either include or omit it. >> >> I didn't find any assembly using any of the instructions from >> the M-extension. However, the BPF JIT is a complicating factor. >> Currently, it emits M-extension instructions to implement various >> BPF operations. For now, I have made HAVE_EBPF_JIT depend on >> CONFIG_RISCV_ISA_M. >> >> I have added the supplementary integer arithmetic functions in >> the file "arch/riscv/lib/ext_m_supplement.c". All the code >> contained in this file is wrapped in an ifndef contingent on the >> presence of CONFIG_RISCV_ISA_M. >> >> Signed-off-by: Michael T. Kloos <michael@michaelkloos.com> > > The patch looks fine to me, but I increasingly get the feeling that the > entire platform feature selection in Kconfig should be guarded with > a global flag that switches between "fully generic" and "fully custom" > builds, where the generic kernel assumes that all the standard > features (64-bit, C, M, FPU, MMU, UEFI, ...) are present, the > incompatible options (XIP, PHYS_RAM_BASE_FIXED, > CMDLINE_FORCE, BUILTIN_DTB, ...) are force-disabled, > and all optional features (V/B/P/H extensions, custom instructions, > platform specific device drivers, ...) are runtime detected. That'd be wonderful, but unfortunately we're trending the other way -- we're at the point where "words in the specification have meaning" is controversial, so trying to talk about which flavors of the specification are standard is just meaningless. I obviously hope that gets sorted out, as we've clearly been pointed straight off a cliff for a while now, but LMKL isn't the place to have that discussion. We've all seen this before, nobody needs to be convinced this leads to a mess. Until we get to the point where "I wrote 'RISC-V' on that potato I found in my couch" can be conclusively determined not compliant with the spec, it's just silly to try and talk about what is. > At the moment, those three types are listed at the same level, > which gives the impression that they can be freely mixed. > > Arnd
On Thu, Mar 10, 2022 at 8:34 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > On Wed, 09 Mar 2022 02:02:27 PST (-0800), Arnd Bergmann wrote: > > On Wed, Mar 9, 2022 at 6:28 AM Michael T. Kloos <michael@michaelkloos.com> wrote: > > That'd be wonderful, but unfortunately we're trending the other way -- > we're at the point where "words in the specification have meaning" is > controversial, so trying to talk about which flavors of the > specification are standard is just meaningless. I obviously hope that > gets sorted out, as we've clearly been pointed straight off a cliff for > a while now, but LMKL isn't the place to have that discussion. We've > all seen this before, nobody needs to be convinced this leads to a mess. > > Until we get to the point where "I wrote 'RISC-V' on that potato I found > in my couch" can be conclusively determined not compliant with the spec, > it's just silly to try and talk about what is. I would argue that codifying the required extensions through kernel source code is much stronger than interpreting a specification. Ideally the specification would match what the kernel requires, but it's not the end of the world if the kernel ends up making decisions that are different: If Linux can do runtime detection of non-M, non-A or pre-standard extensions and handle them correctly without a notable performance impact, it can do that. Or Linux could end up requiring things that are normally there but not in the scope of the spec. Regardless of who determines what the compatible subset is, I think there is value in splitting out Kconfig options that prevent booting on normal RV64GC machines (XIP, NOMMU, 32-bit, ...). This would probably not include the non-M option, as long as a non-M kernel works as expected on CPUs with the M instructions. Arnd
On Wed, Mar 09, 2022 at 11:34:25PM -0800, Palmer Dabbelt wrote: > If someone has hardware that lacks M and users care about running Linux on > that then I'm happy to support it. I'll still point out the silliness of > that decision, but if it's too late to change things then I'd rather support > the hardware. If it's one of these "fill out every possible allowed ISA > flavor, even if nobody has one that runs Linux" then I don't see a reason to > bother -- there's an infinite amount of allowable RISC-V implementations, > we'll all go crazy chasing them around. It's not like Linux had required M mode since the RISC-V port was merged upstream, so any new implementor really should know about it. And performance on anything that requires Linux will just be horrible. I could see a bit of an excuse for a nommu port. Anyway, this kind of patch really does need to state the why. And it better be a really good reason.
As far as I can tell, it's simply a compiler flag to enable or disable it. The only complicating issue that I found was the BPF JIT. That is a special case amounting to a compiler within the kernel itself and can be patched or set to depend on M. There is still the BPF interpreter. The kernel doesn't depend on the C-extension. I have grepped through the source code where that symbol is referenced, the checking of instruction alignment constraints, and various checks used to make that work are much more extensive than I expected, yet that is supported. To quote the commentary from the beginning of chapter 2 (Base-I) of the RISC-V spec: > "RV32I was designed to be sufficient to form a compiler target and to > support modern operating system environments. The ISA was also designed > to reduce the hardware required in a minimal implementation. RV32I > contains 40 unique instructions, though a simple implementation might > cover the ECALL/EBREAK instructions with a single SYSTEM hardware > instruction that always traps and might be able to implement the FENCE > instruction as a NOP, reducing base instruction count to 38 total. > RV32I can emulate almost any other ISA extension (except the A extension, > which requires additional hardware support for atomicity). > > In practice, a hardware implementation including the machine-mode > privileged architecture will also require the 6 CSR instructions. > > Subsets of the base integer ISA might be useful for pedagogical purposes, > but the base has been defined such that there should be little incentive > to subset a real hardware implementation beyond omitting support for > misaligned memory accesses and treating all SYSTEM instructions > as a single trap." This is all a noble goal of RISC-V, to create a minimal, yet robust ISA. I'm not arguing to support no-A, as stated in the commentary, atomic can not be emulated (I mean, maybe you could by taking over sscratch) and the atomic properties of the CSR instructions. But that does seem hacky.) and everything else is handled in the Base-I (Not including Zicsr). The kernel supports rv(32/64)ima so to draw the line in the sand that rv(32/64)ia is unacceptable seems silly. One of my concerns here is that if support is not added, that people may start writing assembly that uses M instructions and other non-portable stuff. This will make a future port more difficult. I do not have real hardware like this but I do have an emulator that implements rv32iasu. Eventually I want to build a HART implementation out of real hardware logic chips. Adding M, adds hardware complexity and I think that it is better to do in software for such a minimal case. Sure it will be slower than having native hardware M support. Firmware emulation will be even slower than that. I'd much rather have the option in the kernel to switch this on or off. I am not trying to argue that the kernel should bend over to support my pet projects, but it seems quite strange and arbitrary to draw the line here over such a beautifully modular and minimal ISA. > there's an infinite amount of > allowable RISC-V implementations, we'll all go crazy chasing them > around. 2 points on that: 1) Then it would seem logical to have the kernel target the most minimal implementation to get the widest support. 2) I actually don't think this is entirely true, or at least not in the sense that it is relevant to the kernel. Atomics are needed for a modern system. The kernel needs to know about the floating-point support because it needs to handle the clean-up and context-switching of the additional floating point registers. The kernel needs to know about the C extension because of the ramifications to alignment. Everything else is just gravy. If sometime in the future the L-extension or the B-extension are finished and implemented in future hardware, unless they make some very strange decisions, I don't see why a kernel built today couldn't run on that hardware with userspace software taking advantage of those native features. Sure, in that situation, the kernel binary wouldn't be taking advantage of them. But then wouldn't you want to add CONFIG_RISCV_ISA_L and CONFIG_RISCV_ISA_B in said future kernel so that the compiler is passed an -march value such that the kernel can take advantage of these new native hardware features? It seems to me that this is the grand vision in the modularity of both the RISC-V ISA and the C programming language. We aren't writing everything in assembly for a reason. I don't think it makes sense to stop one extension away from what the RISC-V spec documents themselves state is the intended minimal system capable of running a full modern operating system. I think it is an arbitrary limit to the portability of the kernel. Michael On 3/10/2022 2:34 AM, Palmer Dabbelt wrote: > On Wed, 09 Mar 2022 23:06:22 PST (-0800), Christoph Hellwig wrote: >> Why? > I have no idea, but this has come up a few times before. > IIRC the original port had a no-M flavor (don't remember if it even made > it to the original patch submission, but it was around for a bit). We > decided to drop this under the theory that nobody would use it: > essentially, if you can afford the handful of MiB of memory required to > run Linux then you've probably got a multiplier. > If someone has hardware that lacks M and users care about running Linux > on that then I'm happy to support it. I'll still point out the > silliness of that decision, but if it's too late to change things then > I'd rather support the hardware. If it's one of these "fill out every > possible allowed ISA flavor, even if nobody has one that runs Linux" > then I don't see a reason to bother -- there's an infinite amount of > allowable RISC-V implementations, we'll all go crazy chasing them > around. > FWIW: to a first order, that applies to the no-A stuff as well (though > that got dropped because the Arm folks pointed out a way to support that > was way better than ours).
Some other thoughts: It sounds like I am not the first person to want this feature and I probably won't be the last. I created the change for my own reasons, the same as any other contributor. I think we all know that I can not pull out some chart and say, "This many people want this and here is why." I live in central Ohio and have been doing this as a hobby. I don't even know anyone else who knows about systems and operating system development. If the justification that you are looking for is that I as some hypothetical developer at a major tech company is about to release a new RISC-V chip without M support but we want it to run Linux, I can not provide that answer. It sounds a bit like some software or hardware, chicken or the egg anyway. Trying to maintain my own fork if people start contributing patches with incompatible assembly scares me. Michael On 3/10/2022 4:31 AM, Michael T. Kloos wrote: > As far as I can tell, it's simply a compiler flag to enable or disable it. > The only complicating issue that I found was the BPF JIT. That is a > special case amounting to a compiler within the kernel itself and can be > patched or set to depend on M. There is still the BPF interpreter. The > kernel doesn't depend on the C-extension. I have grepped through the > source code where that symbol is referenced, the checking of instruction > alignment constraints, and various checks used to make that work are much > more extensive than I expected, yet that is supported. To quote the > commentary from the beginning of chapter 2 (Base-I) of the RISC-V spec: >> "RV32I was designed to be sufficient to form a compiler target and to >> support modern operating system environments. The ISA was also designed >> to reduce the hardware required in a minimal implementation. RV32I >> contains 40 unique instructions, though a simple implementation might >> cover the ECALL/EBREAK instructions with a single SYSTEM hardware >> instruction that always traps and might be able to implement the FENCE >> instruction as a NOP, reducing base instruction count to 38 total. >> RV32I can emulate almost any other ISA extension (except the A extension, >> which requires additional hardware support for atomicity). >> In practice, a hardware implementation including the machine-mode >> privileged architecture will also require the 6 CSR instructions. >> Subsets of the base integer ISA might be useful for pedagogical purposes, >> but the base has been defined such that there should be little incentive >> to subset a real hardware implementation beyond omitting support for >> misaligned memory accesses and treating all SYSTEM instructions >> as a single trap." > This is all a noble goal of RISC-V, to create a minimal, yet robust ISA. > I'm not arguing to support no-A, as stated in the commentary, atomic can > not be emulated (I mean, maybe you could by taking over sscratch) and > the atomic properties of the CSR instructions. But that does seem hacky.) > and everything else is handled in the Base-I (Not including Zicsr). The > kernel supports rv(32/64)ima so to draw the line in the sand that > rv(32/64)ia is unacceptable seems silly. > One of my concerns here is that if support is not added, that people may > start writing assembly that uses M instructions and other non-portable > stuff. This will make a future port more difficult. > I do not have real hardware like this but I do have an emulator that > implements rv32iasu. Eventually I want to build a HART implementation > out of real hardware logic chips. Adding M, adds hardware complexity and > I think that it is better to do in software for such a minimal case. > Sure it will be slower than having native hardware M support. Firmware > emulation will be even slower than that. I'd much rather have the option > in the kernel to switch this on or off. > I am not trying to argue that the kernel should bend over to support my > pet projects, but it seems quite strange and arbitrary to draw the line > here over such a beautifully modular and minimal ISA. >> there's an infinite amount of >> allowable RISC-V implementations, we'll all go crazy chasing them >> around. > 2 points on that: > 1) Then it would seem logical to have the kernel target the most minimal > implementation to get the widest support. > 2) I actually don't think this is entirely true, or at least not in the > sense that it is relevant to the kernel. Atomics are needed for a modern > system. The kernel needs to know about the floating-point support because > it needs to handle the clean-up and context-switching of the additional > floating point registers. The kernel needs to know about the C extension > because of the ramifications to alignment. Everything else is just gravy. > If sometime in the future the L-extension or the B-extension are finished > and implemented in future hardware, unless they make some very strange > decisions, I don't see why a kernel built today couldn't run on that > hardware with userspace software taking advantage of those native features. > Sure, in that situation, the kernel binary wouldn't be taking advantage of > them. But then wouldn't you want to add CONFIG_RISCV_ISA_L and > CONFIG_RISCV_ISA_B in said future kernel so that the compiler is passed an > -march value such that the kernel can take advantage of these new native > hardware features? > It seems to me that this is the grand vision in the modularity of both the > RISC-V ISA and the C programming language. We aren't writing everything in > assembly for a reason. I don't think it makes sense to stop one extension > away from what the RISC-V spec documents themselves state is the intended > minimal system capable of running a full modern operating system. I think > it is an arbitrary limit to the portability of the kernel. > Michael > On 3/10/2022 2:34 AM, Palmer Dabbelt wrote: >> On Wed, 09 Mar 2022 23:06:22 PST (-0800), Christoph Hellwig wrote: >>> Why? >> I have no idea, but this has come up a few times before. >> IIRC the original port had a no-M flavor (don't remember if it even made >> it to the original patch submission, but it was around for a bit). We >> decided to drop this under the theory that nobody would use it: >> essentially, if you can afford the handful of MiB of memory required to >> run Linux then you've probably got a multiplier. >> If someone has hardware that lacks M and users care about running Linux >> on that then I'm happy to support it. I'll still point out the >> silliness of that decision, but if it's too late to change things then >> I'd rather support the hardware. If it's one of these "fill out every >> possible allowed ISA flavor, even if nobody has one that runs Linux" >> then I don't see a reason to bother -- there's an infinite amount of >> allowable RISC-V implementations, we'll all go crazy chasing them >> around. >> FWIW: to a first order, that applies to the no-A stuff as well (though >> that got dropped because the Arm folks pointed out a way to support that >> was way better than ours). > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Is there something I can do that would help alleviate your concerns or apprehension? On 3/10/2022 8:22 AM, Michael T. Kloos wrote: > Some other thoughts: > It sounds like I am not the first person to want this feature and I > probably won't be the last. I created the change for my own reasons, the > same as any other contributor. I think we all know that I can not pull > out some chart and say, "This many people want this and here is why." I > live in central Ohio and have been doing this as a hobby. I don't even > know anyone else who knows about systems and operating system development. > If the justification that you are looking for is that I as some > hypothetical developer at a major tech company is about to release a new > RISC-V chip without M support but we want it to run Linux, I can not > provide that answer. It sounds a bit like some software or hardware, > chicken or the egg anyway. Trying to maintain my own fork if people > start contributing patches with incompatible assembly scares me. > Michael
On Wed, 09 Mar 2022 23:54:17 PST (-0800), Arnd Bergmann wrote: > On Thu, Mar 10, 2022 at 8:34 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> On Wed, 09 Mar 2022 02:02:27 PST (-0800), Arnd Bergmann wrote: >> > On Wed, Mar 9, 2022 at 6:28 AM Michael T. Kloos <michael@michaelkloos.com> wrote: >> >> That'd be wonderful, but unfortunately we're trending the other way -- >> we're at the point where "words in the specification have meaning" is >> controversial, so trying to talk about which flavors of the >> specification are standard is just meaningless. I obviously hope that >> gets sorted out, as we've clearly been pointed straight off a cliff for >> a while now, but LMKL isn't the place to have that discussion. We've >> all seen this before, nobody needs to be convinced this leads to a mess. >> >> Until we get to the point where "I wrote 'RISC-V' on that potato I found >> in my couch" can be conclusively determined not compliant with the spec, >> it's just silly to try and talk about what is. > > I would argue that codifying the required extensions through kernel source The problem here isn't the required extensions, it's that vendors can claim to implement an extension on hardware that doesn't exhibit any of the behavior the specification expresses that systems with those extensions must have. The D1 is a very concrete example of this. > code is much stronger than interpreting a specification. Ideally the > specification > would match what the kernel requires, but it's not the end of the world if > the kernel ends up making decisions that are different: If Linux can do > runtime detection of non-M, non-A or pre-standard extensions and handle > them correctly without a notable performance impact, it can do that. Or > Linux could end up requiring things that are normally there but not > in the scope of the spec. > > Regardless of who determines what the compatible subset is, I think there > is value in splitting out Kconfig options that prevent booting on normal > RV64GC machines (XIP, NOMMU, 32-bit, ...). This would probably > not include the non-M option, as long as a non-M kernel works as > expected on CPUs with the M instructions. I get the value to having an option hiding these things, as users might shoot themselves in the foot. I sent a patch, not sure it's exactly what we want but at least it's something concrete to discuss.
On Thu, 10 Mar 2022 05:37:27 PST (-0800), Michael@MichaelKloos.com wrote: > Is there something I can do that would help alleviate your concerns or > apprehension? IMO this is one of those cases where having hardware is required. I can understand the goal of providing a Linux port for the minimal RISC-V compatible system, but IIUC the minimal RISC-V compatible system is any object associated with a member of the RISC-V foundation that said member attests is a RISC-V system. There's really no way to implement Linux on all such systems so we have to set the bar somewhere, and bar is generally set at "more time will be spent using this than maintaining it". Systems without M have generally not met that bar, and I don't see anything changing now. If you have users then I'm happy to reconsider, the goal here is to make real systems work. That said: we've already got enough trouble trying to make actual shipping hardware function correctly, we're all going to lose our minds trying to chase around everything that could in theory be a RISC-V system but doesn't actually exist. > > On 3/10/2022 8:22 AM, Michael T. Kloos wrote: > >> Some other thoughts: >> It sounds like I am not the first person to want this feature and I >> probably won't be the last. I created the change for my own reasons, the >> same as any other contributor. I think we all know that I can not pull >> out some chart and say, "This many people want this and here is why." I >> live in central Ohio and have been doing this as a hobby. I don't even >> know anyone else who knows about systems and operating system development. >> If the justification that you are looking for is that I as some >> hypothetical developer at a major tech company is about to release a new >> RISC-V chip without M support but we want it to run Linux, I can not >> provide that answer. It sounds a bit like some software or hardware, >> chicken or the egg anyway. Trying to maintain my own fork if people >> start contributing patches with incompatible assembly scares me. >> Michael
Well at least I can take some satisfaction in that it seems that on my 2nd patch sent to the Linux kernel, I got it right the first time. No one has complained about problems with the code itself, just that they don't want the feature. I have also received no errors back from the build bot. For now, I will try to maintain this port on my own. Thank you for the consideration Palmer. Michael On Thu, 2022-03-10 at 20:29 -0800, Palmer Dabbelt wrote: > On Thu, 10 Mar 2022 05:37:27 PST (-0800), Michael@MichaelKloos.com wrote: > > Is there something I can do that would help alleviate your concerns or > > apprehension? > > IMO this is one of those cases where having hardware is required. > > I can understand the goal of providing a Linux port for the minimal > RISC-V compatible system, but IIUC the minimal RISC-V compatible system > is any object associated with a member of the RISC-V foundation that > said member attests is a RISC-V system. There's really no way to > implement Linux on all such systems so we have to set the bar somewhere, > and bar is generally set at "more time will be spent using this than > maintaining it". Systems without M have generally not met that bar, and > I don't see anything changing now. > > If you have users then I'm happy to reconsider, the goal here is to make > real systems work. That said: we've already got enough trouble trying > to make actual shipping hardware function correctly, we're all going to > lose our minds trying to chase around everything that could in theory be > a RISC-V system but doesn't actually exist. > > > > > On 3/10/2022 8:22 AM, Michael T. Kloos wrote: > > > > > Some other thoughts: > > > It sounds like I am not the first person to want this feature and I > > > probably won't be the last. I created the change for my own reasons, the > > > same as any other contributor. I think we all know that I can not pull > > > out some chart and say, "This many people want this and here is why." I > > > live in central Ohio and have been doing this as a hobby. I don't even > > > know anyone else who knows about systems and operating system development. > > > If the justification that you are looking for is that I as some > > > hypothetical developer at a major tech company is about to release a new > > > RISC-V chip without M support but we want it to run Linux, I can not > > > provide that answer. It sounds a bit like some software or hardware, > > > chicken or the egg anyway. Trying to maintain my own fork if people > > > start contributing patches with incompatible assembly scares me. > > > Michael
Hi! > >>That'd be wonderful, but unfortunately we're trending the other way -- > >>we're at the point where "words in the specification have meaning" is > >>controversial, so trying to talk about which flavors of the > >>specification are standard is just meaningless. I obviously hope that > >>gets sorted out, as we've clearly been pointed straight off a cliff for > >>a while now, but LMKL isn't the place to have that discussion. We've > >>all seen this before, nobody needs to be convinced this leads to a mess. > >> > >>Until we get to the point where "I wrote 'RISC-V' on that potato I found > >>in my couch" can be conclusively determined not compliant with the spec, > >>it's just silly to try and talk about what is. > > > >I would argue that codifying the required extensions through kernel source > > The problem here isn't the required extensions, it's that vendors can claim > to implement an extension on hardware that doesn't exhibit any of the > behavior the specification expresses that systems with those extensions must > have. The D1 is a very concrete example of this. Sounds like someone interested should make a webpage listing available CPUs that claim RISC-V compatibility but far short of advertised claims? I'd like to get RISC-V board to play with sometime soon, and some help in what board to get would be welcome... Best regards, Pavel
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 5adcbd9b5e88..40e1110a405c 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -83,7 +83,7 @@ config RISCV select HAVE_CONTEXT_TRACKING select HAVE_DEBUG_KMEMLEAK select HAVE_DMA_CONTIGUOUS if MMU - select HAVE_EBPF_JIT if MMU + select HAVE_EBPF_JIT if (MMU && RISCV_ISA_M) select HAVE_FUNCTION_ERROR_INJECTION select HAVE_GCC_PLUGINS select HAVE_GENERIC_VDSO if MMU && 64BIT @@ -323,15 +323,25 @@ config NODES_SHIFT Specify the maximum number of NUMA Nodes available on the target system. Increases memory reserved to accommodate various tables. +config RISCV_ISA_M + bool "Emit multiplication instructions when building Linux" + default y + help + Adds "M" to the ISA subsets that the toolchain is allowed to emit + when building Linux, which results in multiplication, division, and + remainder instructions in the Linux binary. + + If you don't know what to do here, say Y. + config RISCV_ISA_C bool "Emit compressed instructions when building Linux" default y help - Adds "C" to the ISA subsets that the toolchain is allowed to emit - when building Linux, which results in compressed instructions in the - Linux binary. + Adds "C" to the ISA subsets that the toolchain is allowed to emit + when building Linux, which results in compressed instructions in the + Linux binary. - If you don't know what to do here, say Y. + If you don't know what to do here, say Y. menu "supported PMU type" depends on PERF_EVENTS diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 7d81102cffd4..7e24cfe51ef7 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -46,8 +46,10 @@ endif endif # ISA string setting -riscv-march-$(CONFIG_ARCH_RV32I) := rv32ima -riscv-march-$(CONFIG_ARCH_RV64I) := rv64ima +riscv-march-$(CONFIG_ARCH_RV32I) := rv32i +riscv-march-$(CONFIG_ARCH_RV64I) := rv64i +riscv-march-$(CONFIG_RISCV_ISA_M) := $(riscv-march-y)m +riscv-march-y := $(riscv-march-y)a riscv-march-$(CONFIG_FPU) := $(riscv-march-y)fd riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index 25d5c9664e57..965eebaaa1ce 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -5,5 +5,6 @@ lib-y += memset.o lib-y += memmove.o lib-$(CONFIG_MMU) += uaccess.o lib-$(CONFIG_64BIT) += tishift.o +lib-y += ext_m_supplement.o obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o diff --git a/arch/riscv/lib/ext_m_supplement.c b/arch/riscv/lib/ext_m_supplement.c new file mode 100644 index 000000000000..42ced0ea9fe2 --- /dev/null +++ b/arch/riscv/lib/ext_m_supplement.c @@ -0,0 +1,588 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Michael T. Kloos <michael@michaelkloos.com> + */ + +/* + * The GNU manual page here: + * https://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html + * describes these functions in terms of signed and unsigned, int and long. + * However, these prototypes are wrong when the size of int and long are + * considered per the RISC-V ABI specification. The RISC-V port of the GCC + * complier does not follow the standard of those prototypes. This is + * discussed in this thread: + * https://github.com/riscv-collab/riscv-gcc/issues/324 + * + * On the RISC-V Architecture: + * - __xyyysi3 always refers to 32-bit integers. + * - __xyyydi3 always refers to 64-bit integers. + * - __xyyyti3 always refers to 128-bit integers. (Only implemented for rv64) + * + * Per the RISC-V ABI specification, the C base types are: + * - int: 32-bits wide. + * - long: XLEN-bits wide. It matches the register width. + * - long long: 64-bits wide. + * + * Therefore, the correct RISC-V function prototypes are: + * - signed int __mulsi3(signed int a, signed int b); + * - signed long long __muldi3(signed long long a, signed long long b); + * - signed __int128 __multi3(signed __int128 a, signed __int128 b); + * + * - signed int __divsi3(signed int a, signed int b); + * - signed long long __divdi3(signed long long a, signed long long b); + * - signed __int128 __divti3(signed __int128 a, signed __int128 b); + * + * - unsigned int __udivsi3(unsigned int a, unsigned int b); + * - unsigned long long __udivdi3(unsigned long long a, unsigned long long b); + * - unsigned __int128 __udivti3(unsigned __int128 a, unsigned __int128 b); + * + * - signed int __modsi3(signed int a, signed int b); + * - signed long long __moddi3(signed long long a, signed long long b); + * - signed __int128 __modti3(signed __int128 a, signed __int128 b); + * + * - unsigned int __umodsi3(unsigned int a, unsigned int b); + * - unsigned long long __umoddi3(unsigned long long a, unsigned long long b); + * - unsigned __int128 __umodti3(unsigned __int128 a, unsigned __int128 b); + */ + +/* + * This C code is not portable across architectures. It is RISC-V specific. + */ + +#ifndef CONFIG_RISCV_ISA_M + +#include <linux/export.h> + +signed int __mulsi3(signed int a, signed int b) +{ + unsigned int ua; + unsigned int ub; + unsigned int j; + unsigned int i; + signed int r; + + ua = a; + ub = b; + + j = 0; + for (i = 0; i < sizeof(signed int) * 8; i++) { + if (!ua || !ub) + break; + if (ua & 0x1) + j += ub; + ua >>= 1; + ub <<= 1; + } + + r = j; + + return r; +} +EXPORT_SYMBOL(__mulsi3); + +signed long long __muldi3(signed long long a, signed long long b) +{ + unsigned long long ua; + unsigned long long ub; + unsigned long long j; + unsigned int i; + signed long long r; + + ua = a; + ub = b; + + j = 0; + for (i = 0; i < sizeof(signed long long) * 8; i++) { + if (!ua || !ub) + break; + if (ua & 0x1) + j += ub; + ua >>= 1; + ub <<= 1; + } + + r = j; + + return r; +} +EXPORT_SYMBOL(__muldi3); + +#ifdef CONFIG_64BIT +signed __int128 __multi3(signed __int128 a, signed __int128 b) +{ + unsigned __int128 ua; + unsigned __int128 ub; + unsigned __int128 j; + unsigned int i; + signed __int128 r; + + ua = a; + ub = b; + + j = 0; + for (i = 0; i < sizeof(signed __int128) * 8; i++) { + if (!ua || !ub) + break; + if (ua & 0x1) + j += ub; + ua >>= 1; + ub <<= 1; + } + + r = j; + + return r; +} +EXPORT_SYMBOL(__multi3); +#endif + +signed int __divsi3(signed int a, signed int b) +{ + unsigned int ua; + unsigned int ub; + unsigned int j; + unsigned int i; + signed int r; + + if (b == 0) + return (signed int)(-1); + + ua = a; + ub = b; + if (a < 0) + ua = -a; + if (b < 0) + ub = -b; + + j = 0; + i = 0; + while (ua >= ub) { + if ((signed int)ub < 0) { + ua -= ub; + j |= 1u << i; + break; + } + ub <<= 1; + i++; + } + while (i > 0) { + i--; + ub >>= 1; + if (ua >= ub) { + ua -= ub; + j |= 1u << i; + } + } + + r = j; + a ^= b; + if (a < 0) + r = -r; + + return r; +} +EXPORT_SYMBOL(__divsi3); + +signed long long __divdi3(signed long long a, signed long long b) +{ + unsigned long long ua; + unsigned long long ub; + unsigned long long j; + unsigned int i; + signed long long r; + + if (b == 0) + return (signed long long)(-1); + + ua = a; + ub = b; + if (a < 0) + ua = -a; + if (b < 0) + ub = -b; + + j = 0; + i = 0; + while (ua >= ub) { + if ((signed long long)ub < 0) { + ua -= ub; + j |= 1ull << i; + break; + } + ub <<= 1; + i++; + } + while (i > 0) { + i--; + ub >>= 1; + if (ua >= ub) { + ua -= ub; + j |= 1ull << i; + } + } + + r = j; + a ^= b; + if (a < 0) + r = -r; + + return r; +} +EXPORT_SYMBOL(__divdi3); + +#ifdef CONFIG_64BIT +signed __int128 __divti3(signed __int128 a, signed __int128 b) +{ + unsigned __int128 ua; + unsigned __int128 ub; + unsigned __int128 j; + unsigned int i; + signed __int128 r; + + if (b == 0) + return (signed __int128)(-1); + + ua = a; + ub = b; + if (a < 0) + ua = -a; + if (b < 0) + ub = -b; + + j = 0; + i = 0; + while (ua >= ub) { + if ((signed __int128)ub < 0) { + ua -= ub; + j |= ((unsigned __int128)1) << i; + break; + } + ub <<= 1; + i++; + } + while (i > 0) { + i--; + ub >>= 1; + if (ua >= ub) { + ua -= ub; + j |= ((unsigned __int128)1) << i; + } + } + + r = j; + a ^= b; + if (a < 0) + r = -r; + + return r; +} +EXPORT_SYMBOL(__divti3); +#endif + +unsigned int __udivsi3(unsigned int a, unsigned int b) +{ + unsigned int j; + unsigned int i; + + if (b == 0) + return (signed int)(-1); + + j = 0; + i = 0; + while (a >= b) { + if ((signed int)b < 0) { + a -= b; + j |= 1u << i; + break; + } + b <<= 1; + i++; + } + while (i > 0) { + i--; + b >>= 1; + if (a >= b) { + a -= b; + j |= 1u << i; + } + } + + return j; +} +EXPORT_SYMBOL(__udivsi3); + +unsigned long long __udivdi3(unsigned long long a, unsigned long long b) +{ + unsigned long long j; + unsigned long long i; + + if (b == 0) + return (signed long long)(-1); + + j = 0; + i = 0; + while (a >= b) { + if ((signed long long)b < 0) { + a -= b; + j |= 1ull << i; + break; + } + b <<= 1; + i++; + } + while (i > 0) { + i--; + b >>= 1; + if (a >= b) { + a -= b; + j |= 1ull << i; + } + } + + return j; +} +EXPORT_SYMBOL(__udivdi3); + +#ifdef CONFIG_64BIT +unsigned __int128 __udivti3(unsigned __int128 a, unsigned __int128 b) +{ + unsigned __int128 j; + unsigned __int128 i; + + if (b == 0) + return (signed __int128)(-1); + + j = 0; + i = 0; + while (a >= b) { + if ((signed __int128)b < 0) { + a -= b; + j |= ((unsigned __int128)1) << i; + break; + } + b <<= 1; + i++; + } + while (i > 0) { + i--; + b >>= 1; + if (a >= b) { + a -= b; + j |= ((unsigned __int128)1) << i; + } + } + + return j; +} +EXPORT_SYMBOL(__udivti3); +#endif + +signed int __modsi3(signed int a, signed int b) +{ + unsigned int ua; + unsigned int ub; + unsigned int i; + signed int r; + + if (b == 0) + return a; + + ua = a; + ub = b; + if (a < 0) + ua = -a; + if (b < 0) + ub = -b; + + i = 0; + while (ua >= ub) { + if ((signed int)ub < 0) { + ua -= ub; + break; + } + ub <<= 1; + i++; + } + while (i > 0) { + i--; + ub >>= 1; + if (ua >= ub) + ua -= ub; + } + + r = ua; + if (a < 0) + r = -r; + + return r; +} +EXPORT_SYMBOL(__modsi3); + +signed long long __moddi3(signed long long a, signed long long b) +{ + unsigned long long ua; + unsigned long long ub; + unsigned int i; + signed long long r; + + if (b == 0) + return a; + + ua = a; + ub = b; + if (a < 0) + ua = -a; + if (b < 0) + ub = -b; + + i = 0; + while (ua >= ub) { + if ((signed long long)ub < 0) { + ua -= ub; + break; + } + ub <<= 1; + i++; + } + while (i > 0) { + i--; + ub >>= 1; + if (ua >= ub) + ua -= ub; + } + + r = ua; + if (a < 0) + r = -r; + + return r; +} +EXPORT_SYMBOL(__moddi3); + +#ifdef CONFIG_64BIT +signed __int128 __modti3(signed __int128 a, signed __int128 b) +{ + unsigned __int128 ua; + unsigned __int128 ub; + unsigned int i; + signed __int128 r; + + if (b == 0) + return a; + + ua = a; + ub = b; + if (a < 0) + ua = -a; + if (b < 0) + ub = -b; + + i = 0; + while (ua >= ub) { + if ((signed __int128)ub < 0) { + ua -= ub; + break; + } + ub <<= 1; + i++; + } + while (i > 0) { + i--; + ub >>= 1; + if (ua >= ub) + ua -= ub; + } + + r = ua; + if (a < 0) + r = -r; + + return r; +} +EXPORT_SYMBOL(__modti3); +#endif + +unsigned int __umodsi3(unsigned int a, unsigned int b) +{ + unsigned int i; + + if (b == 0) + return a; + + i = 0; + while (a >= b) { + if ((signed int)b < 0) { + a -= b; + break; + } + b <<= 1; + i++; + } + while (i > 0) { + i--; + b >>= 1; + if (a >= b) + a -= b; + } + + return a; +} +EXPORT_SYMBOL(__umodsi3); + +unsigned long long __umoddi3(unsigned long long a, unsigned long long b) +{ + unsigned long long i; + + if (b == 0) + return a; + + i = 0; + while (a >= b) { + if ((signed long long)b < 0) { + a -= b; + break; + } + b <<= 1; + i++; + } + while (i > 0) { + i--; + b >>= 1; + if (a >= b) + a -= b; + } + + return a; +} +EXPORT_SYMBOL(__umoddi3); + +#ifdef CONFIG_64BIT +unsigned __int128 __umodti3(unsigned __int128 a, unsigned __int128 b) +{ + unsigned __int128 i; + + if (b == 0) + return a; + + i = 0; + while (a >= b) { + if ((signed __int128)b < 0) { + a -= b; + break; + } + b <<= 1; + i++; + } + while (i > 0) { + i--; + b >>= 1; + if (a >= b) + a -= b; + } + + return a; +} +EXPORT_SYMBOL(__umodti3); +#endif + +#endif
Added a new config symbol RISCV_ISA_M to enable the usage of the multiplication, division, and remainder (modulus) instructions from the M-extension. This configures the march build flag to either include or omit it. I didn't find any assembly using any of the instructions from the M-extension. However, the BPF JIT is a complicating factor. Currently, it emits M-extension instructions to implement various BPF operations. For now, I have made HAVE_EBPF_JIT depend on CONFIG_RISCV_ISA_M. I have added the supplementary integer arithmetic functions in the file "arch/riscv/lib/ext_m_supplement.c". All the code contained in this file is wrapped in an ifndef contingent on the presence of CONFIG_RISCV_ISA_M. Signed-off-by: Michael T. Kloos <michael@michaelkloos.com> --- arch/riscv/Kconfig | 20 +- arch/riscv/Makefile | 6 +- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/ext_m_supplement.c | 588 ++++++++++++++++++++++++++++++ 4 files changed, 608 insertions(+), 7 deletions(-) create mode 100644 arch/riscv/lib/ext_m_supplement.c