Subject: Re: [PATCH 2/2] KVM: PPC: Book3E: Emulate MCSRR0/1 SPR and rfmci instruction

From: Scott Wood <[email-masked]>
Date: Tue, 9 Jul 2013 13:29:22 -0500

On 07/09/2013 12:46:32 PM, Alexander Graf wrote:
> On 07/09/2013 07:16 PM, Scott Wood wrote:
>> On 07/08/2013 01:45:58 PM, Alexander Graf wrote:
>>>=20
>>> On 03.07.2013, at 15:30, Mihai Caraman wrote:
>>>=20
>>> > Some guests are making use of return from machine check =20
>>> instruction
>>> > to do crazy things even though the 64-bit kernel doesn't handle =20
>>> yet
>>> > this interrupt. Emulate MCSRR0/1 SPR and rfmci instruction =20
>>> accordingly.
>>> >
>>> > Signed-off-by: Mihai Caraman <[email-masked]>
>>> > ---
>>> > arch/powerpc/include/asm/kvm_host.h |    1 +
>>> > arch/powerpc/kvm/booke_emulate.c    |   25 =20
>>> +++++++++++++++++++++++++
>>> > arch/powerpc/kvm/timing.c           |    1 +
>>> > 3 files changed, 27 insertions(+), 0 deletions(-)
>>> >
>>> > diff --git a/arch/powerpc/include/asm/kvm_host.h =20
>>> b/arch/powerpc/include/asm/kvm_host.h
>>> > index af326cd..0466789 100644
>>> > --- a/arch/powerpc/include/asm/kvm_host.h
>>> > +++ b/arch/powerpc/include/asm/kvm_host.h
>>> > @@ -148,6 +148,7 @@ enum kvm_exit_types {
>>> >     EMULATED_TLBWE_EXITS,
>>> >     EMULATED_RFI_EXITS,
>>> >     EMULATED_RFCI_EXITS,
>>> > +    EMULATED_RFMCI_EXITS,
>>>=20
>>> I would quite frankly prefer to see us abandon the whole exit =20
>>> timing framework in the kernel and instead use trace points. Then =20
>>> we don't have to maintain all of this randomly exercised code.
>>=20
>> Would this map well to tracepoints?  We're not trying to track =20
>> discrete events, so much as accumulated time spent in different =20
>> areas.
>=20
> I think so. We'd just have to emit tracepoints as soon as we enter =20
> handle_exit and in prepare_to_enter. Then a user space program should =20
> have everything it needs to create statistics out of that. It would =20
> certainly simplify the entry/exit path.

I was hoping that wasn't going to be your answer. :-)

Such a change would introduce a new dependency, more complexity, and =20
the possibility for bad totals to result from a ring buffer filling =20
faster than userspace can drain it.

I also don't see how it would simplify entry/exit, since we'd still =20
need to take timestamps in the same places, in order to record a final =20
event that says how long a particular event took.

-Scott=