Re: [RFC v3 1/2] target/loongarch: Add loongson binary translation feature

[Huacai Chen]

On Wed, Jul 3, 2024 at 3:51 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 在2024年7月1日七月 下午2:57，Jiaxun Yang写道：
> > 在2024年5月30日五月 上午7:49，Bibo Mao写道：
> >> Loongson Binary Translation (LBT) is used to accelerate binary
> >> translation, which contains 4 scratch registers (scr0 to scr3), x86/ARM
> >> eflags (eflags) and x87 fpu stack pointer (ftop).
> >>
> >> Now LBT feature is added in kvm mode, not supported in TCG mode since
> >> it is not emulated. Feature variable lbt is added with OnOffAuto type,
> >> If lbt feature is not supported with KVM host, it reports error if there
> >> is lbt=on command line.
> >>
> >> If there is no any command line about lbt parameter, it checks whether
> >> KVM host supports lbt feature and set the corresponding value in cpucfg.
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> > Hi Bibo,
> >
> > I was going across recent LoongArch changes and this comes into my 
> > attention:
> >
> >> ---
> >>  target/loongarch/cpu.c                | 53 +++++++++++++++++++++++++++
> >>  target/loongarch/cpu.h                |  6 +++
> >>  target/loongarch/kvm/kvm.c            | 26 +++++++++++++
> >>  target/loongarch/kvm/kvm_loongarch.h  | 16 ++++++++
> >>  target/loongarch/loongarch-qmp-cmds.c |  2 +-
> >>  5 files changed, 102 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> >> index b5c1ec94af..14265b6667 100644
> >> --- a/target/loongarch/cpu.c
> >> +++ b/target/loongarch/cpu.c
> >> @@ -571,6 +571,30 @@ static void loongarch_cpu_disas_set_info(CPUState
> >> *s, disassemble_info *info)
> >>      info->print_insn = print_insn_loongarch;
> >>  }
> >>
> >> +static void loongarch_cpu_check_lbt(CPUState *cs, Error **errp)
> >> +{
> >> +    CPULoongArchState *env = cpu_env(cs);
> >> +    LoongArchCPU *cpu = LOONGARCH_CPU(cs);
> >> +    bool kvm_supported;
> >> +
> >> +    kvm_supported = kvm_feature_supported(cs, LOONGARCH_FEATURE_LBT);
> >
> > IMHO if there is no global states that should be saved/restored VM wise,
> > this should be handled at per CPU level, preferably with CPUCFG flags hint.
> >
> > We should minimize non-privilege KVM feature bits to prevent hindering
> > asymmetry ISA system.
>
> + Huacai for further discussion
>
> Hi Bibo, Huacai,
>
> I investigated the topic further and went through the thread on kernel side.
>
> I think Huacai and me are all on the same page that we should unify the 
> interface for per-CPU
> level feature probing and setting interface. Huacai purposed converting all 
> features to VM feature
> but I still believe CPUCFG is the best interface.
>
> To probe LBT before actual vcpu creation, we can borrow the approach used by 
> other architectures
> (kvm_arm_create_scratch_host_vcpu() & kvm_riscv_create_scratch_vcpu()).
>
> Kernel will reject setting unknown CPUCFG bits with -EINVAL, so to probe LBT 
> we just need to perform
> KVM_SET_REGS to scratch vcpu with LBT set to see if it's valid for kernel. 
> There is no need for any other
> probing interface.
>
> I do think scratch CPU interface is also necessary if we are going to 
> implement cpu = host.
>
> Huacai, would you agree with me?
For me the important thing is consistency, all vm-features or all
vcpu-features are both accepted.

Huacai

>
> Thanks
> - Jiaxun
>
> >
> > Thanks
> > - Jiaxun
> >
> > --
> > - Jiaxun
>
> --
> - Jiaxun