Received-SPF: pass (google.com: domain of linux-kernel+bounces-65246-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240214101557.2900512-1-kernel@xen0n.name> <20240214101557.2900512-5-kernel@xen0n.name>
In-Reply-To: <20240214101557.2900512-5-kernel@xen0n.name>
From: Huacai Chen <chenhuacai@kernel.org>
Date: Wed, 14 Feb 2024 21:06:06 +0800
Message-ID: <CAAhV-H6htTqd3nHymcEpj2DDdeXU00OwtPJfYNpiSA7ac5JV7w@mail.gmail.com>
Subject: Re: [PATCH for-6.8 4/5] KVM: LoongArch: Streamline control flow of kvm_check_cpucfg
To: WANG Xuerui <kernel@xen0n.name>
Cc: Paolo Bonzini <pbonzini@redhat.com>, Tianrui Zhao <zhaotianrui@loongson.cn>, 
	Bibo Mao <maobibo@loongson.cn>, kvm@vger.kernel.org, loongarch@lists.linux.dev, 
	linux-kernel@vger.kernel.org, WANG Xuerui <git@xen0n.name>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi, Xuerui,

On Wed, Feb 14, 2024 at 6:16=E2=80=AFPM WANG Xuerui <kernel@xen0n.name> wro=
te:
>
> From: WANG Xuerui <git@xen0n.name>
>
> All the checks currently done in kvm_check_cpucfg can be realized with
> early returns, so just do that to avoid extra cognitive burden related
> to the return value handling.
>
> The default branch is unreachable because of the earlier validation by
> _kvm_get_cpucfg_mask, so mark it as such too to make things clearer.
>
> Signed-off-by: WANG Xuerui <git@xen0n.name>
> ---
>  arch/loongarch/kvm/vcpu.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index e973500611b4..9e108ffaba30 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -339,24 +339,23 @@ static int kvm_check_cpucfg(int id, u64 val)
>                 /* CPUCFG2 features checking */
>                 if (val & ~mask)
>                         /* The unsupported features should not be set */
> -                       ret =3D -EINVAL;
> -               else if (!(val & CPUCFG2_LLFTP))
> +                       return -EINVAL;
> +               if (!(val & CPUCFG2_LLFTP))
>                         /* The LLFTP must be set, as guest must has a con=
stant timer */
> -                       ret =3D -EINVAL;
> -               else if ((val & CPUCFG2_FP) && (!(val & CPUCFG2_FPSP) || =
!(val & CPUCFG2_FPDP)))
> +                       return -EINVAL;
> +               if ((val & CPUCFG2_FP) && (!(val & CPUCFG2_FPSP) || !(val=
 & CPUCFG2_FPDP)))
>                         /* Single and double float point must both be set=
 when enable FP */
> -                       ret =3D -EINVAL;
> -               else if ((val & CPUCFG2_LSX) && !(val & CPUCFG2_FP))
> +                       return -EINVAL;
> +               if ((val & CPUCFG2_LSX) && !(val & CPUCFG2_FP))
>                         /* FP should be set when enable LSX */
> -                       ret =3D -EINVAL;
> -               else if ((val & CPUCFG2_LASX) && !(val & CPUCFG2_LSX))
> +                       return -EINVAL;
> +               if ((val & CPUCFG2_LASX) && !(val & CPUCFG2_LSX))
>                         /* LSX, FP should be set when enable LASX, and FP=
 has been checked before. */
> -                       ret =3D -EINVAL;
> -               break;
> +                       return -EINVAL;
> +               return 0;
>         default:
> -               break;
> +               unreachable();
Maybe BUG() is better than unreachable()?

Huacai
>         }
> -       return ret;
>  }
>
>  static int kvm_get_one_reg(struct kvm_vcpu *vcpu,
> --
> 2.43.0
>