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

["Jiaxun Yang" <jiaxun.yang@flygoat.com>]

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

Thanks
- Jiaxun

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

-- 
- Jiaxun