[PATCH v2 1/4] KVM: PPC: e500mc: Revert "add load inst fixup"

[Alexander Graf]

Am 03.05.2014 um 01:14 schrieb "mihai.caraman at freescale.com" <mihai.caraman at freescale.com>:

>> From: Alexander Graf <agraf at suse.de>
>> Sent: Friday, May 2, 2014 12:24 PM
>> To: Caraman Mihai Claudiu-B02008
>> Cc: kvm-ppc at vger.kernel.org; kvm at vger.kernel.org; linuxppc-dev at lists.ozlabs.org
>> Subject: Re: [PATCH v2 1/4] KVM: PPC: e500mc: Revert "add load inst fixup"
>> 
>>> On 05/01/2014 02:45 AM, Mihai Caraman wrote:
>>> The commit 1d628af7 "add load inst fixup" made an attempt to handle
>>> failures generated by reading the guest current instruction. The fixup
>>> code that was added works by chance hiding the real issue.
>>> 
>>> Load external pid (lwepx) instruction, used by KVM to read guest
>>> instructions, is executed in a subsituted guest translation context
>>> (EPLC[EGS] = 1). In consequence lwepx's TLB error and data storage
>>> interrupts need to be handled by KVM, even though these interrupts
>>> are generated from host context (MSR[GS] = 0).
>>> 
>>> Currently, KVM hooks only interrupts generated from guest context
>>> (MSR[GS] = 1), doing minimal checks on the fast path to avoid host
>>> performance degradation. As a result, the host kernel handles lwepx
>>> faults searching the faulting guest data address (loaded in DEAR) in
>>> its own Logical Partition ID (LPID) 0 context. In case a host translation
>>> is found the execution returns to the lwepx instruction instead of the
>>> fixup, the host ending up in an infinite loop.
>>> 
>>> Revert the commit "add load inst fixup". lwepx issue will be addressed
>>> in a subsequent patch without needing fixup code.
>>> 
>>> Signed-off-by: Mihai Caraman <mihai.caraman at freescale.com>
>> 
>> Just a random idea: Could we just switch IVOR2 during the critical lwepx
>> phase? In fact, we could even do that later when we're already in C code
>> and try to recover the last instruction. The code IVOR2 would point to
>> would simply set the register we're trying to read to as LAST_INST_FAIL
>> and rfi.
> 
> This was the first idea that sprang to my mind inspired from how DO_KVM
> is hooked on PR. I actually did a simple POC for e500mc/e5500, but this will
> not work on e6500 which has shared IVORs between HW threads.

What if we combine the ideas? On read we flip the IVOR to a separate handler that checks for a field in the PACA. Only if that field is set, we treat the fault as kvm fault, otherwise we jump into the normal handler.

I suppose we'd have to also take a lock to make sure we don't race with the other thread when it wants to also read a guest instruction, but you get the idea.

I have no idea whether this would be any faster, it's more of a brainstorming thing really. But regardless this patch set would be a move into the right direction.

Btw, do we have any guarantees that we don't get scheduled away before we run kvmppc_get_last_inst()? If we run on a different core we can't read the inst anymore. Hrm.


Alex