Received-SPF: pass (google.com: domain of linux-kernel+bounces-76486-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Subject: Re: [PATCH for-6.8 v4 3/3] LoongArch: KVM: Streamline
 kvm_check_cpucfg and improve comments
To: WANG Xuerui <kernel@xen0n.name>, Paolo Bonzini <pbonzini@redhat.com>,
 Huacai Chen <chenhuacai@kernel.org>
Cc: Tianrui Zhao <zhaotianrui@loongson.cn>, kvm@vger.kernel.org,
 loongarch@lists.linux.dev, linux-kernel@vger.kernel.org,
 WANG Xuerui <git@xen0n.name>
References: <20240222105109.2042732-1-kernel@xen0n.name>
 <20240222105109.2042732-4-kernel@xen0n.name>
From: maobibo <maobibo@loongson.cn>
Message-ID: <719c1a92-6ddc-f73a-0609-aa5d40dbee8a@loongson.cn>
Date: Thu, 22 Feb 2024 20:11:20 +0800
User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20240222105109.2042732-4-kernel@xen0n.name>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit


On 2024/2/22 下午6:51, WANG Xuerui 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.
> 
> While at it, clean up comments of _kvm_get_cpucfg_mask and
> kvm_check_cpucfg, by removing comments that are merely restatement of
> the code nearby, and paraphrasing the rest so they read more natural
> for English speakers (that likely are not familiar with the actual
> Chinese-influenced grammar).
> 
> No functional changes intended.
> 
> Signed-off-by: WANG Xuerui <git@xen0n.name>
> ---
>   arch/loongarch/kvm/vcpu.c | 42 +++++++++++++++++++--------------------
>   1 file changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 9f63bbaf19c1..128b89d00ced 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -305,20 +305,16 @@ 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.
> +		 * For the ISA extensions listed below, if one 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.
> -		 */
>   		if (cpu_has_lasx)
>   			*v |= CPUCFG2_LASX;
>   
> @@ -349,24 +345,26 @@ static int kvm_check_cpucfg(int id, u64 val)
>   
>   	switch (id) {
>   	case 2:
> -		/* CPUCFG2 features checking */
>   		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)))
> -			/* Single and double float point must both be set when enable FP */
> -			ret = -EINVAL;
> -		else if ((val & CPUCFG2_LSX) && !(val & CPUCFG2_FP))
> -			/* FP should be set when enable LSX */
> -			ret = -EINVAL;
> -		else if ((val & CPUCFG2_LASX) && !(val & CPUCFG2_LSX))
> -			/* LSX, FP should be set when enable LASX, and FP has been checked before. */
> -			ret = -EINVAL;
> -		break;
> +			/* 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 FP is enabled */
> +			return -EINVAL;
> +		if ((val & CPUCFG2_LSX) && !(val & CPUCFG2_FP))
> +			/* LSX architecturally implies FP but val does not satisfy that */
> +			return -EINVAL;
> +		if ((val & CPUCFG2_LASX) && !(val & CPUCFG2_LSX))
> +			/* LASX architecturally implies LSX and FP but val does not satisfy that */
> +			return -EINVAL;
> +		return 0;
>   	default:
> -		break;
> +		/*
> +		 * Values for the other CPUCFG IDs are not being further validated
> +		 * besides the mask check above.
> +		 */
> +		return 0;
>   	}
> -	return ret;
>   }
>   
>   static int kvm_get_one_reg(struct kvm_vcpu *vcpu,
> 
Reviewed-by: Bibo Mao <maobibo@loongson.cn>