kvm-userspace: sync icache for more architectures

PATCH In review Christian Ehrhardt 3 participants 7 messages Dec 13, 2007 – Dec 18, 2007 3 messages not in the archive KVM

lore

Share & tools

lore

Christian Ehrhardt

author

2007-12-13 19:36 UTC #0 lore raw

PATCH 1/1 kvm-userspace: sync icache for more architectures

Subject: [PATCH][UPDATE] kvm-userspace: sync icache for more architectures
From: Christian Ehrhardt <[email protected]>

A ia64 patch introduced kvm_sync_icache within a ifdef __ia64__, but the concept of split caches is not bound to ia64 and we would like to implement it for ppc too.
The call to kvm_synch_icache is replaced by the flush_icache_range function that is already available in qemu for ia64 and ppc (noop for x86). The call now depends on USE_KVM.
This patch supersedes my old "[PATCH] kvm-userspace: kvm_sync_icache for more architectures" from this morning.
The current ppc code around is not yet ready (it compiles & works for x86), could someone please test&verify this patch for ia64?

Signed-off-by Christian Ehrhardt <[email protected]>

qemu/exec.c +3 -2

index cf14fdd..d2a8f5b 100644
@@ -35,6 +35,7 @@
 #include "cpu.h"
 #include "exec-all.h"
 #ifdef USE_KVM
+#include "dyngen.h"
 #include "qemu-kvm.h"
 #endif
 #if defined(CONFIG_USER_ONLY)

@@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                     phys_ram_dirty[addr1 >> TARGET_PAGE_BITS] |=
                         (0xff & ~CODE_DIRTY_FLAG);
                 }
-#ifdef __ia64__
-		kvm_sync_icache((unsigned long)ptr, l);
+#ifdef USE_KVM
+		flush_icache_range((unsigned long)ptr, ((unsigned long)ptr)+l);
 #endif
             }
         } else {

qemu/hw/ipf.c +0 ~~-11~~

index 957e831..8686c1e 100644
@@ -105,17 +105,6 @@ static void pc_init_ne2k_isa(NICInfo *nd, qemu_irq *pic)
 #ifdef USE_KVM
 extern kvm_context_t kvm_context;
 extern int kvm_allowed;
-
-void kvm_sync_icache(unsigned long address, int len)
-{
-	int l;
-
-	for(l = 0; l < (len + 32); l += 32)
-		__ia64_fc(address + l);
-
-	ia64_sync_i();
-	ia64_srlz_i();
-}
 #endif
 
 static void main_cpu_reset(void *opaque)

qemu/target-ia64/cpu.h +0 -5

index 7349e94..be409c7 100644
@@ -73,9 +73,4 @@ CPUState *cpu_ia64_init(void);
 
 #include "cpu-all.h"
 
-/* IA64 has seperate I/D cache, with coherence maintained by DMA controller.
- * So to emulate right behavior that guest OS is assumed, we need to flush
- * I/D cache here.
- */
-void kvm_sync_icache(unsigned long address, int len);
 #endif

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

A message that is not in the archive (sent off-list or not yet received): 11975745782686-git-send-email-ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org

Hollis Blanchard

2007-12-14 00:12 UTC #2 lore raw

Quoted 11 lines · Christian Ehrhardt

qemu/exec.c +2 -2

@@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
 addr, uint8_t *buf,
                     phys_ram_dirty[addr1 >> TARGET_PAGE_BITS] |=
                         (0xff & ~CODE_DIRTY_FLAG);
                 }
-#ifdef __ia64__
-		kvm_sync_icache((unsigned long)ptr, l);
+#ifdef USE_KVM
+		flush_icache_range((unsigned long)ptr, ((unsigned long)ptr)+l);
 #endif
             }

