[PATCH v4 0/3] LoongArch: KVM: Add Binary Translation extension support

[PATCH v4 0/3] LoongArch: KVM: Add Binary Translation extension support

[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).

Like FPU extension, here late enabling method is used for LBT. LBT context
is saved/restored on vcpu context switch path.

Also this patch set BT capability detection, and BT register get/set
interface for userspace vmm, so that vm supports migration with BT
extension.

---
v3 ... v4:
  1. Merge LBT feature detection for VM and VCPU into one patch.
  2. Move function declaration such as kvm_lose_lbt()/kvm_check_fcsr()/
kvm_enable_lbt_fpu() from header file to c file, since it is only
used in one c file.

v2 ... v3:
  1. Split KVM_LOONGARCH_VM_FEAT_LBT capability checking into three
sub-features, KVM_LOONGARCH_VM_FEAT_X86BT/KVM_LOONGARCH_VM_FEAT_ARMBT
and KVM_LOONGARCH_VM_FEAT_MIPSBT. Return success only if host supports
the sub-feature.

v1 ... v2:
  1. With LBT register read or write interface to userpace, replace
device attr method with KVM_GET_ONE_REG method, since lbt register is
vcpu register and can be added in kvm_reg_list in future.
  2. Add vm device attr ctrl marcro KVM_LOONGARCH_VM_FEAT_CTRL, it is
used to get supported LBT feature before vm or vcpu is created.
---
Bibo Mao (3):
  LoongArch: KVM: Add HW Binary Translation extension support
  LoongArch: KVM: Add LBT feature detection function
  LoongArch: KVM: Add vm migration support for LBT registers

 arch/loongarch/include/asm/kvm_host.h |   8 ++
 arch/loongarch/include/asm/kvm_vcpu.h |  10 +++
 arch/loongarch/include/uapi/asm/kvm.h |  15 ++++
 arch/loongarch/kvm/exit.c             |   9 ++
 arch/loongarch/kvm/vcpu.c             | 121 +++++++++++++++++++++++++-
 arch/loongarch/kvm/vm.c               |  44 +++++++++-
 6 files changed, 205 insertions(+), 2 deletions(-)


base-commit: 55027e689933ba2e64f3d245fb1ff185b3e7fc81
-- 
2.39.3

[PATCH v4 1/3] LoongArch: KVM: Add HW Binary Translation extension support

[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).

Like FPU extension, here late enabling method is used for LBT. LBT context
is saved/restored on vcpu context switch path.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/include/asm/kvm_host.h |  8 ++++
 arch/loongarch/include/asm/kvm_vcpu.h |  6 +++
 arch/loongarch/kvm/exit.c             |  9 ++++
 arch/loongarch/kvm/vcpu.c             | 64 ++++++++++++++++++++++++++-
 4 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index 11b8d413554e..af695fbb1e23 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -137,6 +137,7 @@ enum emulation_result {
 #define KVM_LARCH_LASX		(0x1 << 2)
 #define KVM_LARCH_SWCSR_LATEST	(0x1 << 3)
 #define KVM_LARCH_HWCSR_USABLE	(0x1 << 4)
+#define KVM_LARCH_LBT		(0x1 << 5)
 
 struct kvm_vcpu_arch {
 	/*
@@ -170,6 +171,7 @@ struct kvm_vcpu_arch {
 
 	/* FPU state */
 	struct loongarch_fpu fpu FPU_ALIGN;
+	struct loongarch_lbt lbt;
 
 	/* CSR state */
 	struct loongarch_csrs *csr;
@@ -240,6 +242,12 @@ static inline bool kvm_guest_has_lasx(struct kvm_vcpu_arch *arch)
 	return arch->cpucfg[2] & CPUCFG2_LASX;
 }
 
+static inline bool kvm_guest_has_lbt(struct kvm_vcpu_arch *arch)
+{
+	return arch->cpucfg[2] & (CPUCFG2_X86BT | CPUCFG2_ARMBT
+					| CPUCFG2_MIPSBT);
+}
+
 /* Debug: dump vcpu state */
 int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu);
 
diff --git a/arch/loongarch/include/asm/kvm_vcpu.h b/arch/loongarch/include/asm/kvm_vcpu.h
index d7e51300a89f..cfa5c348598f 100644
--- a/arch/loongarch/include/asm/kvm_vcpu.h
+++ b/arch/loongarch/include/asm/kvm_vcpu.h
@@ -75,6 +75,12 @@ static inline void kvm_save_lasx(struct loongarch_fpu *fpu) { }
 static inline void kvm_restore_lasx(struct loongarch_fpu *fpu) { }
 #endif
 
+#ifdef CONFIG_CPU_HAS_LBT
+int kvm_own_lbt(struct kvm_vcpu *vcpu);
+#else
+static inline int kvm_own_lbt(struct kvm_vcpu *vcpu) { return -EINVAL; }
+#endif
+
 void kvm_init_timer(struct kvm_vcpu *vcpu, unsigned long hz);
 void kvm_reset_timer(struct kvm_vcpu *vcpu);
 void kvm_save_timer(struct kvm_vcpu *vcpu);
diff --git a/arch/loongarch/kvm/exit.c b/arch/loongarch/kvm/exit.c
index 81429c0b5c35..09eca0966cd2 100644
--- a/arch/loongarch/kvm/exit.c
+++ b/arch/loongarch/kvm/exit.c
@@ -835,6 +835,14 @@ static int kvm_handle_hypercall(struct kvm_vcpu *vcpu)
 	return ret;
 }
 
+static int kvm_handle_lbt_disabled(struct kvm_vcpu *vcpu)
+{
+	if (kvm_own_lbt(vcpu))
+		kvm_queue_exception(vcpu, EXCCODE_INE, 0);
+
+	return RESUME_GUEST;
+}
+
 /*
  * LoongArch KVM callback handling for unimplemented guest exiting
  */
@@ -867,6 +875,7 @@ static exit_handle_fn kvm_fault_tables[EXCCODE_INT_START] = {
 	[EXCCODE_LASXDIS]		= kvm_handle_lasx_disabled,
 	[EXCCODE_GSPR]			= kvm_handle_gspr,
 	[EXCCODE_HVC]			= kvm_handle_hypercall,
+	[EXCCODE_BTDIS]			= kvm_handle_lbt_disabled,
 };
 
 int kvm_handle_fault(struct kvm_vcpu *vcpu, int fault)
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 091e80f8be5e..233d28d0e928 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -6,6 +6,7 @@
 #include <linux/kvm_host.h>
 #include <linux/entry-kvm.h>
 #include <asm/fpu.h>
+#include <asm/lbt.h>
 #include <asm/loongarch.h>
 #include <asm/setup.h>
 #include <asm/time.h>
@@ -980,12 +981,69 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 	return 0;
 }
 
+#ifdef CONFIG_CPU_HAS_LBT
+int kvm_own_lbt(struct kvm_vcpu *vcpu)
+{
+	if (!kvm_guest_has_lbt(&vcpu->arch))
+		return -EINVAL;
+
+	preempt_disable();
+	set_csr_euen(CSR_EUEN_LBTEN);
+
+	_restore_lbt(&vcpu->arch.lbt);
+	vcpu->arch.aux_inuse |= KVM_LARCH_LBT;
+	preempt_enable();
+	return 0;
+}
+
+static void kvm_lose_lbt(struct kvm_vcpu *vcpu)
+{
+	preempt_disable();
+	if (vcpu->arch.aux_inuse & KVM_LARCH_LBT) {
+		_save_lbt(&vcpu->arch.lbt);
+		clear_csr_euen(CSR_EUEN_LBTEN);
+		vcpu->arch.aux_inuse &= ~KVM_LARCH_LBT;
+	}
+	preempt_enable();
+}
+
+static void kvm_enable_lbt_fpu(struct kvm_vcpu *vcpu, unsigned long fcsr)
+{
+	/*
+	 * if TM is enabled, top register save/restore will
+	 * cause lbt exception, here enable lbt in advance
+	 */
+	if (fcsr & FPU_CSR_TM)
+		kvm_own_lbt(vcpu);
+}
+
+static void kvm_check_fcsr(struct kvm_vcpu *vcpu)
+{
+	unsigned long fcsr;
+
+	if (vcpu->arch.aux_inuse & KVM_LARCH_FPU)
+		if (!(vcpu->arch.aux_inuse & KVM_LARCH_LBT)) {
+			fcsr = read_fcsr(LOONGARCH_FCSR0);
+			kvm_enable_lbt_fpu(vcpu, fcsr);
+		}
+}
+#else
+static inline void kvm_lose_lbt(struct kvm_vcpu *vcpu) { }
+static inline void kvm_enable_lbt_fpu(struct kvm_vcpu *vcpu,
+				unsigned long fcsr) { }
+static inline void kvm_check_fcsr(struct kvm_vcpu *vcpu) { }
+#endif
+
 /* Enable FPU and restore context */
 void kvm_own_fpu(struct kvm_vcpu *vcpu)
 {
 	preempt_disable();
 
-	/* Enable FPU */
+	/*
+	 * Enable FPU for guest
+	 * We set FR and FRE according to guest context
+	 */
+	kvm_enable_lbt_fpu(vcpu, vcpu->arch.fpu.fcsr);
 	set_csr_euen(CSR_EUEN_FPEN);
 
 	kvm_restore_fpu(&vcpu->arch.fpu);
@@ -1005,6 +1063,7 @@ int kvm_own_lsx(struct kvm_vcpu *vcpu)
 	preempt_disable();
 
 	/* Enable LSX for guest */
+	kvm_enable_lbt_fpu(vcpu, vcpu->arch.fpu.fcsr);
 	set_csr_euen(CSR_EUEN_LSXEN | CSR_EUEN_FPEN);
 	switch (vcpu->arch.aux_inuse & KVM_LARCH_FPU) {
 	case KVM_LARCH_FPU:
@@ -1039,6 +1098,7 @@ int kvm_own_lasx(struct kvm_vcpu *vcpu)
 
 	preempt_disable();
 
+	kvm_enable_lbt_fpu(vcpu, vcpu->arch.fpu.fcsr);
 	set_csr_euen(CSR_EUEN_FPEN | CSR_EUEN_LSXEN | CSR_EUEN_LASXEN);
 	switch (vcpu->arch.aux_inuse & (KVM_LARCH_FPU | KVM_LARCH_LSX)) {
 	case KVM_LARCH_LSX:
@@ -1070,6 +1130,7 @@ void kvm_lose_fpu(struct kvm_vcpu *vcpu)
 {
 	preempt_disable();
 
+	kvm_check_fcsr(vcpu);
 	if (vcpu->arch.aux_inuse & KVM_LARCH_LASX) {
 		kvm_save_lasx(&vcpu->arch.fpu);
 		vcpu->arch.aux_inuse &= ~(KVM_LARCH_LSX | KVM_LARCH_FPU | KVM_LARCH_LASX);
@@ -1092,6 +1153,7 @@ void kvm_lose_fpu(struct kvm_vcpu *vcpu)
 		/* Disable FPU */
 		clear_csr_euen(CSR_EUEN_FPEN);
 	}
+	kvm_lose_lbt(vcpu);
 
 	preempt_enable();
 }
-- 
2.39.3

[PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Bibo Mao]

Two kinds of LBT feature detection are added here, one is VCPU
feature, the other is VM feature. VCPU feature dection can only
work with VCPU thread itself, and requires VCPU thread is created
already. So LBT feature detection for VM is added also, it can
be done even if VM is not created, and also can be done by any
thread besides VCPU threads.

Loongson Binary Translation (LBT) feature is defined in register
cpucfg2. Here LBT capability detection for VCPU is added.

Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
three sub-features relative with LBT are added as following:
 KVM_LOONGARCH_VM_FEAT_X86BT
 KVM_LOONGARCH_VM_FEAT_ARMBT
 KVM_LOONGARCH_VM_FEAT_MIPSBT

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
 arch/loongarch/kvm/vcpu.c             |  6 ++++
 arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
index ddc5cab0ffd0..c40f7d9ffe13 100644
--- a/arch/loongarch/include/uapi/asm/kvm.h
+++ b/arch/loongarch/include/uapi/asm/kvm.h
@@ -82,6 +82,12 @@ struct kvm_fpu {
 #define KVM_IOC_CSRID(REG)		LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
 #define KVM_IOC_CPUCFG(REG)		LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
 
+/* Device Control API on vm fd */
+#define KVM_LOONGARCH_VM_FEAT_CTRL	0
+#define  KVM_LOONGARCH_VM_FEAT_X86BT	0
+#define  KVM_LOONGARCH_VM_FEAT_ARMBT	1
+#define  KVM_LOONGARCH_VM_FEAT_MIPSBT	2
+
 /* Device Control API on vcpu fd */
 #define KVM_LOONGARCH_VCPU_CPUCFG	0
 #define KVM_LOONGARCH_VCPU_PVTIME_CTRL	1
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 233d28d0e928..9734b4d8db05 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
 			*v |= CPUCFG2_LSX;
 		if (cpu_has_lasx)
 			*v |= CPUCFG2_LASX;
+		if (cpu_has_lbt_x86)
+			*v |= CPUCFG2_X86BT;
+		if (cpu_has_lbt_arm)
+			*v |= CPUCFG2_ARMBT;
+		if (cpu_has_lbt_mips)
+			*v |= CPUCFG2_MIPSBT;
 
 		return 0;
 	case LOONGARCH_CPUCFG3:
diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
index 6b2e4f66ad26..09e05108c68b 100644
--- a/arch/loongarch/kvm/vm.c
+++ b/arch/loongarch/kvm/vm.c
@@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	return r;
 }
 
+static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	switch (attr->attr) {
+	case KVM_LOONGARCH_VM_FEAT_X86BT:
+		if (cpu_has_lbt_x86)
+			return 0;
+		return -ENXIO;
+	case KVM_LOONGARCH_VM_FEAT_ARMBT:
+		if (cpu_has_lbt_arm)
+			return 0;
+		return -ENXIO;
+	case KVM_LOONGARCH_VM_FEAT_MIPSBT:
+		if (cpu_has_lbt_mips)
+			return 0;
+		return -ENXIO;
+	default:
+		return -ENXIO;
+	}
+}
+
+static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	switch (attr->group) {
+	case KVM_LOONGARCH_VM_FEAT_CTRL:
+		return kvm_vm_feature_has_attr(kvm, attr);
+	default:
+		return -ENXIO;
+	}
+}
+
 int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 {
-	return -ENOIOCTLCMD;
+	struct kvm *kvm = filp->private_data;
+	void __user *argp = (void __user *)arg;
+	struct kvm_device_attr attr;
+
+	switch (ioctl) {
+	case KVM_HAS_DEVICE_ATTR:
+		if (copy_from_user(&attr, argp, sizeof(attr)))
+			return -EFAULT;
+
+		return kvm_vm_has_attr(kvm, &attr);
+	default:
+		return -EINVAL;
+	}
 }
-- 
2.39.3

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

Hi, Bibo,

On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> Two kinds of LBT feature detection are added here, one is VCPU
> feature, the other is VM feature. VCPU feature dection can only
> work with VCPU thread itself, and requires VCPU thread is created
> already. So LBT feature detection for VM is added also, it can
> be done even if VM is not created, and also can be done by any
> thread besides VCPU threads.
>
> Loongson Binary Translation (LBT) feature is defined in register
> cpucfg2. Here LBT capability detection for VCPU is added.
>
> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> three sub-features relative with LBT are added as following:
>  KVM_LOONGARCH_VM_FEAT_X86BT
>  KVM_LOONGARCH_VM_FEAT_ARMBT
>  KVM_LOONGARCH_VM_FEAT_MIPSBT
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>  arch/loongarch/kvm/vcpu.c             |  6 ++++
>  arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>  3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> index ddc5cab0ffd0..c40f7d9ffe13 100644
> --- a/arch/loongarch/include/uapi/asm/kvm.h
> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> @@ -82,6 +82,12 @@ struct kvm_fpu {
>  #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>  #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>
> +/* Device Control API on vm fd */
> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> +
>  /* Device Control API on vcpu fd */
>  #define KVM_LOONGARCH_VCPU_CPUCFG      0
>  #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
If you insist that LBT should be a vm feature, then I suggest the
above two also be vm features. Though this is an UAPI change, but
CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
have a chance to change it now.

Huacai

> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 233d28d0e928..9734b4d8db05 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>                         *v |= CPUCFG2_LSX;
>                 if (cpu_has_lasx)
>                         *v |= CPUCFG2_LASX;
> +               if (cpu_has_lbt_x86)
> +                       *v |= CPUCFG2_X86BT;
> +               if (cpu_has_lbt_arm)
> +                       *v |= CPUCFG2_ARMBT;
> +               if (cpu_has_lbt_mips)
> +                       *v |= CPUCFG2_MIPSBT;
>
>                 return 0;
>         case LOONGARCH_CPUCFG3:
> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> index 6b2e4f66ad26..09e05108c68b 100644
> --- a/arch/loongarch/kvm/vm.c
> +++ b/arch/loongarch/kvm/vm.c
> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>         return r;
>  }
>
> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> +{
> +       switch (attr->attr) {
> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> +               if (cpu_has_lbt_x86)
> +                       return 0;
> +               return -ENXIO;
> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> +               if (cpu_has_lbt_arm)
> +                       return 0;
> +               return -ENXIO;
> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> +               if (cpu_has_lbt_mips)
> +                       return 0;
> +               return -ENXIO;
> +       default:
> +               return -ENXIO;
> +       }
> +}
> +
> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> +{
> +       switch (attr->group) {
> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> +               return kvm_vm_feature_has_attr(kvm, attr);
> +       default:
> +               return -ENXIO;
> +       }
> +}
> +
>  int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>  {
> -       return -ENOIOCTLCMD;
> +       struct kvm *kvm = filp->private_data;
> +       void __user *argp = (void __user *)arg;
> +       struct kvm_device_attr attr;
> +
> +       switch (ioctl) {
> +       case KVM_HAS_DEVICE_ATTR:
> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> +                       return -EFAULT;
> +
> +               return kvm_vm_has_attr(kvm, &attr);
> +       default:
> +               return -EINVAL;
> +       }
>  }
> --
> 2.39.3
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

Huacai,

On 2024/6/30 上午10:07, Huacai Chen wrote:
> Hi, Bibo,
> 
> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Two kinds of LBT feature detection are added here, one is VCPU
>> feature, the other is VM feature. VCPU feature dection can only
>> work with VCPU thread itself, and requires VCPU thread is created
>> already. So LBT feature detection for VM is added also, it can
>> be done even if VM is not created, and also can be done by any
>> thread besides VCPU threads.
>>
>> Loongson Binary Translation (LBT) feature is defined in register
>> cpucfg2. Here LBT capability detection for VCPU is added.
>>
>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>> three sub-features relative with LBT are added as following:
>>   KVM_LOONGARCH_VM_FEAT_X86BT
>>   KVM_LOONGARCH_VM_FEAT_ARMBT
>>   KVM_LOONGARCH_VM_FEAT_MIPSBT
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>   arch/loongarch/kvm/vcpu.c             |  6 ++++
>>   arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>   3 files changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>   #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>   #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>
>> +/* Device Control API on vm fd */
>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>> +
>>   /* Device Control API on vcpu fd */
>>   #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>   #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> If you insist that LBT should be a vm feature, then I suggest the
> above two also be vm features. Though this is an UAPI change, but
> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> have a chance to change it now.

KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
has is own different gpa address.

For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
the API even if it is 6.10-rc1, VMM has already used this. Else there is
uapi breaking now, still will be in future if we cannot control this.

How about adding new extra features capability for VM such as?
+#define  KVM_LOONGARCH_VM_FEAT_LSX   3
+#define  KVM_LOONGARCH_VM_FEAT_LASX  4

Regards
Bibo Mao
> 
> Huacai
> 
>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>> index 233d28d0e928..9734b4d8db05 100644
>> --- a/arch/loongarch/kvm/vcpu.c
>> +++ b/arch/loongarch/kvm/vcpu.c
>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>                          *v |= CPUCFG2_LSX;
>>                  if (cpu_has_lasx)
>>                          *v |= CPUCFG2_LASX;
>> +               if (cpu_has_lbt_x86)
>> +                       *v |= CPUCFG2_X86BT;
>> +               if (cpu_has_lbt_arm)
>> +                       *v |= CPUCFG2_ARMBT;
>> +               if (cpu_has_lbt_mips)
>> +                       *v |= CPUCFG2_MIPSBT;
>>
>>                  return 0;
>>          case LOONGARCH_CPUCFG3:
>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>> index 6b2e4f66ad26..09e05108c68b 100644
>> --- a/arch/loongarch/kvm/vm.c
>> +++ b/arch/loongarch/kvm/vm.c
>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>          return r;
>>   }
>>
>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>> +{
>> +       switch (attr->attr) {
>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>> +               if (cpu_has_lbt_x86)
>> +                       return 0;
>> +               return -ENXIO;
>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>> +               if (cpu_has_lbt_arm)
>> +                       return 0;
>> +               return -ENXIO;
>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>> +               if (cpu_has_lbt_mips)
>> +                       return 0;
>> +               return -ENXIO;
>> +       default:
>> +               return -ENXIO;
>> +       }
>> +}
>> +
>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>> +{
>> +       switch (attr->group) {
>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>> +               return kvm_vm_feature_has_attr(kvm, attr);
>> +       default:
>> +               return -ENXIO;
>> +       }
>> +}
>> +
>>   int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>   {
>> -       return -ENOIOCTLCMD;
>> +       struct kvm *kvm = filp->private_data;
>> +       void __user *argp = (void __user *)arg;
>> +       struct kvm_device_attr attr;
>> +
>> +       switch (ioctl) {
>> +       case KVM_HAS_DEVICE_ATTR:
>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>> +                       return -EFAULT;
>> +
>> +               return kvm_vm_has_attr(kvm, &attr);
>> +       default:
>> +               return -EINVAL;
>> +       }
>>   }
>> --
>> 2.39.3
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>
>
> Huacai,
>
> On 2024/6/30 上午10:07, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> Two kinds of LBT feature detection are added here, one is VCPU
> >> feature, the other is VM feature. VCPU feature dection can only
> >> work with VCPU thread itself, and requires VCPU thread is created
> >> already. So LBT feature detection for VM is added also, it can
> >> be done even if VM is not created, and also can be done by any
> >> thread besides VCPU threads.
> >>
> >> Loongson Binary Translation (LBT) feature is defined in register
> >> cpucfg2. Here LBT capability detection for VCPU is added.
> >>
> >> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> >> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> >> three sub-features relative with LBT are added as following:
> >>   KVM_LOONGARCH_VM_FEAT_X86BT
> >>   KVM_LOONGARCH_VM_FEAT_ARMBT
> >>   KVM_LOONGARCH_VM_FEAT_MIPSBT
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >>   arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
> >>   arch/loongarch/kvm/vcpu.c             |  6 ++++
> >>   arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
> >>   3 files changed, 55 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >> index ddc5cab0ffd0..c40f7d9ffe13 100644
> >> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >> @@ -82,6 +82,12 @@ struct kvm_fpu {
> >>   #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
> >>   #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
> >>
> >> +/* Device Control API on vm fd */
> >> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> >> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> >> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> >> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> >> +
> >>   /* Device Control API on vcpu fd */
> >>   #define KVM_LOONGARCH_VCPU_CPUCFG      0
> >>   #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> > If you insist that LBT should be a vm feature, then I suggest the
> > above two also be vm features. Though this is an UAPI change, but
> > CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> > have a chance to change it now.
>
> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
> has is own different gpa address.
Then leave this as a vm feature.

>
> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
> the API even if it is 6.10-rc1, VMM has already used this. Else there is
> uapi breaking now, still will be in future if we cannot control this.
UAPI changing before the first release is allowed, which means, we can
change this before the 6.10-final, but cannot change it after
6.10-final.

>
> How about adding new extra features capability for VM such as?
> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
They should be similar as LBT, if LBT is vcpu feature, they should
also be vcpu features; if LBT is vm feature, they should also be vm
features.

Huacai

>
> Regards
> Bibo Mao
> >
> > Huacai
> >
> >> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >> index 233d28d0e928..9734b4d8db05 100644
> >> --- a/arch/loongarch/kvm/vcpu.c
> >> +++ b/arch/loongarch/kvm/vcpu.c
> >> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
> >>                          *v |= CPUCFG2_LSX;
> >>                  if (cpu_has_lasx)
> >>                          *v |= CPUCFG2_LASX;
> >> +               if (cpu_has_lbt_x86)
> >> +                       *v |= CPUCFG2_X86BT;
> >> +               if (cpu_has_lbt_arm)
> >> +                       *v |= CPUCFG2_ARMBT;
> >> +               if (cpu_has_lbt_mips)
> >> +                       *v |= CPUCFG2_MIPSBT;
> >>
> >>                  return 0;
> >>          case LOONGARCH_CPUCFG3:
> >> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >> index 6b2e4f66ad26..09e05108c68b 100644
> >> --- a/arch/loongarch/kvm/vm.c
> >> +++ b/arch/loongarch/kvm/vm.c
> >> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>          return r;
> >>   }
> >>
> >> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >> +{
> >> +       switch (attr->attr) {
> >> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> >> +               if (cpu_has_lbt_x86)
> >> +                       return 0;
> >> +               return -ENXIO;
> >> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> >> +               if (cpu_has_lbt_arm)
> >> +                       return 0;
> >> +               return -ENXIO;
> >> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> >> +               if (cpu_has_lbt_mips)
> >> +                       return 0;
> >> +               return -ENXIO;
> >> +       default:
> >> +               return -ENXIO;
> >> +       }
> >> +}
> >> +
> >> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >> +{
> >> +       switch (attr->group) {
> >> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> >> +               return kvm_vm_feature_has_attr(kvm, attr);
> >> +       default:
> >> +               return -ENXIO;
> >> +       }
> >> +}
> >> +
> >>   int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >>   {
> >> -       return -ENOIOCTLCMD;
> >> +       struct kvm *kvm = filp->private_data;
> >> +       void __user *argp = (void __user *)arg;
> >> +       struct kvm_device_attr attr;
> >> +
> >> +       switch (ioctl) {
> >> +       case KVM_HAS_DEVICE_ATTR:
> >> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> >> +                       return -EFAULT;
> >> +
> >> +               return kvm_vm_has_attr(kvm, &attr);
> >> +       default:
> >> +               return -EINVAL;
> >> +       }
> >>   }
> >> --
> >> 2.39.3
> >>
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

