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/