Quoted 1 line

         } else {

A comment to explain why the icache needs flushing only in the KVM case would be useful. Other than that I'm fine with it.

Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>

Signature · 10 lines

Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Zhang, Xiantao

2007-12-14 03:12 UTC #3 lore raw

Re: [PATCH][UPDATE] kvm-userspace: sync icache for morearchitectures

In reply to a message not in the archive

Review void cpu_physical_memory_rw(target_phys_addr_t · line 2605

Quoted 12 lines

#include "qemu-kvm.h"
#endif
#if defined(CONFIG_USER_ONLY)

(quoted diff) +2 -2

@@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
                     addr, uint8_t *buf, phys_ram_dirty[addr1 >>
                         TARGET_PAGE_BITS] |= (0xff &
                 ~CODE_DIRTY_FLAG); }
-#ifdef __ia64__
-		kvm_sync_icache((unsigned long)ptr, l);
+#ifdef USE_KVM
+		flush_icache_range((unsigned long)ptr, ((unsigned

long)ptr)+l);

Are you sure ia64 implement this function for applications ? I didn't find it at my side, so I write it.

If not, maybe you can change the patch like the following :)

in target-ia64/cpu.h

#define flush_icache_range(start, end) kvm_sync_icache(start, (end)-(start))

Or upgrade kvm_sync_icach to comply with flush_icache_range, and change the name to flush_icache_range.

Thank you!
Xiantao

A message that is not in the archive (sent off-list or not yet received): 42DFA526FC41B1429CE7279EF83C6BDCAD01E3-wq7ZOvIWXbMAbVU2wMM1CrfspsVTdybXVpNB7YpNyf8@public.gmane.org

Christian Ehrhardt

author

2007-12-14 09:07 UTC #5 lore raw

Re: [PATCH][UPDATE] kvm-userspace: sync icache for morearchitectures

Quoted 1 line · Xiantao

Christian Ehrhardt wrote:

<[...]

Quoted 10 lines

Review void cpu_physical_memory_rw(target_phys_addr_t · line 2605

Quoted 8 lines

(quoted diff) +2 -2

@@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
                     addr, uint8_t *buf, phys_ram_dirty[addr1 >>
                         TARGET_PAGE_BITS] |= (0xff &
                 ~CODE_DIRTY_FLAG); }
-#ifdef __ia64__
-		kvm_sync_icache((unsigned long)ptr, l);
+#ifdef USE_KVM
+		flush_icache_range((unsigned long)ptr, ((unsigned

long)ptr)+l);

Quoted 2 lines

Are you sure ia64 implement this function for applications ? I didn't find it at my side, so I write it.

What do you mean with "implement it for applications" ? Take a look at dyngen.h - it starts with the comment "dyngen helpers" and contains a lot of static functions to use them where you need. I don't see anything obvious that would prevent these functions from being built and the file has no own include statements which may change that. Therefor the include "dyngen.h" in the USE_KVM case should be enough to have this function available for cpu_physical_memory_rw in exec.c where we need it.
[...]

Quoted 1 line

Xiantao

---

Quoted 4 lines · Hollis Blanchard

A comment to explain why the icache needs flushing only in the KVM case would be useful. Other than that I'm fine with it.
Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>

AFAIK Plain qemu does not directly execute guest code on the processor, so the icache is not an issue for it.
Qemu itself has the flush_icache_range function only as helper for the dynamic code generation.
But we may now write executable guest code with our intercepted mmio handling that is directly executed when switching back to the guest context, therefore we need that invalidation in the kvm case.

For the case that I'm overlooking something in plain qemu, so that it
might need it too I add [email protected] for comments from there, but currently I think to have it in #ifdef USE_KVM is the right way.

P.S. Hollis did you mean you would like to see a comment in the code where that call takes place?

Signature · 20 lines

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
[email protected]
[email protected]

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

A message that is not in the archive (sent off-list or not yet received): 476247EB.9040003-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org

Zhang, Xiantao

2007-12-14 10:18 UTC #7 lore raw

Re: [PATCH][UPDATE] kvm-userspace: sync icache for morearchitectures

Quoted 11 lines · Christian Ehrhardt

Quoted 1 line · Xiantao

Christian Ehrhardt wrote:

<[...]

Quoted 8 lines

