On Mon, Apr 28, 2025 at 11:31 PM Radim Krčmář <rkrcmar@xxxxxxxxxxxxxxxx> wrote: > > 2025-04-28T17:46:01+05:30, Anup Patel <anup@xxxxxxxxxxxxxx>: > > On Thu, Apr 3, 2025 at 5:02 PM Radim Krčmář <rkrcmar@xxxxxxxxxxxxxxxx> wrote: > >> > >> The SBI reset state has only two variables -- pc and a1. > >> The rest is known, so keep only the necessary information. > >> > >> The reset structures make sense if we want userspace to control the > >> reset state (which we do), but I'd still remove them now and reintroduce > >> with the userspace interface later -- we could probably have just a > >> single reset state per VM, instead of a reset state for each VCPU. > > > > The SBI spec does not define the reset state of CPUs. The SBI > > implementations (aka KVM RISC-V or OpenSBI) or platform > > firmwares are free to clear additional registers as part system > > reset or CPU. > > > > As part of resetting the VCPU, the in-kernel KVM clears all > > the registers. > > Yes, but instead of doing a simple memset(0), KVM carriers around a lot > of data with minimal information value. Reset is not really a fast > path, so I think it would be good to have the code there as simple as > possible. > > > The setting of PC, A0, and A1 is only an entry condition defined > > for CPUs brought-up using SBI HSM start or SBI System suspend. > > That is why this patch has to add kvm_vcpu_reset_state, to remember the > state of pc and a1. (a0 is hart id and can be figured out.) > > > We should not go ahead with this patch. > > This patch only does refactoring. Do you think the current reset > structures are better? > I am fine getting rid of reset structures as long as we have memset(0) in-place to achieve the same thing. Regards, Anup
