Re: [PATCH 36/37] KVM: PPC: booke: expose guest registers on irq reinject
On 02/26/2012 05:59 AM, Alexander Graf wrote:
On 25.02.2012, at 00:40, Scott Wood wrote:
On 02/24/2012 08:26 AM, Alexander Graf wrote:
+static void kvmppc_fill_pt_regs(struct kvm_vcpu *vcpu, struct pt_regs
*regs)
{
- int r = RESUME_HOST;
+ int i;
- /* update before a new last_exit_type is rewritten */
- kvmppc_update_timing_stats(vcpu);
+ for (i = 0; i 32; i++)
+ regs-gpr[i] = kvmppc_get_gpr(vcpu, i);
+ regs-nip = vcpu-arch.pc;
+ regs-msr = vcpu-arch.shared-msr;
+ regs-ctr = vcpu-arch.ctr;
+ regs-link = vcpu-arch.lr;
+ regs-xer = kvmppc_get_xer(vcpu);
+ regs-ccr = kvmppc_get_cr(vcpu);
+ regs-dar = get_guest_dear(vcpu);
+ regs-dsisr = get_guest_esr(vcpu);
+}
How much overhead does this add to every interrupt? Can't we keep this
to the minimum that perf cares about?
I would rather not make assumptions on what perf cares about - maybe we want
to one day implement perf kvm and then perf could rely on pretty much
anything in there.
In that case I think we should be populating a real pt_regs from the
start, as in my original patchset.
I only agreed to take it out because I thought the set of things we'd
copy would be minimal. This seems like a lot of overhead.
I'm not familiar with perf kvm, but if it's kvm-specific surely the
KVM code should know/dictate what it can rely on? Or maybe there can be
a debug option that enables full pt_regs (similar to exit timing)?
Could we just set regs to NULL when the debug option isn't enabled?
+static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
+unsigned int exit_nr)
+{
+ struct pt_regs regs = *current-thread.regs;
+ kvmppc_fill_pt_regs(vcpu, regs);
Why are you copying out of current-thread.regs? That's old junk data,
set by some previous exception and possibly overwritten since.
Because it gives us good default values for anything we don't set. Do you
have other recommendations?
It does not give good default values for anything. It is junk,
unallocated memory, overwritten by who knows what. Same as the memory
you're copying to.
To avoid garbage in fields we don't set, fill it with zeroes first.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 36/37] KVM: PPC: booke: expose guest registers on irq reinject
On 25.02.2012, at 00:40, Scott Wood wrote:
On 02/24/2012 08:26 AM, Alexander Graf wrote:
+static void kvmppc_fill_pt_regs(struct kvm_vcpu *vcpu, struct pt_regs *regs)
{
-int r = RESUME_HOST;
+int i;
-/* update before a new last_exit_type is rewritten */
-kvmppc_update_timing_stats(vcpu);
+for (i = 0; i 32; i++)
+regs-gpr[i] = kvmppc_get_gpr(vcpu, i);
+regs-nip = vcpu-arch.pc;
+regs-msr = vcpu-arch.shared-msr;
+regs-ctr = vcpu-arch.ctr;
+regs-link = vcpu-arch.lr;
+regs-xer = kvmppc_get_xer(vcpu);
+regs-ccr = kvmppc_get_cr(vcpu);
+regs-dar = get_guest_dear(vcpu);
+regs-dsisr = get_guest_esr(vcpu);
+}
How much overhead does this add to every interrupt? Can't we keep this
to the minimum that perf cares about?
I would rather not make assumptions on what perf cares about - maybe we want to
one day implement perf kvm and then perf could rely on pretty much anything
in there.
+
+static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
+ unsigned int exit_nr)
+{
+struct pt_regs regs = *current-thread.regs;
+kvmppc_fill_pt_regs(vcpu, regs);
Why are you copying out of current-thread.regs? That's old junk data,
set by some previous exception and possibly overwritten since.
Because it gives us good default values for anything we don't set. Do you have
other recommendations?
Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 36/37] KVM: PPC: booke: expose guest registers on irq reinject
When reinjecting an interrupt into the host interrupt handler after we're
back in host kernel land, let's tell the kernel about all the guest state
that the interrupt happened at.
This helps getting reasonable numbers out of perf.
Signed-off-by: Alexander Graf ag...@suse.de
---
arch/powerpc/kvm/booke.c | 54 +
1 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 423701b..709fd45 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -593,37 +593,61 @@ static int emulation_exit(struct kvm_run *run, struct
kvm_vcpu *vcpu)
}
}
-/**
- * kvmppc_handle_exit
- *
- * Return value is in the form (errcode2 | RESUME_FLAG_HOST | RESUME_FLAG_NV)
- */
-int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
- unsigned int exit_nr)
+static void kvmppc_fill_pt_regs(struct kvm_vcpu *vcpu, struct pt_regs *regs)
{
- int r = RESUME_HOST;
+ int i;
- /* update before a new last_exit_type is rewritten */
- kvmppc_update_timing_stats(vcpu);
+ for (i = 0; i 32; i++)
+ regs-gpr[i] = kvmppc_get_gpr(vcpu, i);
+ regs-nip = vcpu-arch.pc;
+ regs-msr = vcpu-arch.shared-msr;
+ regs-ctr = vcpu-arch.ctr;
+ regs-link = vcpu-arch.lr;
+ regs-xer = kvmppc_get_xer(vcpu);
+ regs-ccr = kvmppc_get_cr(vcpu);
+ regs-dar = get_guest_dear(vcpu);
+ regs-dsisr = get_guest_esr(vcpu);
+}
+
+static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
+unsigned int exit_nr)
+{
+ struct pt_regs regs = *current-thread.regs;
+ kvmppc_fill_pt_regs(vcpu, regs);
switch (exit_nr) {
case BOOKE_INTERRUPT_EXTERNAL:
- do_IRQ(current-thread.regs);
+ do_IRQ(regs);
break;
-
case BOOKE_INTERRUPT_DECREMENTER:
- timer_interrupt(current-thread.regs);
+ timer_interrupt(regs);
break;
-
#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64)
case BOOKE_INTERRUPT_DOORBELL:
- doorbell_exception(current-thread.regs);
+ doorbell_exception(regs);
break;
#endif
case BOOKE_INTERRUPT_MACHINE_CHECK:
/* FIXME */
break;
}
+}
+
+/**
+ * kvmppc_handle_exit
+ *
+ * Return value is in the form (errcode2 | RESUME_FLAG_HOST | RESUME_FLAG_NV)
+ */
+int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
+ unsigned int exit_nr)
+{
+ int r = RESUME_HOST;
+
+ /* update before a new last_exit_type is rewritten */
+ kvmppc_update_timing_stats(vcpu);
+
+ /* restart interrupts if they were meant for the host */
+ kvmppc_restart_interrupt(vcpu, exit_nr);
local_irq_enable();
--
1.6.0.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 36/37] KVM: PPC: booke: expose guest registers on irq reinject
On 02/24/2012 08:26 AM, Alexander Graf wrote:
+static void kvmppc_fill_pt_regs(struct kvm_vcpu *vcpu, struct pt_regs *regs)
{
- int r = RESUME_HOST;
+ int i;
- /* update before a new last_exit_type is rewritten */
- kvmppc_update_timing_stats(vcpu);
+ for (i = 0; i 32; i++)
+ regs-gpr[i] = kvmppc_get_gpr(vcpu, i);
+ regs-nip = vcpu-arch.pc;
+ regs-msr = vcpu-arch.shared-msr;
+ regs-ctr = vcpu-arch.ctr;
+ regs-link = vcpu-arch.lr;
+ regs-xer = kvmppc_get_xer(vcpu);
+ regs-ccr = kvmppc_get_cr(vcpu);
+ regs-dar = get_guest_dear(vcpu);
+ regs-dsisr = get_guest_esr(vcpu);
+}
How much overhead does this add to every interrupt? Can't we keep this
to the minimum that perf cares about?
+
+static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
+ unsigned int exit_nr)
+{
+ struct pt_regs regs = *current-thread.regs;
+ kvmppc_fill_pt_regs(vcpu, regs);
Why are you copying out of current-thread.regs? That's old junk data,
set by some previous exception and possibly overwritten since.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
