| 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: <bdcc9ec4-31a8-1438-25c0-be8ba7f49ed0@loongson.cn>
Date: Thu, 4 Jul 2024 09:35:18 +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/3 下午11:35, Huacai Chen wrote:
> On Wed, Jul 3, 2024 at 11:15 AM maobibo <maobibo@...ngson.cn> wrote:
>>
>>
>>
>> On 2024/7/2 下午11:43, Huacai Chen wrote:
>>> On Tue, Jul 2, 2024 at 4:42 PM maobibo <maobibo@...ngson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2024/7/2 下午3:28, Huacai Chen wrote:
>>>>> On Tue, Jul 2, 2024 at 12:13 PM maobibo <maobibo@...ngson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2024/7/2 上午10:34, Huacai Chen wrote:
>>>>>>> On Tue, Jul 2, 2024 at 10:25 AM maobibo <maobibo@...ngson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 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?
>>>>>>> A simple example,
>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c revert
>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 and it changes UAPI.
>>>>>>> 1dccdba084897443d116508a8ed71e0ac8a0 is upstream in 6.9-rc1, and
>>>>>>> e877d705704d7c8fe17b6b5ebdfdb14b84c can revert the behavior before
>>>>>>> 6.9-final, but not after that.
>>>>>>>
>>>>>>> Before the first release, the code status is treated as "unstable", so
>>>>>>> revert, modify is allowed. But after the first release, even if an
>>>>>>> "error" should also be treated as a "bad feature".
>>>>>> Huacai,
>>>>>>
>>>>>> Thanks for showing the example.
>>>>>>
>>>>>> For this issue, Can we adding new uapi and mark the old as deprecated?
>>>>>> so that it can be removed after years.
>>>>> Unnecessary, just remove the old one. Deprecation is for the usage
>>>>> after the first release.
>>>>>
>>>>>>
>>>>>> For me, it is too frequent to revert the old uapi, it is not bug and
>>>>>> only that we have better method now. Also QEMU has synchronized the uapi
>>>>>> to its directory already.
>>>>> QEMU also hasn't been released after synchronizing the uapi, so it is
>>>>> OK to remove the old api now.
>>>> No, I will not do such thing. It is just a joke to revert the uapi.
>>>>
>>>> So just create new world and old world on Loongarch system again?
>>> Again, code status before the first release is *unstable*, that status
>>> is not enough to be a "world".
>>>
>>> It's your responsibility to make a good design at the beginning, but
>>> you fail to do that. Fortunately we are before the first release;
>>> unfortunately you don't want to do that.
>> Yes, this is flaw at the beginning, however it can works and new abi can
>> be added.
>>
>> If there is no serious bug and it is synced to QEMU already, I am not
>> willing to revert uabi. Different projects have its own schedule plan,
>> that is one reason. The most important reason may be that different
>> peoples have different ways handling these issues.
> In another thread I found that Jiaxun said he has a solution to make
> LBT be a vcpu feature and still works well. However, that may take
> some time and is too late for 6.11.
>
> But we have another choice now: just remove the UAPI and vm.c parts in
> this series, let the LBT main parts be upstream in 6.11, and then
> solve other problems after 6.11. Even if Jiaxun's solution isn't
> usable, we can still use this old vm feature solution then.
I am sure it is best if it is VM feature for LBT feature detection,
LSX/LASX feature detection uses CPU feature, we can improve it later.
For host cpu type or migration feature detection, I have no idea now,
also I do not think it will be big issue for me, I will do it with
scheduled time. Of source, welcome Jiaxun and you to implement host cpu
type or migration feature detection.
Regards
Bibo Mao
>
>
> Huacai
>>
>> Regards
>> Bibo, Mao
>>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo, Mao
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo, Mao
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> 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