Re: [PATCH v3 09/17] riscv: kvm: Use generated instruction headers for csr code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Undefined macro `GET_REG` causes a build failure, and intended `get_reg()` usage masks a logic error where an immediate is treated as a register index.
--

--- Patch [9]: [PATCH v3 09/17] riscv: kvm: Use generated instruction headers for csr code ---
commit d70b1983c271eb6387c8473892b04f021becb8ff
Author: Charlie Jenkins <thecharlesjenkins@gmail.com>

riscv: kvm: Use generated instruction headers for csr code

This commit migrates the CSR parsing code in RISC-V KVM to use the generated
instruction headers instead of hand-written instruction composition functions.

> diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
> index f09f9251d1f0a..a1898ab8bbe85 100644
> --- a/arch/riscv/kvm/vcpu_insn.c
> +++ b/arch/riscv/kvm/vcpu_insn.c

[ ... ]

> @@ -144,45 +144,49 @@ static int csr_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
>  	const struct csr_func *tcfn, *cfn = NULL;
>  	ulong val = 0, wr_mask = 0, new_val = 0;
>  
>  	/* Decode the CSR instruction */

[ ... ]

> +	} else if (riscv_insn_is_csrrwi(insn)) {
>  		wr_mask = -1UL;
> +		new_val = riscv_insn_csrrwi_extract_imm(insn);
> +		csr_num = riscv_insn_csrrwi_extract_csr(insn);
> +	} else if (riscv_insn_is_csrrsi(insn)) {
> +		wr_mask = riscv_insn_csrrwi_extract_imm(insn);

[Severity: High]
Should this use a csrrsi specific extraction function? It looks like it is
currently reusing riscv_insn_csrrwi_extract_imm() due to a copy-paste error.

>  		new_val = -1UL;
> +		csr_num = riscv_insn_csrrsi_extract_csr(insn);
> +	} else if (riscv_insn_is_csrrci(insn)) {
> +		wr_mask = GET_REG(riscv_insn_csrrwi_extract_imm(insn));

[Severity: High]
This introduces an undefined macro GET_REG, which will break the build.

If the intention was to use the newly added get_reg() helper, does that
introduce a logic error? The csrrci instruction uses a 5-bit zero-extended
immediate directly as the write mask, rather than using the immediate as a
register index.

Also, similar to the csrrsi branch above, should this use the csrrci
extraction functions instead of riscv_insn_csrrwi_extract_imm() and
riscv_insn_csrrwi_extract_csr()?

>  		new_val = 0;
> +		csr_num = riscv_insn_csrrwi_extract_csr(insn);
> +	} else {
>  		return rc;
>  	}
>  
> +	#undef GET_REG
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-riscv_insn_table-v3-0-3f8e5f75ba97@gmail.com?part=9