Re: [PATCH v3 03/26] target/arm/kvm: Return immediately on error in kvm_arch_init()

[Philippe Mathieu-Daudé]

Hi Jean-Philippe,

On 25/11/24 20:56, Jean-Philippe Brucker wrote:
> Returning an error to kvm_init() is fatal anyway, no need to continue
> the initialization.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   target/arm/kvm.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 8bdf4abeb6..95bcecf804 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -563,7 +563,7 @@ int kvm_arch_get_default_type(MachineState *ms)
>   
>   int kvm_arch_init(MachineState *ms, KVMState *s)
>   {
> -    int ret = 0;
> +    int ret;

With your change we can reduce this variable scope ...

>       /* For ARM interrupt delivery is always asynchronous,
>        * whether we are using an in-kernel VGIC or not.
>        */
> @@ -585,7 +585,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>           !kvm_check_extension(s, KVM_CAP_ARM_IRQ_LINE_LAYOUT_2)) {
>           error_report("Using more than 256 vcpus requires a host kernel "
>                        "with KVM_CAP_ARM_IRQ_LINE_LAYOUT_2");
> -        ret = -EINVAL;
> +        return -EINVAL;
>       }
>   
>       if (kvm_check_extension(s, KVM_CAP_ARM_NISV_TO_USER)) {
> @@ -607,13 +607,14 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>               warn_report("Eager Page Split support not available");
>           } else if (!(s->kvm_eager_split_size & sizes)) {
>               error_report("Eager Page Split requested chunk size not valid");
> -            ret = -EINVAL;
> +            return -EINVAL;
>           } else {
>               ret = kvm_vm_enable_cap(s, KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE, 0,
>                                       s->kvm_eager_split_size);

... by declaring it here.

Otherwise:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>               if (ret < 0) {
>                   error_report("Enabling of Eager Page Split failed: %s",
>                                strerror(-ret));
> +                return ret;
>               }
>           }
>       }
> @@ -626,7 +627,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       hw_breakpoints = g_array_sized_new(true, true,
>                                          sizeof(HWBreakpoint), max_hw_bps);
>   
> -    return ret;
> +    return 0;
>   }
>   
>   unsigned long kvm_arch_vcpu_id(CPUState *cpu)