Re: [Qemu-devel] qemu-kvm: Role of flush_icache_range on PPC

[Scott Wood]

On 09/28/2011 09:45 AM, Jan Kiszka wrote:
> On 2011-09-28 16:26, Alexander Graf wrote:
>>
>> On 28.09.2011, at 16:23, Jan Kiszka wrote:
>>
>>> Alex,
>>>
>>> we have this diff in qemu-kvm:
>>>
>>> diff --git a/exec.c b/exec.c
>>> index c1e045d..f188549 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -3950,6 +3955,11 @@ void cpu_physical_memory_rw(target_phys_addr_t
>>> addr, uint8_t *buf,
>>>                      cpu_physical_memory_set_dirty_flags(
>>>                          addr1, (0xff&  ~CODE_DIRTY_FLAG));
>>>                  }
>>> +        /* qemu doesn't execute guest code directly, but kvm does
>>> +           therefore flush instruction caches */
>>> +        if (kvm_enabled())
>>> +            flush_icache_range((unsigned long)ptr,
>>> +                       ((unsigned long)ptr)+l);
>>>                  qemu_put_ram_ptr(ptr);
>>>              }
>>>          } else {

Been meaning to send a poke about this one:

http://patchwork.ozlabs.org/patch/90403/

I've seen issues with this when the executable images are initially
loaded by qemu.

>>> flush_icache_range() is doing something only on PPC hosts. So do we need
>>> this upstream?
>>
>> This makes sure that when device emulation overwrites code that is
>> already present in the cache of a CPU, it gets flushed from the
>> icache. I'm fairly sure we want that :). But let's ask Ben and David
>> as well.
> 
> /me wondered which write scenario precisely needs this. It could only be
> something synchronous /wrt to some VCPU.

Why would it need to be synchronous?  Even if it's asynchronous emulated
DMA, we don't want it sitting around only in a data cache that
instruction fetches won't snoop.

> Which operations could trigger
> such a write? Does PPC inject software breakpoints in form of trap
> operations or so?

It's not implemented yet in mainline for powerpc (we have something
internal that is on the backlog of things to be cleaned up and sent
out), but this is what we'd do for kvm_arch_insert_sw_breakpoint().

-Scott