Re: [PATCH 2/6] KVM: PPC: Add support for multiple-TCE hcalls
On Mon, May 06, 2013 at 05:25:53PM +1000, Alexey Kardashevskiy wrote:
This adds real mode handlers for the H_PUT_TCE_INDIRECT and
H_STUFF_TCE hypercalls for QEMU emulated devices such as virtio
devices or emulated PCI. These calls allow adding multiple entries
(up to 512) into the TCE table in one call which saves time on
transition to/from real mode.
This adds a guest physical to host real address converter
and calls the existing H_PUT_TCE handler. The converting function
is going to be fully utilized by upcoming VFIO supporting patches.
This also implements the KVM_CAP_PPC_MULTITCE capability,
so in order to support the functionality of this patch, QEMU
needs to query for this capability and set the hcall-multi-tce
hypertas property only if the capability is present, otherwise
there will be serious performance degradation.
Hrm. Clearly I didn't read this carefully enough before. There are
some problems here.
[snip]
diff --git a/arch/powerpc/kvm/book3s_64_vio.c
b/arch/powerpc/kvm/book3s_64_vio.c
index 72ffc89..643ac1e 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -14,6 +14,7 @@
*
* Copyright 2010 Paul Mackerras, IBM Corp. pau...@au1.ibm.com
* Copyright 2011 David Gibson, IBM Corporation d...@au1.ibm.com
+ * Copyright 2013 Alexey Kardashevskiy, IBM Corporation a...@au1.ibm.com
*/
#include linux/types.h
@@ -36,9 +37,14 @@
#include asm/ppc-opcode.h
#include asm/kvm_host.h
#include asm/udbg.h
+#include asm/iommu.h
#define TCES_PER_PAGE(PAGE_SIZE / sizeof(u64))
+#define ERROR_ADDR (~(unsigned long)0x0)
+/*
+ * TCE tables handlers.
+ */
static long kvmppc_stt_npages(unsigned long window_size)
{
return ALIGN((window_size SPAPR_TCE_SHIFT)
@@ -148,3 +154,111 @@ fail:
}
return ret;
}
+
+/*
+ * Virtual mode handling of IOMMU map/unmap.
+ */
+/* Converts guest physical address into host virtual */
+static unsigned long get_virt_address(struct kvm_vcpu *vcpu,
+ unsigned long gpa)
This should probably return a void * rather than an unsigned long.
Well, actually a void __user *.
+{
+ unsigned long hva, gfn = gpa PAGE_SHIFT;
+ struct kvm_memory_slot *memslot;
+
+ memslot = search_memslots(kvm_memslots(vcpu-kvm), gfn);
+ if (!memslot)
+ return ERROR_ADDR;
+
+ /*
+ * Convert gfn to hva preserving flags and an offset
+ * within a system page
+ */
+ hva = __gfn_to_hva_memslot(memslot, gfn) + (gpa ~PAGE_MASK);
+ return hva;
+}
+
+long kvmppc_virtmode_h_put_tce(struct kvm_vcpu *vcpu,
+ unsigned long liobn, unsigned long ioba,
+ unsigned long tce)
+{
+ struct kvmppc_spapr_tce_table *tt;
+
+ tt = kvmppc_find_tce_table(vcpu, liobn);
+ /* Didn't find the liobn, put it to userspace */
+ if (!tt)
+ return H_TOO_HARD;
+
+ /* Emulated IO */
+ return kvmppc_emulated_h_put_tce(tt, ioba, tce);
+}
+
+long kvmppc_virtmode_h_put_tce_indirect(struct kvm_vcpu *vcpu,
+ unsigned long liobn, unsigned long ioba,
+ unsigned long tce_list, unsigned long npages)
+{
+ struct kvmppc_spapr_tce_table *tt;
+ long i;
+ unsigned long tces;
+
+ /* The whole table addressed by tce_list resides in 4K page */
+ if (npages 512)
+ return H_PARAMETER;
So, that doesn't actually verify what the comment says it does - only
that the list is 4K in total. You need to check the alignment of
tce_list as well.
+
+ tt = kvmppc_find_tce_table(vcpu, liobn);
+ /* Didn't find the liobn, put it to userspace */
+ if (!tt)
+ return H_TOO_HARD;
+
+ tces = get_virt_address(vcpu, tce_list);
+ if (tces == ERROR_ADDR)
+ return H_TOO_HARD;
+
+ /* Emulated IO */
This comment doesn't seem to have any bearing on the test which
follows it.
+ if ((ioba + (npages IOMMU_PAGE_SHIFT)) tt-window_size)
+ return H_PARAMETER;
+
+ for (i = 0; i npages; ++i) {
+ unsigned long tce;
+ unsigned long ptce = tces + i * sizeof(unsigned long);
+
+ if (get_user(tce, (unsigned long __user *)ptce))
+ break;
+
+ if (kvmppc_emulated_h_put_tce(tt,
+ ioba + (i IOMMU_PAGE_SHIFT), tce))
+ break;
+ }
+ if (i == npages)
+ return H_SUCCESS;
+
+ /* Failed, do cleanup */
+ do {
+ --i;
+ kvmppc_emulated_h_put_tce(tt, ioba + (i IOMMU_PAGE_SHIFT),
+ 0);
+ } while (i);
Hrm, so, actually PAPR specifies that this hcall is supposed to first
copy the given tces to hypervisor memory, then translate (and
validate) them all, and only then touch the actual TCE table. Rather
more complicated to do, but I guess we should - that would get rid
Re: [PATCH v2 1/4] powerpc: hard_irq_disable(): Call trace_hardirqs_off after disabling
On Thu, 2013-05-09 at 22:09 -0500, Scott Wood wrote:
lockdep.c has this:
/*
* So we're supposed to get called after you mask local IRQs,
* but for some reason the hardware doesn't quite think you did
* a proper job.
*/
if (DEBUG_LOCKS_WARN_ON(!irqs_disabled()))
return;
Since irqs_disabled() is based on soft_enabled(), that (not just the
hard EE bit) needs to be 0 before we call trace_hardirqs_off.
Signed-off-by: Scott Wood scottw...@freescale.com
Oops ;-)
I'll apply that to my tree and will send it to Linus right after -rc1,
the rest will go the normal way for KVM patches.
Cheers,
Ben.
---
arch/powerpc/include/asm/hw_irq.h |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_irq.h
b/arch/powerpc/include/asm/hw_irq.h
index d615b28..ba713f1 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -96,11 +96,12 @@ static inline bool arch_irqs_disabled(void)
#endif
#define hard_irq_disable() do {\
+ u8 _was_enabled = get_paca()-soft_enabled; \
__hard_irq_disable(); \
- if (local_paca-soft_enabled) \
- trace_hardirqs_off(); \
get_paca()-soft_enabled = 0; \
get_paca()-irq_happened |= PACA_IRQ_HARD_DIS; \
+ if (_was_enabled) \
+ trace_hardirqs_off(); \
} while(0)
static inline bool lazy_irq_pending(void)
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] KVM: PPC: Add support for multiple-TCE hcalls
On 05/10/2013 04:51 PM, David Gibson wrote:
On Mon, May 06, 2013 at 05:25:53PM +1000, Alexey Kardashevskiy wrote:
This adds real mode handlers for the H_PUT_TCE_INDIRECT and
H_STUFF_TCE hypercalls for QEMU emulated devices such as virtio
devices or emulated PCI. These calls allow adding multiple entries
(up to 512) into the TCE table in one call which saves time on
transition to/from real mode.
This adds a guest physical to host real address converter
and calls the existing H_PUT_TCE handler. The converting function
is going to be fully utilized by upcoming VFIO supporting patches.
This also implements the KVM_CAP_PPC_MULTITCE capability,
so in order to support the functionality of this patch, QEMU
needs to query for this capability and set the hcall-multi-tce
hypertas property only if the capability is present, otherwise
there will be serious performance degradation.
Hrm. Clearly I didn't read this carefully enough before. There are
some problems here.
?
[snip]
diff --git a/arch/powerpc/kvm/book3s_64_vio.c
b/arch/powerpc/kvm/book3s_64_vio.c
index 72ffc89..643ac1e 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -14,6 +14,7 @@
*
* Copyright 2010 Paul Mackerras, IBM Corp. pau...@au1.ibm.com
* Copyright 2011 David Gibson, IBM Corporation d...@au1.ibm.com
+ * Copyright 2013 Alexey Kardashevskiy, IBM Corporation a...@au1.ibm.com
*/
#include linux/types.h
@@ -36,9 +37,14 @@
#include asm/ppc-opcode.h
#include asm/kvm_host.h
#include asm/udbg.h
+#include asm/iommu.h
#define TCES_PER_PAGE (PAGE_SIZE / sizeof(u64))
+#define ERROR_ADDR (~(unsigned long)0x0)
+/*
+ * TCE tables handlers.
+ */
static long kvmppc_stt_npages(unsigned long window_size)
{
return ALIGN((window_size SPAPR_TCE_SHIFT)
@@ -148,3 +154,111 @@ fail:
}
return ret;
}
+
+/*
+ * Virtual mode handling of IOMMU map/unmap.
+ */
+/* Converts guest physical address into host virtual */
+static unsigned long get_virt_address(struct kvm_vcpu *vcpu,
+unsigned long gpa)
This should probably return a void * rather than an unsigned long.
Well, actually a void __user *.
+{
+unsigned long hva, gfn = gpa PAGE_SHIFT;
+struct kvm_memory_slot *memslot;
+
+memslot = search_memslots(kvm_memslots(vcpu-kvm), gfn);
+if (!memslot)
+return ERROR_ADDR;
+
+/*
+ * Convert gfn to hva preserving flags and an offset
+ * within a system page
+ */
+hva = __gfn_to_hva_memslot(memslot, gfn) + (gpa ~PAGE_MASK);
+return hva;
+}
+
+long kvmppc_virtmode_h_put_tce(struct kvm_vcpu *vcpu,
+unsigned long liobn, unsigned long ioba,
+unsigned long tce)
+{
+struct kvmppc_spapr_tce_table *tt;
+
+tt = kvmppc_find_tce_table(vcpu, liobn);
+/* Didn't find the liobn, put it to userspace */
+if (!tt)
+return H_TOO_HARD;
+
+/* Emulated IO */
+return kvmppc_emulated_h_put_tce(tt, ioba, tce);
+}
+
+long kvmppc_virtmode_h_put_tce_indirect(struct kvm_vcpu *vcpu,
+unsigned long liobn, unsigned long ioba,
+unsigned long tce_list, unsigned long npages)
+{
+struct kvmppc_spapr_tce_table *tt;
+long i;
+unsigned long tces;
+
+/* The whole table addressed by tce_list resides in 4K page */
+if (npages 512)
+return H_PARAMETER;
So, that doesn't actually verify what the comment says it does - only
that the list is 4K in total. You need to check the alignment of
tce_list as well.
The spec says to return H_PARAMETER if 512. I.e. it takes just 1 page and
I do not need to bother if pages may not lay continuously in RAM (matters
for real mode).
/*
* As the spec is saying that maximum possible number of TCEs is 512,
* the whole TCE page is no more than 4K. Therefore we do not have to
* worry if pages do not lie continuously in the RAM
*/
Any better?...
+
+tt = kvmppc_find_tce_table(vcpu, liobn);
+/* Didn't find the liobn, put it to userspace */
+if (!tt)
+return H_TOO_HARD;
+
+tces = get_virt_address(vcpu, tce_list);
+if (tces == ERROR_ADDR)
+return H_TOO_HARD;
+
+/* Emulated IO */
This comment doesn't seem to have any bearing on the test which
follows it.
+if ((ioba + (npages IOMMU_PAGE_SHIFT)) tt-window_size)
+return H_PARAMETER;
+
+for (i = 0; i npages; ++i) {
+unsigned long tce;
+unsigned long ptce = tces + i * sizeof(unsigned long);
+
+if (get_user(tce, (unsigned long __user *)ptce))
+break;
+
+if (kvmppc_emulated_h_put_tce(tt,
+ioba + (i IOMMU_PAGE_SHIFT), tce))
+break;
+}
+if (i == npages)
+return H_SUCCESS;
+
+/* Failed, do cleanup */
+do {
+--i;
+
Re: [PATCH] powerpc: export debug register save function for KVM
On 07.05.2013, at 11:40, Bharat Bhushan wrote:
KVM need this function when switching from vcpu to user-space
thread. My subsequent patch will use this function.
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
arch/powerpc/include/asm/switch_to.h |4
arch/powerpc/kernel/process.c|3 ++-
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/include/asm/switch_to.h
b/arch/powerpc/include/asm/switch_to.h
index 200d763..50b357f 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -30,6 +30,10 @@ extern void enable_kernel_spe(void);
extern void giveup_spe(struct task_struct *);
extern void load_up_spe(struct task_struct *);
+#ifdef CONFIG_PPC_ADV_DEBUG_REGS
+extern void switch_booke_debug_regs(struct thread_struct *new_thread);
+#endif
+
#ifndef CONFIG_SMP
extern void discard_lazy_cpu_state(void);
#else
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ca89375..a938138 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -362,12 +362,13 @@ static void prime_debug_regs(struct thread_struct
*thread)
* debug registers, set the debug registers from the values
* stored in the new thread.
*/
-static void switch_booke_debug_regs(struct thread_struct *new_thread)
+void switch_booke_debug_regs(struct thread_struct *new_thread)
{
if ((current-thread.debug.dbcr0 DBCR0_IDM)
|| (new_thread-debug.dbcr0 DBCR0_IDM))
prime_debug_regs(new_thread);
}
+EXPORT_SYMBOL(switch_booke_debug_regs);
EXPORT_SYMBOL_GPL
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; - else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) - r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; + break; + case BOOKE_INTERRUPT_DATA_STORAGE: kvmppc_core_queue_data_storage(vcpu, vcpu-arch.fault_dear, vcpu-arch.fault_esr); Kconfig should also be updated (in both proposals). -Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: PPC: Add userspace debug stub support
On 07.05.2013, at 11:40, Bharat Bhushan wrote:
This patch adds the debug stub support on booke/bookehv.
Now QEMU debug stub can use hw breakpoint, watchpoint and
software breakpoint to debug guest.
This is how we save/restore debug register context when switching
between guest, userspace and kernel user-process:
When QEMU is running
- thread-debug_reg == QEMU debug register context.
- Kernel will handle switching the debug register on context switch.
- no vcpu_load() called
QEMU makes ioctls (except RUN)
- This will call vcpu_load()
- should not change context.
- Some ioctls can change vcpu debug register, context saved in
vcpu-debug_regs
QEMU Makes RUN ioctl
- Save thread-debug_reg on STACK
- Store thread-debug_reg == vcpu-debug_reg
- load thread-debug_reg
- RUN VCPU ( So thread points to vcpu context )
Context switch happens When VCPU running
- makes vcpu_load() should not load any context
- kernel loads the vcpu context as thread-debug_regs points to vcpu context.
On heavyweight_exit
- Load the context saved on stack in thread-debug_reg
Currently we do not support debug resource emulation to guest,
On debug exception, always exit to user space irrespective of
user space is expecting the debug exception or not. If this is
unexpected exception (breakpoint/watchpoint event not set by
userspace) then let us leave the action on user space. This
is similar to what it was before, only thing is that now we
have proper exit state available to user space.
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
arch/powerpc/include/asm/kvm_host.h |3 +
arch/powerpc/include/uapi/asm/kvm.h |1 +
arch/powerpc/kvm/booke.c| 242 ---
arch/powerpc/kvm/booke.h|5 +
4 files changed, 233 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 838a577..1b29945 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -524,7 +524,10 @@ struct kvm_vcpu_arch {
u32 eptcfg;
u32 epr;
u32 crit_save;
+ /* guest debug registers*/
struct debug_reg dbg_reg;
+ /* shadow debug registers */
Please be more verbose here. What exactly does this contain? Why do we need
shadow and non-shadow registers? The comment as it is reads like
/* Add one plus one */
x = 1 + 1;
+ struct debug_reg shadow_dbg_reg;
#endif
gpa_t paddr_accessed;
gva_t vaddr_accessed;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index ded0607..f5077c2 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -27,6 +27,7 @@
#define __KVM_HAVE_PPC_SMT
#define __KVM_HAVE_IRQCHIP
#define __KVM_HAVE_IRQ_LINE
+#define __KVM_HAVE_GUEST_DEBUG
struct kvm_regs {
__u64 pc;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index ef99536..6a44ad4 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu)
#endif
}
+static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)
+{
+ /* Synchronize guest's desire to get debug interrupts into shadow MSR */
+#ifndef CONFIG_KVM_BOOKE_HV
+ vcpu-arch.shadow_msr = ~MSR_DE;
+ vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE;
+#endif
+
+ /* Force enable debug interrupts when user space wants to debug */
+ if (vcpu-guest_debug) {
+#ifdef CONFIG_KVM_BOOKE_HV
+ /*
+ * Since there is no shadow MSR, sync MSR_DE into the guest
+ * visible MSR.
+ */
+ vcpu-arch.shared-msr |= MSR_DE;
+#else
+ vcpu-arch.shadow_msr |= MSR_DE;
+ vcpu-arch.shared-msr = ~MSR_DE;
+#endif
+ }
+}
+
/*
* Helper function for full MSR writes. No need to call this if only
* EE/CE/ME/DE/RI are changing.
@@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr)
kvmppc_mmu_msr_notify(vcpu, old_msr);
kvmppc_vcpu_sync_spe(vcpu);
kvmppc_vcpu_sync_fpu(vcpu);
+ kvmppc_vcpu_sync_debug(vcpu);
}
static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
@@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu)
int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
{
int ret, s;
+ struct debug_reg debug;
#ifdef CONFIG_PPC_FPU
unsigned int fpscr;
int fpexc_mode;
@@ -699,11 +724,27 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
kvm_vcpu *vcpu)
kvmppc_load_guest_fp(vcpu);
#endif
+ /*
+ * Clear current-thread.dbcr0 so that kernel does not
+ * restore h/w registers on context switch in vcpu running state.
+ */
Incorrect comment?
+ /* Save thread-debug context on stack */
/* Switch to guest debug
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ +SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ +SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; -else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) -r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC This would still be wrong, since running 2 guests with altivec would break each other. Alex +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; + break; + case BOOKE_INTERRUPT_DATA_STORAGE: kvmppc_core_queue_data_storage(vcpu, vcpu-arch.fault_dear, vcpu-arch.fault_esr); Kconfig should also be updated (in both proposals). -Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; - else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) - r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC This would still be wrong, since running 2 guests with altivec would break each other. It's wrong if you expect to have altivec supported in the VM which is not the case. Guests that executes altivec instructions will get a program (unimplemented op) exception see below. Do we really need to remove e6500 all together for this? Alex +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; +
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, May 10, 2013 at 08:07:00AM +1000, Benjamin Herrenschmidt wrote: On Thu, 2013-05-09 at 16:27 -0500, Scott Wood wrote: On 05/09/2013 07:37:42 AM, Benjamin Herrenschmidt wrote: On Thu, 2013-05-09 at 17:44 +0800, tiejun.chen wrote: Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur as I recall. Only if directed to the hypervisor. This is always the case with KVM, right? At least on booke... Hrm, on A2 we could choose iirc. Well not DEC but EXT at least, I don't remember about DBELL. Case 1) - Local_irq_disable() will set soft_enabled = 0 - Now Externel interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. No more other interrupt gated by MSR.EE can happen. Looks like the idea here is to not let a device keep on inserting interrupt till the interrupt condition on device is cleared, right? The external interrupt line is level sensitive normally, so we have to mask MSR:EE in that case or the interrupt would keep re-occurring (note that FSL has this concept of auto-acked interrupts via the on die MPIC for which you can potentially use PACA_IRQ_EE_EDGE instead and avoid having to mask MSR:EE). Note that if we do this, we can no longer leave the interrupt vector in EPR, and would have to track (potentially multiple different) pending external interrupts in software. Right, you can have a little queue in the PACA and leave EE disabled if it's full. You can probably get away with a queue of 1 though :-) So I would assume you will not pick up these two patches, right? http://patchwork.ozlabs.org/patch/235530/ http://patchwork.ozlabs.org/patch/235532/ Anyway it is more easier to enable the external proxy by using this method. But if you insist, I can respin a patch to use the method you suggested since it will definitely reduce the window where the irq is hard disabled. Thanks, Kevin Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html pgpVwG6hJXYWj.pgp Description: PGP signature
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. -Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] KVM: PPC: Add userspace debug stub support
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Friday, May 10, 2013 3:48 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421;
tiejun.c...@windriver.com; Bhushan Bharat-R65777
Subject: Re: [PATCH] KVM: PPC: Add userspace debug stub support
On 07.05.2013, at 11:40, Bharat Bhushan wrote:
This patch adds the debug stub support on booke/bookehv.
Now QEMU debug stub can use hw breakpoint, watchpoint and software
breakpoint to debug guest.
This is how we save/restore debug register context when switching
between guest, userspace and kernel user-process:
When QEMU is running
- thread-debug_reg == QEMU debug register context.
- Kernel will handle switching the debug register on context switch.
- no vcpu_load() called
QEMU makes ioctls (except RUN)
- This will call vcpu_load()
- should not change context.
- Some ioctls can change vcpu debug register, context saved in
- vcpu-debug_regs
QEMU Makes RUN ioctl
- Save thread-debug_reg on STACK
- Store thread-debug_reg == vcpu-debug_reg load thread-debug_reg
- RUN VCPU ( So thread points to vcpu context )
Context switch happens When VCPU running
- makes vcpu_load() should not load any context kernel loads the vcpu
- context as thread-debug_regs points to vcpu context.
On heavyweight_exit
- Load the context saved on stack in thread-debug_reg
Currently we do not support debug resource emulation to guest, On
debug exception, always exit to user space irrespective of user space
is expecting the debug exception or not. If this is unexpected
exception (breakpoint/watchpoint event not set by
userspace) then let us leave the action on user space. This is similar
to what it was before, only thing is that now we have proper exit
state available to user space.
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
arch/powerpc/include/asm/kvm_host.h |3 +
arch/powerpc/include/uapi/asm/kvm.h |1 +
arch/powerpc/kvm/booke.c| 242
---
arch/powerpc/kvm/booke.h|5 +
4 files changed, 233 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 838a577..1b29945 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -524,7 +524,10 @@ struct kvm_vcpu_arch {
u32 eptcfg;
u32 epr;
u32 crit_save;
+ /* guest debug registers*/
struct debug_reg dbg_reg;
+ /* shadow debug registers */
Please be more verbose here. What exactly does this contain? Why do we need
shadow and non-shadow registers? The comment as it is reads like
/* Add one plus one */
x = 1 + 1;
/*
* Shadow debug registers hold the debug register content
* to be written in h/w debug register on behalf of guest
* written value or user space written value.
*/
+ struct debug_reg shadow_dbg_reg;
#endif
gpa_t paddr_accessed;
gva_t vaddr_accessed;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index ded0607..f5077c2 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -27,6 +27,7 @@
#define __KVM_HAVE_PPC_SMT
#define __KVM_HAVE_IRQCHIP
#define __KVM_HAVE_IRQ_LINE
+#define __KVM_HAVE_GUEST_DEBUG
struct kvm_regs {
__u64 pc;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
ef99536..6a44ad4 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu
*vcpu) #endif }
+static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) {
+ /* Synchronize guest's desire to get debug interrupts into shadow
+MSR */ #ifndef CONFIG_KVM_BOOKE_HV
+ vcpu-arch.shadow_msr = ~MSR_DE;
+ vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif
+
+ /* Force enable debug interrupts when user space wants to debug */
+ if (vcpu-guest_debug) {
+#ifdef CONFIG_KVM_BOOKE_HV
+ /*
+* Since there is no shadow MSR, sync MSR_DE into the guest
+* visible MSR.
+*/
+ vcpu-arch.shared-msr |= MSR_DE;
+#else
+ vcpu-arch.shadow_msr |= MSR_DE;
+ vcpu-arch.shared-msr = ~MSR_DE;
+#endif
+ }
+}
+
/*
* Helper function for full MSR writes. No need to call this if
only
* EE/CE/ME/DE/RI are changing.
@@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr)
kvmppc_mmu_msr_notify(vcpu, old_msr);
kvmppc_vcpu_sync_spe(vcpu);
kvmppc_vcpu_sync_fpu(vcpu);
+ kvmppc_vcpu_sync_debug(vcpu);
}
static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@
-655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu)
int kvmppc_vcpu_run(struct
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 04:40:01 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. Exceptions don't get handled all that often, and ideally we catch it when it's added rather than after-the-fact. diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? This is just to fix the build break -- we don't need ESR yet. Though I did look at the ESR documentation and didn't see where Altivec exceptions used it (I do see a couple places now). -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: PPC: Add userspace debug stub support
On 10.05.2013, at 19:31, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Friday, May 10, 2013 3:48 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421;
tiejun.c...@windriver.com; Bhushan Bharat-R65777
Subject: Re: [PATCH] KVM: PPC: Add userspace debug stub support
On 07.05.2013, at 11:40, Bharat Bhushan wrote:
This patch adds the debug stub support on booke/bookehv.
Now QEMU debug stub can use hw breakpoint, watchpoint and software
breakpoint to debug guest.
This is how we save/restore debug register context when switching
between guest, userspace and kernel user-process:
When QEMU is running
- thread-debug_reg == QEMU debug register context.
- Kernel will handle switching the debug register on context switch.
- no vcpu_load() called
QEMU makes ioctls (except RUN)
- This will call vcpu_load()
- should not change context.
- Some ioctls can change vcpu debug register, context saved in
- vcpu-debug_regs
QEMU Makes RUN ioctl
- Save thread-debug_reg on STACK
- Store thread-debug_reg == vcpu-debug_reg load thread-debug_reg
- RUN VCPU ( So thread points to vcpu context )
Context switch happens When VCPU running
- makes vcpu_load() should not load any context kernel loads the vcpu
- context as thread-debug_regs points to vcpu context.
On heavyweight_exit
- Load the context saved on stack in thread-debug_reg
Currently we do not support debug resource emulation to guest, On
debug exception, always exit to user space irrespective of user space
is expecting the debug exception or not. If this is unexpected
exception (breakpoint/watchpoint event not set by
userspace) then let us leave the action on user space. This is similar
to what it was before, only thing is that now we have proper exit
state available to user space.
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
arch/powerpc/include/asm/kvm_host.h |3 +
arch/powerpc/include/uapi/asm/kvm.h |1 +
arch/powerpc/kvm/booke.c| 242
---
arch/powerpc/kvm/booke.h|5 +
4 files changed, 233 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 838a577..1b29945 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -524,7 +524,10 @@ struct kvm_vcpu_arch {
u32 eptcfg;
u32 epr;
u32 crit_save;
+ /* guest debug registers*/
struct debug_reg dbg_reg;
+ /* shadow debug registers */
Please be more verbose here. What exactly does this contain? Why do we need
shadow and non-shadow registers? The comment as it is reads like
/* Add one plus one */
x = 1 + 1;
/*
* Shadow debug registers hold the debug register content
* to be written in h/w debug register on behalf of guest
* written value or user space written value.
*/
/* hardware visible debug registers when in guest state */
+ struct debug_reg shadow_dbg_reg;
#endif
gpa_t paddr_accessed;
gva_t vaddr_accessed;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index ded0607..f5077c2 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -27,6 +27,7 @@
#define __KVM_HAVE_PPC_SMT
#define __KVM_HAVE_IRQCHIP
#define __KVM_HAVE_IRQ_LINE
+#define __KVM_HAVE_GUEST_DEBUG
struct kvm_regs {
__u64 pc;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
ef99536..6a44ad4 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu
*vcpu) #endif }
+static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) {
+ /* Synchronize guest's desire to get debug interrupts into shadow
+MSR */ #ifndef CONFIG_KVM_BOOKE_HV
+ vcpu-arch.shadow_msr = ~MSR_DE;
+ vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif
+
+ /* Force enable debug interrupts when user space wants to debug */
+ if (vcpu-guest_debug) {
+#ifdef CONFIG_KVM_BOOKE_HV
+ /*
+* Since there is no shadow MSR, sync MSR_DE into the guest
+* visible MSR.
+*/
+ vcpu-arch.shared-msr |= MSR_DE;
+#else
+ vcpu-arch.shadow_msr |= MSR_DE;
+ vcpu-arch.shared-msr = ~MSR_DE;
+#endif
+ }
+}
+
/*
* Helper function for full MSR writes. No need to call this if
only
* EE/CE/ME/DE/RI are changing.
@@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr)
kvmppc_mmu_msr_notify(vcpu, old_msr);
kvmppc_vcpu_sync_spe(vcpu);
kvmppc_vcpu_sync_fpu(vcpu);
+ kvmppc_vcpu_sync_debug(vcpu);
}
static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@
-655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu)
int
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? -Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? Alex case BOOKE_INTERRUPT_DOORBELL: kvmppc_fill_pt_regs(regs); doorbell_exception(regs); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 05/10/2013 01:14:27 PM, Alexander Graf wrote: On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? This is an improvement because CONFIG_PPC_DOORBELL is what controls whether the function that is called inside the ifdef exists. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 10.05.2013, at 20:17, Scott Wood wrote: On 05/10/2013 01:14:27 PM, Alexander Graf wrote: On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? This is an improvement because CONFIG_PPC_DOORBELL is what controls whether the function that is called inside the ifdef exists. Aha! Now that's a good reason. Tiejun, please adjust your patch description accordingly. Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 8:42 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. I wouldn't want to see e6500 removed for this reason. Thanks, Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. I doubt you'll manage to break anything there :). So what you should ensure is 1) 32bit e500 still compiles 2) 32bit e500 still executes fine, albeit without altivec Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 01:20:33 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 8:42 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. Program exception injection still doesn't deal with state corruption. I wouldn't want to see e6500 removed for this reason. It's not being removed, just closed while under construction to prevent damage to people passing through. :-) -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 9:06 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) That's exactly what I have said. That patch was intended as a RFC I forgot to mark it as such under time constraints, however it reached its goal. -Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 9:23 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- p...@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. Huh ... I forgot that G4 in handled under Book3S, not so obvious. I doubt you'll manage to break anything there :). If you have stated breaks instead of works would have been more clear where you were heading :-J -Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
Am 10.05.2013 um 20:51 schrieb Caraman Mihai Claudiu-B02008 b02...@freescale.com: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 9:23 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- p...@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. Huh ... I forgot that G4 in handled under Book3S, not so obvious. I doubt you'll manage to break anything there :). If you have stated breaks instead of works would have been more clear where you were heading :-J Ok, next time I'll make it more obvious :). Alex -Mike -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 02:06:53 PM, Caraman Mihai Claudiu-B02008 wrote: I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. Program exception injection still doesn't deal with state corruption. Yes but it's not critical for this particular case since nobody is able to effectively use that state via altivec instructions. Leaking state however can be a real issue. Depending on guest behavior it could look like things are working even though they aren't (e.g. a guest just enables MSR[VEC] and uses altivec instructions, not relying on exceptions). This really isn't worth spending a lot of time debating... Once Altivec is fixed properly (you said that'd be soon, right?), we can add e6500 back to the list. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, 2013-05-10 at 22:12 +0800, Kevin Hao wrote: So I would assume you will not pick up these two patches, right? http://patchwork.ozlabs.org/patch/235530/ http://patchwork.ozlabs.org/patch/235532/ Anyway it is more easier to enable the external proxy by using this method. But if you insist, I can respin a patch to use the method you suggested since it will definitely reduce the window where the irq is hard disabled. I would keep the EE_EDGE bit definition. I have no objection to a gradual approach however for the other one where we apply it as is now to enable coreint while you do a rework to make it better :-) Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Sat, 2013-05-11 at 07:49 +1000, Benjamin Herrenschmidt wrote: I would keep the EE_EDGE bit definition. I have no objection to a gradual approach however for the other one where we apply it as is now to enable coreint while you do a rework to make it better :-) Note also that I generally don't apply FSL related patches directly, I rely on Kumar Gala picking them up so he's the one ultimately making that choice. Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit()
On 05/10/2013 12:01:19 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Scott Wood Sent: Friday, May 10, 2013 8:40 AM To: Alexander Graf; Benjamin Herrenschmidt Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; Wood Scott-B07421 Subject: [PATCH v2 2/4] kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit() EE is hard-disabled on entry to kvmppc_handle_exit(), so call hard_irq_disable() so that PACA_IRQ_HARD_DIS is set, and soft_enabled is unset. Without this, we get warnings such as arch/powerpc/kernel/time.c:300, and sometimes host kernel hangs. Signed-off-by: Scott Wood scottw...@freescale.com --- arch/powerpc/kvm/booke.c |5 + 1 file changed, 5 insertions(+) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..705fc5c 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -833,6 +833,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, int r = RESUME_HOST; int s; +#ifdef CONFIG_PPC64 + WARN_ON(local_paca-irq_happened != 0); +#endif + hard_irq_disable(); It is not actually to hard disable as EE is already clear but to make it looks like hard_disable to host. Right? If so, should we write a comment here on why we are doing this? Yes, I can add a comment. -Scott -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup
On 05/10/2013 12:01:38 AM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On
Behalf Of Scott Wood
Sent: Friday, May 10, 2013 8:40 AM
To: Alexander Graf; Benjamin Herrenschmidt
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
linuxppc-...@lists.ozlabs.org;
Wood Scott-B07421
Subject: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup
- WARN_ON_ONCE(!irqs_disabled());
+ WARN_ON(irqs_disabled());
+ hard_irq_disable();
Here we hard disable in kvmppc_prepare_to_enter(), so my comment in
other patch about interrupt loss is no more valid.
So here
MSR.EE = 0
local_paca-soft_enabled = 0
local_paca-irq_happened |= PACA_IRQ_HARD_DIS;
+
while (true) {
if (need_resched()) {
local_irq_enable();
This will make the state:
MSR.EE = 1
local_paca-soft_enabled = 1
local_paca-irq_happened = PACA_IRQ_HARD_DIS; //same as before
Is that a valid state where interrupts are fully enabled and
irq_happend in not 0?
PACA_IRQ_HARD_DIS will have been cleared by local_irq_enable(), as
Tiejun pointed out.
int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
{
int r = 0;
WARN_ON_ONCE(!irqs_disabled());
kvmppc_core_check_exceptions(vcpu);
if (vcpu-requests) {
/* Exception delivery raised request; start over */
return 1;
}
if (vcpu-arch.shared-msr MSR_WE) {
local_irq_enable();
kvm_vcpu_block(vcpu);
clear_bit(KVM_REQ_UNHALT, vcpu-requests);
local_irq_disable();
^^^
We do not require hard_irq_disable() here?
Yes, that should be changed to hard_irq_disable(), and I'll add a
WARN_ON to double check that interrupts are hard-disabled (eventually
we'll probably want to make these critical-path assertions dependent on
a debug option...). It doesn't really matter all that much, though,
since we don't have MSR_WE on any 64-bit booke chips. :-)
-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
