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

From: Oliver Upton <oliver.upton@linux.dev>
To: Mark Brown <broonie@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>, James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Joey Gouly <joey.gouly@arm.com>,
	linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: arm64: Only save S1PIE registers when dirty
Date: Fri, 1 Mar 2024 19:32:28 +0000	[thread overview]
Message-ID: <ZeItTLQxdxxICw01@linux.dev> (raw)
In-Reply-To: <20240301-kvm-arm64-defer-regs-v1-1-401e3de92e97@kernel.org>

On Fri, Mar 01, 2024 at 06:05:53PM +0000, Mark Brown wrote:
> Currently we save the S1PIE registers every time we exit the guest but
> the expected usage pattern for these registers is that they will be
> written to very infrequently, likely once during initialisation and then
> never updated again.  This means that most likely most of our saves of
> these registers are redundant.  Let's avoid these redundant saves by
> enabling fine grained write traps for the EL0 and EL1 PIE registers when
> switching to the guest and only saving if a write happened.
> 
> We track if the registers have been written by storing a mask of bits
> for HFGWTR_EL2, we may be able to use the same approach for other
> registers with similar access patterns.  We assume that it is likely
> that both registers will be written in quick succession and mark both
> PIR_EL1 and PIRE0_EL1 as dirty if either is written in order to minimise
> overhead.
> 
> This will have a negative performance impact if guests do start updating
> these registers frequently but since the PIE indexes have a wide impact
> on the page tables it seems likely that this will not be the case.
> 
> We do not need to check for FGT support since it is mandatory for
> systems with PIE.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> I don't have a good sense if this is a good idea or not, or if this is a
> desirable implementation of the concept - the patch is based on some
> concerns about the cost of the system register context switching.  We
> should be able to do something similar for some of the other registers.

Is there any data beyond a microbenchmark to suggest save elision
benefits the VM at all? The idea of baking the trap configuration based
on what KVM _thinks_ the guest will do isn't particularly exciting. This
doesn't seem to be a one-size-fits-all solution.

The overheads of guest exits are extremely configuration dependent, and
on VHE the save/restore of EL1 state happens at vcpu_load() / vcpu_put()
rather than every exit. There isn't a whole lot KVM can do to lessen the
blow of sharing EL1 in the nVHE configuration.

Looking a bit further out, the cost of traps will be dramatically higher
when running as a guest hypervisor, so we'd want to avoid them if
possible...

-- 
Thanks,
Oliver