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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> >>
>
>





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux