Re: [PATCH 19/38] KVM: PPC: Add cache flush on page map

From: Scott Wood <scottwood@freescale.com>
To: Alexander Graf <agraf@suse.de>
Cc: <kvm-ppc@vger.kernel.org>, KVM list <kvm@vger.kernel.org>
Subject: Re: [PATCH 19/38] KVM: PPC: Add cache flush on page map
Date: Wed, 15 Aug 2012 13:56:02 -0500	[thread overview]
Message-ID: <502BF0C2.3010101@freescale.com> (raw)
In-Reply-To: <3773C769-16E5-4E95-AB2C-E9B164C9D406@suse.de>

On 08/15/2012 01:51 PM, Alexander Graf wrote:
> 
> On 15.08.2012, at 20:33, Scott Wood wrote:
> 
>> On 08/15/2012 01:29 PM, Alexander Graf wrote:
>>>
>>> On 15.08.2012, at 20:27, Alexander Graf wrote:
>>>
>>>>
>>>> On 15.08.2012, at 20:16, Scott Wood wrote:
>>>>
>>>>> On 08/15/2012 01:01 PM, Alexander Graf wrote:
>>>>>>
>>>>>> On 15.08.2012, at 19:47, Scott Wood wrote:
>>>>>>
>>>>>>> On 08/15/2012 12:27 PM, Alexander Graf wrote:
>>>>>>>>
>>>>>>>> On 15.08.2012, at 19:26, Scott Wood wrote:
>>>>>>>>
>>>>>>>>> On 08/15/2012 04:52 AM, Alexander Graf wrote:
>>>>>>>>>>
>>>>>>>>>> On 15.08.2012, at 03:23, Scott Wood wrote:
>>>>>>>>>>
>>>>>>>>>>> On 08/14/2012 06:04 PM, Alexander Graf wrote:
>>>>>>>>>>>> When we map a page that wasn't icache cleared before, do so when first
>>>>>>>>>>>> mapping it in KVM using the same information bits as the Linux mapping
>>>>>>>>>>>> logic. That way we are 100% sure that any page we map does not have stale
>>>>>>>>>>>> entries in the icache.
>>>>>>>>>>>
>>>>>>>>>>> We're not really 100% sure of that -- this only handles the case where
>>>>>>>>>>> the kernel does the dirtying, not when it's done by QEMU or the guest.
>>>>>>>>>>
>>>>>>>>>> When the guest does it, the guest is responsible for clearing the
>>>>>>>>>> icache. Same for QEMU. It needs to clear it when doing DMA.
>>>>>>>>>
>>>>>>>>> Sure.  I was just worried that that commit message could be taken the
>>>>>>>>> wrong way, as in "we no longer need the QEMU icache flushing patch".
>>>>>>>>>
>>>>>>>>>> However, what is still broken would be a direct /dev/mem map. There
>>>>>>>>>> QEMU should probably clear the icache before starting the guest, in
>>>>>>>>>> case another guest was running on that same memory before.
>>>>>>>>>> Fortunately, we don't have that mode available in upstream QEMU :).
>>>>>>>>>
>>>>>>>>> How is QEMU loading images different if it's /dev/mem versus ordinary
>>>>>>>>> anonymous memory?  You probably won't have stale icache data in the
>>>>>>>>> latter case (which makes it less likely to be a problem in pratice), but
>>>>>>>>> in theory you could have data that still hasn't left the dcache.
>>>>>>>>
>>>>>>>> It's the same. I just talked to Ben about this today in a different context and we should be safe :).
>>>>>>>
>>>>>>> Safe how?
>>>>>>>
>>>>>>> If it's truly the same, we're definitely not safe, since I had problems
>>>>>>> with this using /dev/mem (particularly when changing the kernel image
>>>>>>> without a host reboot) before I put in the icache flush patch.
>>>>>>
>>>>>> QEMU needs to icache flush everything it puts into guest memory.
>>>>>
>>>>> Yes.  I thought you meant we should be safe as things are now.
>>>>
>>>> Hrm. What happened to your patch that flushes the icache on cpu_physical_memory_rw?
>>
>> IIRC Ben wanted it conditionalized to not slow things down on
>> icache-coherent systems, and I never got around to respinning it.
> 
> No, he was saying that DMA doesn't flush the icache:
> 
>   http://thread.gmane.org/gmane.comp.emulators.qemu/119022/focus=119086

I recall someone asking for it to be made conditional, but I don't have
time to look it up right now -- I want to try to get some U-Boot stuff
done before the end of the merge window tomorrow.

>>> Ah, if I read Ben's comment correctly we only need it for rom loads, not always for cpu_physical_memory_rw.
>>
>> Why?
> 
> Because guest Linux apparently assumes that DMA'd memory needs to be icache flushed.

What about breakpoints and other debug modifications?

And it's possible (if not necessarily likely) that other guests are
different.

-Scott