Hi Philippe,

Thanks for reviewing my patch.
I reply inline.

On 2024/5/27 下午6:37, Philippe Mathieu-Daudé wrote:
Hi Bibo,

On 27/5/24 10:35, Bibo Mao wrote:
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. There are two feature flags such as forced_features
and default_features for each vcpu, the real feature is still in cpucfg.
Flag forced_features is parsed from command line, default_features is
parsed from cpu type.

Flag forced_features has higher priority than flag default_features,
default_features will be used if there is no command line option for LBT
feature. If the feature is not supported with KVM host, it reports error
and exits if forced_features is set, else it disables feature and continues
if default_features is set.

Signed-off-by: Bibo Mao <maob...@loongson.cn>
---
  target/loongarch/cpu.c                | 69 +++++++++++++++++++++++++++
  target/loongarch/cpu.h                | 12 +++++
  target/loongarch/kvm/kvm.c            | 26 ++++++++++
  target/loongarch/kvm/kvm_loongarch.h  | 16 +++++++
  target/loongarch/loongarch-qmp-cmds.c |  2 +-
  5 files changed, 124 insertions(+), 1 deletion(-)


+static void loongarch_set_lbt(Object *obj, bool value, Error **errp)
+{
+    LoongArchCPU *cpu = LOONGARCH_CPU(obj);
+
+    if (!kvm_enabled()) {

Either set errp, ...

+        return;
+    }
+
+    if (value) {
+        /* Enable binary translation for all architectures */
+        cpu->env.forced_features |= BIT_ULL(LOONGARCH_FEATURE_LBT);
+    } else {
+        /* Disable default features also */
+        cpu->env.default_features &= ~BIT_ULL(LOONGARCH_FEATURE_LBT);
+    }
+}
+
  void loongarch_cpu_post_init(Object *obj)
  {
      object_property_add_bool(obj, "lsx", loongarch_get_lsx,
                               loongarch_set_lsx);
      object_property_add_bool(obj, "lasx", loongarch_get_lasx,
                               loongarch_set_lasx);

... or only add the property if KVM is enabled:

    if (kvm_enabled()) {
Sure, will do. I think this method is better.

By the way bitmap method forced_features/default_feature is variant
of OnOffAuto method. Bitmap method uses two bit, OnOffAuto method uses separate feature variable. We do not know which method is better or which is the future trend.

Regards
Bibo Mao

+    object_property_add_bool(obj, "lbt", loongarch_get_lbt,
+                             loongarch_set_lbt);
  }


Reply via email to