On Tue, Mar 25, 2025 at 10:38 PM Radim Krčmář rkrcmar@ventanamicro.com wrote:
2025-03-25T22:27:41+05:30, Anup Patel apatel@ventanamicro.com:
On Tue, Mar 25, 2025 at 8:58 PM Anup Patel apatel@ventanamicro.com wrote:
On Tue, Mar 25, 2025 at 12:01 AM Radim Krčmář rkrcmar@ventanamicro.com wrote:
Hi, I'm sending this early to ventana-sw as we hit the issue in today's slack discussion. I only compile-tested it so far and it will take me a while to trigger a bug and verify the solution.
---8<-- The smstateen CSRs control which stateful features are enabled in VU-mode. SU-mode must properly context switch the state of all enabled features.
Reset the smstateen CSRs, because SU-mode might not know that it must context switch the state. Reset unconditionally as it is shorter and safer, and not that much slower.
Fixes: 81f0f314fec9 ("RISCV: KVM: Add sstateen0 context save/restore") Cc: stable@vger.kernel.org Signed-off-by: Radim Krčmář rkrcmar@ventanamicro.com
How about moving "struct kvm_vcpu_smstateen_csr smstateen" from "struct kvm_vcpu_arch" to "struct kvm_vcpu_csr". This way we will not need an extra "struct kvm_vcpu_smstateen_csr reset_smstateen_csr" in "struct kvm_vcpu_csr".
It is tricky, because kvm_riscv_vcpu_general_set_csr calculates the amount of registers accessible to userspace based the on size of kvm_vcpu_csr.
We'd have to make changes to logic before expanding kvm_vcpu_csr with kvm_vcpu_smstateen_csr. At that point, I think it be much easier to just put all csrs to kvm_vcpu_csr directly, which would also simplify future extensions.
Other than my comment, this looks good for upstreaming.
I think the current version is more appropriate for stable, and we can implement your suggestion afterwards.
Sounds good.
Regards, Anup