Re: [PATCH v6 3/3] LoongArch: KVM: Add vm migration support for LBT registers
From: maobibo
Date: Mon Sep 02 2024 - 00:16:27 EST
On 2024/9/2 下午12:04, Huacai Chen wrote:
On Mon, Sep 2, 2024 at 10:45 AM maobibo <maobibo@xxxxxxxxxxx> wrote:
On 2024/9/2 上午10:20, Huacai Chen wrote:
On Mon, Sep 2, 2024 at 9:56 AM maobibo <maobibo@xxxxxxxxxxx> wrote:
Hi Huacai,
On 2024/8/31 下午10:49, Huacai Chen wrote:
Hi, Bibo,
On Tue, Jul 30, 2024 at 3:57 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
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@xxxxxxxxxxx>
---
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 49bafac8b22d..003fb766c93f 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)
I think KVM_REG_LOONGARCH_MASK should contain all above register
classes, so should it be (KVM_REG_LOONGARCH | 0x370000ULL)?
Sorry, maybe I miss something. What is the meaning of 0x370000ULL? How
does the value come from?
It seems I misunderstood the mask, please ignore.
#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)
FTOP is a 32bit register in other place of the kernel, is it correct
to use U64 here?
It is deliberate and there is no 32bit compat requirement for kvm. ALL
regiester interfaces are defined as 64-bit.
On kernel and qemu side, ftop can be defined as 32bit still, however the
interface is 64-bit. So there is forced type conversion between u32 and
u64. There is no problem here.
If you are sure, then no problem. But there is indeed KVM_REG_SIZE_U32
in include/uapi/linux/kvm.h, and if we append more fields after ftop,
define it as U64 may break memcpy().
yes, there is KVM_REG_SIZE_U32 definition, however LoongArch KVM does
not use it, else the safer checking is a little complicated. Now
parameter with KVM_REG_SIZE_U32 is simply treated as illegal.
I think just add a "case KVM_REG_SIZE_U32" in kvm_set_reg/kvm_get_reg
is OK, kvm_set_one_reg/kvm_get_one_reg don't need any modifications.
No?
No, it is not so simple. If KVM_REG_SIZE_U32 is supported, if such user
application passes such parameters KVM_REG_LOONGARCH_LBT_FTOP64 or
KVM_REG_LOONGARCH_LBT_EFLAGS32 , should it be treated as illegal or
normal? If it is illegal, where should the safety checking code be put?
#define KVM_REG_LOONGARCH_LBT_FTOP64 (KVM_REG_LOONGARCH_LBT |
KVM_REG_SIZE_U64 | 6)
#define KVM_REG_LOONGARCH_LBT_FTOP32 (KVM_REG_LOONGARCH_LBT |
KVM_REG_SIZE_U32 | 6)
#define KVM_REG_LOONGARCH_LBT_EFLAGS64 (KVM_REG_LOONGARCH_LBT |
KVM_REG_SIZE_U64 | 5)
#define KVM_REG_LOONGARCH_LBT_EFLAGS32 (KVM_REG_LOONGARCH_LBT |
KVM_REG_SIZE_U32 | 5)
Regards
Bibo Mao
Huacai
And no memcpy() is used for ftop/cpucfg, there is assignment for every
single register like this:
For cpucfg read/write:
vcpu->arch.cpucfg[id] = (u32)v;
*v = vcpu->arch.cpucfg[id];
For ftop read/write:
vcpu->arch.fpu.ftop = v;
*v = vcpu->arch.fpu.ftop;
Regards
Bibo Mao
+
#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 b5324885a81a..b2500d4fa729 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -597,6 +597,34 @@ static int kvm_get_one_reg(struct kvm_vcpu *vcpu,
break;
}
break;
+ case KVM_REG_LOONGARCH_LBT:
What about adding FPU/LSX/LASX registers (if needed for migration) in
kvm_{get, set}_one_reg() here?
If there is 512bit SIMD or other requirement, it will be added in
kvm_{get, set}_one_reg(). For FPU/LSX/LASX registers, there is common
API KVM_GET_FPU/KVM_SET_FPU here. The impmentation of QEMU only gets
FPU, the upper LSX/LASX is lost, we will submit a patch in qemu side,
the kvm kernel side is ok.
OK, no problem.
Huacai
/*
* for KVM_GET_FPU and KVM_SET_FPU
*/
struct kvm_fpu {
__u32 fcsr;
__u64 fcc; /* 8x8 */
struct kvm_fpureg {
__u64 val64[4];
} fpr[32];
};
Regards
Bibo Mao
Huacai
+ 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;
@@ -663,6 +691,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