Marcelo Tosatti wrote:
On Fri, May 15, 2009 at 01:10:34PM -0400, Christoph Hellwig wrote:
On Thu, May 14, 2009 at 05:30:16PM -0300, Marcelo Tosatti wrote:
+ trace_kvm_cr_write(cr, val);
switch (cr) {
case 0:
- kvm_set_cr0(vcpu, kvm_register_read(vcpu, reg));
+ kvm_set_cr0(vcpu, val);
skip_emulated_instruction(vcpu);
Do we really need one trace point covering all cr writes, _and_ one for
each specific register?
There is one tracepoint named kvm_cr that covers cr reads and writes.
kvm_trace_cr_read/kvm_trace_cr_write are macros that expand to
kvm_trace_cr(rw=1 or rw=0). Perhaps that is not a very good idea.
If the framework allows us to filter on arguments, that's fine (I think
it does). We need a way to look at, say, cr4 reads in a sea of cr3
read/writes.
Again this seems a bit cumbersome. Why not just one tracepoint for
page faults, with a flag if we're using npt or not?
Issue is the meaning of these faults is different. With npt disabled the
fault is a guest fault (like a normal pagefault), but with npt enabled
the fault indicates the host pagetables the hardware uses to do the
translation are not set up correctly.
No, with npt disabled a fault is either a guest fault or a shadow fault
or a host (minor/major) fault.
With npt enabled it cannot be a guest fault, but may be a shadow fault
or a host fault.
I did unify them as you suggest but reverted back to separate
tracepoints because the unification might be confusing.
Can be unified later if desirable.
I think you can unify them, especially as they will never occur together
in the same run. We'll need tracepoints later in the mmu code for
dealing with the different fault types. Guest faults are marked by
injecting a #GP, shadow faults by populating an spte, and host faults
somewhere in the bowels of get_user_pages(). Isn't it wonderful how
everything is integrated?
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
