[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e553bbb7-9533-0a09-25b5-23a430817f26@loongson.cn>
Date: Tue, 25 Jun 2024 10:40:33 +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
Subject: Re: [PATCH v3 4/4] LoongArch: KVM: Add VM LBT feature detection
support
On 2024/6/24 下午10:02, Huacai Chen wrote:
> On Mon, Jun 24, 2024 at 10:00 AM maobibo <maobibo@...ngson.cn> wrote:
>>
>>
>>
>> On 2024/6/23 下午6:14, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Mon, May 27, 2024 at 3:46 PM Bibo Mao <maobibo@...ngson.cn> wrote:
>>>>
>>>> Before virt machine or vcpu is created, vmm need check supported
>>>> features from KVM. Here ioctl command KVM_HAS_DEVICE_ATTR is added
>>>> for VM, and macro KVM_LOONGARCH_VM_FEAT_CTRL is added to check
>>>> supported feature.
>>>>
>>>> Three sub-features relative with LBT are added, in later any new
>>>> feature can be added if it is used for vmm. The sub-features is
>>>> KVM_LOONGARCH_VM_FEAT_X86BT
>>>> KVM_LOONGARCH_VM_FEAT_ARMBT
>>>> KVM_LOONGARCH_VM_FEAT_MIPSBT
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@...ngson.cn>
>>>> ---
>>>> arch/loongarch/include/uapi/asm/kvm.h | 6 ++++
>>>> arch/loongarch/kvm/vm.c | 44 ++++++++++++++++++++++++++-
>>>> 2 files changed, 49 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>> index 656aa6a723a6..ed12e509815c 100644
>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>> @@ -91,6 +91,12 @@ struct kvm_fpu {
>>>> #define KVM_IOC_CSRID(REG) LOONGARCH_REG_64(KVM_REG_LOONGARCH_CSR, REG)
>>>> #define KVM_IOC_CPUCFG(REG) LOONGARCH_REG_64(KVM_REG_LOONGARCH_CPUCFG, REG)
>>>>
>>>> +/* Device Control API on vm fd */
>>>> +#define KVM_LOONGARCH_VM_FEAT_CTRL 0
>>>> +#define KVM_LOONGARCH_VM_FEAT_X86BT 0
>>>> +#define KVM_LOONGARCH_VM_FEAT_ARMBT 1
>>>> +#define KVM_LOONGARCH_VM_FEAT_MIPSBT 2
>>> I think LBT should be vcpu features rather than vm features, which is
>>> the same like CPUCFG and FP/SIMD.
>> yes, LBT is part of vcpu feature. Only when VMM check validity about
>> LBT, it is too late if it is vcpu feature. It is only checkable after
>> vcpu is created also, that is too late for qemu VMM.
> But why do we need so early to detect LBT? Why can the CPUCFG attr be
> implemented in vcpu.c?
To be frankly, I do not know neither.
By my understanding, cpu feature validity checking should be called
once, so it is put at cpu instance_init() at beginning. Else if the
checking is put in function kvm_arch_put_registers(), which is called by
many places if its state is changed, such as vcpu pause/resume,
migration, creation etc, it is unnecessary.
So LBT feature is part of VM, it is used for feature validity checking
when starting VM. Also it is part of vcpu, it can be used for
compatibility or post-migration feature checking. So the CPUCFG attr is
implemented in vcpu.c
To now, my ability and energy is to following framework and do
architecture specific; if you have any suggestion with qemu, you can
submit patches to create new things.
Regards
Bibo Mao
>
> Huacai
>
>>
>> However if it is VM feature, this feature can be checked even if VM or
>> VCPU is not created.
>>
>> So here is LBt is treated as VM capability also, You can check function
>> kvm_vm_ioctl_check_extension() on other architectures,
>> KVM_CAP_GUEST_DEBUG_HW_BPS/KVM_CAP_ARM_PMU_V3 are also VM features.
>>
>>>
>>> Moreover, this patch can be merged to the 2nd one.
>> Sure, I will merge it with 2nd patch.
>>
>> Regards
>> Bibo Mao
>>
>>>
>>> Huacai
>>>
>>>> +
>>>> /* Device Control API on vcpu fd */
>>>> #define KVM_LOONGARCH_VCPU_CPUCFG 0
>>>> #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>>> index 6b2e4f66ad26..09e05108c68b 100644
>>>> --- a/arch/loongarch/kvm/vm.c
>>>> +++ b/arch/loongarch/kvm/vm.c
>>>> @@ -99,7 +99,49 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>> return r;
>>>> }
>>>>
>>>> +static int kvm_vm_feature_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>> +{
>>>> + switch (attr->attr) {
>>>> + case KVM_LOONGARCH_VM_FEAT_X86BT:
>>>> + if (cpu_has_lbt_x86)
>>>> + return 0;
>>>> + return -ENXIO;
>>>> + case KVM_LOONGARCH_VM_FEAT_ARMBT:
>>>> + if (cpu_has_lbt_arm)
>>>> + return 0;
>>>> + return -ENXIO;
>>>> + case KVM_LOONGARCH_VM_FEAT_MIPSBT:
>>>> + if (cpu_has_lbt_mips)
>>>> + return 0;
>>>> + return -ENXIO;
>>>> + default:
>>>> + return -ENXIO;
>>>> + }
>>>> +}
>>>> +
>>>> +static int kvm_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
>>>> +{
>>>> + switch (attr->group) {
>>>> + case KVM_LOONGARCH_VM_FEAT_CTRL:
>>>> + return kvm_vm_feature_has_attr(kvm, attr);
>>>> + default:
>>>> + return -ENXIO;
>>>> + }
>>>> +}
>>>> +
>>>> int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>>> {
>>>> - return -ENOIOCTLCMD;
>>>> + struct kvm *kvm = filp->private_data;
>>>> + void __user *argp = (void __user *)arg;
>>>> + struct kvm_device_attr attr;
>>>> +
>>>> + switch (ioctl) {
>>>> + case KVM_HAS_DEVICE_ATTR:
>>>> + if (copy_from_user(&attr, argp, sizeof(attr)))
>>>> + return -EFAULT;
>>>> +
>>>> + return kvm_vm_has_attr(kvm, &attr);
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> }
>>>> --
>>>> 2.39.3
>>>>
>>
Powered by blists - more mailing lists