Liu Yu-B13201 wrote:
-----Original Message-----
From: Christian Ehrhardt [mailto:ehrhardt@xxxxxxxxxxxxxxxxxx]
Sent: Monday, September 01, 2008 8:15 PM
To: Liu Yu-B13201
Cc: kvm-ppc@xxxxxxxxxxxxxxx; hollisb@xxxxxxxxxx
Subject: Re: [PATCH 3/6] kvmppc: rewrite guest code - sprg0-3
+
+ down_read(¤t->mm->mmap_sem);
+ pcpage = gfn_to_page(vcpu->kvm,
+ guest_pcaddr >>
KVM_PPCPV_MAGIC_PAGE_SHIFT);
+ up_read(¤t->mm->mmap_sem);
+ if (pcpage == bad_page)
+ return -EFAULT;
+
+ pcmem = kmap_atomic(pcpage, KM_USER0);
+ if (!pcmem)
+ return -EFAULT;
+ pcaddr = ((unsigned long)pcmem
+ | (vcpu->arch.pc &
KVM_PPCPV_MAGIC_PAGE_MASK));
+ BUG_ON(inst != *((u32 *)pcaddr));
+ create_instruction(pcaddr, newinst);
+ kunmap_atomic(pcpage, KM_USER0);
+ }
I think you are writing new instruction back here.
Why not use kvm_write_guest_page() (kvm_main.c) directly?
hmm I'm exchaning instructions in memory and the "create_instruction"
function I use is exactly doing that.
I expect that the kvm function would miss the needed sync
functionality
to ensure that all old code is dropped from the cpu caches.
Well I'm not
an expert in that, I need to look into that.
It seems there wouldn't be cache consistency problem for PowerPC as long as the memory is updated by processor.
Btw, where is the definition of create_instruction?
-> asm/system.h:521
+
+ return err;
+}
/* XXX to do:
* lhax
@@ -260,6 +415,9 @@
int dcrn;
enum emulation_result emulated = EMULATE_DONE;
int advance = 1;
+
+ if (kvmppc_has_pvmem(vcpu) &&
!kvmppc_pvmem_rewrite_instruction(vcpu))
+ return EMULATE_DONE;
Rewirting instruction is one-off, so it's not that important to pay
attention to performance.
However, putting rewriting at the beginning of the function
add extra
workload to other emulations.
How about this way?
int rewrite = 0;
.......
case mfspr0:
rewrite = 1;
......
// at the end of the function
if(unlikely(rewrite) && kvmppc_has_pvmem(vcpu))
kvmppc_pvmem_rewrite_instruction(vcpu);
This would spread checks with kvmppc_has_pvmem all over the code,
because if we rewrite the instruction it is not allowed to emulate it.
On the other hand we could emulate it and change the instruction for
their "next" hit (not jump back to current pc but at pc+4 as it is
emulated).
Yeah that could work, and once most rewrite actions are done the
"unlikely" is the right thing because later on we won't hit
it anymore.
That should speed up emulation a bit (not much compared to exit/enter
guest, but it's never wrong to code things performance wise).
In fact, not only the unlikely thing, I am worried about that
putting kvmppc_pvmem_rewrite_instruction() at beginning
will cosume some time for normal emulations, ( a lot comparisons and jumps)
especially more and more instructions would be replaced in it in the future.
Sure, I think I already got your point. This is what I described with
"emulate & rewrite it" allowing us to put the
kvmppc_pvmem_rewrite_instruction() at the end guarded by an additional
if that will eventually reduce the overhead to nearly zero once the
trapping code is rewritten.
The code flow I have in mind is that:
emulate instruction
if this is a rewritable one set flag
increase program counter because instruction is emulated
if unlikely(rewritable) flag is set and kvm_has_pvmem call
kvmppc_pvmem_rewrite_instruction
-> The next time guest executes that code it will already use the new
rewritten instruction
-> non rewritable instructions that are emulated don't set the flag and
therefore don't call kvmppc_pvmem_rewrite_instruction saving overhead
I'll submit an updated patch series somewhen this week. Then you can
check if I got your hint right :-)
--
Grüsse / regards,
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
