diff mbox series

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

Message ID 20240730075744.1215856-4-maobibo@loongson.cn (mailing list archive)
State New, archived
Headers show
Series LoongArch: KVM: Add Binary Translation extension support | expand

Commit Message

Bibo Mao July 30, 2024, 7:57 a.m. UTC
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(+)

Comments

Huacai Chen Aug. 31, 2024, 2:49 p.m. UTC | #1
Hi, Bibo,

On Tue, Jul 30, 2024 at 3:57 PM Bibo Mao <maobibo@loongson.cn> 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@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 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)?

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

> +
>  #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?

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
>
Bibo Mao Sept. 2, 2024, 1:56 a.m. UTC | #2
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@loongson.cn> 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@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 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?

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

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

/*
  * 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
>>
Huacai Chen Sept. 2, 2024, 2:20 a.m. UTC | #3
On Mon, Sep 2, 2024 at 9:56 AM maobibo <maobibo@loongson.cn> 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@loongson.cn> 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@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 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
> >>
>
>
Bibo Mao Sept. 2, 2024, 2:45 a.m. UTC | #4
On 2024/9/2 上午10:20, Huacai Chen wrote:
> On Mon, Sep 2, 2024 at 9:56 AM maobibo <maobibo@loongson.cn> 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@loongson.cn> 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@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 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.

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
>>>>
>>
>>
Huacai Chen Sept. 2, 2024, 4:04 a.m. UTC | #5
On Mon, Sep 2, 2024 at 10:45 AM maobibo <maobibo@loongson.cn> wrote:
>
>
>
> On 2024/9/2 上午10:20, Huacai Chen wrote:
> > On Mon, Sep 2, 2024 at 9:56 AM maobibo <maobibo@loongson.cn> 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@loongson.cn> 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@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 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?

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
> >>>>
> >>
> >>
>
>
Bibo Mao Sept. 2, 2024, 4:15 a.m. UTC | #6
On 2024/9/2 下午12:04, Huacai Chen wrote:
> On Mon, Sep 2, 2024 at 10:45 AM maobibo <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2024/9/2 上午10:20, Huacai Chen wrote:
>>> On Mon, Sep 2, 2024 at 9:56 AM maobibo <maobibo@loongson.cn> 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@loongson.cn> 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@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 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
>>>>>>
>>>>
>>>>
>>
>>
diff mbox series

Patch

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)
 #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 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:
+		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;