Re: [RFC PATCH v3 06/19] target/arm/kvm: Read all ID registers from KVM

On 7/16/26 11:38 PM, Khushit Shah wrote:
> Try to read all the ID registers from KVM. KVM may return ENOENT, for
> ID registers it does not support reading from userspace. Ignore those
> registers. As this list is small, in future we may anyway want to
> handle those register specially.
>
> We want to do this so that we can expose more or less complete
> host feature view to management stack, as for migration, we want both
> writable and non-writable ID registers to match.
>
> Also remove the unused 'cpu' args throughout the call-chain.
I will integrate this fix in the parent series
>
> Signed-off-by: Khushit Shah <[email protected]>
> ---
>  target/arm/kvm.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 63b3727e05..42ff731f6e 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -292,14 +292,14 @@ static int kvm_feature_idx_to_idregs_idx(int kidx)
>  }
>  
>  /*
> - * get_host_cpu_idregs: Read all the writable ID reg host values
> + * get_host_cpu_idregs: Read all the ID reg host values
>   *
> - * Need to be called once the writable mask has been populated
> - * Note we may want to read all the known id regs but some of them are not
> - * writable and return an error, hence the choice of reading only those which
> - * are writable. Those are also readable!
> + * We currently try to read all the registers. If the read fails with
> + * ENOENT (KVM does not identify the ID register) ignore it.
> + * Some of the registers that errored with ENOENT will anyway need special
> + * handling.
>   */
> -static int get_host_cpu_idregs(ARMCPU *cpu, int fd, ARMHostCPUFeatures *ahcf)
> +static int get_host_cpu_idregs(int fd, ARMHostCPUFeatures *ahcf)
>  {
>      int err = 0;
>      int i;
> @@ -310,26 +310,26 @@ static int get_host_cpu_idregs(ARMCPU *cpu, int fd, ARMHostCPUFeatures *ahcf)
>          uint64_t *reg;
>          int ret;
>  
> -        if (!sysregdesc->writable_mask) {
> -            continue;
> -        }
> -
>          reg = &ahcf->isar.idregs[i];
>          ret = read_sys_reg64(fd, reg, idregs_sysreg_to_kvm_reg(sysreg));
> -        trace_get_host_cpu_idregs(sysregdesc->name, *reg);
>          if (ret) {
> -            error_report("%s error reading value of host %s register (%m)",
> -                         __func__, sysregdesc->name);
> -
> -            err = ret;
> +            if (errno == ENOENT) {
> +                warn_report("%s error reading value of host %s register "
> +                                "(ENOENT)", __func__, sysregdesc->name);
if there are expected cases where the read is failing I don't think we
want to warn. Add a trace point instead.
> +            } else {
> +                error_report("%s error reading value of host %s register"
> +                            " (%m)", __func__, sysregdesc->name);
> +                err = ret;
> +            }
>          }
> +        trace_get_host_cpu_idregs(sysregdesc->name, *reg);
>      }
>      return err;
>  }
>  
>  
>  static void
> -kvm_arm_get_host_cpu_features(ARMCPU *cpu, ARMHostCPUFeatures *ahcf)
> +kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>  {
>      /* Identify the feature bits corresponding to the host CPU, and
>       * fill out the ARMHostCPUClass fields accordingly. To do this
> @@ -415,8 +415,8 @@ kvm_arm_get_host_cpu_features(ARMCPU *cpu, ARMHostCPUFeatures *ahcf)
>          SET_IDREG(&ahcf->isar, ID_AA64PFR0, 0x00000011); /* EL1&0, AArch64 only */
>          err = 0;
>      } else {
> -        /* Make sure all writable ID reg values are initialized */
> -        err |= get_host_cpu_idregs(cpu, fd, ahcf);
> +        /* Try to read all the ID registers */
> +        err |= get_host_cpu_idregs(fd, ahcf);
>  
>          /*
>           * temporarily override the CLIDR_EL1 value since some host values
> @@ -595,7 +595,7 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
>      g_free(writable_map);
>  
>      if (!arm_host_cpu_features.dtb_compatible) {
> -        kvm_arm_get_host_cpu_features(cpu, &arm_host_cpu_features);
> +        kvm_arm_get_host_cpu_features(&arm_host_cpu_features);
>      }
>  
>      cpu->kvm_target = arm_host_cpu_features.target;
Thanks

Eric