On Mon, Jun 24, 2024 at 10:00 AM maobibo <maobibo@xxxxxxxxxxx> wrote:
>
>
>
> On 2024/6/23 下午6:14, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > On Mon, May 27, 2024 at 3:46 PM Bibo Mao <maobibo@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> >> ---
> >> 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?
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
> >>
>
