Message ID | 20230515021522.2445551-1-zhaotianrui@loongson.cn (mailing list archive) |
---|---|
Headers | show |
Series | Add KVM LoongArch support | expand |
Hi Paolo & Huacai, Sorry to bother you, I do not know flow of kernel code reviewing and merging. I want to know who should give a reviewed-by comments for these piece of code about loongarch kvm patch. It should be kvm maintainer or LoongArch maintianer? And any suggestion is welcome. Regards Bibo, Mao 在 2023/5/15 10:14, Tianrui Zhao 写道: > This series adds KVM LoongArch support. Loongson 3A5000 supports hardware > assisted virtualization. With cpu virtualization, there are separate > hw-supported user mode and kernel mode in guest mode. With memory > virtualization, there are two-level hw mmu table for guest mode and host > mode. Also there is separate hw cpu timer with consant frequency in > guest mode, so that vm can migrate between hosts with different freq. > Currently, we are able to boot LoongArch Linux Guests. > > Few key aspects of KVM LoongArch added by this series are: > 1. Enable kvm hardware function when kvm module is loaded. > 2. Implement VM and vcpu related ioctl interface such as vcpu create, > vcpu run etc. GET_ONE_REG/SET_ONE_REG ioctl commands are use to > get general registers one by one. > 3. Hardware access about MMU, timer and csr are emulated in kernel. > 4. Hardwares such as mmio and iocsr device are emulated in user space > such as APIC, IPI, pci devices etc. > > The running environment of LoongArch virt machine: > 1. Cross tools to build kernel and uefi: > $ wget https://github.com/loongson/build-tools/releases/download/2022.09.06/loongarch64-clfs-6.3-cross-tools-gcc-glibc.tar.xz > tar -vxf loongarch64-clfs-6.3-cross-tools-gcc-glibc.tar.xz -C /opt > export PATH=/opt/cross-tools/bin:$PATH > export LD_LIBRARY_PATH=/opt/cross-tools/lib:$LD_LIBRARY_PATH > export LD_LIBRARY_PATH=/opt/cross-tools/loongarch64-unknown-linux-gnu/lib/:$LD_LIBRARY_PATH > 2. This series is based on the linux source code: > https://github.com/loongson/linux-loongarch-kvm > Build command: > git checkout kvm-loongarch > make ARCH=loongarch CROSS_COMPILE=loongarch64-unknown-linux-gnu- loongson3_defconfig > make ARCH=loongarch CROSS_COMPILE=loongarch64-unknown-linux-gnu- > 3. QEMU hypervisor with LoongArch supported: > https://github.com/loongson/qemu > Build command: > git checkout kvm-loongarch > ./configure --target-list="loongarch64-softmmu" --enable-kvm > make > 4. Uefi bios of LoongArch virt machine: > Link: https://github.com/tianocore/edk2-platforms/tree/master/Platform/Loongson/LoongArchQemuPkg#readme > 5. you can also access the binary files we have already build: > https://github.com/yangxiaojuan-loongson/qemu-binary > The command to boot loongarch virt machine: > $ qemu-system-loongarch64 -machine virt -m 4G -cpu la464 \ > -smp 1 -bios QEMU_EFI.fd -kernel vmlinuz.efi -initrd ramdisk \ > -serial stdio -monitor telnet:localhost:4495,server,nowait \ > -append "root=/dev/ram rdinit=/sbin/init console=ttyS0,115200" \ > --nographic > > Changes for v10: > 1. Fix grammatical problems in LoongArch documentation. > 2. It is not necessary to save or restore the LOONGARCH_CSR_PGD when > vcpu put and vcpu load, so we remove it. > > Changes for v9: > 1. Apply the new defined interrupt number macros in loongarch.h to kvm, > such as INT_SWI0, INT_HWI0, INT_TI, INT_IPI, etc. And remove the > previous unused macros. > 2. Remove unused variables in kvm_vcpu_arch, and reorder the variables > to make them more standard. > > Changes for v8: > 1. Adjust the cpu_data.guest.options structure, add the ases flag into > it, and remove the previous guest.ases. We do this to keep consistent > with host cpu_data.options structure. > 2. Remove the "#include <asm/kvm_host.h>" in some files which also > include the "<linux/kvm_host.h>". As linux/kvm_host.h already include > the asm/kvm_host.h. > 3. Fix some unstandard spelling and grammar errors in comments, and > improve a little code format to make it easier and standard. > > Changes for v7: > 1. Fix the kvm_save/restore_hw_gcsr compiling warnings reported by > kernel test robot. The report link is: > https://lore.kernel.org/oe-kbuild-all/202304131526.iXfLaVZc-lkp@intel.com/ > 2. Fix loongarch kvm trace related compiling problems. > > Changes for v6: > 1. Fix the Documentation/virt/kvm/api.rst compile warning about > loongarch parts. > > Changes for v5: > 1. Implement get/set mp_state ioctl interface, and only the > KVM_MP_STATE_RUNNABLE state is supported now, and other states > will be completed in the future. The state is also used when vcpu > run idle instruction, if vcpu state is changed to RUNNABLE, the > vcpu will have the possibility to be woken up. > 2. Supplement kvm document about loongarch-specific part, such as add > api introduction for GET/SET_ONE_REG, GET/SET_FPU, GET/SET_MP_STATE, > etc. > 3. Improve the kvm_switch_to_guest function in switch.S, remove the > previous tmp,tmp1 arguments and replace it with t0,t1 reg. > > Changes for v4: > 1. Add a csr_need_update flag in _vcpu_put, as most csr registers keep > unchanged during process context switch, so we need not to update it > every time. We can do this only if the soft csr is different form hardware. > That is to say all of csrs should update after vcpu enter guest, as for > set_csr_ioctl, we have written soft csr to keep consistent with hardware. > 2. Improve get/set_csr_ioctl interface, we set SW or HW or INVALID flag > for all csrs according to it's features when kvm init. In get/set_csr_ioctl, > if csr is HW, we use gcsrrd/ gcsrwr instruction to access it, else if csr is > SW, we use software to emulate it, and others return false. > 3. Add set_hw_gcsr function in csr_ops.S, and it is used in set_csr_ioctl. > We have splited hw gcsr into three parts, so we can calculate the code offset > by gcsrid and jump here to run the gcsrwr instruction. We use this function to > make the code easier and avoid to use the previous SET_HW_GCSR(XXX) interface. > 4. Improve kvm mmu functions, such as flush page table and make clean page table > interface. > > Changes for v3: > 1. Remove the vpid array list in kvm_vcpu_arch and use a vpid variable here, > because a vpid will never be recycled if a vCPU migrates from physical CPU A > to B and back to A. > 2. Make some constant variables in kvm_context to global such as vpid_mask, > guest_eentry, enter_guest, etc. > 3. Add some new tracepoints, such as kvm_trace_idle, kvm_trace_cache, > kvm_trace_gspr, etc. > 4. There are some duplicate codes in kvm_handle_exit and kvm_vcpu_run, > so we move it to a new function kvm_pre_enter_guest. > 5. Change the RESUME_HOST, RESUME_GUEST value, return 1 for resume guest > and "<= 0" for resume host. > 6. Fcsr and fpu registers are saved/restored together. > > Changes for v2: > 1. Seprate the original patch-01 and patch-03 into small patches, and the > patches mainly contain kvm module init, module exit, vcpu create, vcpu run, > etc. > 2. Remove the original KVM_{GET,SET}_CSRS ioctl in the kvm uapi header, > and we use the common KVM_{GET,SET}_ONE_REG to access register. > 3. Use BIT(x) to replace the "1 << n_bits" statement. > > Tianrui Zhao (30): > LoongArch: KVM: Add kvm related header files > LoongArch: KVM: Implement kvm module related interface > LoongArch: KVM: Implement kvm hardware enable, disable interface > LoongArch: KVM: Implement VM related functions > LoongArch: KVM: Add vcpu related header files > LoongArch: KVM: Implement vcpu create and destroy interface > LoongArch: KVM: Implement vcpu run interface > LoongArch: KVM: Implement vcpu handle exit interface > LoongArch: KVM: Implement vcpu get, vcpu set registers > LoongArch: KVM: Implement vcpu ENABLE_CAP ioctl interface > LoongArch: KVM: Implement fpu related operations for vcpu > LoongArch: KVM: Implement vcpu interrupt operations > LoongArch: KVM: Implement misc vcpu related interfaces > LoongArch: KVM: Implement vcpu load and vcpu put operations > LoongArch: KVM: Implement vcpu status description > LoongArch: KVM: Implement update VM id function > LoongArch: KVM: Implement virtual machine tlb operations > LoongArch: KVM: Implement vcpu timer operations > LoongArch: KVM: Implement kvm mmu operations > LoongArch: KVM: Implement handle csr excption > LoongArch: KVM: Implement handle iocsr exception > LoongArch: KVM: Implement handle idle exception > LoongArch: KVM: Implement handle gspr exception > LoongArch: KVM: Implement handle mmio exception > LoongArch: KVM: Implement handle fpu exception > LoongArch: KVM: Implement kvm exception vector > LoongArch: KVM: Implement vcpu world switch > LoongArch: KVM: Implement probe virtualization when LoongArch cpu init > LoongArch: KVM: Enable kvm config and add the makefile > LoongArch: KVM: Supplement kvm document about LoongArch-specific part > > Documentation/virt/kvm/api.rst | 71 +- > arch/loongarch/Kbuild | 1 + > arch/loongarch/Kconfig | 2 + > arch/loongarch/configs/loongson3_defconfig | 2 + > arch/loongarch/include/asm/cpu-features.h | 22 + > arch/loongarch/include/asm/cpu-info.h | 10 + > arch/loongarch/include/asm/inst.h | 16 + > arch/loongarch/include/asm/kvm_csr.h | 57 ++ > arch/loongarch/include/asm/kvm_host.h | 252 ++++++ > arch/loongarch/include/asm/kvm_types.h | 11 + > arch/loongarch/include/asm/kvm_vcpu.h | 97 +++ > arch/loongarch/include/asm/loongarch.h | 211 ++++- > arch/loongarch/include/uapi/asm/kvm.h | 107 +++ > arch/loongarch/kernel/asm-offsets.c | 32 + > arch/loongarch/kernel/cpu-probe.c | 53 ++ > arch/loongarch/kvm/Kconfig | 38 + > arch/loongarch/kvm/Makefile | 22 + > arch/loongarch/kvm/csr_ops.S | 76 ++ > arch/loongarch/kvm/exit.c | 707 +++++++++++++++++ > arch/loongarch/kvm/interrupt.c | 126 +++ > arch/loongarch/kvm/main.c | 339 ++++++++ > arch/loongarch/kvm/mmu.c | 729 +++++++++++++++++ > arch/loongarch/kvm/switch.S | 301 +++++++ > arch/loongarch/kvm/timer.c | 266 +++++++ > arch/loongarch/kvm/tlb.c | 31 + > arch/loongarch/kvm/trace.h | 168 ++++ > arch/loongarch/kvm/vcpu.c | 880 +++++++++++++++++++++ > arch/loongarch/kvm/vm.c | 78 ++ > arch/loongarch/kvm/vmid.c | 64 ++ > include/uapi/linux/kvm.h | 9 + > 30 files changed, 4764 insertions(+), 14 deletions(-) > create mode 100644 arch/loongarch/include/asm/kvm_csr.h > create mode 100644 arch/loongarch/include/asm/kvm_host.h > create mode 100644 arch/loongarch/include/asm/kvm_types.h > create mode 100644 arch/loongarch/include/asm/kvm_vcpu.h > create mode 100644 arch/loongarch/include/uapi/asm/kvm.h > create mode 100644 arch/loongarch/kvm/Kconfig > create mode 100644 arch/loongarch/kvm/Makefile > create mode 100644 arch/loongarch/kvm/csr_ops.S > create mode 100644 arch/loongarch/kvm/exit.c > create mode 100644 arch/loongarch/kvm/interrupt.c > create mode 100644 arch/loongarch/kvm/main.c > create mode 100644 arch/loongarch/kvm/mmu.c > create mode 100644 arch/loongarch/kvm/switch.S > create mode 100644 arch/loongarch/kvm/timer.c > create mode 100644 arch/loongarch/kvm/tlb.c > create mode 100644 arch/loongarch/kvm/trace.h > create mode 100644 arch/loongarch/kvm/vcpu.c > create mode 100644 arch/loongarch/kvm/vm.c > create mode 100644 arch/loongarch/kvm/vmid.c >
On 2023/5/18 10:56, maobibo wrote: > Hi Paolo & Huacai, > > Sorry to bother you, I do not know flow of kernel code reviewing and merging. > > I want to know who should give a reviewed-by comments for these piece of code > about loongarch kvm patch. It should be kvm maintainer or LoongArch maintianer? > And any suggestion is welcome. IMO the series should get its R-b from kvm maintainers (because it's kvm after all), and ideally also Acked-by from arch/loongarch maintainers (because it contains arch-specific code), according to common sense. But in order for the various maintainers/reviewers to effectively review, maybe the LoongArch ISA manual Volume 3 (containing details about the virtualization extension) should be put out soon. AFAIK Huacai has access to it, by being a Loongson employee, but I don't know if he can review this series in the public without violating NDAs; Loongson outsiders like me and the kvm reviewers can only trust the commit messages and comments for the time being. (BTW, how do people usually deal with pre-release hardware wit documentation not out yet? I suppose similar situations like this should turn up fairly often.) Aside from this, there's another point: use of undocumented instructions in raw form with ".word". This currently doesn't work in LLVM/Clang, thus will slightly set back the ongoing ClangBuiltLinux enablement effort (currently all such usages in arch/loongarch are related to "invtlb" which has perfect support, and can be removed). AFAIK, such practice dates back to the LoongISA times, when the Loongson extended opcodes weren't supported by the upstream MIPS toolchains for some reason; but LoongArch is independent and not bounded by anyone else now, so it's better in terms of maintainability to just add the instructions to the toolchains. People will not be inconvenienced by having to use bleeding-edge LoongArch toolchains because upstream LoongArch devs have always been doing this.
On Sun, 2023-05-21 at 18:22 +0800, WANG Xuerui wrote: /* snip */ > (BTW, how do people usually deal with pre-release hardware wit > documentation not out yet? I suppose similar situations like this should > turn up fairly often.) Intel normally releases the doc much earlier than shipping the hardware to customers. For example, the x86 Linear Address Masking doc has been released in 2020 (allowing Linus himself to hack the LAM code in kernel :), but AFAIK there is no Intel CPU models released with LAM support yet (at least my Raptor Lake does not indicate LAM in cpuid, or maybe I'm missing the latest server models?) For other vendors I'm not sure. > Aside from this, there's another point: use of undocumented instructions > in raw form with ".word". This currently doesn't work in LLVM/Clang, Hmm, is it an inherent limitation of Clang or it's simply not implemented for LoongArch yet? On x86_64 I tried ".byte 0x90" in the inline assembly and Clang correctly emits a nop instruction. > thus will slightly set back the ongoing ClangBuiltLinux enablement > effort (currently all such usages in arch/loongarch are related to > "invtlb" which has perfect support, and can be removed). AFAIK, such > practice dates back to the LoongISA times, when the Loongson extended > opcodes weren't supported by the upstream MIPS toolchains for some > reason; but LoongArch is independent and not bounded by anyone else now, > so it's better in terms of maintainability to just add the instructions > to the toolchains. People will not be inconvenienced by having to use > bleeding-edge LoongArch toolchains because upstream LoongArch devs have > always been doing this. Or it may be resolved by some fancy #ifdef directives.
在 2023/5/21 18:22, WANG Xuerui 写道: > On 2023/5/18 10:56, maobibo wrote: >> Hi Paolo & Huacai, >> >> Sorry to bother you, I do not know flow of kernel code reviewing and merging. >> >> I want to know who should give a reviewed-by comments for these piece of code >> about loongarch kvm patch. It should be kvm maintainer or LoongArch maintianer? >> And any suggestion is welcome. > > IMO the series should get its R-b from kvm maintainers (because it's kvm after all), and ideally also Acked-by from arch/loongarch maintainers (because it contains arch-specific code), according to common sense. > > But in order for the various maintainers/reviewers to effectively review, maybe the LoongArch ISA manual Volume 3 (containing details about the virtualization extension) should be put out soon. AFAIK Huacai has access to it, by being a Loongson employee, but I don't know if he can review this series in the public without violating NDAs; Loongson outsiders like me and the kvm reviewers can only trust the commit messages and comments for the time being. Yes, it will be best if kvm maintainers can give reviewed-by comments, since they understand kvm component at best and know the future evolution directions. > > (BTW, how do people usually deal with pre-release hardware wit documentation not out yet? I suppose similar situations like this should turn up fairly often.) Manual is actually one issue, however it does not prevent the review process. There are some drivers for *fruit* devices, I can not find the hw manual also. With the manual, it helps to review and points out the further and detailed issues. > > Aside from this, there's another point: use of undocumented instructions in raw form with ".word". This currently doesn't work in LLVM/Clang, thus will slightly set back the ongoing ClangBuiltLinux enablement effort (currently all such usages in arch/loongarch are related to "invtlb" which has perfect support, and can be removed). AFAIK, such practice dates back to the LoongISA times, when the Loongson extended opcodes weren't supported by the upstream MIPS toolchains for some reason; but LoongArch is independent and not bounded by anyone else now, so it's better in terms of maintainability to just add the instructions to the toolchains. People will not be inconvenienced by having to use bleeding-edge LoongArch toolchains because upstream LoongArch devs have always been doing this. As for one new architecture, it is normal to use .word or .insn, instruction will update for the first few years and also compiler may be not supported timely. The other arch has the same phenomenon if you grep "\.insn", also llvm on LoongArch supports ".word" directives. After three or five years, we will remove these ".insn" macro when hw and compiler is matured. Regards Bibo, Mao >
On 2023/5/22 09:39, maobibo wrote: > > > 在 2023/5/21 18:22, WANG Xuerui 写道: >> On 2023/5/18 10:56, maobibo wrote: >>> <snip> >> (BTW, how do people usually deal with pre-release hardware wit documentation not out yet? I suppose similar situations like this should turn up fairly often.) > Manual is actually one issue, however it does not prevent the review > process. There are some drivers for *fruit* devices, I can not find > the hw manual also. With the manual, it helps to review and points > out the further and detailed issues. There's a *slight* difference: the certain vendor you've mentioned is historically uncooperative in providing the documentation, so outside contributors had to reverse-engineer and document the HW themselves; but in Loongson's case, you *are* the vendor, so you are probably in a position that can make everyone's life easier by at least pushing for the docs release... >> >> Aside from this, there's another point: use of undocumented instructions in raw form with ".word". This currently doesn't work in LLVM/Clang <snip> > As for one new architecture, it is normal to use .word or .insn, instruction > will update for the first few years and also compiler may be not supported > timely. The other arch has the same phenomenon if you grep "\.insn", also > llvm on LoongArch supports ".word" directives. > > After three or five years, we will remove these ".insn" macro when hw and > compiler is matured. Sorry for the confusion at my side; `.word` certainly works, what doesn't work currently seems to be the `parse_r` helper. I know because I've tried in the last week with latest LLVM/Clang snapshot. And you can't write ergonomic inline asm with proper register allocator awareness without the helper; the LoongArch assembler isn't capable of assembling in a certain encoding format. With RISC-V `.insn` you can do things like `.insn r 0xNN, 0, 0, a0, a1, a2`, but you cannot simply e.g. express gcsrxchg with `.insn DJK 0x05000000, a0, a1, a2` because no such instruction format convention has been standardized. (The notation demonstrated here is taken from [1].) In any case, it seems best to at least wait for the documentation release a little bit, or you should state clearly that this is not going to happen soon, so people can properly manage their expectation and prioritize. (For example, if I know docs and/or assembler support for the virtualization extension won't come soon, then I'd work on supporting the .word idiom before other things. Otherwise there are more important things than that.) [1]: https://github.com/loongson/LoongArch-Documentation/pull/56
On Mon, 2023-05-22 at 10:37 +0800, WANG Xuerui wrote: > > Manual is actually one issue, however it does not prevent the review > > process. There are some drivers for *fruit* devices, I can not find > > the hw manual also. With the manual, it helps to review and points > > out the further and detailed issues. > > There's a *slight* difference: the certain vendor you've mentioned is > historically uncooperative in providing the documentation, so outside > contributors had to reverse-engineer and document the HW themselves; but > in Loongson's case, you *are* the vendor, so you are probably in a > position that can make everyone's life easier by at least pushing for > the docs release... And some other differences: 1. *fruit* devices use an ARM64 which is already documented anyway. 2. *fruit* devices are more "popular" and the maintainer can test on it more easily. And *fruit* have more fans (esp. more fans with CS/EE skills, while we have more [expletive deleted] fans :( ) so their reverse-engineer effort is easier as well. Yes it may be not fair to others, but the world is just not fair. 3. *fruit* is really a bad example... When people ask me how to build Linux From Scratch on *fruit* I just tell them "try to avoid things from a vendor with no intention to allow you run another OS".
在 2023/5/22 10:37, WANG Xuerui 写道: > On 2023/5/22 09:39, maobibo wrote: >> >> >> 在 2023/5/21 18:22, WANG Xuerui 写道: >>> On 2023/5/18 10:56, maobibo wrote: >>>> <snip> >>> (BTW, how do people usually deal with pre-release hardware wit documentation not out yet? I suppose similar situations like this should turn up fairly often.) >> Manual is actually one issue, however it does not prevent the review >> process. There are some drivers for *fruit* devices, I can not find >> the hw manual also. With the manual, it helps to review and points >> out the further and detailed issues. > > There's a *slight* difference: the certain vendor you've mentioned is historically uncooperative in providing the documentation, so outside contributors had to reverse-engineer and document the HW themselves; but in Loongson's case, you *are* the vendor, so you are probably in a position that can make everyone's life easier by at least pushing for the docs release... > >>> >>> Aside from this, there's another point: use of undocumented instructions in raw form with ".word". This currently doesn't work in LLVM/Clang <snip> >> As for one new architecture, it is normal to use .word or .insn, instruction >> will update for the first few years and also compiler may be not supported >> timely. The other arch has the same phenomenon if you grep "\.insn", also >> llvm on LoongArch supports ".word" directives. >> >> After three or five years, we will remove these ".insn" macro when hw and >> compiler is matured. > > Sorry for the confusion at my side; `.word` certainly works, what doesn't work currently seems to be the `parse_r` helper. I know because I've tried in the last week with latest LLVM/Clang snapshot. And you can't write ergonomic inline asm with proper register allocator awareness without the helper; the LoongArch assembler isn't capable of assembling in a certain encoding format. With RISC-V `.insn` you can do things like `.insn r 0xNN, 0, 0, a0, a1, a2`, but you cannot simply e.g. express gcsrxchg with `.insn DJK 0x05000000, a0, a1, a2` because no such instruction format convention has been standardized. (The notation demonstrated here is taken from [1].) I hate parse_r helper also, it is hard to understand, the kernel about LoongArch has the same issue. How about using a fixed register like this? /* GCSR */ static __always_inline u64 gcsr_read(u32 reg) { u64 val = 0; BUILD_BUG_ON(!__builtin_constant_p(reg)); /* Instructions will be available in binutils later */ asm volatile ( "parse_r __reg, %[val]\n\t" /* * read val from guest csr register %[reg] * gcsrrd %[val], %[reg] */ ".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | __reg\n\t" : [val] "+r" (val) : [reg] "i" (reg) : "memory"); return val; } /* GCSR */ static __always_inline u64 gcsr_read(u32 reg) { register unsigned long val asm("t0"); BUILD_BUG_ON(!__builtin_constant_p(reg)); /* Instructions will be available in binutils later */ asm volatile ( "parse_r __reg, %[val]\n\t" /* * read val from guest csr register %[reg] * gcsrrd %[val], %[reg] */ ".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | 12 \n\t" : : [reg] "i" (reg) : "memory", "t0"); return val; } Regards Bibo, Mao > > In any case, it seems best to at least wait for the documentation release a little bit, or you should state clearly that this is not going to happen soon, so people can properly manage their expectation and prioritize. (For example, if I know docs and/or assembler support for the virtualization extension won't come soon, then I'd work on supporting the .word idiom before other things. Otherwise there are more important things than that.) > > [1]: https://github.com/loongson/LoongArch-Documentation/pull/56 >
On 2023/5/23 10:54, maobibo wrote: > > [snip] > > I hate parse_r helper also, it is hard to understand, the kernel about > LoongArch has the same issue. How about using a fixed register like this? > > /* GCSR */ > static __always_inline u64 gcsr_read(u32 reg) > { > u64 val = 0; > > BUILD_BUG_ON(!__builtin_constant_p(reg)); > /* Instructions will be available in binutils later */ > asm volatile ( > "parse_r __reg, %[val]\n\t" Isn't this still parse_r-ing things? ;-) > /* > * read val from guest csr register %[reg] > * gcsrrd %[val], %[reg] > */ > ".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | __reg\n\t" > : [val] "+r" (val) > : [reg] "i" (reg) > : "memory"); > > return val; > } > > /* GCSR */ > static __always_inline u64 gcsr_read(u32 reg) > { > register unsigned long val asm("t0"); I got what you're trying to accomplish here. At which point you may just refer to how glibc implements its inline syscall templates and hardcode both the input and output arguments, then simply hard-code the whole instruction word. If others don't have opinions about doing things this way, I wouldn't either. (CSR operations are not expected to become performance-sensitive in any case, so you may freely choose registers here, and t0 for output seems okay. I'd recommend stuffing "reg" to a temporary value bound to a0 though.) > > BUILD_BUG_ON(!__builtin_constant_p(reg)); > /* Instructions will be available in binutils later */ > asm volatile ( > "parse_r __reg, %[val]\n\t" > /* > * read val from guest csr register %[reg] > * gcsrrd %[val], %[reg] > */ > ".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | 12 \n\t" > : : [reg] "i" (reg) > : "memory", "t0"); > > return val; > }
在 2023/5/23 18:27, WANG Xuerui 写道: > On 2023/5/23 10:54, maobibo wrote: >> >> [snip] >> >> I hate parse_r helper also, it is hard to understand, the kernel about >> LoongArch has the same issue. How about using a fixed register like this? >> >> /* GCSR */ >> static __always_inline u64 gcsr_read(u32 reg) >> { >> u64 val = 0; >> >> BUILD_BUG_ON(!__builtin_constant_p(reg)); >> /* Instructions will be available in binutils later */ >> asm volatile ( >> "parse_r __reg, %[val]\n\t" > > Isn't this still parse_r-ing things? ;-) > >> /* >> * read val from guest csr register %[reg] >> * gcsrrd %[val], %[reg] >> */ >> ".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | __reg\n\t" >> : [val] "+r" (val) >> : [reg] "i" (reg) >> : "memory"); >> >> return val; >> } >> >> /* GCSR */ >> static __always_inline u64 gcsr_read(u32 reg) >> { >> register unsigned long val asm("t0"); > > I got what you're trying to accomplish here. At which point you may just refer to how glibc implements its inline syscall templates and hardcode both the input and output arguments, then simply hard-code the whole instruction word. If others don't have opinions about doing things this way, I wouldn't either. > > (CSR operations are not expected to become performance-sensitive in any case, so you may freely choose registers here, and t0 for output seems okay. I'd recommend stuffing "reg" to a temporary value bound to a0 though.) a0 is ok for me. riscv has better method than both parse_r helper and using tmp register as follows, maybe we can use the similiar method. .macro insn_r, opcode, func3, func7, rd, rs1, rs2 .4byte ((\opcode << INSN_R_OPCODE_SHIFT) | \ (\func3 << INSN_R_FUNC3_SHIFT) | \ (\func7 << INSN_R_FUNC7_SHIFT) | \ (.L__gpr_num_\rd << INSN_R_RD_SHIFT) | \ (.L__gpr_num_\rs1 << INSN_R_RS1_SHIFT) | \ (.L__gpr_num_\rs2 << INSN_R_RS2_SHIFT)) .endm #define HINVAL_VVMA(vaddr, asid) \ INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(19), \ __RD(0), RS1(vaddr), RS2(asid)) asm volatile(HINVAL_VVMA(%0, %1) : : "r" (pos), "r" (asid) : "memory"); Regards Bibo, Mao > >> >> BUILD_BUG_ON(!__builtin_constant_p(reg)); >> /* Instructions will be available in binutils later */ >> asm volatile ( >> "parse_r __reg, %[val]\n\t" >> /* >> * read val from guest csr register %[reg] >> * gcsrrd %[val], %[reg] >> */ >> ".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | 12 \n\t" >> : : [reg] "i" (reg) >> : "memory", "t0"); >> >> return val; >> } >