Huacai,

On 2024/7/1 下午6:26, Huacai Chen wrote:
> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>> Huacai,
>>
>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>> feature, the other is VM feature. VCPU feature dection can only
>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>> already. So LBT feature detection for VM is added also, it can
>>>> be done even if VM is not created, and also can be done by any
>>>> thread besides VCPU threads.
>>>>
>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>
>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>> three sub-features relative with LBT are added as following:
>>>>    KVM_LOONGARCH_VM_FEAT_X86BT
>>>>    KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>    KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>>    arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>    arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>    arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>    3 files changed, 55 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>    #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>    #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>
>>>> +/* Device Control API on vm fd */
>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>> +
>>>>    /* Device Control API on vcpu fd */
>>>>    #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>    #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>> If you insist that LBT should be a vm feature, then I suggest the
>>> above two also be vm features. Though this is an UAPI change, but
>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>> have a chance to change it now.
>>
>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>> has is own different gpa address.
> Then leave this as a vm feature.
> 
>>
>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>> uapi breaking now, still will be in future if we cannot control this.
> UAPI changing before the first release is allowed, which means, we can
> change this before the 6.10-final, but cannot change it after
> 6.10-final.
Now QEMU has already synced uapi to its own directory, also I never hear 
about this, with my experience with uapi change, there is only newly 
added or removed deprecated years ago.

Is there any documentation about UAPI change rules?
> 
>>
>> How about adding new extra features capability for VM such as?
>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
> They should be similar as LBT, if LBT is vcpu feature, they should
> also be vcpu features; if LBT is vm feature, they should also be vm
> features.
On other architectures, with function kvm_vm_ioctl_check_extension()
    KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
    KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
These features are all cpu features, at the same time they are VM features.

If they are cpu features, how does VMM detect validity of these features 
passing from command line? After all VCPUs are created and send bootup 
command to these VCPUs? That is too late, VMM main thread is easy to 
detect feature validity if they are VM features also.

To be honest, I am not familiar with KVM still, only get further 
understanding after actual problems solving. Welcome to give comments, 
however please read more backgroud if you insist on, else there will be 
endless argument again.

Regards
Bibo, Mao
> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>>
>>> Huacai
>>>
>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>> index 233d28d0e928..9734b4d8db05 100644
>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>                           *v |= CPUCFG2_LSX;
>>>>                   if (cpu_has_lasx)
>>>>                           *v |= CPUCFG2_LASX;
>>>> +               if (cpu_has_lbt_x86)
>>>> +                       *v |= CPUCFG2_X86BT;
>>>> +               if (cpu_has_lbt_arm)
>>>> +                       *v |= CPUCFG2_ARMBT;
>>>> +               if (cpu_has_lbt_mips)
>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>
>>>>                   return 0;
>>>>           case LOONGARCH_CPUCFG3:
>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>> --- a/arch/loongarch/kvm/vm.c
>>>> +++ b/arch/loongarch/kvm/vm.c
>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>           return r;
>>>>    }
>>>>
>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>> +{
>>>> +       switch (attr->attr) {
>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>> +               if (cpu_has_lbt_x86)
>>>> +                       return 0;
>>>> +               return -ENXIO;
>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>> +               if (cpu_has_lbt_arm)
>>>> +                       return 0;
>>>> +               return -ENXIO;
>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>> +               if (cpu_has_lbt_mips)
>>>> +                       return 0;
>>>> +               return -ENXIO;
>>>> +       default:
>>>> +               return -ENXIO;
>>>> +       }
>>>> +}
>>>> +
>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>> +{
>>>> +       switch (attr->group) {
>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>> +       default:
>>>> +               return -ENXIO;
>>>> +       }
>>>> +}
>>>> +
>>>>    int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>    {
>>>> -       return -ENOIOCTLCMD;
>>>> +       struct kvm *kvm = filp->private_data;
>>>> +       void __user *argp = (void __user *)arg;
>>>> +       struct kvm_device_attr attr;
>>>> +
>>>> +       switch (ioctl) {
>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>> +                       return -EFAULT;
>>>> +
>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>> +       default:
>>>> +               return -EINVAL;
>>>> +       }
>>>>    }
>>>> --
>>>> 2.39.3
>>>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>
> Huacai,
>
> On 2024/7/1 下午6:26, Huacai Chen wrote:
> > On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >> Huacai,
> >>
> >> On 2024/6/30 上午10:07, Huacai Chen wrote:
> >>> Hi, Bibo,
> >>>
> >>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>> Two kinds of LBT feature detection are added here, one is VCPU
> >>>> feature, the other is VM feature. VCPU feature dection can only
> >>>> work with VCPU thread itself, and requires VCPU thread is created
> >>>> already. So LBT feature detection for VM is added also, it can
> >>>> be done even if VM is not created, and also can be done by any
> >>>> thread besides VCPU threads.
> >>>>
> >>>> Loongson Binary Translation (LBT) feature is defined in register
> >>>> cpucfg2. Here LBT capability detection for VCPU is added.
> >>>>
> >>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> >>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> >>>> three sub-features relative with LBT are added as following:
> >>>>    KVM_LOONGARCH_VM_FEAT_X86BT
> >>>>    KVM_LOONGARCH_VM_FEAT_ARMBT
> >>>>    KVM_LOONGARCH_VM_FEAT_MIPSBT
> >>>>
> >>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>> ---
> >>>>    arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
> >>>>    arch/loongarch/kvm/vcpu.c             |  6 ++++
> >>>>    arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
> >>>>    3 files changed, 55 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
> >>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
> >>>>    #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
> >>>>    #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
> >>>>
> >>>> +/* Device Control API on vm fd */
> >>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> >>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> >>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> >>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> >>>> +
> >>>>    /* Device Control API on vcpu fd */
> >>>>    #define KVM_LOONGARCH_VCPU_CPUCFG      0
> >>>>    #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> >>> If you insist that LBT should be a vm feature, then I suggest the
> >>> above two also be vm features. Though this is an UAPI change, but
> >>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> >>> have a chance to change it now.
> >>
> >> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
> >> has is own different gpa address.
> > Then leave this as a vm feature.
> >
> >>
> >> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
> >> the API even if it is 6.10-rc1, VMM has already used this. Else there is
> >> uapi breaking now, still will be in future if we cannot control this.
> > UAPI changing before the first release is allowed, which means, we can
> > change this before the 6.10-final, but cannot change it after
> > 6.10-final.
> Now QEMU has already synced uapi to its own directory, also I never hear
> about this, with my experience with uapi change, there is only newly
> added or removed deprecated years ago.
>
> Is there any documentation about UAPI change rules?
No document, but learn from my more than 10 years upstream experience.

> >
> >>
> >> How about adding new extra features capability for VM such as?
> >> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
> >> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
> > They should be similar as LBT, if LBT is vcpu feature, they should
> > also be vcpu features; if LBT is vm feature, they should also be vm
> > features.
> On other architectures, with function kvm_vm_ioctl_check_extension()
>     KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>     KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
> These features are all cpu features, at the same time they are VM features.
>
> If they are cpu features, how does VMM detect validity of these features
> passing from command line? After all VCPUs are created and send bootup
> command to these VCPUs? That is too late, VMM main thread is easy to
> detect feature validity if they are VM features also.
>
> To be honest, I am not familiar with KVM still, only get further
> understanding after actual problems solving. Welcome to give comments,
> however please read more backgroud if you insist on, else there will be
> endless argument again.
I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
haven't insisted on whether they should be vcpu features or vm
features.

Huacai

>
> Regards
> Bibo, Mao
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>>
> >>> Huacai
> >>>
> >>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >>>> index 233d28d0e928..9734b4d8db05 100644
> >>>> --- a/arch/loongarch/kvm/vcpu.c
> >>>> +++ b/arch/loongarch/kvm/vcpu.c
> >>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
> >>>>                           *v |= CPUCFG2_LSX;
> >>>>                   if (cpu_has_lasx)
> >>>>                           *v |= CPUCFG2_LASX;
> >>>> +               if (cpu_has_lbt_x86)
> >>>> +                       *v |= CPUCFG2_X86BT;
> >>>> +               if (cpu_has_lbt_arm)
> >>>> +                       *v |= CPUCFG2_ARMBT;
> >>>> +               if (cpu_has_lbt_mips)
> >>>> +                       *v |= CPUCFG2_MIPSBT;
> >>>>
> >>>>                   return 0;
> >>>>           case LOONGARCH_CPUCFG3:
> >>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >>>> index 6b2e4f66ad26..09e05108c68b 100644
> >>>> --- a/arch/loongarch/kvm/vm.c
> >>>> +++ b/arch/loongarch/kvm/vm.c
> >>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>>>           return r;
> >>>>    }
> >>>>
> >>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>> +{
> >>>> +       switch (attr->attr) {
> >>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> >>>> +               if (cpu_has_lbt_x86)
> >>>> +                       return 0;
> >>>> +               return -ENXIO;
> >>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> >>>> +               if (cpu_has_lbt_arm)
> >>>> +                       return 0;
> >>>> +               return -ENXIO;
> >>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> >>>> +               if (cpu_has_lbt_mips)
> >>>> +                       return 0;
> >>>> +               return -ENXIO;
> >>>> +       default:
> >>>> +               return -ENXIO;
> >>>> +       }
> >>>> +}
> >>>> +
> >>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>> +{
> >>>> +       switch (attr->group) {
> >>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> >>>> +               return kvm_vm_feature_has_attr(kvm, attr);
> >>>> +       default:
> >>>> +               return -ENXIO;
> >>>> +       }
> >>>> +}
> >>>> +
> >>>>    int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >>>>    {
> >>>> -       return -ENOIOCTLCMD;
> >>>> +       struct kvm *kvm = filp->private_data;
> >>>> +       void __user *argp = (void __user *)arg;
> >>>> +       struct kvm_device_attr attr;
> >>>> +
> >>>> +       switch (ioctl) {
> >>>> +       case KVM_HAS_DEVICE_ATTR:
> >>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> >>>> +                       return -EFAULT;
> >>>> +
> >>>> +               return kvm_vm_has_attr(kvm, &attr);
> >>>> +       default:
> >>>> +               return -EINVAL;
> >>>> +       }
> >>>>    }
> >>>> --
> >>>> 2.39.3
> >>>>
> >>
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/2 上午9:59, Huacai Chen wrote:
> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>>
>> Huacai,
>>
>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>> Huacai,
>>>>
>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>> Hi, Bibo,
>>>>>
>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>> be done even if VM is not created, and also can be done by any
>>>>>> thread besides VCPU threads.
>>>>>>
>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>
>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>> three sub-features relative with LBT are added as following:
>>>>>>     KVM_LOONGARCH_VM_FEAT_X86BT
>>>>>>     KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>>>     KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>>>
>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>> ---
>>>>>>     arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>>>     arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>>>     arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>>>     3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>>>     #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>>>     #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>>>
>>>>>> +/* Device Control API on vm fd */
>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>>>> +
>>>>>>     /* Device Control API on vcpu fd */
>>>>>>     #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>>>     #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>> have a chance to change it now.
>>>>
>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>> has is own different gpa address.
>>> Then leave this as a vm feature.
>>>
>>>>
>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>> uapi breaking now, still will be in future if we cannot control this.
>>> UAPI changing before the first release is allowed, which means, we can
>>> change this before the 6.10-final, but cannot change it after
>>> 6.10-final.
>> Now QEMU has already synced uapi to its own directory, also I never hear
>> about this, with my experience with uapi change, there is only newly
>> added or removed deprecated years ago.
>>
>> Is there any documentation about UAPI change rules?
> No document, but learn from my more than 10 years upstream experience.
Can you show me an example about with your rich upstream experience?
> 
>>>
>>>>
>>>> How about adding new extra features capability for VM such as?
>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>> features.
>> On other architectures, with function kvm_vm_ioctl_check_extension()
>>      KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>>      KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>> These features are all cpu features, at the same time they are VM features.
>>
>> If they are cpu features, how does VMM detect validity of these features
>> passing from command line? After all VCPUs are created and send bootup
>> command to these VCPUs? That is too late, VMM main thread is easy to
>> detect feature validity if they are VM features also.
>>
>> To be honest, I am not familiar with KVM still, only get further
>> understanding after actual problems solving. Welcome to give comments,
>> however please read more backgroud if you insist on, else there will be
>> endless argument again.
> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
> haven't insisted on whether they should be vcpu features or vm
> features.
It is reasonable if LSX/LASX/LBT should be in the same class, since 
there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.

What is the usage about CPUCFG capability used for VM feature? It is not 
a detailed feature, it is only feature-set indicator like cpuid.

