On Sun, Dec 22, 2019 at 12:22:10PM +0000, Marc Zyngier wrote:

On Sat, 21 Dec 2019 10:48:16 +0000,
Marc Zyngier [off-list ref] wrote:

quoted

[fixing email addresses]

Hi Andrew,

On 2019-12-20 14:30, Andrew Murray wrote:

quoted

This series implements support for allowing KVM guests to use the Arm
Statistical Profiling Extension (SPE).

Thanks for this. In future, please Cc me and Will on email addresses
we can actually read.

quoted

It has been tested on a model to ensure that both host and guest can
simultaneously use SPE with valid data. E.g.

$ perf record -e arm_spe/ts_enable=1,pa_enable=1,pct_enable=1/ \
        dd if=/dev/zero of=/dev/null count=1000
$ perf report --dump-raw-trace > spe_buf.txt

As we save and restore the SPE context, the guest can access the SPE
registers directly, thus in this version of the series we remove the
trapping and emulation.

In the previous series of this support, when KVM SPE isn't
supported (e.g. via CONFIG_KVM_ARM_SPE) we were able to return a
value of 0 to all reads of the SPE registers - as we can no longer
do this there isn't a mechanism to prevent the guest from using
SPE - thus I'm keen for feedback on the best way of resolving
this.

Surely there is a way to conditionally trap SPE registers, right? You
should still be able to do this if SPE is not configured for a given
guest (as we do for other feature such as PtrAuth).

quoted

It appears necessary to pin the entire guest memory in order to
provide guest SPE access - otherwise it is possible for the guest
to receive Stage-2 faults.

Really? How can the guest receive a stage-2 fault? This doesn't fit
what I understand of the ARMv8 exception model. Or do you mean a SPE
interrupt describing a S2 fault?

Yes the latter.

quoted

And this is not just pinning the memory either. You have to ensure that
all S2 page tables are created ahead of SPE being able to DMA to guest
memory. This may have some impacts on the THP code...

I'll have a look at the actual series ASAP (but that's not very soon).

I found some time to go through the series, and there is clearly a lot
of work left to do:

- There so nothing here to handle memory pinning whatsoever. If it
  works, it is only thanks to some side effect.

- The missing trapping is deeply worrying. Given that this is an
  optional feature, you cannot just let the guest do whatever it wants
  in an uncontrolled manner.

Yes I'll add this.

- The interrupt handling is busted. You mix concepts picked from both
  the PMU and the timer code, while the SPE device doesn't behave like
  any of these two (it is neither a fully emulated device, nor a
  device that is exclusively owned by a guest at any given time).

I expect some level of discussion on the list including at least Will
and myself before you respin this.

Thanks for the quick feedback.

Andrew Murray

	M.

-- 
Jazz is not dead, it just smells funny.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel