Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/2] tcg/ppc*: Move cache initialization to ppc specific code

[Alexander Graf]

On 03.10.2011, at 23:51, Scott Wood wrote:

> On 10/03/2011 04:43 PM, Alexander Graf wrote:
>> 
>> On 03.10.2011, at 23:40, Scott Wood wrote:
>> 
>>> On 10/03/2011 04:10 PM, Stefan Weil wrote:
>>>> Am 03.10.2011 22:52, schrieb Scott Wood:
>>>>> On 10/03/2011 03:43 PM, Stefan Weil wrote:
>>>>>> qemu_cache_utils_init() is only used by ppc / ppc64 tcg targets
>>>>>> to initialize the cache before flush_icache_range() is called.
>>>>>> 
>>>>>> This patch moves the code to tcg/ppc and tcg/ppc64.
>>>>>> Initialisation is called from tcg_target_init() there.
>>>>>> 
>>>>>> Signed-off-by: Stefan Weil <address@hidden>
>>>>> 
>>>>> This is not only needed for TCG. We need flush_icache_range() for KVM.
>>>>> See http://patchwork.ozlabs.org/patch/90403/ and the thread starting
>>>>> with http://lists.gnu.org/archive/html/qemu-ppc/2011-09/msg00180.html
>>>>> 
>>>>> And must this be duplicated between ppc and ppc64?
>>>>> 
>>>>> -Scott
>>>> 
>>>> Your patch 90403 is obviously still missing in QEMU master -
>>>> that's the reason why I did not notice that PPC KVM needs
>>>> flush_icache_range().
>>> 
>>> Yes...
>>> 
>>> Alex, is there any objection to merging 90403?
>> 
>> IIRC Ben was raising concerns that they don't need to flush their icache and 
>> it'd incur some speed penalties.
> 
> Not doing it sometimes invokes crash penalties for us. :-)
> 
> We could add some way to skip the invalidation if we know the host is an
> implementation that doesn't need it, possibly depending on the context
> (is it just DMA he wants to avoid doing this on[1], or do their chips
> have a fully coherent icache?), but IMHO functional correctness should
> come first.

I tend to agree. I really want to give him the chance to suggest something 
clever first. Btw, we could always keep a global variable around to tell us if 
the host we're on needs the flush and only conditionally do it. If I understood 
Ben correctly, P7s do have coherent icaches always, so we could match on the 
PVR on init and then move on :).


Alex