Re: [PATCH v10 00/30] Add KVM LoongArch support

From: maobibo
Date: Sun May 21 2023 - 21:39:32 EST




在 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
>