Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint
On 01.08.14 06:50, Madhavan Srinivasan wrote:
This patch adds kernel side support for software breakpoint.
Design is that, by using an illegal instruction, we trap to hypervisor
via Emulation Assistance interrupt, where we check for the illegal instruction
and accordingly we return to Host or Guest. Patch also adds support for
software breakpoint in PR KVM.
Changes v2-v3:
Changed the debug instructions. Using the all zero opcode in the instruction
word
as illegal instruction as mentioned in Power ISA instead of ABS
Removed reg updated in emulation assist and added a call to
kvmppc_emulate_instruction for reg update.
Changes v1-v2:
Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also
share it.
Added code to use KVM get one reg infrastructure to get debug opcode.
Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
Made changes to commit message.
Signed-off-by: Madhavan Srinivasan ma...@linux.vnet.ibm.com
---
arch/powerpc/include/asm/kvm_book3s.h | 7 +++
arch/powerpc/include/asm/ppc-opcode.h | 5 +
arch/powerpc/kvm/book3s.c | 3 ++-
arch/powerpc/kvm/book3s_hv.c | 12 ++--
arch/powerpc/kvm/book3s_pr.c | 3 +++
arch/powerpc/kvm/emulate.c| 9 +
6 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index f52f656..f17e3fd 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -24,6 +24,13 @@
#include linux/kvm_host.h
#include asm/kvm_book3s_asm.h
+/*
+ * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software
Breakpoint.
+ * Based on PowerISA v2.07, Instruction with opcode 0s will be treated as
illegal
+ * instruction.
+ */
+#define KVMPPC_INST_BOOK3S_DEBUG 0x0000
+
struct kvmppc_bat {
u64 raw;
u32 bepi;
diff --git a/arch/powerpc/include/asm/ppc-opcode.h
b/arch/powerpc/include/asm/ppc-opcode.h
index 3132bb9..56739b3 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -111,6 +111,11 @@
#define OP_31_XOP_LHBRX 790
#define OP_31_XOP_STHBRX918
+/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
+ * 0x0000 -- Primary opcode is 0s
+ */
+#define OP_ZERO 0x0
+
#define OP_LWZ 32
#define OP_LD 58
#define OP_LWZU 33
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index c254c27..b40fe5d 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
- return -EINVAL;
+ vcpu-guest_debug = dbg-control;
+ return 0;
}
void kvmppc_decrementer_func(unsigned long data)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 7a12edb..7c16f4f 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,8 +725,13 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run,
struct kvm_vcpu *vcpu,
* we don't emulate any guest instructions at this stage.
*/
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
- kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
- r = RESUME_GUEST;
+ if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG) {
+ kvmppc_emulate_instruction(run, vcpu);
I changed the emulation code flow very recently, so while I advised you
to write it this way this won't work with recent git versions anymore :(.
Please just create a tiny static function that handles this particular
inst and duplicate the logic in book3s_emulate.c (for PR) as well as
here (for HV).
+ r = RESUME_HOST;
+ } else {
+ kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+ r = RESUME_GUEST;
+ }
break;
/*
* This occurs if the guest (kernel or userspace), does something that
@@ -831,6 +836,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64
id,
long int i;
switch (id) {
+ case KVM_REG_PPC_DEBUG_INST:
+ *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
+ break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, 0);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 8eef1e5..27f5234 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1229,6 +1229,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu,
u64 id,
int r = 0;
switch (id) {
+ case KVM_REG_PPC_DEBUG_INST:
+ *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint
On Mon, 2014-08-11 at 09:26 +0200, Alexander Graf wrote: diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c index da86d9b..d95014e 100644 --- a/arch/powerpc/kvm/emulate.c +++ b/arch/powerpc/kvm/emulate.c This should be book3s_emulate.c. Any reason we can't make that 0000 opcode as breakpoint common to all powerpc variants ? 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 v3] powerpc/kvm: support to handle sw breakpoint
On 11.08.14 10:51, Benjamin Herrenschmidt wrote: On Mon, 2014-08-11 at 09:26 +0200, Alexander Graf wrote: diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c index da86d9b..d95014e 100644 --- a/arch/powerpc/kvm/emulate.c +++ b/arch/powerpc/kvm/emulate.c This should be book3s_emulate.c. Any reason we can't make that 0000 opcode as breakpoint common to all powerpc variants ? I can't think of a good reason. We use a hypercall on booke (which traps into an illegal instruction for pr) today, but I don't think it has to be that way. Given that the user space API allows us to change it dynamically, there should be nothing blocking us from going with 0000 always. 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: e500mc: Add support for single threaded vcpus on e6500 core
On 06.08.14 18:33, Mihai Caraman wrote:
ePAPR represents hardware threads as cpu node properties in device tree.
So with existing QEMU, hardware threads are simply exposed as vcpus with
one hardware thread.
The e6500 core shares TLBs between hardware threads. Without tlb write
conditional instruction, the Linux kernel uses per core mechanisms to
protect against duplicate TLB entries.
The guest is unable to detect real siblings threads, so it can't use a
TLB protection mechanism. An alternative solution is to use the hypervisor
to allocate different lpids to guest's vcpus running simultaneous on real
siblings threads. This patch moves lpid to vcpu level and allocates a pool
of lpids (equal to the number of threads per core) per VM.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
Please rebase this patch before
[PATCH v3 5/5] KVM: PPC: Book3E: Enable e6500 core
to proper handle SMP guests.
arch/powerpc/include/asm/kvm_host.h | 5
arch/powerpc/kernel/asm-offsets.c | 4 +++
arch/powerpc/kvm/e500_mmu_host.c| 15 +-
arch/powerpc/kvm/e500mc.c | 55 +
4 files changed, 55 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 98d9dd5..1b0bb4a 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -227,7 +227,11 @@ struct kvm_arch_memory_slot {
};
struct kvm_arch {
+#ifdef CONFIG_KVM_BOOKE_HV
+ unsigned int lpid_pool[2];
+#else
unsigned int lpid;
+#endif
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
unsigned long hpt_virt;
struct revmap_entry *revmap;
@@ -435,6 +439,7 @@ struct kvm_vcpu_arch {
u32 eplc;
u32 epsc;
u32 oldpir;
+ u32 lpid;
#endif
#if defined(CONFIG_BOOKE)
diff --git a/arch/powerpc/kernel/asm-offsets.c
b/arch/powerpc/kernel/asm-offsets.c
index ab9ae04..5a30b87 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -483,7 +483,11 @@ int main(void)
DEFINE(VCPU_SHARED_MAS6, offsetof(struct kvm_vcpu_arch_shared, mas6));
DEFINE(VCPU_KVM, offsetof(struct kvm_vcpu, kvm));
+#ifdef CONFIG_KVM_BOOKE_HV
+ DEFINE(KVM_LPID, offsetof(struct kvm_vcpu, arch.lpid));
This is a recipe for confusion. Please use a name that indicates that
we're looking at the vcpu - VCPU_LPID for example.
+#else
DEFINE(KVM_LPID, offsetof(struct kvm, arch.lpid));
+#endif
/* book3s */
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 4150826..a233cc6 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -69,7 +69,7 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
* writing shadow tlb entry to host TLB
*/
static inline void __write_host_tlbe(struct kvm_book3e_206_tlb_entry *stlbe,
-uint32_t mas0)
+uint32_t mas0, uint32_t *lpid)
Why a pointer?
{
unsigned long flags;
@@ -80,6 +80,8 @@ static inline void __write_host_tlbe(struct kvm_book3e_206_tlb_entry *stlbe,
mtspr(SPRN_MAS3, (u32)stlbe-mas7_3);
mtspr(SPRN_MAS7, (u32)(stlbe-mas7_3 32));
#ifdef CONFIG_KVM_BOOKE_HV
+ /* populate mas8 with latest LPID */
What is a latest LPID? Really all you're doing is you're populating
mas8 with the thread-specific lpid.
+ stlbe-mas8 = MAS8_TGS | *lpid;
mtspr(SPRN_MAS8, stlbe-mas8);
Just ignore the value in stlbe and directly write MAS8_TGS | lpid into mas8.
#endif
asm volatile(isync; tlbwe : : : memory);
@@ -129,11 +131,12 @@ static inline void write_host_tlbe(struct
kvmppc_vcpu_e500 *vcpu_e500,
if (tlbsel == 0) {
mas0 = get_host_mas0(stlbe-mas2);
- __write_host_tlbe(stlbe, mas0);
+ __write_host_tlbe(stlbe, mas0, vcpu_e500-vcpu.arch.lpid);
} else {
__write_host_tlbe(stlbe,
MAS0_TLBSEL(1) |
- MAS0_ESEL(to_htlb1_esel(sesel)));
+ MAS0_ESEL(to_htlb1_esel(sesel)),
+ vcpu_e500-vcpu.arch.lpid);
}
}
@@ -318,9 +321,7 @@ static void kvmppc_e500_setup_stlbe(
stlbe-mas7_3 = ((u64)pfn PAGE_SHIFT) |
e500_shadow_mas3_attrib(gtlbe-mas7_3, pr);
-#ifdef CONFIG_KVM_BOOKE_HV
- stlbe-mas8 = MAS8_TGS | vcpu-kvm-arch.lpid;
-#endif
+ /* Set mas8 when executing tlbwe since LPID can change dynamically */
Please be more precise in this comment.
}
static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
@@ -632,7 +633,7 @@ int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, enum
instruction_type type,
local_irq_save(flags);
mtspr(SPRN_MAS6,
Re: [PATCH 1/2] powerpc/booke: Restrict SPE exception handlers to e200/e500 cores
On Wed, 2014-08-06 at 11:39 +0300, Mihai Caraman wrote: SPE exception handlers are now defined for 32-bit e500mc cores even though SPE unit is not present and CONFIG_SPE is undefined. Restrict SPE exception handlers to e200/e500 cores adding CONFIG_SPE_POSSIBLE and consequently guard __stup_ivors and __setup_cpu functions. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kernel/cpu_setup_fsl_booke.S | 12 +++- arch/powerpc/kernel/cputable.c| 5 + arch/powerpc/kernel/head_fsl_booke.S | 18 +- arch/powerpc/platforms/Kconfig.cputype| 6 +- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S index 4f1393d..44bb2c9 100644 --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S @@ -91,6 +91,7 @@ _GLOBAL(setup_altivec_idle) blr +#if defined(CONFIG_E500) defined(CONFIG_PPC_E500MC) When would you ever have CONFIG_PPC_E500MC without CONFIG_E500? diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S index b497188..4f8930f 100644 --- a/arch/powerpc/kernel/head_fsl_booke.S +++ b/arch/powerpc/kernel/head_fsl_booke.S @@ -613,6 +613,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) mfspr r10, SPRN_SPRG_RSCRATCH0 b InstructionStorage +/* Define SPE handlers for e200 and e500v2 */ #ifdef CONFIG_SPE /* SPE Unavailable */ START_EXCEPTION(SPEUnavailable) @@ -622,10 +623,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) b fast_exception_return 1: addir3,r1,STACK_FRAME_OVERHEAD EXC_XFER_EE_LITE(0x2010, KernelSPE) -#else +#elif CONFIG_SPE_POSSIBLE #elif defined(CONFIG_SPE_POSSIBLE) Likewise elsewhere -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: e500mc: Add support for single threaded vcpus on e6500 core
On Wed, 2014-08-06 at 19:33 +0300, Mihai Caraman wrote:
@@ -390,19 +400,30 @@ static void kvmppc_core_vcpu_free_e500mc(struct
kvm_vcpu *vcpu)
static int kvmppc_core_init_vm_e500mc(struct kvm *kvm)
{
- int lpid;
+ int i, lpid;
- lpid = kvmppc_alloc_lpid();
- if (lpid 0)
- return lpid;
+ /* The lpid pool supports only 2 entries now */
+ if (threads_per_core 2)
+ return -ENOMEM;
+
+ /* Each VM allocates one LPID per HW thread index */
+ for (i = 0; i threads_per_core; i++) {
+ lpid = kvmppc_alloc_lpid();
+ if (lpid 0)
+ return lpid;
+
+ kvm-arch.lpid_pool[i] = lpid;
+ }
Wouldn't it be simpler to halve the size of the lpid pool that the
allocator sees, and just OR in the high bit based on the low bit of the
cpu number?
-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: e500mc: Add support for single threaded vcpus on e6500 core
Am 12.08.2014 um 01:36 schrieb Scott Wood scottw...@freescale.com:
On Wed, 2014-08-06 at 19:33 +0300, Mihai Caraman wrote:
@@ -390,19 +400,30 @@ static void kvmppc_core_vcpu_free_e500mc(struct
kvm_vcpu *vcpu)
static int kvmppc_core_init_vm_e500mc(struct kvm *kvm)
{
-int lpid;
+int i, lpid;
-lpid = kvmppc_alloc_lpid();
-if (lpid 0)
-return lpid;
+/* The lpid pool supports only 2 entries now */
+if (threads_per_core 2)
+return -ENOMEM;
+
+/* Each VM allocates one LPID per HW thread index */
+for (i = 0; i threads_per_core; i++) {
+lpid = kvmppc_alloc_lpid();
+if (lpid 0)
+return lpid;
+
+kvm-arch.lpid_pool[i] = lpid;
+}
Wouldn't it be simpler to halve the size of the lpid pool that the
allocator sees, and just OR in the high bit based on the low bit of the
cpu number?
Heh, I wrote the same and then removed the section from my reply again. It
wouldn't really make that much of a difference if you think it through
completely.
But yes, it certainly would be quite a bit more natural. I'm ok either way.
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: e500mc: Add support for single threaded vcpus on e6500 core
On Tue, 2014-08-12 at 01:53 +0200, Alexander Graf wrote:
Am 12.08.2014 um 01:36 schrieb Scott Wood scottw...@freescale.com:
On Wed, 2014-08-06 at 19:33 +0300, Mihai Caraman wrote:
@@ -390,19 +400,30 @@ static void kvmppc_core_vcpu_free_e500mc(struct
kvm_vcpu *vcpu)
static int kvmppc_core_init_vm_e500mc(struct kvm *kvm)
{
-int lpid;
+int i, lpid;
-lpid = kvmppc_alloc_lpid();
-if (lpid 0)
-return lpid;
+/* The lpid pool supports only 2 entries now */
+if (threads_per_core 2)
+return -ENOMEM;
+
+/* Each VM allocates one LPID per HW thread index */
+for (i = 0; i threads_per_core; i++) {
+lpid = kvmppc_alloc_lpid();
+if (lpid 0)
+return lpid;
+
+kvm-arch.lpid_pool[i] = lpid;
+}
Wouldn't it be simpler to halve the size of the lpid pool that the
allocator sees, and just OR in the high bit based on the low bit of the
cpu number?
Heh, I wrote the same and then removed the section from my reply again. It
wouldn't really make that much of a difference if you think it through
completely.
But yes, it certainly would be quite a bit more natural. I'm ok either way.
It's not a huge difference, but it would at least get rid of some of the
ifdeffing in the headers. It'd also be nicer when debugging to have the
LPIDs correlated.
-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 7/7 v3] KVM: PPC: BOOKE: Emulate debug registers and exception
On Wed, 2014-08-06 at 12:08 +0530, Bharat Bhushan wrote: @@ -1249,6 +1284,7 @@ int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu) setup_timer(vcpu-arch.wdt_timer, kvmppc_watchdog_func, (unsigned long)vcpu); + kvmppc_clear_dbsr(); return 0; This could use a comment for why we're doing this. Also, I'm a bit uneasy about clearing the whole DBSR here, where we haven't yet switched the debug registers to guest context. It shouldn't actually matter except for deferred debug exceptions which are not actually useful (in fact e6500 removed support for them), but still... -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
