Re: [RFC PATCH RESEND 1/4] target/arm/kvm: Introduce helper to check target impl CPU support

[Eric Auger <eric.auger@redhat.com>]

Hi Shameer,

On 8/1/25 9:47 AM, Shameer Kolothum wrote:
> From: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
>
> The helper function will try to set the SMCCC filters for KVM vendor
> hypercalls related to target implementation CPU support. It also
> checks the kernel support for writable implementation ID registers
> (MIDR/REVIDR/AIDR) and enables it.
>
> Subsequent patches for Target Impl CPU support will make use of this
> helper.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  target/arm/kvm.c     | 72 ++++++++++++++++++++++++++++++++++++++++++++
>  target/arm/kvm_arm.h | 12 ++++++++
>  2 files changed, 84 insertions(+)
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 3f41f99e23..eb04640b50 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -2072,6 +2072,78 @@ bool kvm_arm_mte_supported(void)
>      return kvm_check_extension(kvm_state, KVM_CAP_ARM_MTE);
>  }
>  
> +static bool kvm_arm_set_vm_attr(struct kvm_device_attr *attr, const char *name)
> +{
> +    int err;
> +
> +    err = kvm_vm_ioctl(kvm_state, KVM_HAS_DEVICE_ATTR, attr);
> +    if (err != 0) {
> +        error_report("%s: KVM_HAS_DEVICE_ATTR: %s", name, strerror(-err));
> +        return false;
> +    }
> +
> +    err = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, attr);
> +    if (err != 0) {
> +        error_report("%s: KVM_SET_DEVICE_ATTR: %s", name, strerror(-err));
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static bool kvm_arm_set_smccc_filter(uint64_t func, uint8_t faction)
nit: base is _u32 in the kernel
> +{
> +    struct kvm_smccc_filter filter = {
> +        .base = func,
> +        .nr_functions = 1,
> +        .action = faction,
> +    };
> +    struct kvm_device_attr attr = {
> +        .group = KVM_ARM_VM_SMCCC_CTRL,
> +        .attr = KVM_ARM_VM_SMCCC_FILTER,
> +        .flags = 0,
> +        .addr = (uintptr_t)&filter,
> +    };
> +
> +    if (!kvm_arm_set_vm_attr(&attr, "SMCCC Filter")) {
> +        error_report("failed to set SMCCC filter in KVM Host");
maybe also output @func in the error msg
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +bool kvm_arm_target_impl_cpus_supported(void)
> +{
> +    if (!kvm_arm_set_smccc_filter(
> +        ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_VER_FUNC_ID,
> +        KVM_SMCCC_FILTER_FWD_TO_USER)) {
> +        error_report("ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_VER fwd filter "
> +                     "install failed");
> +        return false;
> +    }
> +
> +    if (!kvm_arm_set_smccc_filter(
> +        ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID,
> +        KVM_SMCCC_FILTER_FWD_TO_USER)) {
> +        error_report("ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_CPUS fwd filter "
> +                     "install failed");
> +        return false;
> +    }
> +
> +    if (!kvm_check_extension(kvm_state, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS)) {
> +        error_report("KVM_CAP_ARM_WRITABLE_IMP_ID_REGS not supported");
> +        return false;
> +    }
> +
> +    if (kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS, 0)) {
> +        error_report("Failed to enable KVM_CAP_ARM_WRITABLE_IMP_ID_REGS cap");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
>  
>  uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index ba5de45f86..3cd6447901 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -207,6 +207,13 @@ bool kvm_arm_sve_supported(void);
>   */
>  bool kvm_arm_mte_supported(void);
>  
> +/**
> + * kvm_arm_target_impl_cpus_supported:
> + *
> + * Returns: true if KVM can enable target impl CPUs, and false otherwise.
> + */
> +bool kvm_arm_target_impl_cpus_supported(void);
> +
>  /**
>   * kvm_arm_get_max_vm_ipa_size:
>   * @ms: Machine state handle
> @@ -263,6 +270,11 @@ static inline bool kvm_arm_mte_supported(void)
>      return false;
>  }
>  
> +static inline bool kvm_arm_target_impl_cpus_supported(void)
> +{
> +    return false;
> +}
> +
>  static inline int kvm_arm_get_writable_id_regs(ARMCPU *cpu, IdRegMap *idregmap)
>  {
>      return -ENOSYS;
Otherwise looks good to me.

Thanks

Eric