| Message ID | 20230803-master-refactor-instructions-v4-v1-0-2128e61fa4ff@rivosinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RISC-V: Refactor instructions | expand |
On Thu, Aug 03, 2023 at 07:10:26PM -0700, Charlie Jenkins wrote: > There are many systems across the kernel that rely on directly creating > and modifying instructions. In order to unify them, create shared > definitions for instructions and registers. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > --- > arch/riscv/include/asm/insn.h | 2742 +++++++++++++++++++++++++++--- "I did a lot of copy-pasting from the RISC-V spec" How is anyone supposed to cross check this when there's 1000s of lines of a diff here? We've had some subtle bugs in some of the definitions in the past, so I would like to be able to check at this opportune moment that things are correct. > arch/riscv/include/asm/reg.h | 88 + > arch/riscv/kernel/kgdb.c | 4 +- > arch/riscv/kernel/probes/simulate-insn.c | 39 +- > arch/riscv/kernel/vector.c | 2 +- You need to at least split this up. I doubt a 2742 change diff for insn.h was required to make the changes in these 4 files. Then after that, it would be so much easier to reason about these changes if the additions to insn.h happened at the same time as the removals from the affected locations. I would probably split this so that things are done in more stages, with the larger patches split between changes that require no new definitions and changes that require moving things to insn.h > 5 files changed, 2629 insertions(+), 246 deletions(-) What you would want to see if this arrived in your inbox as a reviewer? Don't get me wrong, I do like what you are doing here, the BPF JIT especially is filled with "uhh okay, I guess those offsets are right", so I don't mean to be discouraging. Thanks, Conor.
On Thu, Aug 03, 2023 at 07:10:25PM -0700, Charlie Jenkins wrote: > There are numerous systems in the kernel that rely on directly > modifying, creating, and reading instructions. Many of these systems > have rewritten code to do this. This patch will delegate all instruction > handling into insn.h and reg.h. All of the compressed instructions, RVI, > Zicsr, M, A instructions are included, as well as a subset of the F,D,Q > extensions. > > --- > This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ > is also touching. > > --- > Testing: > > There are a lot of subsystems touched and I have not tested every > individual instruction. I did a lot of copy-pasting from the RISC-V spec > so opcodes and such should be correct How about we create macros which generate each of the functions an instruction needs, e.g. riscv_insn_is_*(), etc. based on the output of [1]. I know basically nothing about that project, but it looks like it creates most the defines this series is creating from what we [hope] to be an authoritative source. I also assume that if we don't like the current output format, then we could probably post patches to the project to get the format we want. For example, we could maybe propose an "lc" format for "Linux C". I'd also recommend only importing the generated defines and generating the functions that will actually have immediate consumers or are part of a set of defines that have immediate consumers. Each consumer of new instructions will be responsible for generating and importing the defines and adding the respective macro invocations to generate the functions. This series can also take that approach, i.e. convert one set of instructions at a time, each in a separate patch. [1] https://github.com/riscv/riscv-opcodes Thanks, drew > , but the construction of every > instruction is not fully tested. > > vector: Compiled and booted > > jump_label: Ensured static keys function as expected. > > kgdb: Attempted to run the provided tests but they failed even without > my changes > > module: Loaded and unloaded modules > > patch.c: Ensured kernel booted > > kprobes: Used a kprobing module to probe jalr, auipc, and branch > instructions > > nommu misaligned addresses: Kernel boots > > kvm: Ran KVM selftests > > bpf: Kernel boots. Most of the instructions are exclusively used by BPF > but I am unsure of the best way of testing BPF. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > Charlie Jenkins (10): > RISC-V: Expand instruction definitions > RISC-V: vector: Refactor instructions > RISC-V: Refactor jump label instructions > RISC-V: KGDB: Refactor instructions > RISC-V: module: Refactor instructions > RISC-V: Refactor patch instructions > RISC-V: nommu: Refactor instructions > RISC-V: kvm: Refactor instructions > RISC-V: bpf: Refactor instructions > RISC-V: Refactor bug and traps instructions > > arch/riscv/include/asm/bug.h | 18 +- > arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- > arch/riscv/include/asm/reg.h | 88 + > arch/riscv/kernel/jump_label.c | 13 +- > arch/riscv/kernel/kgdb.c | 13 +- > arch/riscv/kernel/module.c | 80 +- > arch/riscv/kernel/patch.c | 3 +- > arch/riscv/kernel/probes/kprobes.c | 13 +- > arch/riscv/kernel/probes/simulate-insn.c | 100 +- > arch/riscv/kernel/probes/uprobes.c | 5 +- > arch/riscv/kernel/traps.c | 9 +- > arch/riscv/kernel/traps_misaligned.c | 218 +-- > arch/riscv/kernel/vector.c | 5 +- > arch/riscv/kvm/vcpu_insn.c | 281 +-- > arch/riscv/net/bpf_jit.h | 707 +------- > 15 files changed, 2825 insertions(+), 1472 deletions(-) > --- > base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 > change-id: 20230801-master-refactor-instructions-v4-433aa040da03 > -- > - Charlie > > > -- > kvm-riscv mailing list > kvm-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kvm-riscv
On Fri, Aug 04, 2023 at 12:28:28PM +0300, Andrew Jones wrote: > On Thu, Aug 03, 2023 at 07:10:25PM -0700, Charlie Jenkins wrote: > > There are numerous systems in the kernel that rely on directly > > modifying, creating, and reading instructions. Many of these systems > > have rewritten code to do this. This patch will delegate all instruction > > handling into insn.h and reg.h. All of the compressed instructions, RVI, > > Zicsr, M, A instructions are included, as well as a subset of the F,D,Q > > extensions. > > > > --- > > This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ > > is also touching. > > > > --- > > Testing: > > > > There are a lot of subsystems touched and I have not tested every > > individual instruction. I did a lot of copy-pasting from the RISC-V spec > > so opcodes and such should be correct > > How about we create macros which generate each of the functions an > instruction needs, e.g. riscv_insn_is_*(), etc. based on the output of > [1]. I know basically nothing about that project, but it looks like it > creates most the defines this series is creating from what we [hope] to > be an authoritative source. I also assume that if we don't like the > current output format, then we could probably post patches to the project > to get the format we want. For example, we could maybe propose an "lc" > format for "Linux C". That's a great idea, I didn't realize that existed! > > I'd also recommend only importing the generated defines and generating > the functions that will actually have immediate consumers or are part of > a set of defines that have immediate consumers. Each consumer of new > instructions will be responsible for generating and importing the defines > and adding the respective macro invocations to generate the functions. > This series can also take that approach, i.e. convert one set of > instructions at a time, each in a separate patch. Since I was hand-writing everything and copying it wasn't too much effort to just copy all of the instructions from a group. However, from a testing standpoint it makes sense to exclude instructions not yet in use. > > [1] https://github.com/riscv/riscv-opcodes > > Thanks, > drew > > > > , but the construction of every > > instruction is not fully tested. > > > > vector: Compiled and booted > > > > jump_label: Ensured static keys function as expected. > > > > kgdb: Attempted to run the provided tests but they failed even without > > my changes > > > > module: Loaded and unloaded modules > > > > patch.c: Ensured kernel booted > > > > kprobes: Used a kprobing module to probe jalr, auipc, and branch > > instructions > > > > nommu misaligned addresses: Kernel boots > > > > kvm: Ran KVM selftests > > > > bpf: Kernel boots. Most of the instructions are exclusively used by BPF > > but I am unsure of the best way of testing BPF. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > --- > > Charlie Jenkins (10): > > RISC-V: Expand instruction definitions > > RISC-V: vector: Refactor instructions > > RISC-V: Refactor jump label instructions > > RISC-V: KGDB: Refactor instructions > > RISC-V: module: Refactor instructions > > RISC-V: Refactor patch instructions > > RISC-V: nommu: Refactor instructions > > RISC-V: kvm: Refactor instructions > > RISC-V: bpf: Refactor instructions > > RISC-V: Refactor bug and traps instructions > > > > arch/riscv/include/asm/bug.h | 18 +- > > arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- > > arch/riscv/include/asm/reg.h | 88 + > > arch/riscv/kernel/jump_label.c | 13 +- > > arch/riscv/kernel/kgdb.c | 13 +- > > arch/riscv/kernel/module.c | 80 +- > > arch/riscv/kernel/patch.c | 3 +- > > arch/riscv/kernel/probes/kprobes.c | 13 +- > > arch/riscv/kernel/probes/simulate-insn.c | 100 +- > > arch/riscv/kernel/probes/uprobes.c | 5 +- > > arch/riscv/kernel/traps.c | 9 +- > > arch/riscv/kernel/traps_misaligned.c | 218 +-- > > arch/riscv/kernel/vector.c | 5 +- > > arch/riscv/kvm/vcpu_insn.c | 281 +-- > > arch/riscv/net/bpf_jit.h | 707 +------- > > 15 files changed, 2825 insertions(+), 1472 deletions(-) > > --- > > base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 > > change-id: 20230801-master-refactor-instructions-v4-433aa040da03 > > -- > > - Charlie > > > > > > -- > > kvm-riscv mailing list > > kvm-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kvm-riscv
On Fri, Aug 04, 2023 at 08:59:24AM +0100, Conor Dooley wrote: > On Thu, Aug 03, 2023 at 07:10:26PM -0700, Charlie Jenkins wrote: > > There are many systems across the kernel that rely on directly creating > > and modifying instructions. In order to unify them, create shared > > definitions for instructions and registers. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > arch/riscv/include/asm/insn.h | 2742 +++++++++++++++++++++++++++--- > > "I did a lot of copy-pasting from the RISC-V spec" > > How is anyone supposed to cross check this when there's 1000s of lines > of a diff here? We've had some subtle bugs in some of the definitions in > the past, so I would like to be able to check at this opportune moment > that things are correct. > > > arch/riscv/include/asm/reg.h | 88 + > > arch/riscv/kernel/kgdb.c | 4 +- > > arch/riscv/kernel/probes/simulate-insn.c | 39 +- > > arch/riscv/kernel/vector.c | 2 +- > > You need to at least split this up. I doubt a 2742 change diff for > insn.h was required to make the changes in these 4 files. Yeah it is kind of a nightmare to look at, I will split it up. > > Then after that, it would be so much easier to reason about these > changes if the additions to insn.h happened at the same time as the > removals from the affected locations. > > I would probably split this so that things are done in more stages, > with the larger patches split between changes that require no new > definitions and changes that require moving things to insn.h > > > 5 files changed, 2629 insertions(+), 246 deletions(-) > > What you would want to see if this arrived in your inbox as a reviewer? > > Don't get me wrong, I do like what you are doing here, the BPF JIT > especially is filled with "uhh okay, I guess those offsets are right", > so I don't mean to be discouraging. > > Thanks, > Conor.
On Fri, Aug 04, 2023 at 10:24:33AM -0700, Charlie Jenkins wrote: > On Fri, Aug 04, 2023 at 12:28:28PM +0300, Andrew Jones wrote: > > On Thu, Aug 03, 2023 at 07:10:25PM -0700, Charlie Jenkins wrote: > > > There are numerous systems in the kernel that rely on directly > > > modifying, creating, and reading instructions. Many of these systems > > > have rewritten code to do this. This patch will delegate all instruction > > > handling into insn.h and reg.h. All of the compressed instructions, RVI, > > > Zicsr, M, A instructions are included, as well as a subset of the F,D,Q > > > extensions. > > > > > > --- > > > This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ > > > is also touching. > > > > > > --- > > > Testing: > > > > > > There are a lot of subsystems touched and I have not tested every > > > individual instruction. I did a lot of copy-pasting from the RISC-V spec > > > so opcodes and such should be correct > > > > How about we create macros which generate each of the functions an > > instruction needs, e.g. riscv_insn_is_*(), etc. based on the output of > > [1]. I know basically nothing about that project, but it looks like it > > creates most the defines this series is creating from what we [hope] to > > be an authoritative source. I also assume that if we don't like the > > current output format, then we could probably post patches to the project > > to get the format we want. For example, we could maybe propose an "lc" > > format for "Linux C". > That's a great idea, I didn't realize that existed! I have discovered that the riscv-opcodes repository is not in a state that makes it helpful. If it were workable, it would make it easy to include a "Linux C" format. I have had a pull request open on the repo for two weeks now and the person who maintains the repo has not interacted. At minimum, in order for it to be useful it would need an ability to describe the bit order of immediates in an instruction and include script arguments to select which instructions should be included. There is a "C" format, but it is actually just a Spike format. Nonetheless, it seems like it is prohibitive to use it. > > > > I'd also recommend only importing the generated defines and generating > > the functions that will actually have immediate consumers or are part of > > a set of defines that have immediate consumers. Each consumer of new > > instructions will be responsible for generating and importing the defines > > and adding the respective macro invocations to generate the functions. > > This series can also take that approach, i.e. convert one set of > > instructions at a time, each in a separate patch. > Since I was hand-writing everything and copying it wasn't too much > effort to just copy all of the instructions from a group. However, from > a testing standpoint it makes sense to exclude instructions not yet in > use. > > > > [1] https://github.com/riscv/riscv-opcodes > > > > Thanks, > > drew > > > > > > > , but the construction of every > > > instruction is not fully tested. > > > > > > vector: Compiled and booted > > > > > > jump_label: Ensured static keys function as expected. > > > > > > kgdb: Attempted to run the provided tests but they failed even without > > > my changes > > > > > > module: Loaded and unloaded modules > > > > > > patch.c: Ensured kernel booted > > > > > > kprobes: Used a kprobing module to probe jalr, auipc, and branch > > > instructions > > > > > > nommu misaligned addresses: Kernel boots > > > > > > kvm: Ran KVM selftests > > > > > > bpf: Kernel boots. Most of the instructions are exclusively used by BPF > > > but I am unsure of the best way of testing BPF. > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > > > --- > > > Charlie Jenkins (10): > > > RISC-V: Expand instruction definitions > > > RISC-V: vector: Refactor instructions > > > RISC-V: Refactor jump label instructions > > > RISC-V: KGDB: Refactor instructions > > > RISC-V: module: Refactor instructions > > > RISC-V: Refactor patch instructions > > > RISC-V: nommu: Refactor instructions > > > RISC-V: kvm: Refactor instructions > > > RISC-V: bpf: Refactor instructions > > > RISC-V: Refactor bug and traps instructions > > > > > > arch/riscv/include/asm/bug.h | 18 +- > > > arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- > > > arch/riscv/include/asm/reg.h | 88 + > > > arch/riscv/kernel/jump_label.c | 13 +- > > > arch/riscv/kernel/kgdb.c | 13 +- > > > arch/riscv/kernel/module.c | 80 +- > > > arch/riscv/kernel/patch.c | 3 +- > > > arch/riscv/kernel/probes/kprobes.c | 13 +- > > > arch/riscv/kernel/probes/simulate-insn.c | 100 +- > > > arch/riscv/kernel/probes/uprobes.c | 5 +- > > > arch/riscv/kernel/traps.c | 9 +- > > > arch/riscv/kernel/traps_misaligned.c | 218 +-- > > > arch/riscv/kernel/vector.c | 5 +- > > > arch/riscv/kvm/vcpu_insn.c | 281 +-- > > > arch/riscv/net/bpf_jit.h | 707 +------- > > > 15 files changed, 2825 insertions(+), 1472 deletions(-) > > > --- > > > base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 > > > change-id: 20230801-master-refactor-instructions-v4-433aa040da03 > > > -- > > > - Charlie > > > > > > > > > -- > > > kvm-riscv mailing list > > > kvm-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/kvm-riscv
On 17 Aug 2023, at 01:31, Charlie Jenkins <charlie@rivosinc.com> wrote: > > On Fri, Aug 04, 2023 at 10:24:33AM -0700, Charlie Jenkins wrote: >> On Fri, Aug 04, 2023 at 12:28:28PM +0300, Andrew Jones wrote: >>> On Thu, Aug 03, 2023 at 07:10:25PM -0700, Charlie Jenkins wrote: >>>> There are numerous systems in the kernel that rely on directly >>>> modifying, creating, and reading instructions. Many of these systems >>>> have rewritten code to do this. This patch will delegate all instruction >>>> handling into insn.h and reg.h. All of the compressed instructions, RVI, >>>> Zicsr, M, A instructions are included, as well as a subset of the F,D,Q >>>> extensions. >>>> >>>> --- >>>> This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ >>>> is also touching. >>>> >>>> --- >>>> Testing: >>>> >>>> There are a lot of subsystems touched and I have not tested every >>>> individual instruction. I did a lot of copy-pasting from the RISC-V spec >>>> so opcodes and such should be correct >>> >>> How about we create macros which generate each of the functions an >>> instruction needs, e.g. riscv_insn_is_*(), etc. based on the output of >>> [1]. I know basically nothing about that project, but it looks like it >>> creates most the defines this series is creating from what we [hope] to >>> be an authoritative source. I also assume that if we don't like the >>> current output format, then we could probably post patches to the project >>> to get the format we want. For example, we could maybe propose an "lc" >>> format for "Linux C". >> That's a great idea, I didn't realize that existed! > I have discovered that the riscv-opcodes repository is not in a state > that makes it helpful. If it were workable, it would make it easy to > include a "Linux C" format. I have had a pull request open on the repo > for two weeks now and the person who maintains the repo has not > interacted. Huh? Andrew has replied to you twice on your PR, and was the last one to comment. That’s hardly “has not interacted”. > At minimum, in order for it to be useful it would need an ability to > describe the bit order of immediates in an instruction and include script > arguments to select which instructions should be included. There is a > "C" format, but it is actually just a Spike format. So extend it? Or do something with QEMU’s equivalent that expresses it. Jess > Nonetheless, it > seems like it is prohibitive to use it. >>> >>> I'd also recommend only importing the generated defines and generating >>> the functions that will actually have immediate consumers or are part of >>> a set of defines that have immediate consumers. Each consumer of new >>> instructions will be responsible for generating and importing the defines >>> and adding the respective macro invocations to generate the functions. >>> This series can also take that approach, i.e. convert one set of >>> instructions at a time, each in a separate patch. >> Since I was hand-writing everything and copying it wasn't too much >> effort to just copy all of the instructions from a group. However, from >> a testing standpoint it makes sense to exclude instructions not yet in >> use. >>> >>> [1] https://github.com/riscv/riscv-opcodes >>> >>> Thanks, >>> drew >>> >>> >>>> , but the construction of every >>>> instruction is not fully tested. >>>> >>>> vector: Compiled and booted >>>> >>>> jump_label: Ensured static keys function as expected. >>>> >>>> kgdb: Attempted to run the provided tests but they failed even without >>>> my changes >>>> >>>> module: Loaded and unloaded modules >>>> >>>> patch.c: Ensured kernel booted >>>> >>>> kprobes: Used a kprobing module to probe jalr, auipc, and branch >>>> instructions >>>> >>>> nommu misaligned addresses: Kernel boots >>>> >>>> kvm: Ran KVM selftests >>>> >>>> bpf: Kernel boots. Most of the instructions are exclusively used by BPF >>>> but I am unsure of the best way of testing BPF. >>>> >>>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> >>>> >>>> --- >>>> Charlie Jenkins (10): >>>> RISC-V: Expand instruction definitions >>>> RISC-V: vector: Refactor instructions >>>> RISC-V: Refactor jump label instructions >>>> RISC-V: KGDB: Refactor instructions >>>> RISC-V: module: Refactor instructions >>>> RISC-V: Refactor patch instructions >>>> RISC-V: nommu: Refactor instructions >>>> RISC-V: kvm: Refactor instructions >>>> RISC-V: bpf: Refactor instructions >>>> RISC-V: Refactor bug and traps instructions >>>> >>>> arch/riscv/include/asm/bug.h | 18 +- >>>> arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- >>>> arch/riscv/include/asm/reg.h | 88 + >>>> arch/riscv/kernel/jump_label.c | 13 +- >>>> arch/riscv/kernel/kgdb.c | 13 +- >>>> arch/riscv/kernel/module.c | 80 +- >>>> arch/riscv/kernel/patch.c | 3 +- >>>> arch/riscv/kernel/probes/kprobes.c | 13 +- >>>> arch/riscv/kernel/probes/simulate-insn.c | 100 +- >>>> arch/riscv/kernel/probes/uprobes.c | 5 +- >>>> arch/riscv/kernel/traps.c | 9 +- >>>> arch/riscv/kernel/traps_misaligned.c | 218 +-- >>>> arch/riscv/kernel/vector.c | 5 +- >>>> arch/riscv/kvm/vcpu_insn.c | 281 +-- >>>> arch/riscv/net/bpf_jit.h | 707 +------- >>>> 15 files changed, 2825 insertions(+), 1472 deletions(-) >>>> --- >>>> base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 >>>> change-id: 20230801-master-refactor-instructions-v4-433aa040da03 >>>> -- >>>> - Charlie >>>> >>>> >>>> -- >>>> kvm-riscv mailing list >>>> kvm-riscv@lists.infradead.org >>>> http://lists.infradead.org/mailman/listinfo/kvm-riscv > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 17 Aug 2023, at 04:57, Jessica Clarke <jrtc27@jrtc27.com> wrote: > > On 17 Aug 2023, at 01:31, Charlie Jenkins <charlie@rivosinc.com> wrote: >> >> On Fri, Aug 04, 2023 at 10:24:33AM -0700, Charlie Jenkins wrote: >>> On Fri, Aug 04, 2023 at 12:28:28PM +0300, Andrew Jones wrote: >>>> On Thu, Aug 03, 2023 at 07:10:25PM -0700, Charlie Jenkins wrote: >>>>> There are numerous systems in the kernel that rely on directly >>>>> modifying, creating, and reading instructions. Many of these systems >>>>> have rewritten code to do this. This patch will delegate all instruction >>>>> handling into insn.h and reg.h. All of the compressed instructions, RVI, >>>>> Zicsr, M, A instructions are included, as well as a subset of the F,D,Q >>>>> extensions. >>>>> >>>>> --- >>>>> This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ >>>>> is also touching. >>>>> >>>>> --- >>>>> Testing: >>>>> >>>>> There are a lot of subsystems touched and I have not tested every >>>>> individual instruction. I did a lot of copy-pasting from the RISC-V spec >>>>> so opcodes and such should be correct >>>> >>>> How about we create macros which generate each of the functions an >>>> instruction needs, e.g. riscv_insn_is_*(), etc. based on the output of >>>> [1]. I know basically nothing about that project, but it looks like it >>>> creates most the defines this series is creating from what we [hope] to >>>> be an authoritative source. I also assume that if we don't like the >>>> current output format, then we could probably post patches to the project >>>> to get the format we want. For example, we could maybe propose an "lc" >>>> format for "Linux C". >>> That's a great idea, I didn't realize that existed! >> I have discovered that the riscv-opcodes repository is not in a state >> that makes it helpful. If it were workable, it would make it easy to >> include a "Linux C" format. I have had a pull request open on the repo >> for two weeks now and the person who maintains the repo has not >> interacted. > > Huh? Andrew has replied to you twice on your PR, and was the last one to > comment. That’s hardly “has not interacted”. > >> At minimum, in order for it to be useful it would need an ability to >> describe the bit order of immediates in an instruction and include script >> arguments to select which instructions should be included. There is a >> "C" format, but it is actually just a Spike format. > > So extend it? Or do something with QEMU’s equivalent that expresses it. Note that every field already identifies the bit order (or, for the case of compressed instructions, register restrictions) since that’s needed to produce the old LaTeX instruction set listings; that’s why there’s jimm20 vs imm20, for example. One could surely encode that in Python and generate the LaTeX strings from the Python, making the details of the encodings available elsewhere. Or just have your own mapping from name to whatever you need. But, either way, the information should all be there today in the input files, it’s just a matter of extending the script to produce whatever you want from them. > Jess > >> Nonetheless, it >> seems like it is prohibitive to use it. >>>> >>>> I'd also recommend only importing the generated defines and generating >>>> the functions that will actually have immediate consumers or are part of >>>> a set of defines that have immediate consumers. Each consumer of new >>>> instructions will be responsible for generating and importing the defines >>>> and adding the respective macro invocations to generate the functions. >>>> This series can also take that approach, i.e. convert one set of >>>> instructions at a time, each in a separate patch. >>> Since I was hand-writing everything and copying it wasn't too much >>> effort to just copy all of the instructions from a group. However, from >>> a testing standpoint it makes sense to exclude instructions not yet in >>> use. >>>> >>>> [1] https://github.com/riscv/riscv-opcodes >>>> >>>> Thanks, >>>> drew >>>> >>>> >>>>> , but the construction of every >>>>> instruction is not fully tested. >>>>> >>>>> vector: Compiled and booted >>>>> >>>>> jump_label: Ensured static keys function as expected. >>>>> >>>>> kgdb: Attempted to run the provided tests but they failed even without >>>>> my changes >>>>> >>>>> module: Loaded and unloaded modules >>>>> >>>>> patch.c: Ensured kernel booted >>>>> >>>>> kprobes: Used a kprobing module to probe jalr, auipc, and branch >>>>> instructions >>>>> >>>>> nommu misaligned addresses: Kernel boots >>>>> >>>>> kvm: Ran KVM selftests >>>>> >>>>> bpf: Kernel boots. Most of the instructions are exclusively used by BPF >>>>> but I am unsure of the best way of testing BPF. >>>>> >>>>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> >>>>> >>>>> --- >>>>> Charlie Jenkins (10): >>>>> RISC-V: Expand instruction definitions >>>>> RISC-V: vector: Refactor instructions >>>>> RISC-V: Refactor jump label instructions >>>>> RISC-V: KGDB: Refactor instructions >>>>> RISC-V: module: Refactor instructions >>>>> RISC-V: Refactor patch instructions >>>>> RISC-V: nommu: Refactor instructions >>>>> RISC-V: kvm: Refactor instructions >>>>> RISC-V: bpf: Refactor instructions >>>>> RISC-V: Refactor bug and traps instructions >>>>> >>>>> arch/riscv/include/asm/bug.h | 18 +- >>>>> arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- >>>>> arch/riscv/include/asm/reg.h | 88 + >>>>> arch/riscv/kernel/jump_label.c | 13 +- >>>>> arch/riscv/kernel/kgdb.c | 13 +- >>>>> arch/riscv/kernel/module.c | 80 +- >>>>> arch/riscv/kernel/patch.c | 3 +- >>>>> arch/riscv/kernel/probes/kprobes.c | 13 +- >>>>> arch/riscv/kernel/probes/simulate-insn.c | 100 +- >>>>> arch/riscv/kernel/probes/uprobes.c | 5 +- >>>>> arch/riscv/kernel/traps.c | 9 +- >>>>> arch/riscv/kernel/traps_misaligned.c | 218 +-- >>>>> arch/riscv/kernel/vector.c | 5 +- >>>>> arch/riscv/kvm/vcpu_insn.c | 281 +-- >>>>> arch/riscv/net/bpf_jit.h | 707 +------- >>>>> 15 files changed, 2825 insertions(+), 1472 deletions(-) >>>>> --- >>>>> base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 >>>>> change-id: 20230801-master-refactor-instructions-v4-433aa040da03 >>>>> -- >>>>> - Charlie >>>>> >>>>> >>>>> -- >>>>> kvm-riscv mailing list >>>>> kvm-riscv@lists.infradead.org >>>>> http://lists.infradead.org/mailman/listinfo/kvm-riscv >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, Aug 17, 2023 at 05:05:45AM +0100, Jessica Clarke wrote: > On 17 Aug 2023, at 04:57, Jessica Clarke <jrtc27@jrtc27.com> wrote: > > > > On 17 Aug 2023, at 01:31, Charlie Jenkins <charlie@rivosinc.com> wrote: > >> > >> On Fri, Aug 04, 2023 at 10:24:33AM -0700, Charlie Jenkins wrote: > >>> On Fri, Aug 04, 2023 at 12:28:28PM +0300, Andrew Jones wrote: > >>>> On Thu, Aug 03, 2023 at 07:10:25PM -0700, Charlie Jenkins wrote: > >>>>> There are numerous systems in the kernel that rely on directly > >>>>> modifying, creating, and reading instructions. Many of these systems > >>>>> have rewritten code to do this. This patch will delegate all instruction > >>>>> handling into insn.h and reg.h. All of the compressed instructions, RVI, > >>>>> Zicsr, M, A instructions are included, as well as a subset of the F,D,Q > >>>>> extensions. > >>>>> > >>>>> --- > >>>>> This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ > >>>>> is also touching. > >>>>> > >>>>> --- > >>>>> Testing: > >>>>> > >>>>> There are a lot of subsystems touched and I have not tested every > >>>>> individual instruction. I did a lot of copy-pasting from the RISC-V spec > >>>>> so opcodes and such should be correct > >>>> > >>>> How about we create macros which generate each of the functions an > >>>> instruction needs, e.g. riscv_insn_is_*(), etc. based on the output of > >>>> [1]. I know basically nothing about that project, but it looks like it > >>>> creates most the defines this series is creating from what we [hope] to > >>>> be an authoritative source. I also assume that if we don't like the > >>>> current output format, then we could probably post patches to the project > >>>> to get the format we want. For example, we could maybe propose an "lc" > >>>> format for "Linux C". > >>> That's a great idea, I didn't realize that existed! > >> I have discovered that the riscv-opcodes repository is not in a state > >> that makes it helpful. If it were workable, it would make it easy to > >> include a "Linux C" format. I have had a pull request open on the repo > >> for two weeks now and the person who maintains the repo has not > >> interacted. > > > > Huh? Andrew has replied to you twice on your PR, and was the last one to > > comment. That’s hardly “has not interacted”. > > I should have been more clear because Andrew was very responsive. However, Neel Gala appears to be the "maintainer" in the sense that Andrew defers what gets merged into the repo to him. Neel has not provided any feedback, and he needs to comment before Andrew will merge anything in. > >> At minimum, in order for it to be useful it would need an ability to > >> describe the bit order of immediates in an instruction and include script > >> arguments to select which instructions should be included. There is a > >> "C" format, but it is actually just a Spike format. > > > > So extend it? Or do something with QEMU’s equivalent that expresses it. Yes, that is a possibility. To my knowledge GCC and the spec generator have moved away from using this repo. Is it still used by QEMU? > > Note that every field already identifies the bit order (or, for the > case of compressed instructions, register restrictions) since that’s > needed to produce the old LaTeX instruction set listings; that’s why > there’s jimm20 vs imm20, for example. One could surely encode that in > Python and generate the LaTeX strings from the Python, making the > details of the encodings available elsewhere. Or just have your own > mapping from name to whatever you need. But, either way, the > information should all be there today in the input files, it’s just a > matter of extending the script to produce whatever you want from them. All of the LaTeX bit orders were hardcoded in strings. As such, the bit order is described for the LaTeX format but not in general. It would not make sense to hardcode them a second time for the output of the Linux generation. You can see the strings by searching for 'latex_mapping' in the constants.py file. It seems to me that it will be significantly more challenging to use riscv-opcodes than it would for people to just hand create the macros that they need. - Charlie > > > Jess > > > >> Nonetheless, it > >> seems like it is prohibitive to use it. > >>>> > >>>> I'd also recommend only importing the generated defines and generating > >>>> the functions that will actually have immediate consumers or are part of > >>>> a set of defines that have immediate consumers. Each consumer of new > >>>> instructions will be responsible for generating and importing the defines > >>>> and adding the respective macro invocations to generate the functions. > >>>> This series can also take that approach, i.e. convert one set of > >>>> instructions at a time, each in a separate patch. > >>> Since I was hand-writing everything and copying it wasn't too much > >>> effort to just copy all of the instructions from a group. However, from > >>> a testing standpoint it makes sense to exclude instructions not yet in > >>> use. > >>>> > >>>> [1] https://github.com/riscv/riscv-opcodes > >>>> > >>>> Thanks, > >>>> drew > >>>> > >>>> > >>>>> , but the construction of every > >>>>> instruction is not fully tested. > >>>>> > >>>>> vector: Compiled and booted > >>>>> > >>>>> jump_label: Ensured static keys function as expected. > >>>>> > >>>>> kgdb: Attempted to run the provided tests but they failed even without > >>>>> my changes > >>>>> > >>>>> module: Loaded and unloaded modules > >>>>> > >>>>> patch.c: Ensured kernel booted > >>>>> > >>>>> kprobes: Used a kprobing module to probe jalr, auipc, and branch > >>>>> instructions > >>>>> > >>>>> nommu misaligned addresses: Kernel boots > >>>>> > >>>>> kvm: Ran KVM selftests > >>>>> > >>>>> bpf: Kernel boots. Most of the instructions are exclusively used by BPF > >>>>> but I am unsure of the best way of testing BPF. > >>>>> > >>>>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > >>>>> > >>>>> --- > >>>>> Charlie Jenkins (10): > >>>>> RISC-V: Expand instruction definitions > >>>>> RISC-V: vector: Refactor instructions > >>>>> RISC-V: Refactor jump label instructions > >>>>> RISC-V: KGDB: Refactor instructions > >>>>> RISC-V: module: Refactor instructions > >>>>> RISC-V: Refactor patch instructions > >>>>> RISC-V: nommu: Refactor instructions > >>>>> RISC-V: kvm: Refactor instructions > >>>>> RISC-V: bpf: Refactor instructions > >>>>> RISC-V: Refactor bug and traps instructions > >>>>> > >>>>> arch/riscv/include/asm/bug.h | 18 +- > >>>>> arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- > >>>>> arch/riscv/include/asm/reg.h | 88 + > >>>>> arch/riscv/kernel/jump_label.c | 13 +- > >>>>> arch/riscv/kernel/kgdb.c | 13 +- > >>>>> arch/riscv/kernel/module.c | 80 +- > >>>>> arch/riscv/kernel/patch.c | 3 +- > >>>>> arch/riscv/kernel/probes/kprobes.c | 13 +- > >>>>> arch/riscv/kernel/probes/simulate-insn.c | 100 +- > >>>>> arch/riscv/kernel/probes/uprobes.c | 5 +- > >>>>> arch/riscv/kernel/traps.c | 9 +- > >>>>> arch/riscv/kernel/traps_misaligned.c | 218 +-- > >>>>> arch/riscv/kernel/vector.c | 5 +- > >>>>> arch/riscv/kvm/vcpu_insn.c | 281 +-- > >>>>> arch/riscv/net/bpf_jit.h | 707 +------- > >>>>> 15 files changed, 2825 insertions(+), 1472 deletions(-) > >>>>> --- > >>>>> base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 > >>>>> change-id: 20230801-master-refactor-instructions-v4-433aa040da03 > >>>>> -- > >>>>> - Charlie > >>>>> > >>>>> > >>>>> -- > >>>>> kvm-riscv mailing list > >>>>> kvm-riscv@lists.infradead.org > >>>>> http://lists.infradead.org/mailman/listinfo/kvm-riscv > >> > >> _______________________________________________ > >> linux-riscv mailing list > >> linux-riscv@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-riscv > >
On Thu, 17 Aug 2023 09:43:16 PDT (-0700), Charlie Jenkins wrote: > On Thu, Aug 17, 2023 at 05:05:45AM +0100, Jessica Clarke wrote: >> On 17 Aug 2023, at 04:57, Jessica Clarke <jrtc27@jrtc27.com> wrote: >> > >> > On 17 Aug 2023, at 01:31, Charlie Jenkins <charlie@rivosinc.com> wrote: >> >> >> >> On Fri, Aug 04, 2023 at 10:24:33AM -0700, Charlie Jenkins wrote: >> >>> On Fri, Aug 04, 2023 at 12:28:28PM +0300, Andrew Jones wrote: >> >>>> On Thu, Aug 03, 2023 at 07:10:25PM -0700, Charlie Jenkins wrote: >> >>>>> There are numerous systems in the kernel that rely on directly >> >>>>> modifying, creating, and reading instructions. Many of these systems >> >>>>> have rewritten code to do this. This patch will delegate all instruction >> >>>>> handling into insn.h and reg.h. All of the compressed instructions, RVI, >> >>>>> Zicsr, M, A instructions are included, as well as a subset of the F,D,Q >> >>>>> extensions. >> >>>>> >> >>>>> --- >> >>>>> This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ >> >>>>> is also touching. >> >>>>> >> >>>>> --- >> >>>>> Testing: >> >>>>> >> >>>>> There are a lot of subsystems touched and I have not tested every >> >>>>> individual instruction. I did a lot of copy-pasting from the RISC-V spec >> >>>>> so opcodes and such should be correct >> >>>> >> >>>> How about we create macros which generate each of the functions an >> >>>> instruction needs, e.g. riscv_insn_is_*(), etc. based on the output of >> >>>> [1]. I know basically nothing about that project, but it looks like it >> >>>> creates most the defines this series is creating from what we [hope] to >> >>>> be an authoritative source. I also assume that if we don't like the >> >>>> current output format, then we could probably post patches to the project >> >>>> to get the format we want. For example, we could maybe propose an "lc" >> >>>> format for "Linux C". >> >>> That's a great idea, I didn't realize that existed! >> >> I have discovered that the riscv-opcodes repository is not in a state >> >> that makes it helpful. If it were workable, it would make it easy to >> >> include a "Linux C" format. I have had a pull request open on the repo >> >> for two weeks now and the person who maintains the repo has not >> >> interacted. >> > >> > Huh? Andrew has replied to you twice on your PR, and was the last one to >> > comment. That’s hardly “has not interacted”. >> > > I should have been more clear because Andrew was very responsive. > However, Neel Gala appears to be the "maintainer" in the sense that > Andrew defers what gets merged into the repo to him. Neel has not > provided any feedback, and he needs to comment before Andrew will merge > anything in. >> >> At minimum, in order for it to be useful it would need an ability to >> >> describe the bit order of immediates in an instruction and include script >> >> arguments to select which instructions should be included. There is a >> >> "C" format, but it is actually just a Spike format. >> > >> > So extend it? Or do something with QEMU’s equivalent that expresses it. > Yes, that is a possibility. To my knowledge GCC and the spec generator > have moved away from using this repo. Is it still used by QEMU? >> >> Note that every field already identifies the bit order (or, for the >> case of compressed instructions, register restrictions) since that’s >> needed to produce the old LaTeX instruction set listings; that’s why >> there’s jimm20 vs imm20, for example. One could surely encode that in >> Python and generate the LaTeX strings from the Python, making the >> details of the encodings available elsewhere. Or just have your own >> mapping from name to whatever you need. But, either way, the >> information should all be there today in the input files, it’s just a >> matter of extending the script to produce whatever you want from them. > All of the LaTeX bit orders were hardcoded in strings. As such, the bit > order is described for the LaTeX format but not in general. It would not > make sense to hardcode them a second time for the output of the Linux > generation. You can see the strings by searching for 'latex_mapping' in > the constants.py file. > > It seems to me that it will be significantly more challenging to use > riscv-opcodes than it would for people to just hand create the macros > that they need. Ya, riscv-opcodes is pretty custy. We stopped using it elsewhere ages ago. > - Charlie >> >> > Jess >> > >> >> Nonetheless, it >> >> seems like it is prohibitive to use it. >> >>>> >> >>>> I'd also recommend only importing the generated defines and generating >> >>>> the functions that will actually have immediate consumers or are part of >> >>>> a set of defines that have immediate consumers. Each consumer of new >> >>>> instructions will be responsible for generating and importing the defines >> >>>> and adding the respective macro invocations to generate the functions. >> >>>> This series can also take that approach, i.e. convert one set of >> >>>> instructions at a time, each in a separate patch. >> >>> Since I was hand-writing everything and copying it wasn't too much >> >>> effort to just copy all of the instructions from a group. However, from >> >>> a testing standpoint it makes sense to exclude instructions not yet in >> >>> use. >> >>>> >> >>>> [1] https://github.com/riscv/riscv-opcodes >> >>>> >> >>>> Thanks, >> >>>> drew >> >>>> >> >>>> >> >>>>> , but the construction of every >> >>>>> instruction is not fully tested. >> >>>>> >> >>>>> vector: Compiled and booted >> >>>>> >> >>>>> jump_label: Ensured static keys function as expected. >> >>>>> >> >>>>> kgdb: Attempted to run the provided tests but they failed even without >> >>>>> my changes >> >>>>> >> >>>>> module: Loaded and unloaded modules >> >>>>> >> >>>>> patch.c: Ensured kernel booted >> >>>>> >> >>>>> kprobes: Used a kprobing module to probe jalr, auipc, and branch >> >>>>> instructions >> >>>>> >> >>>>> nommu misaligned addresses: Kernel boots >> >>>>> >> >>>>> kvm: Ran KVM selftests >> >>>>> >> >>>>> bpf: Kernel boots. Most of the instructions are exclusively used by BPF >> >>>>> but I am unsure of the best way of testing BPF. >> >>>>> >> >>>>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> >> >>>>> >> >>>>> --- >> >>>>> Charlie Jenkins (10): >> >>>>> RISC-V: Expand instruction definitions >> >>>>> RISC-V: vector: Refactor instructions >> >>>>> RISC-V: Refactor jump label instructions >> >>>>> RISC-V: KGDB: Refactor instructions >> >>>>> RISC-V: module: Refactor instructions >> >>>>> RISC-V: Refactor patch instructions >> >>>>> RISC-V: nommu: Refactor instructions >> >>>>> RISC-V: kvm: Refactor instructions >> >>>>> RISC-V: bpf: Refactor instructions >> >>>>> RISC-V: Refactor bug and traps instructions >> >>>>> >> >>>>> arch/riscv/include/asm/bug.h | 18 +- >> >>>>> arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- >> >>>>> arch/riscv/include/asm/reg.h | 88 + >> >>>>> arch/riscv/kernel/jump_label.c | 13 +- >> >>>>> arch/riscv/kernel/kgdb.c | 13 +- >> >>>>> arch/riscv/kernel/module.c | 80 +- >> >>>>> arch/riscv/kernel/patch.c | 3 +- >> >>>>> arch/riscv/kernel/probes/kprobes.c | 13 +- >> >>>>> arch/riscv/kernel/probes/simulate-insn.c | 100 +- >> >>>>> arch/riscv/kernel/probes/uprobes.c | 5 +- >> >>>>> arch/riscv/kernel/traps.c | 9 +- >> >>>>> arch/riscv/kernel/traps_misaligned.c | 218 +-- >> >>>>> arch/riscv/kernel/vector.c | 5 +- >> >>>>> arch/riscv/kvm/vcpu_insn.c | 281 +-- >> >>>>> arch/riscv/net/bpf_jit.h | 707 +------- >> >>>>> 15 files changed, 2825 insertions(+), 1472 deletions(-) >> >>>>> --- >> >>>>> base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 >> >>>>> change-id: 20230801-master-refactor-instructions-v4-433aa040da03 >> >>>>> -- >> >>>>> - Charlie >> >>>>> >> >>>>> >> >>>>> -- >> >>>>> kvm-riscv mailing list >> >>>>> kvm-riscv@lists.infradead.org >> >>>>> http://lists.infradead.org/mailman/listinfo/kvm-riscv >> >> >> >> _______________________________________________ >> >> linux-riscv mailing list >> >> linux-riscv@lists.infradead.org >> >> http://lists.infradead.org/mailman/listinfo/linux-riscv >> >>
On Thu, Aug 17, 2023 at 10:52:22AM -0700, Palmer Dabbelt wrote: > On Thu, 17 Aug 2023 09:43:16 PDT (-0700), Charlie Jenkins wrote: ... > > It seems to me that it will be significantly more challenging to use > > riscv-opcodes than it would for people to just hand create the macros > > that they need. > > Ya, riscv-opcodes is pretty custy. We stopped using it elsewhere ages ago. Ah, pity I didn't know the history of it or I wouldn't have suggested it, wasting Charlie's time (sorry, Charlie!). So everywhere that needs encodings are manually scraping them from the PDFs? Or maybe we can write our own parser which converts adoc/wavedrom files[1] to Linux C? [1] https://github.com/riscv/riscv-isa-manual/tree/main/src/images/wavedrom Thanks, drew
On Fri, Aug 18, 2023 at 09:30:32AM +0200, Andrew Jones wrote: > On Thu, Aug 17, 2023 at 10:52:22AM -0700, Palmer Dabbelt wrote: > > On Thu, 17 Aug 2023 09:43:16 PDT (-0700), Charlie Jenkins wrote: > ... > > > It seems to me that it will be significantly more challenging to use > > > riscv-opcodes than it would for people to just hand create the macros > > > that they need. > > > > Ya, riscv-opcodes is pretty custy. We stopped using it elsewhere ages ago. > > Ah, pity I didn't know the history of it or I wouldn't have suggested it, > wasting Charlie's time (sorry, Charlie!). So everywhere that needs > encodings are manually scraping them from the PDFs? Or maybe we can write > our own parser which converts adoc/wavedrom files[1] to Linux C? > > [1] https://github.com/riscv/riscv-isa-manual/tree/main/src/images/wavedrom The problem with the wavedrom files is that there are no standard for how each instruction is identified. The title of of the adoc gives some insight and there is generally a funct3 or specific opcode that is associated with the instruction but it would be kind of messy to write a script to parse that. I think manually constructing the instructions is fine. When somebody wants to add a new instruction they probably will not need to add very many at a time, so it should be only a couple of lines that they will be able to test. > > Thanks, > drew
On Wed, Sep 06, 2023 at 11:51:05AM -0700, Charlie Jenkins wrote: > On Fri, Aug 18, 2023 at 09:30:32AM +0200, Andrew Jones wrote: > > On Thu, Aug 17, 2023 at 10:52:22AM -0700, Palmer Dabbelt wrote: > > > On Thu, 17 Aug 2023 09:43:16 PDT (-0700), Charlie Jenkins wrote: > > ... > > > > It seems to me that it will be significantly more challenging to use > > > > riscv-opcodes than it would for people to just hand create the macros > > > > that they need. > > > > > > Ya, riscv-opcodes is pretty custy. We stopped using it elsewhere ages ago. > > > > Ah, pity I didn't know the history of it or I wouldn't have suggested it, > > wasting Charlie's time (sorry, Charlie!). So everywhere that needs > > encodings are manually scraping them from the PDFs? Or maybe we can write > > our own parser which converts adoc/wavedrom files[1] to Linux C? > > > > [1] https://github.com/riscv/riscv-isa-manual/tree/main/src/images/wavedrom > > The problem with the wavedrom files is that there are no standard for > how each instruction is identified. The title of of the adoc gives some > insight and there is generally a funct3 or specific opcode that is > associated with the instruction but it would be kind of messy to write a > script to parse that. I think manually constructing the instructions is > fine. When somebody wants to add a new instruction they probably will > not need to add very many at a time, so it should be only a couple of > lines that they will be able to test. > OK, we'll just have to prop our eyelids open with toothpicks to get through the review of the initial mass conversion. Thanks, drew
There are numerous systems in the kernel that rely on directly modifying, creating, and reading instructions. Many of these systems have rewritten code to do this. This patch will delegate all instruction handling into insn.h and reg.h. All of the compressed instructions, RVI, Zicsr, M, A instructions are included, as well as a subset of the F,D,Q extensions. --- This is modifying code that https://lore.kernel.org/lkml/20230731183925.152145-1-namcaov@gmail.com/ is also touching. --- Testing: There are a lot of subsystems touched and I have not tested every individual instruction. I did a lot of copy-pasting from the RISC-V spec so opcodes and such should be correct, but the construction of every instruction is not fully tested. vector: Compiled and booted jump_label: Ensured static keys function as expected. kgdb: Attempted to run the provided tests but they failed even without my changes module: Loaded and unloaded modules patch.c: Ensured kernel booted kprobes: Used a kprobing module to probe jalr, auipc, and branch instructions nommu misaligned addresses: Kernel boots kvm: Ran KVM selftests bpf: Kernel boots. Most of the instructions are exclusively used by BPF but I am unsure of the best way of testing BPF. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- Charlie Jenkins (10): RISC-V: Expand instruction definitions RISC-V: vector: Refactor instructions RISC-V: Refactor jump label instructions RISC-V: KGDB: Refactor instructions RISC-V: module: Refactor instructions RISC-V: Refactor patch instructions RISC-V: nommu: Refactor instructions RISC-V: kvm: Refactor instructions RISC-V: bpf: Refactor instructions RISC-V: Refactor bug and traps instructions arch/riscv/include/asm/bug.h | 18 +- arch/riscv/include/asm/insn.h | 2744 +++++++++++++++++++++++++++--- arch/riscv/include/asm/reg.h | 88 + arch/riscv/kernel/jump_label.c | 13 +- arch/riscv/kernel/kgdb.c | 13 +- arch/riscv/kernel/module.c | 80 +- arch/riscv/kernel/patch.c | 3 +- arch/riscv/kernel/probes/kprobes.c | 13 +- arch/riscv/kernel/probes/simulate-insn.c | 100 +- arch/riscv/kernel/probes/uprobes.c | 5 +- arch/riscv/kernel/traps.c | 9 +- arch/riscv/kernel/traps_misaligned.c | 218 +-- arch/riscv/kernel/vector.c | 5 +- arch/riscv/kvm/vcpu_insn.c | 281 +-- arch/riscv/net/bpf_jit.h | 707 +------- 15 files changed, 2825 insertions(+), 1472 deletions(-) --- base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 change-id: 20230801-master-refactor-instructions-v4-433aa040da03