[PATCH for-6.8 4/5] KVM: LoongArch: Streamline control flow of kvm_check_cpucfg

WANG Xuerui posted 5 patches 1 year, 12 months ago
There is a newer version of this series
[PATCH for-6.8 4/5] KVM: LoongArch: Streamline control flow of kvm_check_cpucfg
Posted by WANG Xuerui 1 year, 12 months ago
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 = -EINVAL;
-		else if (!(val & CPUCFG2_LLFTP))
+			return -EINVAL;
+		if (!(val & CPUCFG2_LLFTP))
 			/* The LLFTP must be set, as guest must has a constant timer */
-			ret = -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 = -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 = -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 = -EINVAL;
-		break;
+			return -EINVAL;
+		return 0;
 	default:
-		break;
+		unreachable();
 	}
-	return ret;
 }
 
 static int kvm_get_one_reg(struct kvm_vcpu *vcpu,
-- 
2.43.0
Re: [PATCH for-6.8 4/5] KVM: LoongArch: Streamline control flow of kvm_check_cpucfg
Posted by Huacai Chen 1 year, 12 months ago
Hi, Xuerui,

On Wed, Feb 14, 2024 at 6:16 PM WANG Xuerui <kernel@xen0n.name> wrote:
>
> 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 = -EINVAL;
> -               else if (!(val & CPUCFG2_LLFTP))
> +                       return -EINVAL;
> +               if (!(val & CPUCFG2_LLFTP))
>                         /* The LLFTP must be set, as guest must has a constant timer */
> -                       ret = -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 = -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 = -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 = -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
>