diff mbox series

[for-6.8,5/5] KVM: LoongArch: Clean up comments of _kvm_get_cpucfg_mask and kvm_check_cpucfg

Message ID 20240214101557.2900512-6-kernel@xen0n.name
State New
Headers
Series KVM: LoongArch: Fix wrong CPUCFG ID handling |

Commit Message

WANG Xuerui Feb. 14, 2024, 10:15 a.m. UTC 
  From: WANG Xuerui <git@xen0n.name>

Remove comments that are merely restatement of the code nearby, and
paraphrase the rest so they read more natural for English speakers (that
lack understanding of Chinese grammar). No functional changes.

Signed-off-by: WANG Xuerui <git@xen0n.name>
---
 arch/loongarch/kvm/vcpu.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

Comments

Huacai Chen Feb. 14, 2024, 1:08 p.m. UTC | #1 
Hi, Xuerui,

On Wed, Feb 14, 2024 at 6:16 PM WANG Xuerui <kernel@xen0n.name> wrote:
>
> From: WANG Xuerui <git@xen0n.name>
>
> Remove comments that are merely restatement of the code nearby, and
> paraphrase the rest so they read more natural for English speakers (that
> lack understanding of Chinese grammar). No functional changes.
>
> Signed-off-by: WANG Xuerui <git@xen0n.name>
> ---
>  arch/loongarch/kvm/vcpu.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 9e108ffaba30..ff51d6ba59aa 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -302,20 +302,14 @@ static int _kvm_get_cpucfg_mask(int id, u64 *v)
>  {
>         switch (id) {
>         case 2:
> -               /* Return CPUCFG2 features which have been supported by KVM */
> +               /* CPUCFG2 features unconditionally supported by KVM */
>                 *v = CPUCFG2_FP     | CPUCFG2_FPSP  | CPUCFG2_FPDP     |
>                      CPUCFG2_FPVERS | CPUCFG2_LLFTP | CPUCFG2_LLFTPREV |
>                      CPUCFG2_LAM;
> -               /*
> -                * If LSX is supported by CPU, it is also supported by KVM,
> -                * as we implement it.
> -                */
> +               /* If LSX is supported by the host, then it is also supported by KVM */
>                 if (cpu_has_lsx)
>                         *v |= CPUCFG2_LSX;
> -               /*
> -                * if LASX is supported by CPU, it is also supported by KVM,
> -                * as we implement it.
> -                */
> +               /* Same with LASX */
Consider a full description "If LASX is supported by the host, then it
is also supported by KVM"?

>                 if (cpu_has_lasx)
>                         *v |= CPUCFG2_LASX;
>
> @@ -336,21 +330,23 @@ static int kvm_check_cpucfg(int id, u64 val)
>
>         switch (id) {
>         case 2:
> -               /* CPUCFG2 features checking */
>                 if (val & ~mask)
> -                       /* The unsupported features should not be set */
> +                       /* Unsupported features should not be set */
>                         return -EINVAL;
>                 if (!(val & CPUCFG2_LLFTP))
> -                       /* The LLFTP must be set, as guest must has a constant timer */
> +                       /* Guests must have a constant timer */
>                         return -EINVAL;
>                 if ((val & CPUCFG2_FP) && (!(val & CPUCFG2_FPSP) || !(val & CPUCFG2_FPDP)))
> -                       /* Single and double float point must both be set when enable FP */
> +                       /* Single and double float point must both be set when FP is enabled */
>                         return -EINVAL;
>                 if ((val & CPUCFG2_LSX) && !(val & CPUCFG2_FP))
> -                       /* FP should be set when enable LSX */
> +                       /* LSX is architecturally defined to imply FP */
>                         return -EINVAL;
>                 if ((val & CPUCFG2_LASX) && !(val & CPUCFG2_LSX))
> -                       /* LSX, FP should be set when enable LASX, and FP has been checked before. */
> +                       /*
> +                        * LASX is architecturally defined to imply LSX and FP
> +                        * FP is checked just above
I think "LASX is architecturally defined to imply LSX and FP" is enough here.

> +                        */
>                         return -EINVAL;
>                 return 0;
>         default:
And I prefer to squash the last two patches together.

Huacai

> --
> 2.43.0
>
>
diff mbox series

Patch

diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 9e108ffaba30..ff51d6ba59aa 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -302,20 +302,14 @@  static int _kvm_get_cpucfg_mask(int id, u64 *v)
 {
 	switch (id) {
 	case 2:
-		/* Return CPUCFG2 features which have been supported by KVM */
+		/* CPUCFG2 features unconditionally supported by KVM */
 		*v = CPUCFG2_FP     | CPUCFG2_FPSP  | CPUCFG2_FPDP     |
 		     CPUCFG2_FPVERS | CPUCFG2_LLFTP | CPUCFG2_LLFTPREV |
 		     CPUCFG2_LAM;
-		/*
-		 * If LSX is supported by CPU, it is also supported by KVM,
-		 * as we implement it.
-		 */
+		/* If LSX is supported by the host, then it is also supported by KVM */
 		if (cpu_has_lsx)
 			*v |= CPUCFG2_LSX;
-		/*
-		 * if LASX is supported by CPU, it is also supported by KVM,
-		 * as we implement it.
-		 */
+		/* Same with LASX */
 		if (cpu_has_lasx)
 			*v |= CPUCFG2_LASX;
 
@@ -336,21 +330,23 @@  static int kvm_check_cpucfg(int id, u64 val)
 
 	switch (id) {
 	case 2:
-		/* CPUCFG2 features checking */
 		if (val & ~mask)
-			/* The unsupported features should not be set */
+			/* Unsupported features should not be set */
 			return -EINVAL;
 		if (!(val & CPUCFG2_LLFTP))
-			/* The LLFTP must be set, as guest must has a constant timer */
+			/* Guests must have a constant timer */
 			return -EINVAL;
 		if ((val & CPUCFG2_FP) && (!(val & CPUCFG2_FPSP) || !(val & CPUCFG2_FPDP)))
-			/* Single and double float point must both be set when enable FP */
+			/* Single and double float point must both be set when FP is enabled */
 			return -EINVAL;
 		if ((val & CPUCFG2_LSX) && !(val & CPUCFG2_FP))
-			/* FP should be set when enable LSX */
+			/* LSX is architecturally defined to imply FP */
 			return -EINVAL;
 		if ((val & CPUCFG2_LASX) && !(val & CPUCFG2_LSX))
-			/* LSX, FP should be set when enable LASX, and FP has been checked before. */
+			/*
+			 * LASX is architecturally defined to imply LSX and FP
+			 * FP is checked just above
+			 */
 			return -EINVAL;
 		return 0;
 	default: