[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <13276416-c62b-b33d-1824-7764122ef863@loongson.cn>
Date: Mon, 2 Sep 2024 09:56:06 +0800
From: maobibo <maobibo@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>, WANG Xuerui <kernel@...0n.name>,
kvm@...r.kernel.org, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org, Jiaxun Yang <jiaxun.yang@...goat.com>
Subject: Re: [PATCH v6 3/3] LoongArch: KVM: Add vm migration support for LBT
registers
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@...ngson.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@...ngson.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
>>
Powered by blists - more mailing lists