| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1daacc02-2bdc-928b-6291-7604c841d219@loongson.cn>
Date: Mon, 2 Sep 2024 10:45:08 +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
On 2024/9/2 上午10:20, Huacai Chen wrote:
> On Mon, Sep 2, 2024 at 9:56 AM maobibo <maobibo@...ngson.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@...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?
> 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
>>>>
>>
>>
Powered by blists - more mailing lists