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

[maobibo]

On 2024/7/3 下午5:43, Huacai Chen wrote:
On Wed, Jul 3, 2024 at 3:51 PM Jiaxun Yang <jiaxun.y...@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 <maob...@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.
To understand features immediately is difficult job for me. There is 
supported features/used features usages etc, overall feature detection 
should be VM relative by my knowledge.

Maybe after host machine type and migration feature detection and 
checking is finished, there will be further upstanding -:(

Regards
Bibo Mao


Huacai


Thanks
- Jiaxun


Thanks
- Jiaxun

--
- Jiaxun

--
- Jiaxun