Re: [PATCH 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation

[Alexander Graf <agraf@suse.de>]

On 04/01/2014 06:58 PM, Scott Wood wrote:
> On Tue, 2014-04-01 at 07:47 +0200, Alexander Graf wrote:
>>> Am 01.04.2014 um 01:03 schrieb Scott Wood <scottwood@freescale.com>:
>>>
>>>> On Mon, 2014-03-31 at 15:41 +0200, Alexander Graf wrote:
>>>>> On 03/26/2014 10:17 PM, Scott Wood wrote:
>>>>>> On Thu, 2014-02-20 at 18:30 +0200, Mihai Caraman wrote:
>>>>>> +    /*
>>>>>> +     * Another thread may rewrite the TLB entry in parallel, don't
>>>>>> +     * execute from the address if the execute permission is not set
>>>>>> +     */
>>>> What happens when another thread rewrites the TLB entry in parallel?
>>>> Does tlbsx succeed? Does it fail? Do we see failure indicated somehow?
>>>> Are the contents of the MAS registers consistent at this point or
>>>> inconsistent?
>>> If another thread rewrites the TLB entry, then we use the new TLB entry,
>>> just as if it had raced in hardware.  This check ensures that we don't
>>> execute from the new TLB entry if it doesn't have execute permissions
>>> (just as hardware would).
>>>
>>> What happens if the new TLB entry is valid and executable, and the
>>> instruction pointed to is valid, but doesn't trap (and thus we don't
>>> have emulation for it)?
>>>
>>>> There has to be a good way to detect such a race and deal with it, no?
>>> How would you detect it?  We don't get any information from the trap
>>> about what physical address the instruction was fetched from, or what
>>> the instruction was.
>> Ah, this is a guest race where the guest modifies exactly the TLB in question. I see.
>>
>> Why would this ever happen in practice (in a case where we're not executing malicious code)?
> I don't know.  It probably wouldn't.  But it seems wrong to not check
> the exec bit.

Right, I'm just saying that a program interrupt is not too bad of an 
answer in case the guest does something as stupid as this in kernel 
space :). It's definitely good practice to check for the exec bit.


Alex