Regards
Bibo Mao
> 
> Huacai
> 
>>
>> Regards
>> Bibo, Mao
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>>                            *v |= CPUCFG2_LSX;
>>>>>>                    if (cpu_has_lasx)
>>>>>>                            *v |= CPUCFG2_LASX;
>>>>>> +               if (cpu_has_lbt_x86)
>>>>>> +                       *v |= CPUCFG2_X86BT;
>>>>>> +               if (cpu_has_lbt_arm)
>>>>>> +                       *v |= CPUCFG2_ARMBT;
>>>>>> +               if (cpu_has_lbt_mips)
>>>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>>>
>>>>>>                    return 0;
>>>>>>            case LOONGARCH_CPUCFG3:
>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>>>> --- a/arch/loongarch/kvm/vm.c
>>>>>> +++ b/arch/loongarch/kvm/vm.c
>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>>>            return r;
>>>>>>     }
>>>>>>
>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>> +{
>>>>>> +       switch (attr->attr) {
>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>>>> +               if (cpu_has_lbt_x86)
>>>>>> +                       return 0;
>>>>>> +               return -ENXIO;
>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>>>> +               if (cpu_has_lbt_arm)
>>>>>> +                       return 0;
>>>>>> +               return -ENXIO;
>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>>>> +               if (cpu_has_lbt_mips)
>>>>>> +                       return 0;
>>>>>> +               return -ENXIO;
>>>>>> +       default:
>>>>>> +               return -ENXIO;
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>> +{
>>>>>> +       switch (attr->group) {
>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>>>> +       default:
>>>>>> +               return -ENXIO;
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>>     int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>>>     {
>>>>>> -       return -ENOIOCTLCMD;
>>>>>> +       struct kvm *kvm = filp->private_data;
>>>>>> +       void __user *argp = (void __user *)arg;
>>>>>> +       struct kvm_device_attr attr;
>>>>>> +
>>>>>> +       switch (ioctl) {
>>>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>>>> +                       return -EFAULT;
>>>>>> +
>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>>>> +       default:
>>>>>> +               return -EINVAL;
>>>>>> +       }
>>>>>>     }
>>>>>> --
>>>>>> 2.39.3
>>>>>>
>>>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/7/2 上午9:59, Huacai Chen wrote:
> > On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
> >>
> >> Huacai,
> >>
> >> On 2024/7/1 下午6:26, Huacai Chen wrote:
> >>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>> Huacai,
> >>>>
> >>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
> >>>>> Hi, Bibo,
> >>>>>
> >>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>> Two kinds of LBT feature detection are added here, one is VCPU
> >>>>>> feature, the other is VM feature. VCPU feature dection can only
> >>>>>> work with VCPU thread itself, and requires VCPU thread is created
> >>>>>> already. So LBT feature detection for VM is added also, it can
> >>>>>> be done even if VM is not created, and also can be done by any
> >>>>>> thread besides VCPU threads.
> >>>>>>
> >>>>>> Loongson Binary Translation (LBT) feature is defined in register
> >>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
> >>>>>>
> >>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> >>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> >>>>>> three sub-features relative with LBT are added as following:
> >>>>>>     KVM_LOONGARCH_VM_FEAT_X86BT
> >>>>>>     KVM_LOONGARCH_VM_FEAT_ARMBT
> >>>>>>     KVM_LOONGARCH_VM_FEAT_MIPSBT
> >>>>>>
> >>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>> ---
> >>>>>>     arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
> >>>>>>     arch/loongarch/kvm/vcpu.c             |  6 ++++
> >>>>>>     arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
> >>>>>>     3 files changed, 55 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
> >>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
> >>>>>>     #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
> >>>>>>     #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
> >>>>>>
> >>>>>> +/* Device Control API on vm fd */
> >>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> >>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> >>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> >>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> >>>>>> +
> >>>>>>     /* Device Control API on vcpu fd */
> >>>>>>     #define KVM_LOONGARCH_VCPU_CPUCFG      0
> >>>>>>     #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> >>>>> If you insist that LBT should be a vm feature, then I suggest the
> >>>>> above two also be vm features. Though this is an UAPI change, but
> >>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> >>>>> have a chance to change it now.
> >>>>
> >>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
> >>>> has is own different gpa address.
> >>> Then leave this as a vm feature.
> >>>
> >>>>
> >>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
> >>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
> >>>> uapi breaking now, still will be in future if we cannot control this.
> >>> UAPI changing before the first release is allowed, which means, we can
> >>> change this before the 6.10-final, but cannot change it after
> >>> 6.10-final.
> >> Now QEMU has already synced uapi to its own directory, also I never hear
> >> about this, with my experience with uapi change, there is only newly
> >> added or removed deprecated years ago.
> >>
> >> Is there any documentation about UAPI change rules?
> > No document, but learn from my more than 10 years upstream experience.
> Can you show me an example about with your rich upstream experience?
A simple example,
e877d705704d7c8fe17b6b5ebdfdb14b84c revert
1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
6.9-final, but not after that.

Before the first release, the code status is treated as "unstable", so
revert, modify is allowed. But after the first release, even if an
"error" should also be treated as a "bad feature".

Huacai


> >
> >>>
> >>>>
> >>>> How about adding new extra features capability for VM such as?
> >>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
> >>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
> >>> They should be similar as LBT, if LBT is vcpu feature, they should
> >>> also be vcpu features; if LBT is vm feature, they should also be vm
> >>> features.
> >> On other architectures, with function kvm_vm_ioctl_check_extension()
> >>      KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
> >>      KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
> >> These features are all cpu features, at the same time they are VM features.
> >>
> >> If they are cpu features, how does VMM detect validity of these features
> >> passing from command line? After all VCPUs are created and send bootup
> >> command to these VCPUs? That is too late, VMM main thread is easy to
> >> detect feature validity if they are VM features also.
> >>
> >> To be honest, I am not familiar with KVM still, only get further
> >> understanding after actual problems solving. Welcome to give comments,
> >> however please read more backgroud if you insist on, else there will be
> >> endless argument again.
> > I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
> > haven't insisted on whether they should be vcpu features or vm
> > features.
> It is reasonable if LSX/LASX/LBT should be in the same class, since
> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
>
> What is the usage about CPUCFG capability used for VM feature? It is not
> a detailed feature, it is only feature-set indicator like cpuid.
>
> Regards
> Bibo Mao
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo, Mao
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo Mao
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >>>>>> index 233d28d0e928..9734b4d8db05 100644
> >>>>>> --- a/arch/loongarch/kvm/vcpu.c
> >>>>>> +++ b/arch/loongarch/kvm/vcpu.c
> >>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
> >>>>>>                            *v |= CPUCFG2_LSX;
> >>>>>>                    if (cpu_has_lasx)
> >>>>>>                            *v |= CPUCFG2_LASX;
> >>>>>> +               if (cpu_has_lbt_x86)
> >>>>>> +                       *v |= CPUCFG2_X86BT;
> >>>>>> +               if (cpu_has_lbt_arm)
> >>>>>> +                       *v |= CPUCFG2_ARMBT;
> >>>>>> +               if (cpu_has_lbt_mips)
> >>>>>> +                       *v |= CPUCFG2_MIPSBT;
> >>>>>>
> >>>>>>                    return 0;
> >>>>>>            case LOONGARCH_CPUCFG3:
> >>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >>>>>> index 6b2e4f66ad26..09e05108c68b 100644
> >>>>>> --- a/arch/loongarch/kvm/vm.c
> >>>>>> +++ b/arch/loongarch/kvm/vm.c
> >>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>>>>>            return r;
> >>>>>>     }
> >>>>>>
> >>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>> +{
> >>>>>> +       switch (attr->attr) {
> >>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> >>>>>> +               if (cpu_has_lbt_x86)
> >>>>>> +                       return 0;
> >>>>>> +               return -ENXIO;
> >>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> >>>>>> +               if (cpu_has_lbt_arm)
> >>>>>> +                       return 0;
> >>>>>> +               return -ENXIO;
> >>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> >>>>>> +               if (cpu_has_lbt_mips)
> >>>>>> +                       return 0;
> >>>>>> +               return -ENXIO;
> >>>>>> +       default:
> >>>>>> +               return -ENXIO;
> >>>>>> +       }
> >>>>>> +}
> >>>>>> +
> >>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>> +{
> >>>>>> +       switch (attr->group) {
> >>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> >>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
> >>>>>> +       default:
> >>>>>> +               return -ENXIO;
> >>>>>> +       }
> >>>>>> +}
> >>>>>> +
> >>>>>>     int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >>>>>>     {
> >>>>>> -       return -ENOIOCTLCMD;
> >>>>>> +       struct kvm *kvm = filp->private_data;
> >>>>>> +       void __user *argp = (void __user *)arg;
> >>>>>> +       struct kvm_device_attr attr;
> >>>>>> +
> >>>>>> +       switch (ioctl) {
> >>>>>> +       case KVM_HAS_DEVICE_ATTR:
> >>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> >>>>>> +                       return -EFAULT;
> >>>>>> +
> >>>>>> +               return kvm_vm_has_attr(kvm, &attr);
> >>>>>> +       default:
> >>>>>> +               return -EINVAL;
> >>>>>> +       }
> >>>>>>     }
> >>>>>> --
> >>>>>> 2.39.3
> >>>>>>
> >>>>
> >>
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/2 上午10:34, Huacai Chen wrote:
> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/7/2 上午9:59, Huacai Chen wrote:
>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>> Huacai,
>>>>
>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>> Huacai,
>>>>>>
>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>>>> Hi, Bibo,
>>>>>>>
>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>>>> be done even if VM is not created, and also can be done by any
>>>>>>>> thread besides VCPU threads.
>>>>>>>>
>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>>>
>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>>>> three sub-features relative with LBT are added as following:
>>>>>>>>      KVM_LOONGARCH_VM_FEAT_X86BT
>>>>>>>>      KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>>>>>      KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>>>>>
>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>> ---
>>>>>>>>      arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>>>>>      arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>>>>>      arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>>>>>      3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>>>>>      #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>>>>>      #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>>>>>
>>>>>>>> +/* Device Control API on vm fd */
>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>>>>>> +
>>>>>>>>      /* Device Control API on vcpu fd */
>>>>>>>>      #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>>>>>      #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>>>> have a chance to change it now.
>>>>>>
>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>>>> has is own different gpa address.
>>>>> Then leave this as a vm feature.
>>>>>
>>>>>>
>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>>>> uapi breaking now, still will be in future if we cannot control this.
>>>>> UAPI changing before the first release is allowed, which means, we can
>>>>> change this before the 6.10-final, but cannot change it after
>>>>> 6.10-final.
>>>> Now QEMU has already synced uapi to its own directory, also I never hear
>>>> about this, with my experience with uapi change, there is only newly
>>>> added or removed deprecated years ago.
>>>>
>>>> Is there any documentation about UAPI change rules?
>>> No document, but learn from my more than 10 years upstream experience.
>> Can you show me an example about with your rich upstream experience?
> A simple example,
> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
> 6.9-final, but not after that.
> 
> Before the first release, the code status is treated as "unstable", so
> revert, modify is allowed. But after the first release, even if an
> "error" should also be treated as a "bad feature".
Huacai,

Thanks for showing the example.

For this issue, Can we adding new uapi and mark the old as deprecated?
so that it can be removed after years.

For me, it is too frequent to revert the old uapi, it is not bug and
only that we have better method now. Also QEMU has synchronized the uapi
to its directory already.

Regards
Bibo, Mao
> 
> Huacai
> 
> 
>>>
>>>>>
>>>>>>
>>>>>> How about adding new extra features capability for VM such as?
>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>>>> features.
>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
>>>>       KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>>>>       KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>>>> These features are all cpu features, at the same time they are VM features.
>>>>
>>>> If they are cpu features, how does VMM detect validity of these features
>>>> passing from command line? After all VCPUs are created and send bootup
>>>> command to these VCPUs? That is too late, VMM main thread is easy to
>>>> detect feature validity if they are VM features also.
>>>>
>>>> To be honest, I am not familiar with KVM still, only get further
>>>> understanding after actual problems solving. Welcome to give comments,
>>>> however please read more backgroud if you insist on, else there will be
>>>> endless argument again.
>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
>>> haven't insisted on whether they should be vcpu features or vm
>>> features.
>> It is reasonable if LSX/LASX/LBT should be in the same class, since
>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
>>
>> What is the usage about CPUCFG capability used for VM feature? It is not
>> a detailed feature, it is only feature-set indicator like cpuid.
>>
>> Regards
>> Bibo Mao
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo, Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo Mao
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>>>>                             *v |= CPUCFG2_LSX;
>>>>>>>>                     if (cpu_has_lasx)
>>>>>>>>                             *v |= CPUCFG2_LASX;
>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>> +                       *v |= CPUCFG2_X86BT;
>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>>>>>
>>>>>>>>                     return 0;
>>>>>>>>             case LOONGARCH_CPUCFG3:
>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>>>>>> --- a/arch/loongarch/kvm/vm.c
>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>>>>>             return r;
>>>>>>>>      }
>>>>>>>>
>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>> +{
>>>>>>>> +       switch (attr->attr) {
>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>> +                       return 0;
>>>>>>>> +               return -ENXIO;
>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>> +                       return 0;
>>>>>>>> +               return -ENXIO;
>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>> +                       return 0;
>>>>>>>> +               return -ENXIO;
>>>>>>>> +       default:
>>>>>>>> +               return -ENXIO;
>>>>>>>> +       }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>> +{
>>>>>>>> +       switch (attr->group) {
>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>>>>>> +       default:
>>>>>>>> +               return -ENXIO;
>>>>>>>> +       }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>      int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>>>>>      {
>>>>>>>> -       return -ENOIOCTLCMD;
>>>>>>>> +       struct kvm *kvm = filp->private_data;
>>>>>>>> +       void __user *argp = (void __user *)arg;
>>>>>>>> +       struct kvm_device_attr attr;
>>>>>>>> +
>>>>>>>> +       switch (ioctl) {
>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>>>>>> +                       return -EFAULT;
>>>>>>>> +
>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>>>>>> +       default:
>>>>>>>> +               return -EINVAL;
>>>>>>>> +       }
>>>>>>>>      }
>>>>>>>> --
>>>>>>>> 2.39.3
>>>>>>>>
>>>>>>
>>>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/7/2 上午10:34, Huacai Chen wrote:
> > On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/7/2 上午9:59, Huacai Chen wrote:
> >>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>> Huacai,
> >>>>
> >>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
> >>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>>
> >>>>>> Huacai,
> >>>>>>
> >>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
> >>>>>>> Hi, Bibo,
> >>>>>>>
> >>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
> >>>>>>>> feature, the other is VM feature. VCPU feature dection can only
> >>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
> >>>>>>>> already. So LBT feature detection for VM is added also, it can
> >>>>>>>> be done even if VM is not created, and also can be done by any
> >>>>>>>> thread besides VCPU threads.
> >>>>>>>>
> >>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
> >>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
> >>>>>>>>
> >>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> >>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> >>>>>>>> three sub-features relative with LBT are added as following:
> >>>>>>>>      KVM_LOONGARCH_VM_FEAT_X86BT
> >>>>>>>>      KVM_LOONGARCH_VM_FEAT_ARMBT
> >>>>>>>>      KVM_LOONGARCH_VM_FEAT_MIPSBT
> >>>>>>>>
> >>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>> ---
> >>>>>>>>      arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
> >>>>>>>>      arch/loongarch/kvm/vcpu.c             |  6 ++++
> >>>>>>>>      arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
> >>>>>>>>      3 files changed, 55 insertions(+), 1 deletion(-)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
> >>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
> >>>>>>>>      #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
> >>>>>>>>      #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
> >>>>>>>>
> >>>>>>>> +/* Device Control API on vm fd */
> >>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> >>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> >>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> >>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> >>>>>>>> +
> >>>>>>>>      /* Device Control API on vcpu fd */
> >>>>>>>>      #define KVM_LOONGARCH_VCPU_CPUCFG      0
> >>>>>>>>      #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> >>>>>>> If you insist that LBT should be a vm feature, then I suggest the
> >>>>>>> above two also be vm features. Though this is an UAPI change, but
> >>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> >>>>>>> have a chance to change it now.
> >>>>>>
> >>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
> >>>>>> has is own different gpa address.
> >>>>> Then leave this as a vm feature.
> >>>>>
> >>>>>>
> >>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
> >>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
> >>>>>> uapi breaking now, still will be in future if we cannot control this.
> >>>>> UAPI changing before the first release is allowed, which means, we can
> >>>>> change this before the 6.10-final, but cannot change it after
> >>>>> 6.10-final.
> >>>> Now QEMU has already synced uapi to its own directory, also I never hear
> >>>> about this, with my experience with uapi change, there is only newly
> >>>> added or removed deprecated years ago.
> >>>>
> >>>> Is there any documentation about UAPI change rules?
> >>> No document, but learn from my more than 10 years upstream experience.
> >> Can you show me an example about with your rich upstream experience?
> > A simple example,
> > e877d705704d7c8fe17b6b5ebdfdb14b84c revert
> > 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
> > 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
> > e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
> > 6.9-final, but not after that.
> >
> > Before the first release, the code status is treated as "unstable", so
> > revert, modify is allowed. But after the first release, even if an
> > "error" should also be treated as a "bad feature".
> Huacai,
>
> Thanks for showing the example.
>
> For this issue, Can we adding new uapi and mark the old as deprecated?
> so that it can be removed after years.
Unnecessary, just remove the old one. Deprecation is for the usage
after the first release.

>
> For me, it is too frequent to revert the old uapi, it is not bug and
> only that we have better method now. Also QEMU has synchronized the uapi
> to its directory already.
QEMU also hasn't been released after synchronizing the uapi, so it is
OK to remove the old api now.

Huacai

>
> Regards
> Bibo, Mao
> >
> > Huacai
> >
> >
> >>>
> >>>>>
> >>>>>>
> >>>>>> How about adding new extra features capability for VM such as?
> >>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
> >>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
> >>>>> They should be similar as LBT, if LBT is vcpu feature, they should
> >>>>> also be vcpu features; if LBT is vm feature, they should also be vm
> >>>>> features.
> >>>> On other architectures, with function kvm_vm_ioctl_check_extension()
> >>>>       KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
> >>>>       KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
> >>>> These features are all cpu features, at the same time they are VM features.
> >>>>
> >>>> If they are cpu features, how does VMM detect validity of these features
> >>>> passing from command line? After all VCPUs are created and send bootup
> >>>> command to these VCPUs? That is too late, VMM main thread is easy to
> >>>> detect feature validity if they are VM features also.
> >>>>
> >>>> To be honest, I am not familiar with KVM still, only get further
> >>>> understanding after actual problems solving. Welcome to give comments,
> >>>> however please read more backgroud if you insist on, else there will be
> >>>> endless argument again.
> >>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
> >>> haven't insisted on whether they should be vcpu features or vm
> >>> features.
> >> It is reasonable if LSX/LASX/LBT should be in the same class, since
> >> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
> >>
> >> What is the usage about CPUCFG capability used for VM feature? It is not
> >> a detailed feature, it is only feature-set indicator like cpuid.
> >>
> >> Regards
> >> Bibo Mao
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo, Mao
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>>
> >>>>>> Regards
> >>>>>> Bibo Mao
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >>>>>>>> index 233d28d0e928..9734b4d8db05 100644
> >>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
> >>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
> >>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
> >>>>>>>>                             *v |= CPUCFG2_LSX;
> >>>>>>>>                     if (cpu_has_lasx)
> >>>>>>>>                             *v |= CPUCFG2_LASX;
> >>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>> +                       *v |= CPUCFG2_X86BT;
> >>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>> +                       *v |= CPUCFG2_ARMBT;
> >>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
> >>>>>>>>
> >>>>>>>>                     return 0;
> >>>>>>>>             case LOONGARCH_CPUCFG3:
> >>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
> >>>>>>>> --- a/arch/loongarch/kvm/vm.c
> >>>>>>>> +++ b/arch/loongarch/kvm/vm.c
> >>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>>>>>>>             return r;
> >>>>>>>>      }
> >>>>>>>>
> >>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>> +{
> >>>>>>>> +       switch (attr->attr) {
> >>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> >>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>> +                       return 0;
> >>>>>>>> +               return -ENXIO;
> >>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> >>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>> +                       return 0;
> >>>>>>>> +               return -ENXIO;
> >>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> >>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>> +                       return 0;
> >>>>>>>> +               return -ENXIO;
> >>>>>>>> +       default:
> >>>>>>>> +               return -ENXIO;
> >>>>>>>> +       }
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>> +{
> >>>>>>>> +       switch (attr->group) {
> >>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> >>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
> >>>>>>>> +       default:
> >>>>>>>> +               return -ENXIO;
> >>>>>>>> +       }
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>>      int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >>>>>>>>      {
> >>>>>>>> -       return -ENOIOCTLCMD;
> >>>>>>>> +       struct kvm *kvm = filp->private_data;
> >>>>>>>> +       void __user *argp = (void __user *)arg;
> >>>>>>>> +       struct kvm_device_attr attr;
> >>>>>>>> +
> >>>>>>>> +       switch (ioctl) {
> >>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
> >>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> >>>>>>>> +                       return -EFAULT;
> >>>>>>>> +
> >>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
> >>>>>>>> +       default:
> >>>>>>>> +               return -EINVAL;
> >>>>>>>> +       }
> >>>>>>>>      }
> >>>>>>>> --
> >>>>>>>> 2.39.3
> >>>>>>>>
> >>>>>>
> >>>>
> >>
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/2 下午3:28, Huacai Chen wrote:
> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/7/2 上午10:34, Huacai Chen wrote:
>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
>>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> Huacai,
>>>>>>
>>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Huacai,
>>>>>>>>
>>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>>>>>> Hi, Bibo,
>>>>>>>>>
>>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>>>>>> be done even if VM is not created, and also can be done by any
>>>>>>>>>> thread besides VCPU threads.
>>>>>>>>>>
>>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>>>>>
>>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>>>>>> three sub-features relative with LBT are added as following:
>>>>>>>>>>       KVM_LOONGARCH_VM_FEAT_X86BT
>>>>>>>>>>       KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>>>>>>>       KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>> ---
>>>>>>>>>>       arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>>>>>>>       arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>>>>>>>       arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>>>>>>>       3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>>>>>>>       #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>>>>>>>       #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>>>>>>>
>>>>>>>>>> +/* Device Control API on vm fd */
>>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>>>>>>>> +
>>>>>>>>>>       /* Device Control API on vcpu fd */
>>>>>>>>>>       #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>>>>>>>       #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>>>>>> have a chance to change it now.
>>>>>>>>
>>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>>>>>> has is own different gpa address.
>>>>>>> Then leave this as a vm feature.
>>>>>>>
>>>>>>>>
>>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>>>>>> uapi breaking now, still will be in future if we cannot control this.
>>>>>>> UAPI changing before the first release is allowed, which means, we can
>>>>>>> change this before the 6.10-final, but cannot change it after
>>>>>>> 6.10-final.
>>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
>>>>>> about this, with my experience with uapi change, there is only newly
>>>>>> added or removed deprecated years ago.
>>>>>>
>>>>>> Is there any documentation about UAPI change rules?
>>>>> No document, but learn from my more than 10 years upstream experience.
>>>> Can you show me an example about with your rich upstream experience?
>>> A simple example,
>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
>>> 6.9-final, but not after that.
>>>
>>> Before the first release, the code status is treated as "unstable", so
>>> revert, modify is allowed. But after the first release, even if an
>>> "error" should also be treated as a "bad feature".
>> Huacai,
>>
>> Thanks for showing the example.
>>
>> For this issue, Can we adding new uapi and mark the old as deprecated?
>> so that it can be removed after years.
> Unnecessary, just remove the old one. Deprecation is for the usage
> after the first release.
> 
>>
>> For me, it is too frequent to revert the old uapi, it is not bug and
>> only that we have better method now. Also QEMU has synchronized the uapi
>> to its directory already.
> QEMU also hasn't been released after synchronizing the uapi, so it is
> OK to remove the old api now.
No, I will not do such thing. It is just a joke to revert the uapi.

So just create new world and old world on Loongarch system again?

Regards
Bibo, Mao

> 
> Huacai
> 
>>
>> Regards
>> Bibo, Mao
>>>
>>> Huacai
>>>
>>>
>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> How about adding new extra features capability for VM such as?
>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
>>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>>>>>> features.
>>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
>>>>>>        KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>>>>>>        KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>>>>>> These features are all cpu features, at the same time they are VM features.
>>>>>>
>>>>>> If they are cpu features, how does VMM detect validity of these features
>>>>>> passing from command line? After all VCPUs are created and send bootup
>>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
>>>>>> detect feature validity if they are VM features also.
>>>>>>
>>>>>> To be honest, I am not familiar with KVM still, only get further
>>>>>> understanding after actual problems solving. Welcome to give comments,
>>>>>> however please read more backgroud if you insist on, else there will be
>>>>>> endless argument again.
>>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
>>>>> haven't insisted on whether they should be vcpu features or vm
>>>>> features.
>>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
>>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
>>>>
>>>> What is the usage about CPUCFG capability used for VM feature? It is not
>>>> a detailed feature, it is only feature-set indicator like cpuid.
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo, Mao
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Bibo Mao
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>>>>>>                              *v |= CPUCFG2_LSX;
>>>>>>>>>>                      if (cpu_has_lasx)
>>>>>>>>>>                              *v |= CPUCFG2_LASX;
>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>>>>>>>
>>>>>>>>>>                      return 0;
>>>>>>>>>>              case LOONGARCH_CPUCFG3:
>>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
>>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
>>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>>>>>>>              return r;
>>>>>>>>>>       }
>>>>>>>>>>
>>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>> +{
>>>>>>>>>> +       switch (attr->attr) {
>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>> +                       return 0;
>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>> +                       return 0;
>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>> +                       return 0;
>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>> +       default:
>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>> +       }
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>> +{
>>>>>>>>>> +       switch (attr->group) {
>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>>>>>>>> +       default:
>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>> +       }
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>>       int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>>>>>>>       {
>>>>>>>>>> -       return -ENOIOCTLCMD;
>>>>>>>>>> +       struct kvm *kvm = filp->private_data;
>>>>>>>>>> +       void __user *argp = (void __user *)arg;
>>>>>>>>>> +       struct kvm_device_attr attr;
>>>>>>>>>> +
>>>>>>>>>> +       switch (ioctl) {
>>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>>>>>>>> +                       return -EFAULT;
>>>>>>>>>> +
>>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>>>>>>>> +       default:
>>>>>>>>>> +               return -EINVAL;
>>>>>>>>>> +       }
>>>>>>>>>>       }
>>>>>>>>>> --
>>>>>>>>>> 2.39.3
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/7/2 下午3:28, Huacai Chen wrote:
> > On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/7/2 上午10:34, Huacai Chen wrote:
> >>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
> >>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>> Huacai,
> >>>>>>
> >>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
> >>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Huacai,
> >>>>>>>>
> >>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
> >>>>>>>>> Hi, Bibo,
> >>>>>>>>>
> >>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
> >>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
> >>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
> >>>>>>>>>> already. So LBT feature detection for VM is added also, it can
> >>>>>>>>>> be done even if VM is not created, and also can be done by any
> >>>>>>>>>> thread besides VCPU threads.
> >>>>>>>>>>
> >>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
> >>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
> >>>>>>>>>>
> >>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> >>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> >>>>>>>>>> three sub-features relative with LBT are added as following:
> >>>>>>>>>>       KVM_LOONGARCH_VM_FEAT_X86BT
> >>>>>>>>>>       KVM_LOONGARCH_VM_FEAT_ARMBT
> >>>>>>>>>>       KVM_LOONGARCH_VM_FEAT_MIPSBT
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>>>> ---
> >>>>>>>>>>       arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
> >>>>>>>>>>       arch/loongarch/kvm/vcpu.c             |  6 ++++
> >>>>>>>>>>       arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
> >>>>>>>>>>       3 files changed, 55 insertions(+), 1 deletion(-)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
> >>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
> >>>>>>>>>>       #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
> >>>>>>>>>>       #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
> >>>>>>>>>>
> >>>>>>>>>> +/* Device Control API on vm fd */
> >>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> >>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> >>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> >>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> >>>>>>>>>> +
> >>>>>>>>>>       /* Device Control API on vcpu fd */
> >>>>>>>>>>       #define KVM_LOONGARCH_VCPU_CPUCFG      0
> >>>>>>>>>>       #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> >>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
> >>>>>>>>> above two also be vm features. Though this is an UAPI change, but
> >>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> >>>>>>>>> have a chance to change it now.
> >>>>>>>>
> >>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
> >>>>>>>> has is own different gpa address.
> >>>>>>> Then leave this as a vm feature.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
> >>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
> >>>>>>>> uapi breaking now, still will be in future if we cannot control this.
> >>>>>>> UAPI changing before the first release is allowed, which means, we can
> >>>>>>> change this before the 6.10-final, but cannot change it after
> >>>>>>> 6.10-final.
> >>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
> >>>>>> about this, with my experience with uapi change, there is only newly
> >>>>>> added or removed deprecated years ago.
> >>>>>>
> >>>>>> Is there any documentation about UAPI change rules?
> >>>>> No document, but learn from my more than 10 years upstream experience.
> >>>> Can you show me an example about with your rich upstream experience?
> >>> A simple example,
> >>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
> >>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
> >>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
> >>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
> >>> 6.9-final, but not after that.
> >>>
> >>> Before the first release, the code status is treated as "unstable", so
> >>> revert, modify is allowed. But after the first release, even if an
> >>> "error" should also be treated as a "bad feature".
> >> Huacai,
> >>
> >> Thanks for showing the example.
> >>
> >> For this issue, Can we adding new uapi and mark the old as deprecated?
> >> so that it can be removed after years.
> > Unnecessary, just remove the old one. Deprecation is for the usage
> > after the first release.
> >
> >>
> >> For me, it is too frequent to revert the old uapi, it is not bug and
> >> only that we have better method now. Also QEMU has synchronized the uapi
> >> to its directory already.
> > QEMU also hasn't been released after synchronizing the uapi, so it is
> > OK to remove the old api now.
> No, I will not do such thing. It is just a joke to revert the uapi.
>
> So just create new world and old world on Loongarch system again?
Again, code status before the first release is *unstable*, that status
is not enough to be a "world".

It's your responsibility to make a good design at the beginning, but
you fail to do that. Fortunately we are before the first release;
unfortunately you don't want to do that.


Huacai

>
> Regards
> Bibo, Mao
>
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo, Mao
> >>>
> >>> Huacai
> >>>
> >>>
> >>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> How about adding new extra features capability for VM such as?
> >>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
> >>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
> >>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
> >>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
> >>>>>>> features.
> >>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
> >>>>>>        KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
> >>>>>>        KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
> >>>>>> These features are all cpu features, at the same time they are VM features.
> >>>>>>
> >>>>>> If they are cpu features, how does VMM detect validity of these features
> >>>>>> passing from command line? After all VCPUs are created and send bootup
> >>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
> >>>>>> detect feature validity if they are VM features also.
> >>>>>>
> >>>>>> To be honest, I am not familiar with KVM still, only get further
> >>>>>> understanding after actual problems solving. Welcome to give comments,
> >>>>>> however please read more backgroud if you insist on, else there will be
> >>>>>> endless argument again.
> >>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
> >>>>> haven't insisted on whether they should be vcpu features or vm
> >>>>> features.
> >>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
> >>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
> >>>>
> >>>> What is the usage about CPUCFG capability used for VM feature? It is not
> >>>> a detailed feature, it is only feature-set indicator like cpuid.
> >>>>
> >>>> Regards
> >>>> Bibo Mao
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>>
> >>>>>> Regards
> >>>>>> Bibo, Mao
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Regards
> >>>>>>>> Bibo Mao
> >>>>>>>>>
> >>>>>>>>> Huacai
> >>>>>>>>>
> >>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
> >>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
> >>>>>>>>>>                              *v |= CPUCFG2_LSX;
> >>>>>>>>>>                      if (cpu_has_lasx)
> >>>>>>>>>>                              *v |= CPUCFG2_LASX;
> >>>>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
> >>>>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
> >>>>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
> >>>>>>>>>>
> >>>>>>>>>>                      return 0;
> >>>>>>>>>>              case LOONGARCH_CPUCFG3:
> >>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
> >>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
> >>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
> >>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>>>>>>>>>              return r;
> >>>>>>>>>>       }
> >>>>>>>>>>
> >>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>>>> +{
> >>>>>>>>>> +       switch (attr->attr) {
> >>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> >>>>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>>>> +                       return 0;
> >>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> >>>>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>>>> +                       return 0;
> >>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> >>>>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>>>> +                       return 0;
> >>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>> +       default:
> >>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>> +       }
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>>>> +{
> >>>>>>>>>> +       switch (attr->group) {
> >>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> >>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
> >>>>>>>>>> +       default:
> >>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>> +       }
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>>       int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >>>>>>>>>>       {
> >>>>>>>>>> -       return -ENOIOCTLCMD;
> >>>>>>>>>> +       struct kvm *kvm = filp->private_data;
> >>>>>>>>>> +       void __user *argp = (void __user *)arg;
> >>>>>>>>>> +       struct kvm_device_attr attr;
> >>>>>>>>>> +
> >>>>>>>>>> +       switch (ioctl) {
> >>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
> >>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> >>>>>>>>>> +                       return -EFAULT;
> >>>>>>>>>> +
> >>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
> >>>>>>>>>> +       default:
> >>>>>>>>>> +               return -EINVAL;
> >>>>>>>>>> +       }
> >>>>>>>>>>       }
> >>>>>>>>>> --
> >>>>>>>>>> 2.39.3
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/2 下午11:43, Huacai Chen wrote:
> On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/7/2 下午3:28, Huacai Chen wrote:
>>> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/7/2 上午10:34, Huacai Chen wrote:
>>>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
>>>>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>> Huacai,
>>>>>>>>
>>>>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>>>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Huacai,
>>>>>>>>>>
>>>>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>>>>>>>> Hi, Bibo,
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>>>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>>>>>>>> be done even if VM is not created, and also can be done by any
>>>>>>>>>>>> thread besides VCPU threads.
>>>>>>>>>>>>
>>>>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>>>>>>>
>>>>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>>>>>>>> three sub-features relative with LBT are added as following:
>>>>>>>>>>>>        KVM_LOONGARCH_VM_FEAT_X86BT
>>>>>>>>>>>>        KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>>>>>>>>>        KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>>>> ---
>>>>>>>>>>>>        arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>>>>>>>>>        arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>>>>>>>>>        arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>>>>>>>>>        3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>>>>>>>>>        #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>>>>>>>>>        #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>>>>>>>>>
>>>>>>>>>>>> +/* Device Control API on vm fd */
>>>>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>>>>>>>>>> +
>>>>>>>>>>>>        /* Device Control API on vcpu fd */
>>>>>>>>>>>>        #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>>>>>>>>>        #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>>>>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>>>>>>>> have a chance to change it now.
>>>>>>>>>>
>>>>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>>>>>>>> has is own different gpa address.
>>>>>>>>> Then leave this as a vm feature.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>>>>>>>> uapi breaking now, still will be in future if we cannot control this.
>>>>>>>>> UAPI changing before the first release is allowed, which means, we can
>>>>>>>>> change this before the 6.10-final, but cannot change it after
>>>>>>>>> 6.10-final.
>>>>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
>>>>>>>> about this, with my experience with uapi change, there is only newly
>>>>>>>> added or removed deprecated years ago.
>>>>>>>>
>>>>>>>> Is there any documentation about UAPI change rules?
>>>>>>> No document, but learn from my more than 10 years upstream experience.
>>>>>> Can you show me an example about with your rich upstream experience?
>>>>> A simple example,
>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
>>>>> 6.9-final, but not after that.
>>>>>
>>>>> Before the first release, the code status is treated as "unstable", so
>>>>> revert, modify is allowed. But after the first release, even if an
>>>>> "error" should also be treated as a "bad feature".
>>>> Huacai,
>>>>
>>>> Thanks for showing the example.
>>>>
>>>> For this issue, Can we adding new uapi and mark the old as deprecated?
>>>> so that it can be removed after years.
>>> Unnecessary, just remove the old one. Deprecation is for the usage
>>> after the first release.
>>>
>>>>
>>>> For me, it is too frequent to revert the old uapi, it is not bug and
>>>> only that we have better method now. Also QEMU has synchronized the uapi
>>>> to its directory already.
>>> QEMU also hasn't been released after synchronizing the uapi, so it is
>>> OK to remove the old api now.
>> No, I will not do such thing. It is just a joke to revert the uapi.
>>
>> So just create new world and old world on Loongarch system again?
> Again, code status before the first release is *unstable*, that status
> is not enough to be a "world".
> 
> It's your responsibility to make a good design at the beginning, but
> you fail to do that. Fortunately we are before the first release;
> unfortunately you don't want to do that.
Yes, this is flaw at the beginning, however it can works and new abi can 
be added.

If there is no serious bug and it is synced to QEMU already, I am not 
willing to revert uabi. Different projects have its own schedule plan, 
that is one reason. The most important reason may be that different 
peoples have different ways handling these issues.

Regards
Bibo, Mao
> 
> 
> Huacai
> 
>>
>> Regards
>> Bibo, Mao
>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo, Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> How about adding new extra features capability for VM such as?
>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
>>>>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>>>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>>>>>>>> features.
>>>>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
>>>>>>>>         KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>>>>>>>>         KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>>>>>>>> These features are all cpu features, at the same time they are VM features.
>>>>>>>>
>>>>>>>> If they are cpu features, how does VMM detect validity of these features
>>>>>>>> passing from command line? After all VCPUs are created and send bootup
>>>>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
>>>>>>>> detect feature validity if they are VM features also.
>>>>>>>>
>>>>>>>> To be honest, I am not familiar with KVM still, only get further
>>>>>>>> understanding after actual problems solving. Welcome to give comments,
>>>>>>>> however please read more backgroud if you insist on, else there will be
>>>>>>>> endless argument again.
>>>>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
>>>>>>> haven't insisted on whether they should be vcpu features or vm
>>>>>>> features.
>>>>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
>>>>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
>>>>>>
>>>>>> What is the usage about CPUCFG capability used for VM feature? It is not
>>>>>> a detailed feature, it is only feature-set indicator like cpuid.
>>>>>>
>>>>>> Regards
>>>>>> Bibo Mao
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Bibo, Mao
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Bibo Mao
>>>>>>>>>>>
>>>>>>>>>>> Huacai
>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>>>>>>>>                               *v |= CPUCFG2_LSX;
>>>>>>>>>>>>                       if (cpu_has_lasx)
>>>>>>>>>>>>                               *v |= CPUCFG2_LASX;
>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>>>>>>>>>
>>>>>>>>>>>>                       return 0;
>>>>>>>>>>>>               case LOONGARCH_CPUCFG3:
>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>>>>>>>>>               return r;
>>>>>>>>>>>>        }
>>>>>>>>>>>>
>>>>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +       switch (attr->attr) {
>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>> +       default:
>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>> +       }
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +       switch (attr->group) {
>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>>>>>>>>>> +       default:
>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>> +       }
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>>        int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>>>>>>>>>        {
>>>>>>>>>>>> -       return -ENOIOCTLCMD;
>>>>>>>>>>>> +       struct kvm *kvm = filp->private_data;
>>>>>>>>>>>> +       void __user *argp = (void __user *)arg;
>>>>>>>>>>>> +       struct kvm_device_attr attr;
>>>>>>>>>>>> +
>>>>>>>>>>>> +       switch (ioctl) {
>>>>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>>>>>>>>>> +                       return -EFAULT;
>>>>>>>>>>>> +
>>>>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>>>>>>>>>> +       default:
>>>>>>>>>>>> +               return -EINVAL;
>>>>>>>>>>>> +       }
>>>>>>>>>>>>        }
>>>>>>>>>>>> --
>>>>>>>>>>>> 2.39.3
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

On Wed, Jul 3, 2024 at 11:15 AM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/7/2 下午11:43, Huacai Chen wrote:
> > On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/7/2 下午3:28, Huacai Chen wrote:
> >>> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2024/7/2 上午10:34, Huacai Chen wrote:
> >>>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
> >>>>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>> Huacai,
> >>>>>>>>
> >>>>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
> >>>>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Huacai,
> >>>>>>>>>>
> >>>>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
> >>>>>>>>>>> Hi, Bibo,
> >>>>>>>>>>>
> >>>>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
> >>>>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
> >>>>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
> >>>>>>>>>>>> already. So LBT feature detection for VM is added also, it can
> >>>>>>>>>>>> be done even if VM is not created, and also can be done by any
> >>>>>>>>>>>> thread besides VCPU threads.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
> >>>>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> >>>>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> >>>>>>>>>>>> three sub-features relative with LBT are added as following:
> >>>>>>>>>>>>        KVM_LOONGARCH_VM_FEAT_X86BT
> >>>>>>>>>>>>        KVM_LOONGARCH_VM_FEAT_ARMBT
> >>>>>>>>>>>>        KVM_LOONGARCH_VM_FEAT_MIPSBT
> >>>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>>>>>> ---
> >>>>>>>>>>>>        arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
> >>>>>>>>>>>>        arch/loongarch/kvm/vcpu.c             |  6 ++++
> >>>>>>>>>>>>        arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
> >>>>>>>>>>>>        3 files changed, 55 insertions(+), 1 deletion(-)
> >>>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
> >>>>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
> >>>>>>>>>>>>        #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
> >>>>>>>>>>>>        #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
> >>>>>>>>>>>>
> >>>>>>>>>>>> +/* Device Control API on vm fd */
> >>>>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> >>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> >>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> >>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> >>>>>>>>>>>> +
> >>>>>>>>>>>>        /* Device Control API on vcpu fd */
> >>>>>>>>>>>>        #define KVM_LOONGARCH_VCPU_CPUCFG      0
> >>>>>>>>>>>>        #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> >>>>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
> >>>>>>>>>>> above two also be vm features. Though this is an UAPI change, but
> >>>>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> >>>>>>>>>>> have a chance to change it now.
> >>>>>>>>>>
> >>>>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
> >>>>>>>>>> has is own different gpa address.
> >>>>>>>>> Then leave this as a vm feature.
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
> >>>>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
> >>>>>>>>>> uapi breaking now, still will be in future if we cannot control this.
> >>>>>>>>> UAPI changing before the first release is allowed, which means, we can
> >>>>>>>>> change this before the 6.10-final, but cannot change it after
> >>>>>>>>> 6.10-final.
> >>>>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
> >>>>>>>> about this, with my experience with uapi change, there is only newly
> >>>>>>>> added or removed deprecated years ago.
> >>>>>>>>
> >>>>>>>> Is there any documentation about UAPI change rules?
> >>>>>>> No document, but learn from my more than 10 years upstream experience.
> >>>>>> Can you show me an example about with your rich upstream experience?
> >>>>> A simple example,
> >>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
> >>>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
> >>>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
> >>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
> >>>>> 6.9-final, but not after that.
> >>>>>
> >>>>> Before the first release, the code status is treated as "unstable", so
> >>>>> revert, modify is allowed. But after the first release, even if an
> >>>>> "error" should also be treated as a "bad feature".
> >>>> Huacai,
> >>>>
> >>>> Thanks for showing the example.
> >>>>
> >>>> For this issue, Can we adding new uapi and mark the old as deprecated?
> >>>> so that it can be removed after years.
> >>> Unnecessary, just remove the old one. Deprecation is for the usage
> >>> after the first release.
> >>>
> >>>>
> >>>> For me, it is too frequent to revert the old uapi, it is not bug and
> >>>> only that we have better method now. Also QEMU has synchronized the uapi
> >>>> to its directory already.
> >>> QEMU also hasn't been released after synchronizing the uapi, so it is
> >>> OK to remove the old api now.
> >> No, I will not do such thing. It is just a joke to revert the uapi.
> >>
> >> So just create new world and old world on Loongarch system again?
> > Again, code status before the first release is *unstable*, that status
> > is not enough to be a "world".
> >
> > It's your responsibility to make a good design at the beginning, but
> > you fail to do that. Fortunately we are before the first release;
> > unfortunately you don't want to do that.
> Yes, this is flaw at the beginning, however it can works and new abi can
> be added.
>
> If there is no serious bug and it is synced to QEMU already, I am not
> willing to revert uabi. Different projects have its own schedule plan,
> that is one reason. The most important reason may be that different
> peoples have different ways handling these issues.
In another thread I found that Jiaxun said he has a solution to make
LBT be a vcpu feature and still works well. However, that may take
some time and is too late for 6.11.

But we have another choice now: just remove the UAPI and vm.c parts in
this series, let the LBT main parts be upstream in 6.11, and then
solve other problems after 6.11. Even if Jiaxun's solution isn't
usable, we can still use this old vm feature solution then.


Huacai
>
> Regards
> Bibo, Mao
> >
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo, Mao
> >>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo, Mao
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>
> >>>>>>>
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> How about adding new extra features capability for VM such as?
> >>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
> >>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
> >>>>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
> >>>>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
> >>>>>>>>> features.
> >>>>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
> >>>>>>>>         KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
> >>>>>>>>         KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
> >>>>>>>> These features are all cpu features, at the same time they are VM features.
> >>>>>>>>
> >>>>>>>> If they are cpu features, how does VMM detect validity of these features
> >>>>>>>> passing from command line? After all VCPUs are created and send bootup
> >>>>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
> >>>>>>>> detect feature validity if they are VM features also.
> >>>>>>>>
> >>>>>>>> To be honest, I am not familiar with KVM still, only get further
> >>>>>>>> understanding after actual problems solving. Welcome to give comments,
> >>>>>>>> however please read more backgroud if you insist on, else there will be
> >>>>>>>> endless argument again.
> >>>>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
> >>>>>>> haven't insisted on whether they should be vcpu features or vm
> >>>>>>> features.
> >>>>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
> >>>>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
> >>>>>>
> >>>>>> What is the usage about CPUCFG capability used for VM feature? It is not
> >>>>>> a detailed feature, it is only feature-set indicator like cpuid.
> >>>>>>
> >>>>>> Regards
> >>>>>> Bibo Mao
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Regards
> >>>>>>>> Bibo, Mao
> >>>>>>>>>
> >>>>>>>>> Huacai
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Regards
> >>>>>>>>>> Bibo Mao
> >>>>>>>>>>>
> >>>>>>>>>>> Huacai
> >>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
> >>>>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
> >>>>>>>>>>>>                               *v |= CPUCFG2_LSX;
> >>>>>>>>>>>>                       if (cpu_has_lasx)
> >>>>>>>>>>>>                               *v |= CPUCFG2_LASX;
> >>>>>>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
> >>>>>>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
> >>>>>>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
> >>>>>>>>>>>>
> >>>>>>>>>>>>                       return 0;
> >>>>>>>>>>>>               case LOONGARCH_CPUCFG3:
> >>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >>>>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
> >>>>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
> >>>>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
> >>>>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>>>>>>>>>>>               return r;
> >>>>>>>>>>>>        }
> >>>>>>>>>>>>
> >>>>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> +       switch (attr->attr) {
> >>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> >>>>>>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>>>>>> +                       return 0;
> >>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> >>>>>>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>>>>>> +                       return 0;
> >>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> >>>>>>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>>>>>> +                       return 0;
> >>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>> +       default:
> >>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>> +       }
> >>>>>>>>>>>> +}
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> +       switch (attr->group) {
> >>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> >>>>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
> >>>>>>>>>>>> +       default:
> >>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>> +       }
> >>>>>>>>>>>> +}
> >>>>>>>>>>>> +
> >>>>>>>>>>>>        int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >>>>>>>>>>>>        {
> >>>>>>>>>>>> -       return -ENOIOCTLCMD;
> >>>>>>>>>>>> +       struct kvm *kvm = filp->private_data;
> >>>>>>>>>>>> +       void __user *argp = (void __user *)arg;
> >>>>>>>>>>>> +       struct kvm_device_attr attr;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +       switch (ioctl) {
> >>>>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
> >>>>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> >>>>>>>>>>>> +                       return -EFAULT;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
> >>>>>>>>>>>> +       default:
> >>>>>>>>>>>> +               return -EINVAL;
> >>>>>>>>>>>> +       }
> >>>>>>>>>>>>        }
> >>>>>>>>>>>> --
> >>>>>>>>>>>> 2.39.3
> >>>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
>
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/3 下午11:35, Huacai Chen wrote:
> On Wed, Jul 3, 2024 at 11:15 AM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/7/2 下午11:43, Huacai Chen wrote:
>>> On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/7/2 下午3:28, Huacai Chen wrote:
>>>>> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2024/7/2 上午10:34, Huacai Chen wrote:
>>>>>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
>>>>>>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>> Huacai,
>>>>>>>>>>
>>>>>>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>>>>>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Huacai,
>>>>>>>>>>>>
>>>>>>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>>>>>>>>>> Hi, Bibo,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>>>>>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>>>>>>>>>> be done even if VM is not created, and also can be done by any
>>>>>>>>>>>>>> thread besides VCPU threads.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>>>>>>>>>> three sub-features relative with LBT are added as following:
>>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_X86BT
>>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>         arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>>>>>>>>>>>         arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>>>>>>>>>>>         arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>>>>>>>>>>>         3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>>>>>>>>>>>         #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>>>>>>>>>>>         #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> +/* Device Control API on vm fd */
>>>>>>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>         /* Device Control API on vcpu fd */
>>>>>>>>>>>>>>         #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>>>>>>>>>>>         #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>>>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>>>>>>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>>>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>>>>>>>>>> have a chance to change it now.
>>>>>>>>>>>>
>>>>>>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>>>>>>>>>> has is own different gpa address.
>>>>>>>>>>> Then leave this as a vm feature.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>>>>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>>>>>>>>>> uapi breaking now, still will be in future if we cannot control this.
>>>>>>>>>>> UAPI changing before the first release is allowed, which means, we can
>>>>>>>>>>> change this before the 6.10-final, but cannot change it after
>>>>>>>>>>> 6.10-final.
>>>>>>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
>>>>>>>>>> about this, with my experience with uapi change, there is only newly
>>>>>>>>>> added or removed deprecated years ago.
>>>>>>>>>>
>>>>>>>>>> Is there any documentation about UAPI change rules?
>>>>>>>>> No document, but learn from my more than 10 years upstream experience.
>>>>>>>> Can you show me an example about with your rich upstream experience?
>>>>>>> A simple example,
>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
>>>>>>> 6.9-final, but not after that.
>>>>>>>
>>>>>>> Before the first release, the code status is treated as "unstable", so
>>>>>>> revert, modify is allowed. But after the first release, even if an
>>>>>>> "error" should also be treated as a "bad feature".
>>>>>> Huacai,
>>>>>>
>>>>>> Thanks for showing the example.
>>>>>>
>>>>>> For this issue, Can we adding new uapi and mark the old as deprecated?
>>>>>> so that it can be removed after years.
>>>>> Unnecessary, just remove the old one. Deprecation is for the usage
>>>>> after the first release.
>>>>>
>>>>>>
>>>>>> For me, it is too frequent to revert the old uapi, it is not bug and
>>>>>> only that we have better method now. Also QEMU has synchronized the uapi
>>>>>> to its directory already.
>>>>> QEMU also hasn't been released after synchronizing the uapi, so it is
>>>>> OK to remove the old api now.
>>>> No, I will not do such thing. It is just a joke to revert the uapi.
>>>>
>>>> So just create new world and old world on Loongarch system again?
>>> Again, code status before the first release is *unstable*, that status
>>> is not enough to be a "world".
>>>
>>> It's your responsibility to make a good design at the beginning, but
>>> you fail to do that. Fortunately we are before the first release;
>>> unfortunately you don't want to do that.
>> Yes, this is flaw at the beginning, however it can works and new abi can
>> be added.
>>
>> If there is no serious bug and it is synced to QEMU already, I am not
>> willing to revert uabi. Different projects have its own schedule plan,
>> that is one reason. The most important reason may be that different
>> peoples have different ways handling these issues.
> In another thread I found that Jiaxun said he has a solution to make
> LBT be a vcpu feature and still works well. However, that may take
> some time and is too late for 6.11.

It is welcome if Jiaxun provide patch for host machine type, I have no 
time give any feedback with suggestion of Jianxun now.

> 
> But we have another choice now: just remove the UAPI and vm.c parts in
> this series, let the LBT main parts be upstream in 6.11, and then
> solve other problems after 6.11. Even if Jiaxun's solution isn't
> usable, we can still use this old vm feature solution then.

There is not useul if only LBT main part goes upstream. VMM cannot use 
LBT if control part is not merged.

 From another side, what do you like to do? Reviewing patch of others 
and give comments whatever grammar spelling or useful suggestions, or 
Writing patch which needs much efforts rather than somethings like 
feature configuration, BSP drivers.

Regards
Bibo Mao

> 
> 
> Huacai
>>
>> Regards
>> Bibo, Mao
>>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo, Mao
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo, Mao
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> How about adding new extra features capability for VM such as?
>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
>>>>>>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>>>>>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>>>>>>>>>> features.
>>>>>>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
>>>>>>>>>>          KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>>>>>>>>>>          KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>>>>>>>>>> These features are all cpu features, at the same time they are VM features.
>>>>>>>>>>
>>>>>>>>>> If they are cpu features, how does VMM detect validity of these features
>>>>>>>>>> passing from command line? After all VCPUs are created and send bootup
>>>>>>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
>>>>>>>>>> detect feature validity if they are VM features also.
>>>>>>>>>>
>>>>>>>>>> To be honest, I am not familiar with KVM still, only get further
>>>>>>>>>> understanding after actual problems solving. Welcome to give comments,
>>>>>>>>>> however please read more backgroud if you insist on, else there will be
>>>>>>>>>> endless argument again.
>>>>>>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
>>>>>>>>> haven't insisted on whether they should be vcpu features or vm
>>>>>>>>> features.
>>>>>>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
>>>>>>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
>>>>>>>>
>>>>>>>> What is the usage about CPUCFG capability used for VM feature? It is not
>>>>>>>> a detailed feature, it is only feature-set indicator like cpuid.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Bibo Mao
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Bibo, Mao
>>>>>>>>>>>
>>>>>>>>>>> Huacai
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Regards
>>>>>>>>>>>> Bibo Mao
>>>>>>>>>>>>>
>>>>>>>>>>>>> Huacai
>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>>>>>>>>>>                                *v |= CPUCFG2_LSX;
>>>>>>>>>>>>>>                        if (cpu_has_lasx)
>>>>>>>>>>>>>>                                *v |= CPUCFG2_LASX;
>>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
>>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
>>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>                        return 0;
>>>>>>>>>>>>>>                case LOONGARCH_CPUCFG3:
>>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>>>>>>>>>>>                return r;
>>>>>>>>>>>>>>         }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>> +       switch (attr->attr) {
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>> +       switch (attr->group) {
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>>>>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>         int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>>>>>>>>>>>         {
>>>>>>>>>>>>>> -       return -ENOIOCTLCMD;
>>>>>>>>>>>>>> +       struct kvm *kvm = filp->private_data;
>>>>>>>>>>>>>> +       void __user *argp = (void __user *)arg;
>>>>>>>>>>>>>> +       struct kvm_device_attr attr;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +       switch (ioctl) {
>>>>>>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>>>>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>>>>>>>>>>>> +                       return -EFAULT;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>> +               return -EINVAL;
>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>>         }
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> 2.39.3
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Huacai Chen]

On Thu, Jul 4, 2024 at 9:24 AM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/7/3 下午11:35, Huacai Chen wrote:
> > On Wed, Jul 3, 2024 at 11:15 AM maobibo <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2024/7/2 下午11:43, Huacai Chen wrote:
> >>> On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2024/7/2 下午3:28, Huacai Chen wrote:
> >>>>> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 2024/7/2 上午10:34, Huacai Chen wrote:
> >>>>>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
> >>>>>>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Huacai,
> >>>>>>>>>>
> >>>>>>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
> >>>>>>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Huacai,
> >>>>>>>>>>>>
> >>>>>>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
> >>>>>>>>>>>>> Hi, Bibo,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
> >>>>>>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
> >>>>>>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
> >>>>>>>>>>>>>> already. So LBT feature detection for VM is added also, it can
> >>>>>>>>>>>>>> be done even if VM is not created, and also can be done by any
> >>>>>>>>>>>>>> thread besides VCPU threads.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
> >>>>>>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
> >>>>>>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
> >>>>>>>>>>>>>> three sub-features relative with LBT are added as following:
> >>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_X86BT
> >>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_ARMBT
> >>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_MIPSBT
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>         arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
> >>>>>>>>>>>>>>         arch/loongarch/kvm/vcpu.c             |  6 ++++
> >>>>>>>>>>>>>>         arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
> >>>>>>>>>>>>>>         3 files changed, 55 insertions(+), 1 deletion(-)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
> >>>>>>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> >>>>>>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
> >>>>>>>>>>>>>>         #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
> >>>>>>>>>>>>>>         #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> +/* Device Control API on vm fd */
> >>>>>>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
> >>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
> >>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
> >>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>         /* Device Control API on vcpu fd */
> >>>>>>>>>>>>>>         #define KVM_LOONGARCH_VCPU_CPUCFG      0
> >>>>>>>>>>>>>>         #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
> >>>>>>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
> >>>>>>>>>>>>> above two also be vm features. Though this is an UAPI change, but
> >>>>>>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
> >>>>>>>>>>>>> have a chance to change it now.
> >>>>>>>>>>>>
> >>>>>>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
> >>>>>>>>>>>> has is own different gpa address.
> >>>>>>>>>>> Then leave this as a vm feature.
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
> >>>>>>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
> >>>>>>>>>>>> uapi breaking now, still will be in future if we cannot control this.
> >>>>>>>>>>> UAPI changing before the first release is allowed, which means, we can
> >>>>>>>>>>> change this before the 6.10-final, but cannot change it after
> >>>>>>>>>>> 6.10-final.
> >>>>>>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
> >>>>>>>>>> about this, with my experience with uapi change, there is only newly
> >>>>>>>>>> added or removed deprecated years ago.
> >>>>>>>>>>
> >>>>>>>>>> Is there any documentation about UAPI change rules?
> >>>>>>>>> No document, but learn from my more than 10 years upstream experience.
> >>>>>>>> Can you show me an example about with your rich upstream experience?
> >>>>>>> A simple example,
> >>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
> >>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
> >>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
> >>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
> >>>>>>> 6.9-final, but not after that.
> >>>>>>>
> >>>>>>> Before the first release, the code status is treated as "unstable", so
> >>>>>>> revert, modify is allowed. But after the first release, even if an
> >>>>>>> "error" should also be treated as a "bad feature".
> >>>>>> Huacai,
> >>>>>>
> >>>>>> Thanks for showing the example.
> >>>>>>
> >>>>>> For this issue, Can we adding new uapi and mark the old as deprecated?
> >>>>>> so that it can be removed after years.
> >>>>> Unnecessary, just remove the old one. Deprecation is for the usage
> >>>>> after the first release.
> >>>>>
> >>>>>>
> >>>>>> For me, it is too frequent to revert the old uapi, it is not bug and
> >>>>>> only that we have better method now. Also QEMU has synchronized the uapi
> >>>>>> to its directory already.
> >>>>> QEMU also hasn't been released after synchronizing the uapi, so it is
> >>>>> OK to remove the old api now.
> >>>> No, I will not do such thing. It is just a joke to revert the uapi.
> >>>>
> >>>> So just create new world and old world on Loongarch system again?
> >>> Again, code status before the first release is *unstable*, that status
> >>> is not enough to be a "world".
> >>>
> >>> It's your responsibility to make a good design at the beginning, but
> >>> you fail to do that. Fortunately we are before the first release;
> >>> unfortunately you don't want to do that.
> >> Yes, this is flaw at the beginning, however it can works and new abi can
> >> be added.
> >>
> >> If there is no serious bug and it is synced to QEMU already, I am not
> >> willing to revert uabi. Different projects have its own schedule plan,
> >> that is one reason. The most important reason may be that different
> >> peoples have different ways handling these issues.
> > In another thread I found that Jiaxun said he has a solution to make
> > LBT be a vcpu feature and still works well. However, that may take
> > some time and is too late for 6.11.
>
> It is welcome if Jiaxun provide patch for host machine type, I have no
> time give any feedback with suggestion of Jianxun now.
>
> >
> > But we have another choice now: just remove the UAPI and vm.c parts in
> > this series, let the LBT main parts be upstream in 6.11, and then
> > solve other problems after 6.11. Even if Jiaxun's solution isn't
> > usable, we can still use this old vm feature solution then.
>
> There is not useul if only LBT main part goes upstream. VMM cannot use
> LBT if control part is not merged.
There is no control part UAPI for LSX/LASX, but it works.
If you insist that all should be merged together, there is probably
not enough time for the 6.11 merge window.

Huacai

>
>  From another side, what do you like to do? Reviewing patch of others
> and give comments whatever grammar spelling or useful suggestions, or
> Writing patch which needs much efforts rather than somethings like
> feature configuration, BSP drivers.
>
> Regards
> Bibo Mao
>
> >
> >
> > Huacai
> >>
> >> Regards
> >> Bibo, Mao
> >>>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo, Mao
> >>>>
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>>
> >>>>>> Regards
> >>>>>> Bibo, Mao
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>>
> >>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> How about adding new extra features capability for VM such as?
> >>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
> >>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
> >>>>>>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
> >>>>>>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
> >>>>>>>>>>> features.
> >>>>>>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
> >>>>>>>>>>          KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
> >>>>>>>>>>          KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
> >>>>>>>>>> These features are all cpu features, at the same time they are VM features.
> >>>>>>>>>>
> >>>>>>>>>> If they are cpu features, how does VMM detect validity of these features
> >>>>>>>>>> passing from command line? After all VCPUs are created and send bootup
> >>>>>>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
> >>>>>>>>>> detect feature validity if they are VM features also.
> >>>>>>>>>>
> >>>>>>>>>> To be honest, I am not familiar with KVM still, only get further
> >>>>>>>>>> understanding after actual problems solving. Welcome to give comments,
> >>>>>>>>>> however please read more backgroud if you insist on, else there will be
> >>>>>>>>>> endless argument again.
> >>>>>>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
> >>>>>>>>> haven't insisted on whether they should be vcpu features or vm
> >>>>>>>>> features.
> >>>>>>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
> >>>>>>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
> >>>>>>>>
> >>>>>>>> What is the usage about CPUCFG capability used for VM feature? It is not
> >>>>>>>> a detailed feature, it is only feature-set indicator like cpuid.
> >>>>>>>>
> >>>>>>>> Regards
> >>>>>>>> Bibo Mao
> >>>>>>>>>
> >>>>>>>>> Huacai
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Regards
> >>>>>>>>>> Bibo, Mao
> >>>>>>>>>>>
> >>>>>>>>>>> Huacai
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Regards
> >>>>>>>>>>>> Bibo Mao
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Huacai
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
> >>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
> >>>>>>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
> >>>>>>>>>>>>>>                                *v |= CPUCFG2_LSX;
> >>>>>>>>>>>>>>                        if (cpu_has_lasx)
> >>>>>>>>>>>>>>                                *v |= CPUCFG2_LASX;
> >>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
> >>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
> >>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>                        return 0;
> >>>>>>>>>>>>>>                case LOONGARCH_CPUCFG3:
> >>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> >>>>>>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
> >>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
> >>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
> >>>>>>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >>>>>>>>>>>>>>                return r;
> >>>>>>>>>>>>>>         }
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>>>>>>>> +{
> >>>>>>>>>>>>>> +       switch (attr->attr) {
> >>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
> >>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
> >>>>>>>>>>>>>> +                       return 0;
> >>>>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
> >>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
> >>>>>>>>>>>>>> +                       return 0;
> >>>>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
> >>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
> >>>>>>>>>>>>>> +                       return 0;
> >>>>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>>>> +       default:
> >>>>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>>>> +       }
> >>>>>>>>>>>>>> +}
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> >>>>>>>>>>>>>> +{
> >>>>>>>>>>>>>> +       switch (attr->group) {
> >>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
> >>>>>>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
> >>>>>>>>>>>>>> +       default:
> >>>>>>>>>>>>>> +               return -ENXIO;
> >>>>>>>>>>>>>> +       }
> >>>>>>>>>>>>>> +}
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>         int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >>>>>>>>>>>>>>         {
> >>>>>>>>>>>>>> -       return -ENOIOCTLCMD;
> >>>>>>>>>>>>>> +       struct kvm *kvm = filp->private_data;
> >>>>>>>>>>>>>> +       void __user *argp = (void __user *)arg;
> >>>>>>>>>>>>>> +       struct kvm_device_attr attr;
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>> +       switch (ioctl) {
> >>>>>>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
> >>>>>>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
> >>>>>>>>>>>>>> +                       return -EFAULT;
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
> >>>>>>>>>>>>>> +       default:
> >>>>>>>>>>>>>> +               return -EINVAL;
> >>>>>>>>>>>>>> +       }
> >>>>>>>>>>>>>>         }
> >>>>>>>>>>>>>> --
> >>>>>>>>>>>>>> 2.39.3
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> >>
>
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/8 下午5:51, Huacai Chen wrote:
> On Thu, Jul 4, 2024 at 9:24 AM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/7/3 下午11:35, Huacai Chen wrote:
>>> On Wed, Jul 3, 2024 at 11:15 AM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/7/2 下午11:43, Huacai Chen wrote:
>>>>> On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2024/7/2 下午3:28, Huacai Chen wrote:
>>>>>>> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2024/7/2 上午10:34, Huacai Chen wrote:
>>>>>>>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
>>>>>>>>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Huacai,
>>>>>>>>>>>>
>>>>>>>>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>>>>>>>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Huacai,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>>>>>>>>>>>> Hi, Bibo,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>>>>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>>>>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>>>>>>>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>>>>>>>>>>>> be done even if VM is not created, and also can be done by any
>>>>>>>>>>>>>>>> thread besides VCPU threads.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>>>>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>>>>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>>>>>>>>>>>> three sub-features relative with LBT are added as following:
>>>>>>>>>>>>>>>>          KVM_LOONGARCH_VM_FEAT_X86BT
>>>>>>>>>>>>>>>>          KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>>>>>>>>>>>>>          KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>          arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>>>>>>>>>>>>>          arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>>>>>>>>>>>>>          arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>>>>>>>>>>>>>          3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>>>>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>>>>>>>>>>>>>          #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>>>>>>>>>>>>>          #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> +/* Device Control API on vm fd */
>>>>>>>>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>          /* Device Control API on vcpu fd */
>>>>>>>>>>>>>>>>          #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>>>>>>>>>>>>>          #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>>>>>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>>>>>>>>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>>>>>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>>>>>>>>>>>> have a chance to change it now.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>>>>>>>>>>>> has is own different gpa address.
>>>>>>>>>>>>> Then leave this as a vm feature.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>>>>>>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>>>>>>>>>>>> uapi breaking now, still will be in future if we cannot control this.
>>>>>>>>>>>>> UAPI changing before the first release is allowed, which means, we can
>>>>>>>>>>>>> change this before the 6.10-final, but cannot change it after
>>>>>>>>>>>>> 6.10-final.
>>>>>>>>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
>>>>>>>>>>>> about this, with my experience with uapi change, there is only newly
>>>>>>>>>>>> added or removed deprecated years ago.
>>>>>>>>>>>>
>>>>>>>>>>>> Is there any documentation about UAPI change rules?
>>>>>>>>>>> No document, but learn from my more than 10 years upstream experience.
>>>>>>>>>> Can you show me an example about with your rich upstream experience?
>>>>>>>>> A simple example,
>>>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
>>>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
>>>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
>>>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
>>>>>>>>> 6.9-final, but not after that.
>>>>>>>>>
>>>>>>>>> Before the first release, the code status is treated as "unstable", so
>>>>>>>>> revert, modify is allowed. But after the first release, even if an
>>>>>>>>> "error" should also be treated as a "bad feature".
>>>>>>>> Huacai,
>>>>>>>>
>>>>>>>> Thanks for showing the example.
>>>>>>>>
>>>>>>>> For this issue, Can we adding new uapi and mark the old as deprecated?
>>>>>>>> so that it can be removed after years.
>>>>>>> Unnecessary, just remove the old one. Deprecation is for the usage
>>>>>>> after the first release.
>>>>>>>
>>>>>>>>
>>>>>>>> For me, it is too frequent to revert the old uapi, it is not bug and
>>>>>>>> only that we have better method now. Also QEMU has synchronized the uapi
>>>>>>>> to its directory already.
>>>>>>> QEMU also hasn't been released after synchronizing the uapi, so it is
>>>>>>> OK to remove the old api now.
>>>>>> No, I will not do such thing. It is just a joke to revert the uapi.
>>>>>>
>>>>>> So just create new world and old world on Loongarch system again?
>>>>> Again, code status before the first release is *unstable*, that status
>>>>> is not enough to be a "world".
>>>>>
>>>>> It's your responsibility to make a good design at the beginning, but
>>>>> you fail to do that. Fortunately we are before the first release;
>>>>> unfortunately you don't want to do that.
>>>> Yes, this is flaw at the beginning, however it can works and new abi can
>>>> be added.
>>>>
>>>> If there is no serious bug and it is synced to QEMU already, I am not
>>>> willing to revert uabi. Different projects have its own schedule plan,
>>>> that is one reason. The most important reason may be that different
>>>> peoples have different ways handling these issues.
>>> In another thread I found that Jiaxun said he has a solution to make
>>> LBT be a vcpu feature and still works well. However, that may take
>>> some time and is too late for 6.11.
>>
>> It is welcome if Jiaxun provide patch for host machine type, I have no
>> time give any feedback with suggestion of Jianxun now.
>>
>>>
>>> But we have another choice now: just remove the UAPI and vm.c parts in
>>> this series, let the LBT main parts be upstream in 6.11, and then
>>> solve other problems after 6.11. Even if Jiaxun's solution isn't
>>> usable, we can still use this old vm feature solution then.
>>
>> There is not useul if only LBT main part goes upstream. VMM cannot use
>> LBT if control part is not merged.
> There is no control part UAPI for LSX/LASX, but it works.
LSX/LASX feature probing comes from CPU feature detecting.

> If you insist that all should be merged together, there is probably
> not enough time for the 6.11 merge window.
It is not so hurry for LBT feature, only that PMU function uses the same 
method, that will be delayed also. Almost all user visible features need 
feature detection.

Regards
Bibo Mao

> 
> Huacai
> 
>>
>>   From another side, what do you like to do? Reviewing patch of others
>> and give comments whatever grammar spelling or useful suggestions, or
>> Writing patch which needs much efforts rather than somethings like
>> feature configuration, BSP drivers.
>>
>> Regards
>> Bibo Mao
>>
>>>
>>>
>>> Huacai
>>>>
>>>> Regards
>>>> Bibo, Mao
>>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo, Mao
>>>>>>
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Bibo, Mao
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> How about adding new extra features capability for VM such as?
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
>>>>>>>>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>>>>>>>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>>>>>>>>>>>> features.
>>>>>>>>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
>>>>>>>>>>>>           KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>>>>>>>>>>>>           KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>>>>>>>>>>>> These features are all cpu features, at the same time they are VM features.
>>>>>>>>>>>>
>>>>>>>>>>>> If they are cpu features, how does VMM detect validity of these features
>>>>>>>>>>>> passing from command line? After all VCPUs are created and send bootup
>>>>>>>>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
>>>>>>>>>>>> detect feature validity if they are VM features also.
>>>>>>>>>>>>
>>>>>>>>>>>> To be honest, I am not familiar with KVM still, only get further
>>>>>>>>>>>> understanding after actual problems solving. Welcome to give comments,
>>>>>>>>>>>> however please read more backgroud if you insist on, else there will be
>>>>>>>>>>>> endless argument again.
>>>>>>>>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
>>>>>>>>>>> haven't insisted on whether they should be vcpu features or vm
>>>>>>>>>>> features.
>>>>>>>>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
>>>>>>>>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
>>>>>>>>>>
>>>>>>>>>> What is the usage about CPUCFG capability used for VM feature? It is not
>>>>>>>>>> a detailed feature, it is only feature-set indicator like cpuid.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Bibo Mao
>>>>>>>>>>>
>>>>>>>>>>> Huacai
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Regards
>>>>>>>>>>>> Bibo, Mao
>>>>>>>>>>>>>
>>>>>>>>>>>>> Huacai
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>> Bibo Mao
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Huacai
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>>>>>>>>>>>>                                 *v |= CPUCFG2_LSX;
>>>>>>>>>>>>>>>>                         if (cpu_has_lasx)
>>>>>>>>>>>>>>>>                                 *v |= CPUCFG2_LASX;
>>>>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
>>>>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
>>>>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>                         return 0;
>>>>>>>>>>>>>>>>                 case LOONGARCH_CPUCFG3:
>>>>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>>>>>>>>>>>>>                 return r;
>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>> +       switch (attr->attr) {
>>>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>> +       switch (attr->group) {
>>>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>>>>>>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>          int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>>>>>>>>>>>>>          {
>>>>>>>>>>>>>>>> -       return -ENOIOCTLCMD;
>>>>>>>>>>>>>>>> +       struct kvm *kvm = filp->private_data;
>>>>>>>>>>>>>>>> +       void __user *argp = (void __user *)arg;
>>>>>>>>>>>>>>>> +       struct kvm_device_attr attr;
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> +       switch (ioctl) {
>>>>>>>>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>>>>>>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>>>>>>>>>>>>>> +                       return -EFAULT;
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>>>> +               return -EINVAL;
>>>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>>>>          }
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>> 2.39.3
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>>
>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/3 下午11:35, Huacai Chen wrote:
> On Wed, Jul 3, 2024 at 11:15 AM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/7/2 下午11:43, Huacai Chen wrote:
>>> On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/7/2 下午3:28, Huacai Chen wrote:
>>>>> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2024/7/2 上午10:34, Huacai Chen wrote:
>>>>>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2024/7/2 上午9:59, Huacai Chen wrote:
>>>>>>>>> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>> Huacai,
>>>>>>>>>>
>>>>>>>>>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>>>>>>>>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Huacai,
>>>>>>>>>>>>
>>>>>>>>>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>>>>>>>>>> Hi, Bibo,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>>>>>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>>>>>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>>>>>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>>>>>>>>>> be done even if VM is not created, and also can be done by any
>>>>>>>>>>>>>> thread besides VCPU threads.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>>>>>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>>>>>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>>>>>>>>>> three sub-features relative with LBT are added as following:
>>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_X86BT
>>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_ARMBT
>>>>>>>>>>>>>>         KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>         arch/loongarch/include/uapi/asm/kvm.h |  6 ++++
>>>>>>>>>>>>>>         arch/loongarch/kvm/vcpu.c             |  6 ++++
>>>>>>>>>>>>>>         arch/loongarch/kvm/vm.c               | 44 ++++++++++++++++++++++++++-
>>>>>>>>>>>>>>         3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>>>>>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>>>>>>>>>> @@ -82,6 +82,12 @@ struct kvm_fpu {
>>>>>>>>>>>>>>         #define KVM_IOC_CSRID(REG)             LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>>>>>>>>>>>>         #define KVM_IOC_CPUCFG(REG)            LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> +/* Device Control API on vm fd */
>>>>>>>>>>>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL     0
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_X86BT   0
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_ARMBT   1
>>>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_MIPSBT  2
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>         /* Device Control API on vcpu fd */
>>>>>>>>>>>>>>         #define KVM_LOONGARCH_VCPU_CPUCFG      0
>>>>>>>>>>>>>>         #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>>>>>>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>>>>>>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>>>>>>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>>>>>>>>>> have a chance to change it now.
>>>>>>>>>>>>
>>>>>>>>>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>>>>>>>>>> has is own different gpa address.
>>>>>>>>>>> Then leave this as a vm feature.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>>>>>>>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>>>>>>>>>> uapi breaking now, still will be in future if we cannot control this.
>>>>>>>>>>> UAPI changing before the first release is allowed, which means, we can
>>>>>>>>>>> change this before the 6.10-final, but cannot change it after
>>>>>>>>>>> 6.10-final.
>>>>>>>>>> Now QEMU has already synced uapi to its own directory, also I never hear
>>>>>>>>>> about this, with my experience with uapi change, there is only newly
>>>>>>>>>> added or removed deprecated years ago.
>>>>>>>>>>
>>>>>>>>>> Is there any documentation about UAPI change rules?
>>>>>>>>> No document, but learn from my more than 10 years upstream experience.
>>>>>>>> Can you show me an example about with your rich upstream experience?
>>>>>>> A simple example,
>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
>>>>>>> 6.9-final, but not after that.
>>>>>>>
>>>>>>> Before the first release, the code status is treated as "unstable", so
>>>>>>> revert, modify is allowed. But after the first release, even if an
>>>>>>> "error" should also be treated as a "bad feature".
>>>>>> Huacai,
>>>>>>
>>>>>> Thanks for showing the example.
>>>>>>
>>>>>> For this issue, Can we adding new uapi and mark the old as deprecated?
>>>>>> so that it can be removed after years.
>>>>> Unnecessary, just remove the old one. Deprecation is for the usage
>>>>> after the first release.
>>>>>
>>>>>>
>>>>>> For me, it is too frequent to revert the old uapi, it is not bug and
>>>>>> only that we have better method now. Also QEMU has synchronized the uapi
>>>>>> to its directory already.
>>>>> QEMU also hasn't been released after synchronizing the uapi, so it is
>>>>> OK to remove the old api now.
>>>> No, I will not do such thing. It is just a joke to revert the uapi.
>>>>
>>>> So just create new world and old world on Loongarch system again?
>>> Again, code status before the first release is *unstable*, that status
>>> is not enough to be a "world".
>>>
>>> It's your responsibility to make a good design at the beginning, but
>>> you fail to do that. Fortunately we are before the first release;
>>> unfortunately you don't want to do that.
>> Yes, this is flaw at the beginning, however it can works and new abi can
>> be added.
>>
>> If there is no serious bug and it is synced to QEMU already, I am not
>> willing to revert uabi. Different projects have its own schedule plan,
>> that is one reason. The most important reason may be that different
>> peoples have different ways handling these issues.
> In another thread I found that Jiaxun said he has a solution to make
> LBT be a vcpu feature and still works well. However, that may take
> some time and is too late for 6.11.
> 
> But we have another choice now: just remove the UAPI and vm.c parts in
> this series, let the LBT main parts be upstream in 6.11, and then
> solve other problems after 6.11. Even if Jiaxun's solution isn't
> usable, we can still use this old vm feature solution then.

I am sure it is best if it is VM feature for LBT feature detection, 
LSX/LASX feature detection uses CPU feature, we can improve it later.

For host cpu type or migration feature detection, I have no idea now, 
also I do not think it will be big issue for me, I will do it with 
scheduled time. Of source, welcome Jiaxun and you to implement host cpu 
type or migration feature detection.

Regards
Bibo Mao
> 
> 
> Huacai
>>
>> Regards
>> Bibo, Mao
>>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo, Mao
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo, Mao
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> How about adding new extra features capability for VM such as?
>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LSX   3
>>>>>>>>>>>> +#define  KVM_LOONGARCH_VM_FEAT_LASX  4
>>>>>>>>>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>>>>>>>>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>>>>>>>>>> features.
>>>>>>>>>> On other architectures, with function kvm_vm_ioctl_check_extension()
>>>>>>>>>>          KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>>>>>>>>>>          KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>>>>>>>>>> These features are all cpu features, at the same time they are VM features.
>>>>>>>>>>
>>>>>>>>>> If they are cpu features, how does VMM detect validity of these features
>>>>>>>>>> passing from command line? After all VCPUs are created and send bootup
>>>>>>>>>> command to these VCPUs? That is too late, VMM main thread is easy to
>>>>>>>>>> detect feature validity if they are VM features also.
>>>>>>>>>>
>>>>>>>>>> To be honest, I am not familiar with KVM still, only get further
>>>>>>>>>> understanding after actual problems solving. Welcome to give comments,
>>>>>>>>>> however please read more backgroud if you insist on, else there will be
>>>>>>>>>> endless argument again.
>>>>>>>>> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
>>>>>>>>> haven't insisted on whether they should be vcpu features or vm
>>>>>>>>> features.
>>>>>>>> It is reasonable if LSX/LASX/LBT should be in the same class, since
>>>>>>>> there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
>>>>>>>>
>>>>>>>> What is the usage about CPUCFG capability used for VM feature? It is not
>>>>>>>> a detailed feature, it is only feature-set indicator like cpuid.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Bibo Mao
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Bibo, Mao
>>>>>>>>>>>
>>>>>>>>>>> Huacai
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Regards
>>>>>>>>>>>> Bibo Mao
>>>>>>>>>>>>>
>>>>>>>>>>>>> Huacai
>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>>>>>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>>>>>>>>>>                                *v |= CPUCFG2_LSX;
>>>>>>>>>>>>>>                        if (cpu_has_lasx)
>>>>>>>>>>>>>>                                *v |= CPUCFG2_LASX;
>>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>>>> +                       *v |= CPUCFG2_X86BT;
>>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>>>> +                       *v |= CPUCFG2_ARMBT;
>>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>>>> +                       *v |= CPUCFG2_MIPSBT;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>                        return 0;
>>>>>>>>>>>>>>                case LOONGARCH_CPUCFG3:
>>>>>>>>>>>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>>>>>>>>>>>> --- a/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>> +++ b/arch/loongarch/kvm/vm.c
>>>>>>>>>>>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>>>>>>>>>>>>                return r;
>>>>>>>>>>>>>>         }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>> +       switch (attr->attr) {
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>>>>>>>>>>>> +               if (cpu_has_lbt_x86)
>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>>>>>>>>>>>> +               if (cpu_has_lbt_arm)
>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>>>>>>>>>>>> +               if (cpu_has_lbt_mips)
>>>>>>>>>>>>>> +                       return 0;
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>> +       switch (attr->group) {
>>>>>>>>>>>>>> +       case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>>>>>>>>>>>> +               return kvm_vm_feature_has_attr(kvm, attr);
>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>> +               return -ENXIO;
>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>         int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>>>>>>>>>>>>         {
>>>>>>>>>>>>>> -       return -ENOIOCTLCMD;
>>>>>>>>>>>>>> +       struct kvm *kvm = filp->private_data;
>>>>>>>>>>>>>> +       void __user *argp = (void __user *)arg;
>>>>>>>>>>>>>> +       struct kvm_device_attr attr;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +       switch (ioctl) {
>>>>>>>>>>>>>> +       case KVM_HAS_DEVICE_ATTR:
>>>>>>>>>>>>>> +               if (copy_from_user(&attr, argp, sizeof(attr)))
>>>>>>>>>>>>>> +                       return -EFAULT;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +               return kvm_vm_has_attr(kvm, &attr);
>>>>>>>>>>>>>> +       default:
>>>>>>>>>>>>>> +               return -EINVAL;
>>>>>>>>>>>>>> +       }
>>>>>>>>>>>>>>         }
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> 2.39.3
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Jiaxun Yang]

On 2024/7/4 09:35, maobibo wrote:
> In another thread I found that Jiaxun said he has a solution to make
>> LBT be a vcpu feature and still works well. However, that may take
>> some time and is too late for 6.11.
>>
>> But we have another choice now: just remove the UAPI and vm.c parts in
>> this series, let the LBT main parts be upstream in 6.11, and then
>> solve other problems after 6.11. Even if Jiaxun's solution isn't
>> usable, we can still use this old vm feature solution then.

IMO this is the best approach to make some progress.

>
> I am sure it is best if it is VM feature for LBT feature detection, 
> LSX/LASX feature detection uses CPU feature, we can improve it later.

Please justify the reason, we should always be serious on UAPI design 
choices.
I don't really understand why the approach worked so well on Arm & 
RISC-V is not working
for you.

I understand you may have some plans in your mind, please elaborate so 
we can smash
them together. That's how community work.

>
> For host cpu type or migration feature detection, I have no idea now, 
> also I do not think it will be big issue for me, I will do it with 
> scheduled time. Of source, welcome Jiaxun and you to implement host 
> cpu type or migration feature detection.

My concern is if you allow CPU features to have "auto" property you are 
risking create
inconsistency among migration. Once you've done that it's pretty hard to 
get rid of it.

Please check how RISC-V dealing with CPU features at QMP side.

I'm not meant to hinder your development work, but we should always 
think ahead.

Thanks
- Jiaxun
>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/5 上午3:44, Jiaxun Yang wrote:
> 
> 
> On 2024/7/4 09:35, maobibo wrote:
>> In another thread I found that Jiaxun said he has a solution to make
>>> LBT be a vcpu feature and still works well. However, that may take
>>> some time and is too late for 6.11.
>>>
>>> But we have another choice now: just remove the UAPI and vm.c parts in
>>> this series, let the LBT main parts be upstream in 6.11, and then
>>> solve other problems after 6.11. Even if Jiaxun's solution isn't
>>> usable, we can still use this old vm feature solution then.
> 
> IMO this is the best approach to make some progress.
> 
>>
>> I am sure it is best if it is VM feature for LBT feature detection, 
>> LSX/LASX feature detection uses CPU feature, we can improve it later.
> 
> Please justify the reason, we should always be serious on UAPI design 
> choices.
> I don't really understand why the approach worked so well on Arm & 
> RISC-V is not working
> for you.
On the other hand, can you list benefits or disadvantage of approaches 
on different architecture?

Or you post patch about host cpu support, I list its disadvantage. Or I 
post patch about host cpu support with scheduled time, then we talk 
about it. Is that fair for you?

It is unfair that you list some approaches and let others spend time to 
do, else you are my top boss :)
> 
> I understand you may have some plans in your mind, please elaborate so 
> we can smash
> them together. That's how community work.
> 
>>
>> For host cpu type or migration feature detection, I have no idea now, 
>> also I do not think it will be big issue for me, I will do it with 
>> scheduled time. Of source, welcome Jiaxun and you to implement host 
>> cpu type or migration feature detection.
> 
> My concern is if you allow CPU features to have "auto" property you are 
> risking create
> inconsistency among migration. Once you've done that it's pretty hard to 
> get rid of it.
> 
> Please check how RISC-V dealing with CPU features at QMP side.
> 
> I'm not meant to hinder your development work, but we should always 
> think ahead.
Yes, it is potential issue and we will solve it. Another potential issue 
is that PV features may different on host, you cannot disable PV 
features directly.  The best way is that you post patch about it, then 
we can talk about together, else it may be kindly reminder, also may be 
waste of time, everyone is busy working for boss :)

Regards
Bibo Mao
> 
> Thanks
> - Jiaxun
>>

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Jiaxun Yang]

在2024年7月5日七月 上午9:21，maobibo写道：
[...]
>> for you.
> On the other hand, can you list benefits or disadvantage of approaches 
> on different architecture?

So the obvious benefit of scratch vCPU would be maintaining consistency and simpleness
for UAPI.

It can also maximum code reuse probabilities in other user space hypervisor projects.

Also, it can benefit a potential asymmetrical system. I understand that it won't appear
in near future, but we should always be prepared, especially in UAPI design.

>
> Or you post patch about host cpu support, I list its disadvantage. Or I 
> post patch about host cpu support with scheduled time, then we talk 
> about it. Is that fair for you?

I'm not committed to development work, I can try, but I can't promise.

Regarding the fairness, IMO that's not how community works. If you observe reviewing
process happening all the place, it's always about addressing other's concern.

Still, it's up to maintainers to decide what's reasonable, I'm just trying to help.

>
> It is unfair that you list some approaches and let others spend time to 
> do, else you are my top boss :)

I mean, I'm just trying to make some progress here. I saw you have some disagreement
with Huacai.

I know QEMU side implementation better than Huacai, so I'm trying to propose a solution
that would address Huacai's concern and may work for you.

>> 
>> I understand you may have some plans in your mind, please elaborate so 
>> we can smash
>> them together. That's how community work.
>> 
>>>
>>> For host cpu type or migration feature detection, I have no idea now, 
>>> also I do not think it will be big issue for me, I will do it with 
>>> scheduled time. Of source, welcome Jiaxun and you to implement host 
>>> cpu type or migration feature detection.
>> 
>> My concern is if you allow CPU features to have "auto" property you are 
>> risking create
>> inconsistency among migration. Once you've done that it's pretty hard to 
>> get rid of it.
>> 
>> Please check how RISC-V dealing with CPU features at QMP side.We are working on
>> 
>> I'm not meant to hinder your development work, but we should always 
>> think ahead.
> Yes, it is potential issue and we will solve it. Another potential issue 
> is that PV features may different on host, you cannot disable PV 
> features directly.  The best way is that you post patch about it, then 
> we can talk about together, else it may be kindly reminder, also may be 
> waste of time, everyone is busy working for boss :)

Sigh, so you meant you submitted something known to be problematic?

-- 
- Jiaxun

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/5 上午11:19, Jiaxun Yang wrote:
> 
> 
> 在2024年7月5日七月 上午9:21，maobibo写道：
> [...]
>>> for you.
>> On the other hand, can you list benefits or disadvantage of approaches
>> on different architecture?
> 
> So the obvious benefit of scratch vCPU would be maintaining consistency and simpleness
> for UAPI.
I do not find the simpleness, for the same feature function, both VM 
feature and CPU feature is define as follows.  Do you think it is for 
simple :)

VM CAPBILITY:
     KVM_CAP_ARM_PTRAUTH_ADDRESS
     KVM_CAP_ARM_PTRAUTH_GENERIC
     KVM_CAP_ARM_EL1_32BIT
     KVM_CAP_ARM_PMU_V3
     KVM_CAP_ARM_SVE
     KVM_CAP_ARM_PSCI
     KVM_CAP_ARM_PSCI_0_2

CPU:
     KVM_ARM_VCPU_POWER_OFF
     KVM_ARM_VCPU_EL1_32BIT
     KVM_ARM_VCPU_PSCI_0_2
     KVM_ARM_VCPU_PMU_V3
     KVM_ARM_VCPU_SVE
     KVM_ARM_VCPU_PTRAUTH_ADDRESS
     KVM_ARM_VCPU_PTRAUTH_GENERIC
     KVM_ARM_VCPU_HAS_EL2

Also why scratch vcpu is created and tested on host cpu type rather than 
other cpu type?  It wastes much time for host cpu type to detect capability.
> 
> It can also maximum code reuse probabilities in other user space hypervisor projects.
> 
> Also, it can benefit a potential asymmetrical system. I understand that it won't appear
> in near future, but we should always be prepared, especially in UAPI design.
If for potential asymmetrical system, however there is only one scratch 
vcpu. is that right? how does only one scratch vcpu detect ASMP 
capability, and it is not bind to physical cpu to detect ASMP capability.

In generic big.little is HMP rather than ASMP, are you agree.

> 
>>
>> Or you post patch about host cpu support, I list its disadvantage. Or I
>> post patch about host cpu support with scheduled time, then we talk
>> about it. Is that fair for you?
> 
> I'm not committed to development work, I can try, but I can't promise.
> 
> Regarding the fairness, IMO that's not how community works. If you observe reviewing
> process happening all the place, it's always about addressing other's concern.
> 
> Still, it's up to maintainers to decide what's reasonable, I'm just trying to help.
yes I agree. I and Tianrui are maintainer also, we write all the kvm 
loongarch code. We are familiar with it from the beginning.

Regards
Bibo Mao
> 
>>
>> It is unfair that you list some approaches and let others spend time to
>> do, else you are my top boss :)
> 
> I mean, I'm just trying to make some progress here. I saw you have some disagreement
> with Huacai.
> 
> I know QEMU side implementation better than Huacai, so I'm trying to propose a solution
> that would address Huacai's concern and may work for you.
> 
>>>
>>> I understand you may have some plans in your mind, please elaborate so
>>> we can smash
>>> them together. That's how community work.
>>>
>>>>
>>>> For host cpu type or migration feature detection, I have no idea now,
>>>> also I do not think it will be big issue for me, I will do it with
>>>> scheduled time. Of source, welcome Jiaxun and you to implement host
>>>> cpu type or migration feature detection.
>>>
>>> My concern is if you allow CPU features to have "auto" property you are
>>> risking create
>>> inconsistency among migration. Once you've done that it's pretty hard to
>>> get rid of it.
>>>
>>> Please check how RISC-V dealing with CPU features at QMP side.We are working on
>>>
>>> I'm not meant to hinder your development work, but we should always
>>> think ahead.
>> Yes, it is potential issue and we will solve it. Another potential issue
>> is that PV features may different on host, you cannot disable PV
>> features directly.  The best way is that you post patch about it, then
>> we can talk about together, else it may be kindly reminder, also may be
>> waste of time, everyone is busy working for boss :)
> 
> Sigh, so you meant you submitted something known to be problematic?
> 

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[Jiaxun Yang]

在2024年7月5日七月 上午11:46，maobibo写道：
> On 2024/7/5 上午11:19, Jiaxun Yang wrote:
>> 
>> 
>> 在2024年7月5日七月 上午9:21，maobibo写道：
>> [...]
>>>> for you.
>>> On the other hand, can you list benefits or disadvantage of approaches
>>> on different architecture?
>> 
>> So the obvious benefit of scratch vCPU would be maintaining consistency and simpleness
>> for UAPI.
> I do not find the simpleness, for the same feature function, both VM 
> feature and CPU feature is define as follows.  Do you think it is for 
> simple :)

So they made a mistake here :-(

We don't even need vCPU flag, just probing CPUCFG bits is sufficient.

Note that in Arm's case, some CPU features have system dependencies, that's why
they need to be entitled twice.

For us, we don't have such burden.

If Arm doesn't set a good example here, please check RISC-V's CONFIG reg on dealing
ISA extensions. We don't even need such register because our CPUCFG can perfectly
describe ISA status.

>
> VM CAPBILITY:
>      KVM_CAP_ARM_PTRAUTH_ADDRESS
>      KVM_CAP_ARM_PTRAUTH_GENERIC
>      KVM_CAP_ARM_EL1_32BIT
>      KVM_CAP_ARM_PMU_V3
>      KVM_CAP_ARM_SVE
>      KVM_CAP_ARM_PSCI
>      KVM_CAP_ARM_PSCI_0_2
>
> CPU:
>      KVM_ARM_VCPU_POWER_OFF
>      KVM_ARM_VCPU_EL1_32BIT
>      KVM_ARM_VCPU_PSCI_0_2
>      KVM_ARM_VCPU_PMU_V3
>      KVM_ARM_VCPU_SVE
>      KVM_ARM_VCPU_PTRAUTH_ADDRESS
>      KVM_ARM_VCPU_PTRAUTH_GENERIC
>      KVM_ARM_VCPU_HAS_EL2
>
> Also why scratch vcpu is created and tested on host cpu type rather than 
> other cpu type?  It wastes much time for host cpu type to detect capability.

To maximize supported features, on Arm there is KVM_ARM_VCPU_INIT ioctl.
For us that's unnecessary, our kernel does not need to be aware of CPU type,
only CPUCFG bits are necessary. RISC-V is following the same convention.

>> 
>> It can also maximum code reuse probabilities in other user space hypervisor projects.
>> 
>> Also, it can benefit a potential asymmetrical system. I understand that it won't appear
>> in near future, but we should always be prepared, especially in UAPI design.
> If for potential asymmetrical system, however there is only one scratch 
> vcpu. is that right? how does only one scratch vcpu detect ASMP 
> capability, and it is not bind to physical cpu to detect ASMP capability.
>
> In generic big.little is HMP rather than ASMP, are you agree.

So I was talking about emulating asymmetrical guest. Each guest CPU should have
it's own copy of properties. That's the lesson learnt.

[...]

Anyway I'm just trying to help out here, feel free to go ahead without taking my advice.

I've seen so many pitfalls on all other arches and I don't want them to repeat on LoongArch.
But sometimes people only learn from mistakes.

-- 
- Jiaxun

Re: [PATCH v4 2/3] LoongArch: KVM: Add LBT feature detection function

[maobibo]

On 2024/7/5 下午12:09, Jiaxun Yang wrote:
> 
> 
> 在2024年7月5日七月 上午11:46，maobibo写道：
>> On 2024/7/5 上午11:19, Jiaxun Yang wrote:
>>>
>>>
>>> 在2024年7月5日七月 上午9:21，maobibo写道：
>>> [...]
>>>>> for you.
>>>> On the other hand, can you list benefits or disadvantage of approaches
>>>> on different architecture?
>>>
>>> So the obvious benefit of scratch vCPU would be maintaining consistency and simpleness
>>> for UAPI.
>> I do not find the simpleness, for the same feature function, both VM
>> feature and CPU feature is define as follows.  Do you think it is for
>> simple :)
> 
> So they made a mistake here :-(
> 
> We don't even need vCPU flag, just probing CPUCFG bits is sufficient.
> 
> Note that in Arm's case, some CPU features have system dependencies, that's why
> they need to be entitled twice.
> 
> For us, we don't have such burden.
> 
> If Arm doesn't set a good example here, please check RISC-V's CONFIG reg on dealing
> ISA extensions. We don't even need such register because our CPUCFG can perfectly
> describe ISA status.
> 
>>
>> VM CAPBILITY:
>>       KVM_CAP_ARM_PTRAUTH_ADDRESS
>>       KVM_CAP_ARM_PTRAUTH_GENERIC
>>       KVM_CAP_ARM_EL1_32BIT
>>       KVM_CAP_ARM_PMU_V3
>>       KVM_CAP_ARM_SVE
>>       KVM_CAP_ARM_PSCI
>>       KVM_CAP_ARM_PSCI_0_2
>>
>> CPU:
>>       KVM_ARM_VCPU_POWER_OFF
>>       KVM_ARM_VCPU_EL1_32BIT
>>       KVM_ARM_VCPU_PSCI_0_2
>>       KVM_ARM_VCPU_PMU_V3
>>       KVM_ARM_VCPU_SVE
>>       KVM_ARM_VCPU_PTRAUTH_ADDRESS
>>       KVM_ARM_VCPU_PTRAUTH_GENERIC
>>       KVM_ARM_VCPU_HAS_EL2
>>
>> Also why scratch vcpu is created and tested on host cpu type rather than
>> other cpu type?  It wastes much time for host cpu type to detect capability.
> 
> To maximize supported features, on Arm there is KVM_ARM_VCPU_INIT ioctl.
> For us that's unnecessary, our kernel does not need to be aware of CPU type,
> only CPUCFG bits are necessary. RISC-V is following the same convention.
> 
>>>
>>> It can also maximum code reuse probabilities in other user space hypervisor projects.
>>>
>>> Also, it can benefit a potential asymmetrical system. I understand that it won't appear
>>> in near future, but we should always be prepared, especially in UAPI design.
>> If for potential asymmetrical system, however there is only one scratch
>> vcpu. is that right? how does only one scratch vcpu detect ASMP
>> capability, and it is not bind to physical cpu to detect ASMP capability.
>>
>> In generic big.little is HMP rather than ASMP, are you agree.
> 
> So I was talking about emulating asymmetrical guest. Each guest CPU should have
> it's own copy of properties. That's the lesson learnt.
For ASMP there may be problem if instruction set is different. However I 
think it is a little far from now with LoongArch.
> 
> [...]
> 
> Anyway I'm just trying to help out here, feel free to go ahead without taking my advice.
It is really nice to talk with you , hope more people taking part in 
community from my heart.
> 
> I've seen so many pitfalls on all other arches and I don't want them to repeat on LoongArch.
> But sometimes people only learn from mistakes.
Different arch has different history, LoongArch should take pitfalls 
again, it is normal rules just from my thoughts, only that it is new and 
has no much burden now.

Regards
Bibo Mao

[PATCH v4 3/3] LoongArch: KVM: Add vm migration support for LBT registers

[Bibo Mao]

Every vcpu has separate LBT registers. And there are four scr registers,
one flags and ftop register for LBT extension. When VM migrates, VMM
needs to get LBT registers for every vcpu.

Here macro KVM_REG_LOONGARCH_LBT is added for new vcpu lbt register type,
the following macro is added to get/put LBT registers.
  KVM_REG_LOONGARCH_LBT_SCR0
  KVM_REG_LOONGARCH_LBT_SCR1
  KVM_REG_LOONGARCH_LBT_SCR2
  KVM_REG_LOONGARCH_LBT_SCR3
  KVM_REG_LOONGARCH_LBT_EFLAGS
  KVM_REG_LOONGARCH_LBT_FTOP

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/include/uapi/asm/kvm.h |  9 +++++
 arch/loongarch/kvm/vcpu.c             | 56 +++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
index c40f7d9ffe13..ed12e509815c 100644
--- a/arch/loongarch/include/uapi/asm/kvm.h
+++ b/arch/loongarch/include/uapi/asm/kvm.h
@@ -64,6 +64,7 @@ struct kvm_fpu {
 #define KVM_REG_LOONGARCH_KVM		(KVM_REG_LOONGARCH | 0x20000ULL)
 #define KVM_REG_LOONGARCH_FPSIMD	(KVM_REG_LOONGARCH | 0x30000ULL)
 #define KVM_REG_LOONGARCH_CPUCFG	(KVM_REG_LOONGARCH | 0x40000ULL)
+#define KVM_REG_LOONGARCH_LBT		(KVM_REG_LOONGARCH | 0x50000ULL)
 #define KVM_REG_LOONGARCH_MASK		(KVM_REG_LOONGARCH | 0x70000ULL)
 #define KVM_CSR_IDX_MASK		0x7fff
 #define KVM_CPUCFG_IDX_MASK		0x7fff
@@ -77,6 +78,14 @@ struct kvm_fpu {
 /* Debugging: Special instruction for software breakpoint */
 #define KVM_REG_LOONGARCH_DEBUG_INST	(KVM_REG_LOONGARCH_KVM | KVM_REG_SIZE_U64 | 3)
 
+/* LBT registers */
+#define KVM_REG_LOONGARCH_LBT_SCR0	(KVM_REG_LOONGARCH_LBT | KVM_REG_SIZE_U64 | 1)
+#define KVM_REG_LOONGARCH_LBT_SCR1	(KVM_REG_LOONGARCH_LBT | KVM_REG_SIZE_U64 | 2)
+#define KVM_REG_LOONGARCH_LBT_SCR2	(KVM_REG_LOONGARCH_LBT | KVM_REG_SIZE_U64 | 3)
+#define KVM_REG_LOONGARCH_LBT_SCR3	(KVM_REG_LOONGARCH_LBT | KVM_REG_SIZE_U64 | 4)
+#define KVM_REG_LOONGARCH_LBT_EFLAGS	(KVM_REG_LOONGARCH_LBT | KVM_REG_SIZE_U64 | 5)
+#define KVM_REG_LOONGARCH_LBT_FTOP	(KVM_REG_LOONGARCH_LBT | KVM_REG_SIZE_U64 | 6)
+
 #define LOONGARCH_REG_SHIFT		3
 #define LOONGARCH_REG_64(TYPE, REG)	(TYPE | KVM_REG_SIZE_U64 | (REG << LOONGARCH_REG_SHIFT))
 #define KVM_IOC_CSRID(REG)		LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 9734b4d8db05..1937233cc473 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -664,6 +664,34 @@ static int kvm_get_one_reg(struct kvm_vcpu *vcpu,
 			break;
 		}
 		break;
+	case KVM_REG_LOONGARCH_LBT:
+		if (!kvm_guest_has_lbt(&vcpu->arch))
+			return -ENXIO;
+
+		switch (reg->id) {
+		case KVM_REG_LOONGARCH_LBT_SCR0:
+			*v = vcpu->arch.lbt.scr0;
+			break;
+		case KVM_REG_LOONGARCH_LBT_SCR1:
+			*v = vcpu->arch.lbt.scr1;
+			break;
+		case KVM_REG_LOONGARCH_LBT_SCR2:
+			*v = vcpu->arch.lbt.scr2;
+			break;
+		case KVM_REG_LOONGARCH_LBT_SCR3:
+			*v = vcpu->arch.lbt.scr3;
+			break;
+		case KVM_REG_LOONGARCH_LBT_EFLAGS:
+			*v = vcpu->arch.lbt.eflags;
+			break;
+		case KVM_REG_LOONGARCH_LBT_FTOP:
+			*v = vcpu->arch.fpu.ftop;
+			break;
+		default:
+			ret = -EINVAL;
+			break;
+		}
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -730,6 +758,34 @@ static int kvm_set_one_reg(struct kvm_vcpu *vcpu,
 			break;
 		}
 		break;
+	case KVM_REG_LOONGARCH_LBT:
+		if (!kvm_guest_has_lbt(&vcpu->arch))
+			return -ENXIO;
+
+		switch (reg->id) {
+		case KVM_REG_LOONGARCH_LBT_SCR0:
+			vcpu->arch.lbt.scr0 = v;
+			break;
+		case KVM_REG_LOONGARCH_LBT_SCR1:
+			vcpu->arch.lbt.scr1 = v;
+			break;
+		case KVM_REG_LOONGARCH_LBT_SCR2:
+			vcpu->arch.lbt.scr2 = v;
+			break;
+		case KVM_REG_LOONGARCH_LBT_SCR3:
+			vcpu->arch.lbt.scr3 = v;
+			break;
+		case KVM_REG_LOONGARCH_LBT_EFLAGS:
+			vcpu->arch.lbt.eflags = v;
+			break;
+		case KVM_REG_LOONGARCH_LBT_FTOP:
+			vcpu->arch.fpu.ftop = v;
+			break;
+		default:
+			ret = -EINVAL;
+			break;
+		}
+		break;
 	default:
 		ret = -EINVAL;
 		break;
-- 
2.39.3