| 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: <059d66e4-dd5d-0091-01d9-11aaba9297bd@loongson.cn>
Date: Tue, 2 Jul 2024 10:25:11 +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 v4 2/3] LoongArch: KVM: Add LBT feature detection function
On 2024/7/2 上午9:59, Huacai Chen wrote:
> On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@...ngson.cn> wrote:
>>
>> Huacai,
>>
>> On 2024/7/1 下午6:26, Huacai Chen wrote:
>>> On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@...ngson.cn> wrote:
>>>>
>>>>
>>>> Huacai,
>>>>
>>>> On 2024/6/30 上午10:07, Huacai Chen wrote:
>>>>> Hi, Bibo,
>>>>>
>>>>> On Wed, Jun 26, 2024 at 2:32 PM Bibo Mao <maobibo@...ngson.cn> wrote:
>>>>>>
>>>>>> Two kinds of LBT feature detection are added here, one is VCPU
>>>>>> feature, the other is VM feature. VCPU feature dection can only
>>>>>> work with VCPU thread itself, and requires VCPU thread is created
>>>>>> already. So LBT feature detection for VM is added also, it can
>>>>>> be done even if VM is not created, and also can be done by any
>>>>>> thread besides VCPU threads.
>>>>>>
>>>>>> Loongson Binary Translation (LBT) feature is defined in register
>>>>>> cpucfg2. Here LBT capability detection for VCPU is added.
>>>>>>
>>>>>> Here ioctl command KVM_HAS_DEVICE_ATTR is added for VM, and macro
>>>>>> KVM_LOONGARCH_VM_FEAT_CTRL is added to check supported feature. And
>>>>>> three sub-features relative with LBT are added as following:
>>>>>> 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/vcpu.c | 6 ++++
>>>>>> arch/loongarch/kvm/vm.c | 44 ++++++++++++++++++++++++++-
>>>>>> 3 files changed, 55 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>> index ddc5cab0ffd0..c40f7d9ffe13 100644
>>>>>> --- a/arch/loongarch/include/uapi/asm/kvm.h
>>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm.h
>>>>>> @@ -82,6 +82,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
>>>>>> +
>>>>>> /* Device Control API on vcpu fd */
>>>>>> #define KVM_LOONGARCH_VCPU_CPUCFG 0
>>>>>> #define KVM_LOONGARCH_VCPU_PVTIME_CTRL 1
>>>>> If you insist that LBT should be a vm feature, then I suggest the
>>>>> above two also be vm features. Though this is an UAPI change, but
>>>>> CPUCFG is upstream in 6.10-rc1 and 6.10-final hasn't been released. We
>>>>> have a chance to change it now.
>>>>
>>>> KVM_LOONGARCH_VCPU_PVTIME_CTRL need be attr percpu since every vcpu
>>>> has is own different gpa address.
>>> Then leave this as a vm feature.
>>>
>>>>
>>>> For KVM_LOONGARCH_VCPU_CPUCFG attr, it will not changed. We cannot break
>>>> the API even if it is 6.10-rc1, VMM has already used this. Else there is
>>>> uapi breaking now, still will be in future if we cannot control this.
>>> UAPI changing before the first release is allowed, which means, we can
>>> change this before the 6.10-final, but cannot change it after
>>> 6.10-final.
>> Now QEMU has already synced uapi to its own directory, also I never hear
>> about this, with my experience with uapi change, there is only newly
>> added or removed deprecated years ago.
>>
>> Is there any documentation about UAPI change rules?
> No document, but learn from my more than 10 years upstream experience.
Can you show me an example about with your rich upstream experience?
>
>>>
>>>>
>>>> How about adding new extra features capability for VM such as?
>>>> +#define KVM_LOONGARCH_VM_FEAT_LSX 3
>>>> +#define KVM_LOONGARCH_VM_FEAT_LASX 4
>>> They should be similar as LBT, if LBT is vcpu feature, they should
>>> also be vcpu features; if LBT is vm feature, they should also be vm
>>> features.
>> On other architectures, with function kvm_vm_ioctl_check_extension()
>> KVM_CAP_XSAVE2/KVM_CAP_PMU_CAPABILITY on x86
>> KVM_CAP_ARM_PMU_V3/KVM_CAP_ARM_SVE on arm64
>> These features are all cpu features, at the same time they are VM features.
>>
>> If they are cpu features, how does VMM detect validity of these features
>> passing from command line? After all VCPUs are created and send bootup
>> command to these VCPUs? That is too late, VMM main thread is easy to
>> detect feature validity if they are VM features also.
>>
>> To be honest, I am not familiar with KVM still, only get further
>> understanding after actual problems solving. Welcome to give comments,
>> however please read more backgroud if you insist on, else there will be
>> endless argument again.
> I just say CPUCFG/LSX/LASX and LBT should be in the same class, I
> haven't insisted on whether they should be vcpu features or vm
> features.
It is reasonable if LSX/LASX/LBT should be in the same class, since
there is feature options such as lsx=on/off,lasx=on/off,lbt=on/off.
What is the usage about CPUCFG capability used for VM feature? It is not
a detailed feature, it is only feature-set indicator like cpuid.
Regards
Bibo Mao
>
> Huacai
>
>>
>> Regards
>> Bibo, Mao
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>>
>>>>> Huacai
>>>>>
>>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>>>> index 233d28d0e928..9734b4d8db05 100644
>>>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>>>> @@ -565,6 +565,12 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>>>>>> *v |= CPUCFG2_LSX;
>>>>>> if (cpu_has_lasx)
>>>>>> *v |= CPUCFG2_LASX;
>>>>>> + if (cpu_has_lbt_x86)
>>>>>> + *v |= CPUCFG2_X86BT;
>>>>>> + if (cpu_has_lbt_arm)
>>>>>> + *v |= CPUCFG2_ARMBT;
>>>>>> + if (cpu_has_lbt_mips)
>>>>>> + *v |= CPUCFG2_MIPSBT;
>>>>>>
>>>>>> return 0;
>>>>>> case LOONGARCH_CPUCFG3:
>>>>>> 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