On Tue, Jul 2, 2024 at 9:51 AM maobibo <maobibo@xxxxxxxxxxx> wrote:
>
> Huacai,
>
> On 2024/7/1 下午6:26, Huacai Chen wrote:
> > On Mon, Jul 1, 2024 at 9:27 AM maobibo <maobibo@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> >>>> ---
> >>>> 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.
> >
> >>
> >> 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.
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
> >>>>
> >>
>
