Re: [PATCH] RISC-V: KVM: Zicbo[m|z|p] block sizes should be always present in ONE_REG

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally exposing Zicbo[m|z|p] block size registers breaks cross-host live migration from hosts without these extensions to hosts with them.
--

commit 1377d7b8ff798dd58124162657e04db537839784
Author: Anup Patel <anup.patel@oss.qualcomm.com>

RISC-V: KVM: Zicbo[m|z|p] block sizes should be always present in ONE_REG

This commit drops the ISA extension checks for the Zicbo[m|z|p] block size
config registers in the KVM RISC-V ONE_REG interface. This makes these
registers unconditionally available to KVM user-space.

> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index bb920e8923c93..26d78987f2c23 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c

[ ... ]

> @@ -144,20 +138,14 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
>  		}
>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
> -		if (kvm_riscv_isa_check_host(ZICBOM))
> -			return -ENOENT;
>  		if (reg_val != riscv_cbom_block_size)
>  			return -EINVAL;
>  		break;

[Severity: High]
Does removing this extension check break live migration from a host without
the extension to a host with the extension?

If a VM is migrated from a source host lacking the Zicbo* extensions, the
block size saved by userspace will be 0. When restoring this VM on a
destination host that has the extension (where riscv_cbom_block_size > 0),
userspace will call KVM_SET_ONE_REG with 0. 

Because the host extension check is now removed, KVM will unconditionally
execute the equality check and return -EINVAL since 0 does not match the
destination's block size, causing the migration to fatally fail.

>  	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
> -		if (kvm_riscv_isa_check_host(ZICBOZ))
> -			return -ENOENT;
>  		if (reg_val != riscv_cboz_block_size)
>  			return -EINVAL;
>  		break;

[ ... ]

> @@ -614,20 +602,6 @@ static int copy_config_reg_indices(const struct kvm_vcpu *vcpu,
>  		u64 size;
>  		u64 reg;
>  
> -		/*
> -		 * Avoid reporting config reg if the corresponding extension
> -		 * was not available.
> -		 */
> -		if (i == KVM_REG_RISCV_CONFIG_REG(zicbom_block_size) &&
> -		    kvm_riscv_isa_check_host(ZICBOM))
> -			continue;
> -		else if (i == KVM_REG_RISCV_CONFIG_REG(zicboz_block_size) &&
> -			 kvm_riscv_isa_check_host(ZICBOZ))
> -			continue;
> -		else if (i == KVM_REG_RISCV_CONFIG_REG(zicbop_block_size) &&
> -			 kvm_riscv_isa_check_host(ZICBOP))
> -			continue;
> -
>  		size = IS_ENABLED(CONFIG_32BIT) ? KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
>  		reg = KVM_REG_RISCV | size | KVM_REG_RISCV_CONFIG | i;

[Severity: High]
Does unconditionally including these registers here force userspace to always
read and migrate them?

By always reporting these registers, userspace on a host without the
extensions will now read their value as 0 and migrate that value to the new
host, triggering the -EINVAL failure in kvm_riscv_vcpu_set_reg_config()
mentioned above.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706054002.1089394-1-anup.patel@oss.qualcomm.com?part=1