[PATCH v7 07/12] KVM: arm64: arm_spe: Give SPE enabled state to KVM

[James Clark]

On 20/11/2024 9:16 am, Oliver Upton wrote:
> Hi James,
> 
> On Tue, Nov 12, 2024 at 10:37:06AM +0000, James Clark wrote:
>> Currently in nVHE, KVM has to check if SPE is enabled on every guest
>> switch even if it was never used. Because it's a debug feature and is
>> more likely to not be used than used, give KVM the SPE buffer status to
>> allow a much simpler and faster do-nothing path in the hyp.
>>
>> This is always called with preemption disabled except for probe/hotplug
>> which gets wrapped with preempt_disable().
> 
> Unless the performance penalty of checking if SPE is measurably bad, I'd
> rather we keep things as-is.
> 
> Folks that want to go fast are probably using VHE to begin with. As you
> note below, we need the hypervisor to decide if SPE is enabled based on
> hardware in protected mode anyway. Using a common flow for protected and
> non-protected configs keeps complexity down and increases the likelihood
> SPE save/restore code actually gets tested.
> 

I'm not sure if there is any measurable difference. This change was 
actually in response to this review from Marc here [1]:

   > Why do we need to save anything if nothing was enabled, which is
   > *all the time*? I'm sorry to break it to you, but nobody uses these
   > features.  So I'd like them to have zero cost when not in use.

   > Surely there is something there that should say "yup, tracing" or
   > not (such as the enable bits), which would avoid hitting the sysreg
   > pointlessly?

I suppose I could have taken the "zero cost" bit a bit too literally and 
maybe there were some simpler optimizations that didn't involve strongly 
coupling the driver to KVM. At least for enable/disable, for filtering 
it would still be required.

I'm trying to think if there is some middle ground where there is a 
systemwide flag or static key that gets set on the very first SPE or 
trace session. In theory it could be simpler than this per-cpu enable 
disable stuff, but in the end it pretty much ends up needing the same 
info from the driver (and has the same protected mode issue). So you 
might as well do it as fine grained as this or not at all like you suggest.

[1]: https://lore.kernel.org/linux-arm-kernel/86bk832jza.wl-maz@kernel.org/