Re: [PATCH] KVM: arm64: Only save S1PIE registers when dirty

[Mark Brown]

On Mon, Mar 04, 2024 at 02:39:19PM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@xxxxxxxxxx> wrote:
> > On Sat, Mar 02, 2024 at 10:28:18AM +0000, Marc Zyngier wrote:

> > > Complains from whom? I can't see anything in my inbox, so it my
> > > conclusion that these "issues" are not serious enough to be publicly
> > > mentioned.

> > This was you saying that adding more registers to be context switched
> > here needed special explanation, rather than just being the default and
> > generally unremarkable place to put context switching of registers for
> > EL0/1.

> What I remember saying is that it is wrong to add extra registers to
> the context switch without gating them with the VM configuration.
> Which is a very different thing.

You said both things separately.  This is specifically addressing your
comment:

| For the benefit of the unsuspecting reviewers, and in the absence of a
| public specification (which the XML drop isn't), it would be good to
| have the commit message explaining the rationale of what gets saved
| when.

which did not seem obviously tied to your separate comments about using
your at the time still in flight patches to add support for parsing
features out of the ID registers, it seemed like a separate concern.

> What I want to see explained in all cases is why a register has to be
> eagerly switched and not deferred to the load/put phases, specially on
> VHE. because that has a very visible impact on the overall performance.

I'm confused here, the specific register save/restores that you were
asking for an explanation of are as far as I can tell switched in
load/put (eg, the specific complaint was attached to loads in
__sysreg_restore_el1_state() which for VHE is called from 

	__vcpu_load_switch_sysregs()
	kvm_vcpu_load_vhe()
	kvm_arch_vcpu_load()

which is to my understanding part of the load/put phase).  This should
be true for all the GCS registers, the explanation would be something
along the lines of "these are completely unremarkable EL0/1 registers
and are switched in the default place where we switch all the other
EL0/1 registers".

Attachment: signature.asc
Description: PGP signature