Re: [PATCH] riscv/kvm: Only context-switch senvcfg where it exists

From: Bo Gan <ganboing@gmail.com>
To: Davidlohr Bueso <dave@stgolabs.net>,
	anup@brainfault.org, atish.patra@linux.dev
Cc: pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu,
	linux-riscv@lists.infradead.org, kvm-riscv@lists.infradead.org
Subject: Re: [PATCH] riscv/kvm: Only context-switch senvcfg where it exists
Date: Fri, 11 Sep 2026 02:50:17 -0700	[thread overview]
Message-ID: <4ad187c9-44ae-461a-a29e-2531f0906a1b@gmail.com> (raw)
In-Reply-To: <20260827211209.2871124-1-dave@stgolabs.net>

Hi Davidlohr,

Thanks for attempting to fix this long standing issue on P550. While
you are at it, can you add the conditional write of henvcfg as well?
P550 lacks both henvcfg and senvcfg, so needs to avoid touch either
of them. Be ware that the vendor's OpenSBI carries a patch to silently
ignore the access of h/senvcfg:
https://github.com/sifiveinc/meta-sifive/commit/942256244c61517d375d9359577dafeb04e258e8

However, this patch is just wrong -- it doesn't set return values
properly; it doesn't check for previous privilege levels...

You can just use the latest upstream OpenSBI, where I've contributed the
EIC7700/P550 support code, and get rid of all these vendor shenanigans.
I've thought about doing a proper emulation of henvcfg/senvcfg and hard-
wire them to 0 in OpenSBI, but later it's deemed not spec compliant --
if henvcfg is available, then FIOM bits must be writable.

Thus, you are doing the right thing avoiding accessing them in HS mode.
Please add the henvcfg handling, and I can help testing it. It should be
a pretty simple change.

Bo

On 8/27/26 14:12, Davidlohr Bueso wrote:
> senvcfg arrived in privileged spec 1.12, but H-capable cores on a
> 1.11 base exist, ie: sifive P500 (ESWIN EIC7700X) implements the
> hypervisor extension yet has no senvcfg. KVM swaps the CSR
> unconditionally on every vCPU entry, panicing the host upon a guest
> executing its first instruction:
> 
>    Oops - illegal instruction [#1]
>    Modules linked in: kvm
>    CPU: 2 UID: 0 PID: 127 Comm: qemu-system-ris Not tainted 7.2.0-kvm73bug+ #4 PREEMPTLAZY
>    Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2024.01 01/01/2024
>    epc : kvm_riscv_vcpu_enter_exit+0x26/0xcf8 [kvm]
>     ra : kvm_arch_vcpu_ioctl_run+0x4ce/0x5a6 [kvm]
>    status: 0000000200000100 badaddr: 0000000010a797f3 cause: 0000000000000002
>    [<ffffffff01df432e>] kvm_riscv_vcpu_enter_exit+0x26/0xcf8 [kvm]
>    [<ffffffff01deb3b0>] kvm_arch_vcpu_ioctl_run+0x4ce/0x5a6 [kvm]
>    [<ffffffff01dd7914>] kvm_vcpu_ioctl+0x180/0x5e4 [kvm]
>    [<ffffffff8025702e>] __riscv_sys_ioctl+0x84/0x9a
>    [<ffffffff80dfdc72>] do_trap_ecall_u+0x1d8/0x38a
>    [<ffffffff80e08644>] handle_exception+0x168/0x174
>    Code: 84aa 892e b703 8887 1773 1067 3023 16e5 b783 8907 (97f3) 10a7
> 
> Touch the CSR only when RISCV_ISA_EXT_XLINUXENVCFG is set, similar
> to __switch_to_envcfg() as well as other swaps conditional on
> their extentions.
> 
> Fixes: db3c01c7a308 ("RISCV: KVM: Add senvcfg context save/restore")
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>   arch/riscv/kvm/vcpu.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index e062ca19f9d8..0c2d58db13ab 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -773,7 +773,8 @@ static __always_inline void kvm_riscv_vcpu_swap_in_guest_state(struct kvm_vcpu *
>   	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
>   
>   	vcpu->arch.host_scounteren = csr_swap(CSR_SCOUNTEREN, csr->scounteren);
> -	vcpu->arch.host_senvcfg = csr_swap(CSR_SENVCFG, csr->senvcfg);
> +	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_XLINUXENVCFG))
> +		vcpu->arch.host_senvcfg = csr_swap(CSR_SENVCFG, csr->senvcfg);
>   	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN))
>   		vcpu->arch.host_sstateen0 = csr_swap(CSR_SSTATEEN0, smcsr->sstateen0);
>   	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICFISS))
> @@ -787,7 +788,8 @@ static __always_inline void kvm_riscv_vcpu_swap_in_host_state(struct kvm_vcpu *v
>   	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
>   
>   	csr->scounteren = csr_swap(CSR_SCOUNTEREN, vcpu->arch.host_scounteren);
> -	csr->senvcfg = csr_swap(CSR_SENVCFG, vcpu->arch.host_senvcfg);
> +	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_XLINUXENVCFG))
> +		csr->senvcfg = csr_swap(CSR_SENVCFG, vcpu->arch.host_senvcfg);
>   	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN))
>   		smcsr->sstateen0 = csr_swap(CSR_SSTATEEN0, vcpu->arch.host_sstateen0);
>   	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICFISS))

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv