Re: [PATCH] RISC-V: KVM: fix the Zicbo[m|z|p] block sizes in GET_ONE_REG

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: riscv: reversed logic in GET_ONE_REG for Zicbo block sizes
--

commit 66e4542b7a02ac3b1e7a491baa6d04283e1ccc3a
Author: Yong-Xuan Wang <yongxuan.wang@sifive.com>

RISC-V: KVM: fix the Zicbo[m|z|p] block sizes in GET_ONE_REG

This commit modifies the ternary operator logic used to determine the Zicbo
block sizes reported to KVM userspace via GET_ONE_REG. It changes the
previous behavior to return the block size or 0 based on host extension
support.

> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index 877ba823fc747..f2b1255694491 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c
> @@ -50,13 +50,13 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
>  		reg_val = vcpu->arch.isa[0] & KVM_RISCV_BASE_ISA_MASK;
>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
> -		reg_val = (kvm_riscv_isa_check_host(ZICBOM)) ? 0 : riscv_cbom_block_size;
> +		reg_val = (kvm_riscv_isa_check_host(ZICBOM)) ? riscv_cbom_block_size : 0;

[Severity: High]
Does this change reverse the intended logic and cause a regression in the KVM
userspace ABI?

Looking at kvm_riscv_vcpu_get_reg_config() in arch/riscv/kvm/vcpu_onereg.c,
kvm_riscv_isa_check_host() returns 0 on success (supported) and -ENOENT on
failure.

Because 0 evaluates to false in C, the updated ternary operator returns 0
when the extension is supported, and the block size when unsupported (-ENOENT
evaluates to true).

This causes KVM userspace to receive a 0 block size for supported extensions,
which can prevent VMs from starting or cause guest crashes when performing
cache operations.

>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
> -		reg_val = (kvm_riscv_isa_check_host(ZICBOZ)) ? 0 : riscv_cboz_block_size;
> +		reg_val = (kvm_riscv_isa_check_host(ZICBOZ)) ? riscv_cboz_block_size : 0;

[Severity: High]
Is this subject to the same logic reversal as above?

In kvm_riscv_vcpu_get_reg_config(), the ternary operator for the ZICBOZ
extension similarly returns 0 when the host extension is supported.

>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(zicbop_block_size):
> -		reg_val = (kvm_riscv_isa_check_host(ZICBOP)) ? 0 : riscv_cbop_block_size;
> +		reg_val = (kvm_riscv_isa_check_host(ZICBOP)) ? riscv_cbop_block_size : 0;

[Severity: High]
Does this also share the same logic reversal?

For the ZICBOP extension in kvm_riscv_vcpu_get_reg_config(), the ternary
operator evaluates to 0 when the host supports the extension.

>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(mvendorid):
>  		reg_val = vcpu->arch.mvendorid;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-kvm_fix_cbosize-v1-1-e57eec1b19cb@sifive.com?part=1