Review void cpu_physical_memory_rw(target_phys_addr_t · line 2605

Quoted 8 lines

(quoted diff) +2 -2

@@ -2600,8 +2601,8 @@ void cpu_physical_memory_rw(target_phys_addr_t
                     addr, uint8_t *buf, phys_ram_dirty[addr1 >>
                         TARGET_PAGE_BITS] |= (0xff &
                 ~CODE_DIRTY_FLAG); }
-#ifdef __ia64__
-		kvm_sync_icache((unsigned long)ptr, l);
+#ifdef USE_KVM
+		flush_icache_range((unsigned long)ptr, ((unsigned

long)ptr)+l);

Quoted 11 lines

Quoted 3 lines
Are you sure ia64 implement this function for applications ? I didn't find it at my side, so I write it.
What do you mean with "implement it for applications" ? Take a look at dyngen.h - it starts with the comment "dyngen helpers" and contains a lot of static functions to use them where you need. I don't see anything obvious that would prevent these functions from being built and the file has no own include statements which may change that. Therefor the include "dyngen.h" in the USE_KVM case should be enough to have this function available for cpu_physical_memory_rw in exec.c where we need it.

Good! I didn't notice this definition. Originally, I think you mean it is a library functions. :)

Quoted 14 lines

Quoted 4 lines · Hollis Blanchard
A comment to explain why the icache needs flushing only in the KVM case would be useful. Other than that I'm fine with it.
Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>
AFAIK Plain qemu does not directly execute guest code on the processor, so the icache is not an issue for it.
Qemu itself has the flush_icache_range function only as helper for the dynamic code generation.
But we may now write executable guest code with our intercepted mmio handling that is directly executed when switching back to the guest context, therefore we need that invalidation in the kvm case.

Quoted 7 lines

For the case that I'm overlooking something in plain qemu, so that it
might need it too I add [email protected] for comments from there, but currently I think to have it in #ifdef USE_KVM is the right way.
P.S. Hollis did you mean you would like to see a comment in the code where that call takes place?

Hollis Blanchard

2007-12-18 01:56 UTC #8 lore raw

Re: [PATCH][UPDATE] kvm-userspace: sync icache for morearchitectures

In reply to a message not in the archive

Quoted 16 lines · Christian Ehrhardt

Quoted 1 line · Hollis Blanchard
A comment to explain why the icache needs flushing only in the KVM
case
Quoted 3 lines
would be useful. Other than that I'm fine with it.
Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>
AFAIK Plain qemu does not directly execute guest code on the processor,
so the icache is not an issue for it.
Qemu itself has the flush_icache_range function only as helper for the dynamic code generation.
But we may now write executable guest code with our intercepted mmio handling that is directly executed when switching back to the guest context, therefore we need that invalidation in the kvm case.

Quoted 7 lines

For the case that I'm overlooking something in plain qemu, so that it
might need it too I add [email protected] for comments from there, but currently I think to have it in #ifdef USE_KVM is the right way.
P.S. Hollis did you mean you would like to see a comment in the code where that call takes place?

Yes! Hopefully much shorter than this email... :-P

Signature · 10 lines

Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Christian Ehrhardt

author

2007-12-18 12:58 UTC #9 lore raw

Re: [PATCH][UPDATE] kvm-userspace: sync icache for morearchitectures

Quoted 21 lines · Hollis Blanchard

Quoted 20 lines · Christian Ehrhardt
Quoted 1 line · Hollis Blanchard
A comment to explain why the icache needs flushing only in the KVM
case
Quoted 3 lines
would be useful. Other than that I'm fine with it.
Signed-off-by Hollis Blanchard <hollisb-r/[email protected]>
AFAIK Plain qemu does not directly execute guest code on the processor,
so the icache is not an issue for it.
Qemu itself has the flush_icache_range function only as helper for the dynamic code generation.
But we may now write executable guest code with our intercepted mmio handling that is directly executed when switching back to the guest context, therefore we need that invalidation in the kvm case.
For the case that I'm overlooking something in plain qemu, so that it
might need it too I add [email protected] for comments from there, but currently I think to have it in #ifdef USE_KVM is the right way.

Quoted 5 lines

Quoted 2 lines
P.S. Hollis did you mean you would like to see a comment in the code where that call takes place?
Yes! Hopefully much shorter than this email... :-P

comment added, rebased and resent together with a updated mmio callback simplification patch - I hope I didn't overlook a response to the mmio callback thread again this time ;-)

Signature · 20 lines

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
[email protected]
[email protected]

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace