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().
>
> >
> >> +
> >> #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
> >>
>
>
