On Thu, 29 Oct 2026 at 09:23, Eric Auger <[email protected]> wrote:
>
> Pass an error handle to kvm_arm_init_cpreg_list(). A Subsequent patch
> will do the same with kvm_arch_init_vcpu().
>
> Signed-off-by: Eric Auger <[email protected]>
I wondered if putting patches 6 and 7 the other way around
would be better but it doesn't seem worth the effort of
rearranging them now.
> ---
> target/arm/kvm.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 28f199c03f..d454346037 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -872,6 +872,7 @@ static bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t
> regidx)
> /**
> * kvm_arm_init_cpreg_list:
> * @cpu: ARMCPU
> + * @errp: Error handle
> *
> * Initialize the ARMCPU cpreg list according to the kernel's
> * definition of what CPU registers it knows about (and throw away
> @@ -879,7 +880,7 @@ static bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t
> regidx)
> *
> * Returns: 0 if success, else < 0 error code
> */
> -static int kvm_arm_init_cpreg_list(ARMCPU *cpu)
> +static int kvm_arm_init_cpreg_list(ARMCPU *cpu, Error **errp)
> {
> struct kvm_reg_list rl;
> struct kvm_reg_list *rlp;
> @@ -889,12 +890,17 @@ static int kvm_arm_init_cpreg_list(ARMCPU *cpu)
> rl.n = 0;
> ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
> if (ret != -E2BIG) {
> + if (ret < 0) {
> + error_setg_errno(errp, -ret,
> + "failed to get KVM register list size");
> + }
This made me wonder about what the "returns success" case here
would mean. I think that's in practice a "can't-happen" case,
because it would mean KVM had no registers in its register list
(i.e. we handed it a 0-sized list and it said "sure, that's big
enough"). But we should treat it as an error, because otherwise
we'll return success here and then later code will assume that
cpu->cpreg_indexes and so on have been set up for KVM when they
haven't been.
But this is a pre-existing bug.
Reviewed-by: Peter Maydell <[email protected]>
thanks
-- PMM
