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

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

[Anup Patel]

All config and core registers of the KVM RISC-V ONE_REG interface are
expected to be always available to the KVM user-space and the KVM
get-reg-list selftest assumes these registers to be as base registers.

Currently, the Zicbo[m|z|p] block size config registers are only
available when corresponding ISA extension is present on the host
which breaks the above expectation. In fact, KVM get-reg-list selftest
fails when any of the Zicbo[m|z|p] ISA extension is not present on
host. To address this issue, drop the ISA extension checks from
kvm_riscv_vcpu_get/set_reg_config() and copy_config_reg_indices()
functions.

Fixes: 031f9efafc08 ("KVM: riscv: Add KVM_GET_REG_LIST API support")
Fixes: a044ef71043e ("RISC-V: KVM: use ENOENT in *_one_reg() when extension is unavailable")
Fixes: 48e2febcda74 ("RISC-V: KVM: Provide UAPI for Zicbop block size")
Fixes: cf05b059d59f ("RISC-V: KVM: Introduce common kvm_riscv_isa_check_host()")
Signed-off-by: Anup Patel <anup.patel@oss.qualcomm.com>
---
 arch/riscv/kvm/vcpu_onereg.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index bb920e8923c9..26d78987f2c2 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -50,18 +50,12 @@ 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):
-		if (kvm_riscv_isa_check_host(ZICBOM))
-			return -ENOENT;
 		reg_val = riscv_cbom_block_size;
 		break;
 	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
-		if (kvm_riscv_isa_check_host(ZICBOZ))
-			return -ENOENT;
 		reg_val = riscv_cboz_block_size;
 		break;
 	case KVM_REG_RISCV_CONFIG_REG(zicbop_block_size):
-		if (kvm_riscv_isa_check_host(ZICBOP))
-			return -ENOENT;
 		reg_val = riscv_cbop_block_size;
 		break;
 	case KVM_REG_RISCV_CONFIG_REG(mvendorid):
@@ -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;
 	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;
 	case KVM_REG_RISCV_CONFIG_REG(zicbop_block_size):
-		if (kvm_riscv_isa_check_host(ZICBOP))
-			return -ENOENT;
 		if (reg_val != riscv_cbop_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;
 
-- 
2.43.0

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

[sashiko-bot]

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