linux-kernel - Re: [PATCH for-6.8 v4 3/3] LoongArch: KVM: Streamline kvm_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Thu, 22 Feb 2024 20:11:20 +0800
From: maobibo <maobibo@...ngson.cn>
To: WANG Xuerui <kernel@...0n.name>, Paolo Bonzini <pbonzini@...hat.com>,
 Huacai Chen <chenhuacai@...nel.org>
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>, kvm@...r.kernel.org,
 loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
 WANG Xuerui <git@...0n.name>
Subject: Re: [PATCH for-6.8 v4 3/3] LoongArch: KVM: Streamline
 kvm_check_cpucfg and improve comments



On 2024/2/22 下午6:51, WANG Xuerui wrote:
> From: WANG Xuerui <git@...0n.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@...0n.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@...ngson.cn>