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

[PATCH v2] 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>
---
Changes since v1:
 - Update kvm_riscv_vcpu_set_reg_config() to return -EINVAL
   only when Zicbo[m|z|p] ISA extension is enabled for VCPU
---
 arch/riscv/kvm/vcpu_onereg.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index bb920e8923c9..a8333441192c 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,21 +138,18 @@ 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)
+		if (riscv_isa_extension_available(vcpu->arch.isa, ZICBOM) &&
+		    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)
+		if (riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ) &&
+		    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)
+		if (riscv_isa_extension_available(vcpu->arch.isa, ZICBOP) &&
+		    reg_val != riscv_cbop_block_size)
 			return -EINVAL;
 		break;
 	case KVM_REG_RISCV_CONFIG_REG(mvendorid):
@@ -614,20 +605,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 v2] 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 2 potential issue(s) to consider:
- [High] Silently ignoring writes to Zicbo block size registers when the extension is disabled allows userspace to bypass hardware block size validation, leading to guest memory corruption.
- [High] Unconditionally exposing Zicbo block sizes in KVM_GET_REG_LIST and validating them against current VCPU ISA state breaks VM migration from non-Zicbo hosts.
--

commit ae62f61922ea3db59d443a539b06f94dce05e2f4
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 removes the host ISA extension checks for Zicbo[m|z|p] block size
registers in the ONE_REG interface. It changes the register set logic to
validate the block size only if the extension is currently enabled on the
VCPU, and unconditionally exposes these registers in KVM_GET_REG_LIST.

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

[ ... ]

> @@ -144,21 +138,18 @@ 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)
> +		if (riscv_isa_extension_available(vcpu->arch.isa, ZICBOM) &&
> +		    reg_val != riscv_cbom_block_size)
>  			return -EINVAL;

[Severity: High]
Could this logic silently ignore writes and bypass validation?

If userspace temporarily disables the Zicbom extension on the VCPU and then
writes an unsupported block size to this register, the evaluation of:

    riscv_isa_extension_available(vcpu->arch.isa, ZICBOM)

will be false. KVM will then return 0 (success) instead of -EINVAL,
effectively ignoring the write.

If userspace later re-enables the extension, it might incorrectly believe
the unsupported block size was successfully configured and advertise it to
the guest. Since kvm_riscv_vcpu_get_reg_config() unconditionally returns
riscv_cbom_block_size, wouldn't this read/write asymmetry result in the
guest using an incorrect cache block size and potentially corrupting memory?

Additionally, could this temporal dependency on the VCPU ISA state break
VM migration?

When migrating from a host without Zicbo extensions (where block size is 0)
to a host with them, Zicbom might be enabled by default on the new VCPUs.
Userspace typically restores CONFIG registers before ISA_EXT registers.
If userspace attempts to restore zicbom_block_size as 0 while the extension
is enabled on the destination VCPU, this check will return -EINVAL and
abort the migration.

>  		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)
> +		if (riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ) &&
> +		    reg_val != riscv_cboz_block_size)
>  			return -EINVAL;

[ ... ]

> @@ -614,20 +605,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;

[Severity: High]
By removing this host ISA check, won't the register be unconditionally
exposed to KVM_GET_REG_LIST even on hosts that do not support the extension?

If a host lacks Zicbom, riscv_cbom_block_size will be 0. Exposing this 
register will cause userspace to serialize the value 0 during migration.
As noted above, attempting to restore this 0 value on a destination host
that supports Zicbom will then trigger an -EINVAL during restoration in
kvm_riscv_vcpu_set_reg_config().

> -		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;

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