* [PATCH 1/6 v5] powerpc: remove unnecessary line continuations
From: Bharat Bhushan @ 2013-06-26 5:42 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf, scottwood, tiejun.chen, benh, linuxppc-dev,
linux-kernel, mikey
Cc: Bharat Bhushan
In-Reply-To: <1372225346-5029-1-git-send-email-Bharat.Bhushan@freescale.com>
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
arch/powerpc/kernel/process.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ceb4e7b..639a8de 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -325,7 +325,7 @@ static void set_debug_reg_defaults(struct thread_struct *thread)
/*
* Force User/Supervisor bits to b11 (user-only MSR[PR]=1)
*/
- thread->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | \
+ thread->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US |
DBCR1_IAC3US | DBCR1_IAC4US;
/*
* Force Data Address Compare User/Supervisor bits to be User-only
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/6 v5] powerpc: move debug registers in a structure
From: Bharat Bhushan @ 2013-06-26 5:42 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf, scottwood, tiejun.chen, benh, linuxppc-dev,
linux-kernel, mikey
Cc: Bharat Bhushan
In-Reply-To: <1372225346-5029-1-git-send-email-Bharat.Bhushan@freescale.com>
This way we can use same data type struct with KVM and
also help in using other debug related function.
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
arch/powerpc/include/asm/processor.h | 38 +++++----
arch/powerpc/include/asm/reg_booke.h | 8 +-
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/process.c | 42 +++++-----
arch/powerpc/kernel/ptrace.c | 154 +++++++++++++++++-----------------
arch/powerpc/kernel/signal_32.c | 6 +-
arch/powerpc/kernel/traps.c | 35 ++++----
7 files changed, 146 insertions(+), 139 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index d7e67ca..5b8a7f1 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -147,22 +147,7 @@ typedef struct {
#define TS_FPR(i) fpr[i][TS_FPROFFSET]
#define TS_TRANS_FPR(i) transact_fpr[i][TS_FPROFFSET]
-struct thread_struct {
- unsigned long ksp; /* Kernel stack pointer */
- unsigned long ksp_limit; /* if ksp <= ksp_limit stack overflow */
-
-#ifdef CONFIG_PPC64
- unsigned long ksp_vsid;
-#endif
- struct pt_regs *regs; /* Pointer to saved register state */
- mm_segment_t fs; /* for get_fs() validation */
-#ifdef CONFIG_BOOKE
- /* BookE base exception scratch space; align on cacheline */
- unsigned long normsave[8] ____cacheline_aligned;
-#endif
-#ifdef CONFIG_PPC32
- void *pgdir; /* root of page-table tree */
-#endif
+struct debug_reg {
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
/*
* The following help to manage the use of Debug Control Registers
@@ -199,6 +184,27 @@ struct thread_struct {
unsigned long dvc2;
#endif
#endif
+};
+
+struct thread_struct {
+ unsigned long ksp; /* Kernel stack pointer */
+ unsigned long ksp_limit; /* if ksp <= ksp_limit stack overflow */
+
+#ifdef CONFIG_PPC64
+ unsigned long ksp_vsid;
+#endif
+ struct pt_regs *regs; /* Pointer to saved register state */
+ mm_segment_t fs; /* for get_fs() validation */
+#ifdef CONFIG_BOOKE
+ /* BookE base exception scratch space; align on cacheline */
+ unsigned long normsave[8] ____cacheline_aligned;
+#endif
+#ifdef CONFIG_PPC32
+ void *pgdir; /* root of page-table tree */
+#endif
+ /* Debug Registers */
+ struct debug_reg debug;
+
/* FP and VSX 0-31 register set */
double fpr[32][TS_FPRWIDTH];
struct {
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index b417de3..455dc89 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -381,7 +381,7 @@
#define DBCR0_IA34T 0x00004000 /* Instr Addr 3-4 range Toggle */
#define DBCR0_FT 0x00000001 /* Freeze Timers on debug event */
-#define dbcr_iac_range(task) ((task)->thread.dbcr0)
+#define dbcr_iac_range(task) ((task)->thread.debug.dbcr0)
#define DBCR_IAC12I DBCR0_IA12 /* Range Inclusive */
#define DBCR_IAC12X (DBCR0_IA12 | DBCR0_IA12X) /* Range Exclusive */
#define DBCR_IAC12MODE (DBCR0_IA12 | DBCR0_IA12X) /* IAC 1-2 Mode Bits */
@@ -395,7 +395,7 @@
#define DBCR1_DAC1W 0x20000000 /* DAC1 Write Debug Event */
#define DBCR1_DAC2W 0x10000000 /* DAC2 Write Debug Event */
-#define dbcr_dac(task) ((task)->thread.dbcr1)
+#define dbcr_dac(task) ((task)->thread.debug.dbcr1)
#define DBCR_DAC1R DBCR1_DAC1R
#define DBCR_DAC1W DBCR1_DAC1W
#define DBCR_DAC2R DBCR1_DAC2R
@@ -441,7 +441,7 @@
#define DBCR0_CRET 0x00000020 /* Critical Return Debug Event */
#define DBCR0_FT 0x00000001 /* Freeze Timers on debug event */
-#define dbcr_dac(task) ((task)->thread.dbcr0)
+#define dbcr_dac(task) ((task)->thread.debug.dbcr0)
#define DBCR_DAC1R DBCR0_DAC1R
#define DBCR_DAC1W DBCR0_DAC1W
#define DBCR_DAC2R DBCR0_DAC2R
@@ -475,7 +475,7 @@
#define DBCR1_IAC34MX 0x000000C0 /* Instr Addr 3-4 range eXclusive */
#define DBCR1_IAC34AT 0x00000001 /* Instr Addr 3-4 range Toggle */
-#define dbcr_iac_range(task) ((task)->thread.dbcr1)
+#define dbcr_iac_range(task) ((task)->thread.debug.dbcr1)
#define DBCR_IAC12I DBCR1_IAC12M /* Range Inclusive */
#define DBCR_IAC12X DBCR1_IAC12MX /* Range Exclusive */
#define DBCR_IAC12MODE DBCR1_IAC12MX /* IAC 1-2 Mode Bits */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index b51a97c..c241c60 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -106,7 +106,7 @@ int main(void)
#else /* CONFIG_PPC64 */
DEFINE(PGDIR, offsetof(struct thread_struct, pgdir));
#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
- DEFINE(THREAD_DBCR0, offsetof(struct thread_struct, dbcr0));
+ DEFINE(THREAD_DBCR0, offsetof(struct thread_struct, debug.dbcr0));
#endif
#ifdef CONFIG_SPE
DEFINE(THREAD_EVR0, offsetof(struct thread_struct, evr[0]));
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 639a8de..01ff496 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -312,49 +312,49 @@ static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk);
*/
static void set_debug_reg_defaults(struct thread_struct *thread)
{
- thread->iac1 = thread->iac2 = 0;
+ thread->debug.iac1 = thread->debug.iac2 = 0;
#if CONFIG_PPC_ADV_DEBUG_IACS > 2
- thread->iac3 = thread->iac4 = 0;
+ thread->debug.iac3 = thread->debug.iac4 = 0;
#endif
- thread->dac1 = thread->dac2 = 0;
+ thread->debug.dac1 = thread->debug.dac2 = 0;
#if CONFIG_PPC_ADV_DEBUG_DVCS > 0
- thread->dvc1 = thread->dvc2 = 0;
+ thread->debug.dvc1 = thread->debug.dvc2 = 0;
#endif
- thread->dbcr0 = 0;
+ thread->debug.dbcr0 = 0;
#ifdef CONFIG_BOOKE
/*
* Force User/Supervisor bits to b11 (user-only MSR[PR]=1)
*/
- thread->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US |
+ thread->debug.dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US |
DBCR1_IAC3US | DBCR1_IAC4US;
/*
* Force Data Address Compare User/Supervisor bits to be User-only
* (0b11 MSR[PR]=1) and set all other bits in DBCR2 register to be 0.
*/
- thread->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US;
+ thread->debug.dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US;
#else
- thread->dbcr1 = 0;
+ thread->debug.dbcr1 = 0;
#endif
}
static void prime_debug_regs(struct thread_struct *thread)
{
- mtspr(SPRN_IAC1, thread->iac1);
- mtspr(SPRN_IAC2, thread->iac2);
+ mtspr(SPRN_IAC1, thread->debug.iac1);
+ mtspr(SPRN_IAC2, thread->debug.iac2);
#if CONFIG_PPC_ADV_DEBUG_IACS > 2
- mtspr(SPRN_IAC3, thread->iac3);
- mtspr(SPRN_IAC4, thread->iac4);
+ mtspr(SPRN_IAC3, thread->debug.iac3);
+ mtspr(SPRN_IAC4, thread->debug.iac4);
#endif
- mtspr(SPRN_DAC1, thread->dac1);
- mtspr(SPRN_DAC2, thread->dac2);
+ mtspr(SPRN_DAC1, thread->debug.dac1);
+ mtspr(SPRN_DAC2, thread->debug.dac2);
#if CONFIG_PPC_ADV_DEBUG_DVCS > 0
- mtspr(SPRN_DVC1, thread->dvc1);
- mtspr(SPRN_DVC2, thread->dvc2);
+ mtspr(SPRN_DVC1, thread->debug.dvc1);
+ mtspr(SPRN_DVC2, thread->debug.dvc2);
#endif
- mtspr(SPRN_DBCR0, thread->dbcr0);
- mtspr(SPRN_DBCR1, thread->dbcr1);
+ mtspr(SPRN_DBCR0, thread->debug.dbcr0);
+ mtspr(SPRN_DBCR1, thread->debug.dbcr1);
#ifdef CONFIG_BOOKE
- mtspr(SPRN_DBCR2, thread->dbcr2);
+ mtspr(SPRN_DBCR2, thread->debug.dbcr2);
#endif
}
/*
@@ -364,8 +364,8 @@ static void prime_debug_regs(struct thread_struct *thread)
*/
static void switch_booke_debug_regs(struct thread_struct *new_thread)
{
- if ((current->thread.dbcr0 & DBCR0_IDM)
- || (new_thread->dbcr0 & DBCR0_IDM))
+ if ((current->thread.debug.dbcr0 & DBCR0_IDM)
+ || (new_thread->debug.dbcr0 & DBCR0_IDM))
prime_debug_regs(new_thread);
}
#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 3b14d32..80b902f 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -853,8 +853,8 @@ void user_enable_single_step(struct task_struct *task)
if (regs != NULL) {
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- task->thread.dbcr0 &= ~DBCR0_BT;
- task->thread.dbcr0 |= DBCR0_IDM | DBCR0_IC;
+ task->thread.debug.dbcr0 &= ~DBCR0_BT;
+ task->thread.debug.dbcr0 |= DBCR0_IDM | DBCR0_IC;
regs->msr |= MSR_DE;
#else
regs->msr &= ~MSR_BE;
@@ -870,8 +870,8 @@ void user_enable_block_step(struct task_struct *task)
if (regs != NULL) {
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- task->thread.dbcr0 &= ~DBCR0_IC;
- task->thread.dbcr0 = DBCR0_IDM | DBCR0_BT;
+ task->thread.debug.dbcr0 &= ~DBCR0_IC;
+ task->thread.debug.dbcr0 = DBCR0_IDM | DBCR0_BT;
regs->msr |= MSR_DE;
#else
regs->msr &= ~MSR_SE;
@@ -893,16 +893,16 @@ void user_disable_single_step(struct task_struct *task)
* And, after doing so, if all debug flags are off, turn
* off DBCR0(IDM) and MSR(DE) .... Torez
*/
- task->thread.dbcr0 &= ~DBCR0_IC;
+ task->thread.debug.dbcr0 &= ~DBCR0_IC;
/*
* Test to see if any of the DBCR_ACTIVE_EVENTS bits are set.
*/
- if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
- task->thread.dbcr1)) {
+ if (!DBCR_ACTIVE_EVENTS(task->thread.debug.dbcr0,
+ task->thread.debug.dbcr1)) {
/*
* All debug events were off.....
*/
- task->thread.dbcr0 &= ~DBCR0_IDM;
+ task->thread.debug.dbcr0 &= ~DBCR0_IDM;
regs->msr &= ~MSR_DE;
}
#else
@@ -1030,14 +1030,14 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
*/
/* DAC's hold the whole address without any mode flags */
- task->thread.dac1 = data & ~0x3UL;
+ task->thread.debug.dac1 = data & ~0x3UL;
- if (task->thread.dac1 == 0) {
+ if (task->thread.debug.dac1 == 0) {
dbcr_dac(task) &= ~(DBCR_DAC1R | DBCR_DAC1W);
- if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
- task->thread.dbcr1)) {
+ if (!DBCR_ACTIVE_EVENTS(task->thread.debug.dbcr0,
+ task->thread.debug.dbcr1)) {
task->thread.regs->msr &= ~MSR_DE;
- task->thread.dbcr0 &= ~DBCR0_IDM;
+ task->thread.debug.dbcr0 &= ~DBCR0_IDM;
}
return 0;
}
@@ -1049,7 +1049,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
/* Set the Internal Debugging flag (IDM bit 1) for the DBCR0
register */
- task->thread.dbcr0 |= DBCR0_IDM;
+ task->thread.debug.dbcr0 |= DBCR0_IDM;
/* Check for write and read flags and set DBCR0
accordingly */
@@ -1079,10 +1079,10 @@ static long set_instruction_bp(struct task_struct *child,
struct ppc_hw_breakpoint *bp_info)
{
int slot;
- int slot1_in_use = ((child->thread.dbcr0 & DBCR0_IAC1) != 0);
- int slot2_in_use = ((child->thread.dbcr0 & DBCR0_IAC2) != 0);
- int slot3_in_use = ((child->thread.dbcr0 & DBCR0_IAC3) != 0);
- int slot4_in_use = ((child->thread.dbcr0 & DBCR0_IAC4) != 0);
+ int slot1_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC1) != 0);
+ int slot2_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC2) != 0);
+ int slot3_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC3) != 0);
+ int slot4_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC4) != 0);
if (dbcr_iac_range(child) & DBCR_IAC12MODE)
slot2_in_use = 1;
@@ -1101,9 +1101,9 @@ static long set_instruction_bp(struct task_struct *child,
/* We need a pair of IAC regsisters */
if ((!slot1_in_use) && (!slot2_in_use)) {
slot = 1;
- child->thread.iac1 = bp_info->addr;
- child->thread.iac2 = bp_info->addr2;
- child->thread.dbcr0 |= DBCR0_IAC1;
+ child->thread.debug.iac1 = bp_info->addr;
+ child->thread.debug.iac2 = bp_info->addr2;
+ child->thread.debug.dbcr0 |= DBCR0_IAC1;
if (bp_info->addr_mode ==
PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
dbcr_iac_range(child) |= DBCR_IAC12X;
@@ -1112,9 +1112,9 @@ static long set_instruction_bp(struct task_struct *child,
#if CONFIG_PPC_ADV_DEBUG_IACS > 2
} else if ((!slot3_in_use) && (!slot4_in_use)) {
slot = 3;
- child->thread.iac3 = bp_info->addr;
- child->thread.iac4 = bp_info->addr2;
- child->thread.dbcr0 |= DBCR0_IAC3;
+ child->thread.debug.iac3 = bp_info->addr;
+ child->thread.debug.iac4 = bp_info->addr2;
+ child->thread.debug.dbcr0 |= DBCR0_IAC3;
if (bp_info->addr_mode ==
PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
dbcr_iac_range(child) |= DBCR_IAC34X;
@@ -1134,30 +1134,30 @@ static long set_instruction_bp(struct task_struct *child,
*/
if (slot2_in_use || (slot3_in_use == slot4_in_use)) {
slot = 1;
- child->thread.iac1 = bp_info->addr;
- child->thread.dbcr0 |= DBCR0_IAC1;
+ child->thread.debug.iac1 = bp_info->addr;
+ child->thread.debug.dbcr0 |= DBCR0_IAC1;
goto out;
}
}
if (!slot2_in_use) {
slot = 2;
- child->thread.iac2 = bp_info->addr;
- child->thread.dbcr0 |= DBCR0_IAC2;
+ child->thread.debug.iac2 = bp_info->addr;
+ child->thread.debug.dbcr0 |= DBCR0_IAC2;
#if CONFIG_PPC_ADV_DEBUG_IACS > 2
} else if (!slot3_in_use) {
slot = 3;
- child->thread.iac3 = bp_info->addr;
- child->thread.dbcr0 |= DBCR0_IAC3;
+ child->thread.debug.iac3 = bp_info->addr;
+ child->thread.debug.dbcr0 |= DBCR0_IAC3;
} else if (!slot4_in_use) {
slot = 4;
- child->thread.iac4 = bp_info->addr;
- child->thread.dbcr0 |= DBCR0_IAC4;
+ child->thread.debug.iac4 = bp_info->addr;
+ child->thread.debug.dbcr0 |= DBCR0_IAC4;
#endif
} else
return -ENOSPC;
}
out:
- child->thread.dbcr0 |= DBCR0_IDM;
+ child->thread.debug.dbcr0 |= DBCR0_IDM;
child->thread.regs->msr |= MSR_DE;
return slot;
@@ -1167,49 +1167,49 @@ static int del_instruction_bp(struct task_struct *child, int slot)
{
switch (slot) {
case 1:
- if ((child->thread.dbcr0 & DBCR0_IAC1) == 0)
+ if ((child->thread.debug.dbcr0 & DBCR0_IAC1) == 0)
return -ENOENT;
if (dbcr_iac_range(child) & DBCR_IAC12MODE) {
/* address range - clear slots 1 & 2 */
- child->thread.iac2 = 0;
+ child->thread.debug.iac2 = 0;
dbcr_iac_range(child) &= ~DBCR_IAC12MODE;
}
- child->thread.iac1 = 0;
- child->thread.dbcr0 &= ~DBCR0_IAC1;
+ child->thread.debug.iac1 = 0;
+ child->thread.debug.dbcr0 &= ~DBCR0_IAC1;
break;
case 2:
- if ((child->thread.dbcr0 & DBCR0_IAC2) == 0)
+ if ((child->thread.debug.dbcr0 & DBCR0_IAC2) == 0)
return -ENOENT;
if (dbcr_iac_range(child) & DBCR_IAC12MODE)
/* used in a range */
return -EINVAL;
- child->thread.iac2 = 0;
- child->thread.dbcr0 &= ~DBCR0_IAC2;
+ child->thread.debug.iac2 = 0;
+ child->thread.debug.dbcr0 &= ~DBCR0_IAC2;
break;
#if CONFIG_PPC_ADV_DEBUG_IACS > 2
case 3:
- if ((child->thread.dbcr0 & DBCR0_IAC3) == 0)
+ if ((child->thread.debug.dbcr0 & DBCR0_IAC3) == 0)
return -ENOENT;
if (dbcr_iac_range(child) & DBCR_IAC34MODE) {
/* address range - clear slots 3 & 4 */
- child->thread.iac4 = 0;
+ child->thread.debug.iac4 = 0;
dbcr_iac_range(child) &= ~DBCR_IAC34MODE;
}
- child->thread.iac3 = 0;
- child->thread.dbcr0 &= ~DBCR0_IAC3;
+ child->thread.debug.iac3 = 0;
+ child->thread.debug.dbcr0 &= ~DBCR0_IAC3;
break;
case 4:
- if ((child->thread.dbcr0 & DBCR0_IAC4) == 0)
+ if ((child->thread.debug.dbcr0 & DBCR0_IAC4) == 0)
return -ENOENT;
if (dbcr_iac_range(child) & DBCR_IAC34MODE)
/* Used in a range */
return -EINVAL;
- child->thread.iac4 = 0;
- child->thread.dbcr0 &= ~DBCR0_IAC4;
+ child->thread.debug.iac4 = 0;
+ child->thread.debug.dbcr0 &= ~DBCR0_IAC4;
break;
#endif
default:
@@ -1239,18 +1239,18 @@ static int set_dac(struct task_struct *child, struct ppc_hw_breakpoint *bp_info)
dbcr_dac(child) |= DBCR_DAC1R;
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
dbcr_dac(child) |= DBCR_DAC1W;
- child->thread.dac1 = (unsigned long)bp_info->addr;
+ child->thread.debug.dac1 = (unsigned long)bp_info->addr;
#if CONFIG_PPC_ADV_DEBUG_DVCS > 0
if (byte_enable) {
- child->thread.dvc1 =
+ child->thread.debug.dvc1 =
(unsigned long)bp_info->condition_value;
- child->thread.dbcr2 |=
+ child->thread.debug.dbcr2 |=
((byte_enable << DBCR2_DVC1BE_SHIFT) |
(condition_mode << DBCR2_DVC1M_SHIFT));
}
#endif
#ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
- } else if (child->thread.dbcr2 & DBCR2_DAC12MODE) {
+ } else if (child->thread.debug.dbcr2 & DBCR2_DAC12MODE) {
/* Both dac1 and dac2 are part of a range */
return -ENOSPC;
#endif
@@ -1260,19 +1260,19 @@ static int set_dac(struct task_struct *child, struct ppc_hw_breakpoint *bp_info)
dbcr_dac(child) |= DBCR_DAC2R;
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
dbcr_dac(child) |= DBCR_DAC2W;
- child->thread.dac2 = (unsigned long)bp_info->addr;
+ child->thread.debug.dac2 = (unsigned long)bp_info->addr;
#if CONFIG_PPC_ADV_DEBUG_DVCS > 0
if (byte_enable) {
- child->thread.dvc2 =
+ child->thread.debug.dvc2 =
(unsigned long)bp_info->condition_value;
- child->thread.dbcr2 |=
+ child->thread.debug.dbcr2 |=
((byte_enable << DBCR2_DVC2BE_SHIFT) |
(condition_mode << DBCR2_DVC2M_SHIFT));
}
#endif
} else
return -ENOSPC;
- child->thread.dbcr0 |= DBCR0_IDM;
+ child->thread.debug.dbcr0 |= DBCR0_IDM;
child->thread.regs->msr |= MSR_DE;
return slot + 4;
@@ -1284,32 +1284,32 @@ static int del_dac(struct task_struct *child, int slot)
if ((dbcr_dac(child) & (DBCR_DAC1R | DBCR_DAC1W)) == 0)
return -ENOENT;
- child->thread.dac1 = 0;
+ child->thread.debug.dac1 = 0;
dbcr_dac(child) &= ~(DBCR_DAC1R | DBCR_DAC1W);
#ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
- if (child->thread.dbcr2 & DBCR2_DAC12MODE) {
- child->thread.dac2 = 0;
- child->thread.dbcr2 &= ~DBCR2_DAC12MODE;
+ if (child->thread.debug.dbcr2 & DBCR2_DAC12MODE) {
+ child->thread.debug.dac2 = 0;
+ child->thread.debug.dbcr2 &= ~DBCR2_DAC12MODE;
}
- child->thread.dbcr2 &= ~(DBCR2_DVC1M | DBCR2_DVC1BE);
+ child->thread.debug.dbcr2 &= ~(DBCR2_DVC1M | DBCR2_DVC1BE);
#endif
#if CONFIG_PPC_ADV_DEBUG_DVCS > 0
- child->thread.dvc1 = 0;
+ child->thread.debug.dvc1 = 0;
#endif
} else if (slot == 2) {
if ((dbcr_dac(child) & (DBCR_DAC2R | DBCR_DAC2W)) == 0)
return -ENOENT;
#ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
- if (child->thread.dbcr2 & DBCR2_DAC12MODE)
+ if (child->thread.debug.dbcr2 & DBCR2_DAC12MODE)
/* Part of a range */
return -EINVAL;
- child->thread.dbcr2 &= ~(DBCR2_DVC2M | DBCR2_DVC2BE);
+ child->thread.debug.dbcr2 &= ~(DBCR2_DVC2M | DBCR2_DVC2BE);
#endif
#if CONFIG_PPC_ADV_DEBUG_DVCS > 0
- child->thread.dvc2 = 0;
+ child->thread.debug.dvc2 = 0;
#endif
- child->thread.dac2 = 0;
+ child->thread.debug.dac2 = 0;
dbcr_dac(child) &= ~(DBCR_DAC2R | DBCR_DAC2W);
} else
return -EINVAL;
@@ -1351,22 +1351,22 @@ static int set_dac_range(struct task_struct *child,
return -EIO;
}
- if (child->thread.dbcr0 &
+ if (child->thread.debug.dbcr0 &
(DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W))
return -ENOSPC;
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
- child->thread.dbcr0 |= (DBCR0_DAC1R | DBCR0_IDM);
+ child->thread.debug.dbcr0 |= (DBCR0_DAC1R | DBCR0_IDM);
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
- child->thread.dbcr0 |= (DBCR0_DAC1W | DBCR0_IDM);
- child->thread.dac1 = bp_info->addr;
- child->thread.dac2 = bp_info->addr2;
+ child->thread.debug.dbcr0 |= (DBCR0_DAC1W | DBCR0_IDM);
+ child->thread.debug.dac1 = bp_info->addr;
+ child->thread.debug.dac2 = bp_info->addr2;
if (mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE)
- child->thread.dbcr2 |= DBCR2_DAC12M;
+ child->thread.debug.dbcr2 |= DBCR2_DAC12M;
else if (mode == PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
- child->thread.dbcr2 |= DBCR2_DAC12MX;
+ child->thread.debug.dbcr2 |= DBCR2_DAC12MX;
else /* PPC_BREAKPOINT_MODE_MASK */
- child->thread.dbcr2 |= DBCR2_DAC12MM;
+ child->thread.debug.dbcr2 |= DBCR2_DAC12MM;
child->thread.regs->msr |= MSR_DE;
return 5;
@@ -1504,9 +1504,9 @@ static long ppc_del_hwdebug(struct task_struct *child, long data)
rc = del_dac(child, (int)data - 4);
if (!rc) {
- if (!DBCR_ACTIVE_EVENTS(child->thread.dbcr0,
- child->thread.dbcr1)) {
- child->thread.dbcr0 &= ~DBCR0_IDM;
+ if (!DBCR_ACTIVE_EVENTS(child->thread.debug.dbcr0,
+ child->thread.debug.dbcr1)) {
+ child->thread.debug.dbcr0 &= ~DBCR0_IDM;
child->thread.regs->msr &= ~MSR_DE;
}
}
@@ -1688,7 +1688,7 @@ long arch_ptrace(struct task_struct *child, long request,
if (addr > 0)
break;
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- ret = put_user(child->thread.dac1, datalp);
+ ret = put_user(child->thread.debug.dac1, datalp);
#else
dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
(child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 95068bf..3e46b2c 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1294,7 +1294,7 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
unsigned char tmp;
unsigned long new_msr = regs->msr;
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- unsigned long new_dbcr0 = current->thread.dbcr0;
+ unsigned long new_dbcr0 = current->thread.debug.dbcr0;
#endif
for (i=0; i<ndbg; i++) {
@@ -1309,7 +1309,7 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
} else {
new_dbcr0 &= ~DBCR0_IC;
if (!DBCR_ACTIVE_EVENTS(new_dbcr0,
- current->thread.dbcr1)) {
+ current->thread.debug.dbcr1)) {
new_msr &= ~MSR_DE;
new_dbcr0 &= ~DBCR0_IDM;
}
@@ -1344,7 +1344,7 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
the user is really doing something wrong. */
regs->msr = new_msr;
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- current->thread.dbcr0 = new_dbcr0;
+ current->thread.debug.dbcr0 = new_dbcr0;
#endif
if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx))
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 83efa2f..86d5bd8 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -350,8 +350,8 @@ static inline int check_io_access(struct pt_regs *regs)
#define REASON_TRAP ESR_PTR
/* single-step stuff */
-#define single_stepping(regs) (current->thread.dbcr0 & DBCR0_IC)
-#define clear_single_step(regs) (current->thread.dbcr0 &= ~DBCR0_IC)
+#define single_stepping(regs) (current->thread.debug.dbcr0 & DBCR0_IC)
+#define clear_single_step(regs) (current->thread.debug.dbcr0 &= ~DBCR0_IC)
#else
/* On non-4xx, the reason for the machine check or program
@@ -1389,7 +1389,7 @@ static void handle_debug(struct pt_regs *regs, unsigned long debug_status)
if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
dbcr_dac(current) &= ~(DBCR_DAC1R | DBCR_DAC1W);
#ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
- current->thread.dbcr2 &= ~DBCR2_DAC12MODE;
+ current->thread.debug.dbcr2 &= ~DBCR2_DAC12MODE;
#endif
do_send_trap(regs, mfspr(SPRN_DAC1), debug_status, TRAP_HWBKPT,
5);
@@ -1400,24 +1400,24 @@ static void handle_debug(struct pt_regs *regs, unsigned long debug_status)
6);
changed |= 0x01;
} else if (debug_status & DBSR_IAC1) {
- current->thread.dbcr0 &= ~DBCR0_IAC1;
+ current->thread.debug.dbcr0 &= ~DBCR0_IAC1;
dbcr_iac_range(current) &= ~DBCR_IAC12MODE;
do_send_trap(regs, mfspr(SPRN_IAC1), debug_status, TRAP_HWBKPT,
1);
changed |= 0x01;
} else if (debug_status & DBSR_IAC2) {
- current->thread.dbcr0 &= ~DBCR0_IAC2;
+ current->thread.debug.dbcr0 &= ~DBCR0_IAC2;
do_send_trap(regs, mfspr(SPRN_IAC2), debug_status, TRAP_HWBKPT,
2);
changed |= 0x01;
} else if (debug_status & DBSR_IAC3) {
- current->thread.dbcr0 &= ~DBCR0_IAC3;
+ current->thread.debug.dbcr0 &= ~DBCR0_IAC3;
dbcr_iac_range(current) &= ~DBCR_IAC34MODE;
do_send_trap(regs, mfspr(SPRN_IAC3), debug_status, TRAP_HWBKPT,
3);
changed |= 0x01;
} else if (debug_status & DBSR_IAC4) {
- current->thread.dbcr0 &= ~DBCR0_IAC4;
+ current->thread.debug.dbcr0 &= ~DBCR0_IAC4;
do_send_trap(regs, mfspr(SPRN_IAC4), debug_status, TRAP_HWBKPT,
4);
changed |= 0x01;
@@ -1427,19 +1427,20 @@ static void handle_debug(struct pt_regs *regs, unsigned long debug_status)
* Check all other debug flags and see if that bit needs to be turned
* back on or not.
*/
- if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0, current->thread.dbcr1))
+ if (DBCR_ACTIVE_EVENTS(current->thread.debug.dbcr0,
+ current->thread.debug.dbcr1))
regs->msr |= MSR_DE;
else
/* Make sure the IDM flag is off */
- current->thread.dbcr0 &= ~DBCR0_IDM;
+ current->thread.debug.dbcr0 &= ~DBCR0_IDM;
if (changed & 0x01)
- mtspr(SPRN_DBCR0, current->thread.dbcr0);
+ mtspr(SPRN_DBCR0, current->thread.debug.dbcr0);
}
void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status)
{
- current->thread.dbsr = debug_status;
+ current->thread.debug.dbsr = debug_status;
/* Hack alert: On BookE, Branch Taken stops on the branch itself, while
* on server, it stops on the target of the branch. In order to simulate
@@ -1456,8 +1457,8 @@ void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status)
/* Do the single step trick only when coming from userspace */
if (user_mode(regs)) {
- current->thread.dbcr0 &= ~DBCR0_BT;
- current->thread.dbcr0 |= DBCR0_IDM | DBCR0_IC;
+ current->thread.debug.dbcr0 &= ~DBCR0_BT;
+ current->thread.debug.dbcr0 |= DBCR0_IDM | DBCR0_IC;
regs->msr |= MSR_DE;
return;
}
@@ -1485,13 +1486,13 @@ void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status)
return;
if (user_mode(regs)) {
- current->thread.dbcr0 &= ~DBCR0_IC;
- if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0,
- current->thread.dbcr1))
+ current->thread.debug.dbcr0 &= ~DBCR0_IC;
+ if (DBCR_ACTIVE_EVENTS(current->thread.debug.dbcr0,
+ current->thread.debug.dbcr1))
regs->msr |= MSR_DE;
else
/* Make sure the IDM bit is off */
- current->thread.dbcr0 &= ~DBCR0_IDM;
+ current->thread.debug.dbcr0 &= ~DBCR0_IDM;
}
_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/6 v5] powerpc: export debug registers save function for KVM
From: Bharat Bhushan @ 2013-06-26 5:42 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf, scottwood, tiejun.chen, benh, linuxppc-dev,
linux-kernel, mikey
Cc: Bharat Bhushan
In-Reply-To: <1372225346-5029-1-git-send-email-Bharat.Bhushan@freescale.com>
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.bhushan@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 01ff496..da586aa 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_GPL(switch_booke_debug_regs);
#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
#ifndef CONFIG_HAVE_HW_BREAKPOINT
static void set_debug_reg_defaults(struct thread_struct *thread)
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction
From: Bharat Bhushan @ 2013-06-26 5:42 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf, scottwood, tiejun.chen, benh, linuxppc-dev,
linux-kernel, mikey
Cc: Bharat Bhushan
In-Reply-To: <1372225346-5029-1-git-send-email-Bharat.Bhushan@freescale.com>
"ehpriv" instruction is used for setting software breakpoints
by user space. This patch adds support to exit to user space
with "run->debug" have relevant information.
As this is the first point we are using run->debug, also defined
the run->debug structure.
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
arch/powerpc/include/asm/disassemble.h | 4 ++++
arch/powerpc/include/uapi/asm/kvm.h | 21 +++++++++++++++++----
arch/powerpc/kvm/e500_emulate.c | 27 +++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
index 9b198d1..856f8de 100644
--- a/arch/powerpc/include/asm/disassemble.h
+++ b/arch/powerpc/include/asm/disassemble.h
@@ -77,4 +77,8 @@ static inline unsigned int get_d(u32 inst)
return inst & 0xffff;
}
+static inline unsigned int get_oc(u32 inst)
+{
+ return (inst >> 11) & 0x7fff;
+}
#endif /* __ASM_PPC_DISASSEMBLE_H__ */
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 0fb1a6e..ded0607 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -269,7 +269,24 @@ struct kvm_fpu {
__u64 fpr[32];
};
+/*
+ * Defines for h/w breakpoint, watchpoint (read, write or both) and
+ * software breakpoint.
+ * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status"
+ * for KVM_DEBUG_EXIT.
+ */
+#define KVMPPC_DEBUG_NONE 0x0
+#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
+#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
+#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
struct kvm_debug_exit_arch {
+ __u64 address;
+ /*
+ * exiting to userspace because of h/w breakpoint, watchpoint
+ * (read, write or both) and software breakpoint.
+ */
+ __u32 status;
+ __u32 reserved;
};
/* for KVM_SET_GUEST_DEBUG */
@@ -281,10 +298,6 @@ struct kvm_guest_debug_arch {
* Type denotes h/w breakpoint, read watchpoint, write
* watchpoint or watchpoint (both read and write).
*/
-#define KVMPPC_DEBUG_NONE 0x0
-#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
-#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
-#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
__u32 type;
__u32 reserved;
} bp[16];
diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
index b10a012..dab9d07 100644
--- a/arch/powerpc/kvm/e500_emulate.c
+++ b/arch/powerpc/kvm/e500_emulate.c
@@ -26,6 +26,8 @@
#define XOP_TLBRE 946
#define XOP_TLBWE 978
#define XOP_TLBILX 18
+#define XOP_EHPRIV 270
+#define EHPRIV_OC_DEBUG 0
#ifdef CONFIG_KVM_E500MC
static int dbell2prio(ulong param)
@@ -82,6 +84,26 @@ static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu, int rb)
}
#endif
+static int kvmppc_e500_emul_ehpriv(struct kvm_run *run, struct kvm_vcpu *vcpu,
+ unsigned int inst, int *advance)
+{
+ int emulated = EMULATE_DONE;
+
+ switch (get_oc(inst)) {
+ case EHPRIV_OC_DEBUG:
+ run->exit_reason = KVM_EXIT_DEBUG;
+ run->debug.arch.address = vcpu->arch.pc;
+ run->debug.arch.status = 0;
+ kvmppc_account_exit(vcpu, DEBUG_EXITS);
+ emulated = EMULATE_EXIT_USER;
+ *advance = 0;
+ break;
+ default:
+ emulated = EMULATE_FAIL;
+ }
+ return emulated;
+}
+
int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
unsigned int inst, int *advance)
{
@@ -130,6 +152,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
emulated = kvmppc_e500_emul_tlbivax(vcpu, ea);
break;
+ case XOP_EHPRIV:
+ emulated = kvmppc_e500_emul_ehpriv(run, vcpu, inst,
+ advance);
+ break;
+
default:
emulated = EMULATE_FAIL;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 5/6 v5] KVM: PPC: Using "struct debug_reg"
From: Bharat Bhushan @ 2013-06-26 5:42 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf, scottwood, tiejun.chen, benh, linuxppc-dev,
linux-kernel, mikey
Cc: Bharat Bhushan
In-Reply-To: <1372225346-5029-1-git-send-email-Bharat.Bhushan@freescale.com>
For KVM also use the "struct debug_reg" defined in asm/processor.h
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
arch/powerpc/include/asm/kvm_host.h | 13 +------------
arch/powerpc/kvm/booke.c | 34 ++++++++++++++++++++++++----------
2 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index af326cd..838a577 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -381,17 +381,6 @@ struct kvmppc_slb {
#define KVMPPC_EPR_USER 1 /* exit to userspace to fill EPR */
#define KVMPPC_EPR_KERNEL 2 /* in-kernel irqchip */
-struct kvmppc_booke_debug_reg {
- u32 dbcr0;
- u32 dbcr1;
- u32 dbcr2;
-#ifdef CONFIG_KVM_E500MC
- u32 dbcr4;
-#endif
- u64 iac[KVMPPC_BOOKE_MAX_IAC];
- u64 dac[KVMPPC_BOOKE_MAX_DAC];
-};
-
#define KVMPPC_IRQ_DEFAULT 0
#define KVMPPC_IRQ_MPIC 1
#define KVMPPC_IRQ_XICS 2
@@ -535,7 +524,7 @@ struct kvm_vcpu_arch {
u32 eptcfg;
u32 epr;
u32 crit_save;
- struct kvmppc_booke_debug_reg dbg_reg;
+ struct debug_reg dbg_reg;
#endif
gpa_t paddr_accessed;
gva_t vaddr_accessed;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 62d4ece..3e9fc1d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1424,7 +1424,6 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
int r = 0;
union kvmppc_one_reg val;
int size;
- long int i;
size = one_reg_size(reg->id);
if (size > sizeof(val))
@@ -1432,16 +1431,24 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
switch (reg->id) {
case KVM_REG_PPC_IAC1:
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.iac1);
+ break;
case KVM_REG_PPC_IAC2:
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.iac2);
+ break;
+#if CONFIG_PPC_ADV_DEBUG_IACS > 2
case KVM_REG_PPC_IAC3:
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.iac3);
+ break;
case KVM_REG_PPC_IAC4:
- i = reg->id - KVM_REG_PPC_IAC1;
- val = get_reg_val(reg->id, vcpu->arch.dbg_reg.iac[i]);
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.iac4);
break;
+#endif
case KVM_REG_PPC_DAC1:
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.dac1);
+ break;
case KVM_REG_PPC_DAC2:
- i = reg->id - KVM_REG_PPC_DAC1;
- val = get_reg_val(reg->id, vcpu->arch.dbg_reg.dac[i]);
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.dac2);
break;
case KVM_REG_PPC_EPR: {
u32 epr = get_guest_epr(vcpu);
@@ -1481,7 +1488,6 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
int r = 0;
union kvmppc_one_reg val;
int size;
- long int i;
size = one_reg_size(reg->id);
if (size > sizeof(val))
@@ -1492,16 +1498,24 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
switch (reg->id) {
case KVM_REG_PPC_IAC1:
+ vcpu->arch.dbg_reg.iac1 = set_reg_val(reg->id, val);
+ break;
case KVM_REG_PPC_IAC2:
+ vcpu->arch.dbg_reg.iac2 = set_reg_val(reg->id, val);
+ break;
+#if CONFIG_PPC_ADV_DEBUG_IACS > 2
case KVM_REG_PPC_IAC3:
+ vcpu->arch.dbg_reg.iac3 = set_reg_val(reg->id, val);
+ break;
case KVM_REG_PPC_IAC4:
- i = reg->id - KVM_REG_PPC_IAC1;
- vcpu->arch.dbg_reg.iac[i] = set_reg_val(reg->id, val);
+ vcpu->arch.dbg_reg.iac4 = set_reg_val(reg->id, val);
break;
+#endif
case KVM_REG_PPC_DAC1:
+ vcpu->arch.dbg_reg.dac1 = set_reg_val(reg->id, val);
+ break;
case KVM_REG_PPC_DAC2:
- i = reg->id - KVM_REG_PPC_DAC1;
- vcpu->arch.dbg_reg.dac[i] = set_reg_val(reg->id, val);
+ vcpu->arch.dbg_reg.dac2 = set_reg_val(reg->id, val);
break;
case KVM_REG_PPC_EPR: {
u32 new_epr = set_reg_val(reg->id, val);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 6/6 v5] KVM: PPC: Add userspace debug stub support
From: Bharat Bhushan @ 2013-06-26 5:42 UTC (permalink / raw)
To: kvm-ppc, kvm, agraf, scottwood, tiejun.chen, benh, linuxppc-dev,
linux-kernel, mikey
Cc: Bharat Bhushan
In-Reply-To: <1372225346-5029-1-git-send-email-Bharat.Bhushan@freescale.com>
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.bhushan@freescale.com>
---
arch/powerpc/include/asm/kvm_host.h | 3 +
arch/powerpc/include/uapi/asm/kvm.h | 1 +
arch/powerpc/kvm/booke.c | 239 ++++++++++++++++++++++++++++++++---
arch/powerpc/kvm/booke.h | 5 +
4 files changed, 230 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 838a577..aeb490d 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;
+ /* 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 3e9fc1d..8cd8d41 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 thread_struct thread;
#ifdef CONFIG_PPC_FPU
unsigned int fpscr;
int fpexc_mode;
@@ -698,12 +723,21 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
kvmppc_load_guest_fp(vcpu);
#endif
+ /* Switch to guest debug context */
+ thread.debug = vcpu->arch.shadow_dbg_reg;
+ switch_booke_debug_regs(&thread);
+ thread.debug = current->thread.debug;
+ current->thread.debug = vcpu->arch.shadow_dbg_reg;
ret = __kvmppc_vcpu_run(kvm_run, vcpu);
/* No need for kvm_guest_exit. It's done in handle_exit.
We also get here with interrupts enabled. */
+ /* Switch back to user space debug context */
+ switch_booke_debug_regs(&thread);
+ current->thread.debug = thread.debug;
+
#ifdef CONFIG_PPC_FPU
kvmppc_save_guest_fp(vcpu);
@@ -759,6 +793,30 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
}
}
+static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu)
+{
+ struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
+ u32 dbsr = vcpu->arch.dbsr;
+
+ run->debug.arch.status = 0;
+ run->debug.arch.address = vcpu->arch.pc;
+
+ if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) {
+ run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT;
+ } else {
+ if (dbsr & (DBSR_DAC1W | DBSR_DAC2W))
+ run->debug.arch.status |= KVMPPC_DEBUG_WATCH_WRITE;
+ else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R))
+ run->debug.arch.status |= KVMPPC_DEBUG_WATCH_READ;
+ if (dbsr & (DBSR_DAC1R | DBSR_DAC1W))
+ run->debug.arch.address = dbg_reg->dac1;
+ else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W))
+ run->debug.arch.address = dbg_reg->dac2;
+ }
+
+ return RESUME_HOST;
+}
+
static void kvmppc_fill_pt_regs(struct pt_regs *regs)
{
ulong r1, ip, msr, lr;
@@ -819,6 +877,11 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
case BOOKE_INTERRUPT_CRITICAL:
unknown_exception(®s);
break;
+ case BOOKE_INTERRUPT_DEBUG:
+ /* Save DBSR before preemption is enabled */
+ vcpu->arch.dbsr = mfspr(SPRN_DBSR);
+ kvmppc_clear_dbsr();
+ break;
}
}
@@ -1118,18 +1181,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
}
case BOOKE_INTERRUPT_DEBUG: {
- u32 dbsr;
-
- vcpu->arch.pc = mfspr(SPRN_CSRR0);
-
- /* clear IAC events in DBSR register */
- dbsr = mfspr(SPRN_DBSR);
- dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4;
- mtspr(SPRN_DBSR, dbsr);
-
- run->exit_reason = KVM_EXIT_DEBUG;
+ r = kvmppc_handle_debug(run, vcpu);
+ if (r == RESUME_HOST)
+ run->exit_reason = KVM_EXIT_DEBUG;
kvmppc_account_exit(vcpu, DEBUG_EXITS);
- r = RESUME_HOST;
break;
}
@@ -1180,7 +1235,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
kvmppc_set_msr(vcpu, 0);
#ifndef CONFIG_KVM_BOOKE_HV
- vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS;
+ vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS;
vcpu->arch.shadow_pid = 1;
vcpu->arch.shared->msr = 0;
#endif
@@ -1557,12 +1612,6 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
return r;
}
-int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
- struct kvm_guest_debug *dbg)
-{
- return -EINVAL;
-}
-
int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
{
return -ENOTSUPP;
@@ -1668,6 +1717,157 @@ void kvmppc_decrementer_func(unsigned long data)
kvmppc_set_tsr_bits(vcpu, TSR_DIS);
}
+static int kvmppc_booke_add_breakpoint(struct debug_reg *dbg_reg,
+ uint64_t addr, int index)
+{
+ switch (index) {
+ case 0:
+ dbg_reg->dbcr0 |= DBCR0_IAC1;
+ dbg_reg->iac1 = addr;
+ break;
+ case 1:
+ dbg_reg->dbcr0 |= DBCR0_IAC2;
+ dbg_reg->iac2 = addr;
+ break;
+#if CONFIG_PPC_ADV_DEBUG_IACS > 2
+ case 2:
+ dbg_reg->dbcr0 |= DBCR0_IAC3;
+ dbg_reg->iac3 = addr;
+ break;
+ case 3:
+ dbg_reg->dbcr0 |= DBCR0_IAC4;
+ dbg_reg->iac4 = addr;
+ break;
+#endif
+ default:
+ return -EINVAL;
+ }
+
+ dbg_reg->dbcr0 |= DBCR0_IDM;
+ return 0;
+}
+
+static int kvmppc_booke_add_watchpoint(struct debug_reg *dbg_reg, uint64_t addr,
+ int type, int index)
+{
+ switch (index) {
+ case 0:
+ if (type & KVMPPC_DEBUG_WATCH_READ)
+ dbg_reg->dbcr0 |= DBCR0_DAC1R;
+ if (type & KVMPPC_DEBUG_WATCH_WRITE)
+ dbg_reg->dbcr0 |= DBCR0_DAC1W;
+ dbg_reg->dac1 = addr;
+ break;
+ case 1:
+ if (type & KVMPPC_DEBUG_WATCH_READ)
+ dbg_reg->dbcr0 |= DBCR0_DAC2R;
+ if (type & KVMPPC_DEBUG_WATCH_WRITE)
+ dbg_reg->dbcr0 |= DBCR0_DAC2W;
+ dbg_reg->dac2 = addr;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ dbg_reg->dbcr0 |= DBCR0_IDM;
+ return 0;
+}
+void kvm_guest_protect_msr(struct kvm_vcpu *vcpu, ulong prot_bitmap, bool set)
+{
+ /* XXX: Add similar MSR protection for BookE-PR */
+#ifdef CONFIG_KVM_BOOKE_HV
+ BUG_ON(prot_bitmap & ~(MSRP_UCLEP | MSRP_DEP | MSRP_PMMP));
+ if (set) {
+ if (prot_bitmap & MSR_UCLE)
+ vcpu->arch.shadow_msrp |= MSRP_UCLEP;
+ if (prot_bitmap & MSR_DE)
+ vcpu->arch.shadow_msrp |= MSRP_DEP;
+ if (prot_bitmap & MSR_PMM)
+ vcpu->arch.shadow_msrp |= MSRP_PMMP;
+ } else {
+ if (prot_bitmap & MSR_UCLE)
+ vcpu->arch.shadow_msrp &= ~MSRP_UCLEP;
+ if (prot_bitmap & MSR_DE)
+ vcpu->arch.shadow_msrp &= ~MSRP_DEP;
+ if (prot_bitmap & MSR_PMM)
+ vcpu->arch.shadow_msrp &= ~MSRP_PMMP;
+ }
+#endif
+}
+
+int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
+ struct kvm_guest_debug *dbg)
+{
+ struct debug_reg *dbg_reg;
+ int n, b = 0, w = 0;
+
+ if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
+ vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
+ vcpu->guest_debug = 0;
+ kvm_guest_protect_msr(vcpu, MSR_DE, false);
+ return 0;
+ }
+
+ kvm_guest_protect_msr(vcpu, MSR_DE, true);
+ vcpu->guest_debug = dbg->control;
+ vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
+ /* Set DBCR0_EDM in guest visible DBCR0 register. */
+ vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM;
+
+ if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
+ vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC;
+
+ /* Code below handles only HW breakpoints */
+ dbg_reg = &(vcpu->arch.shadow_dbg_reg);
+
+#ifdef CONFIG_KVM_BOOKE_HV
+ /*
+ * On BookE-HV (e500mc) the guest is always executed with MSR.GS=1
+ * DBCR1 and DBCR2 are set to trigger debug events when MSR.PR is 0
+ */
+ dbg_reg->dbcr1 = 0;
+ dbg_reg->dbcr2 = 0;
+#else
+ /*
+ * On BookE-PR (e500v2) the guest is always executed with MSR.PR=1
+ * We set DBCR1 and DBCR2 to only trigger debug events when MSR.PR
+ * is set.
+ */
+ dbg_reg->dbcr1 = DBCR1_IAC1US | DBCR1_IAC2US | DBCR1_IAC3US |
+ DBCR1_IAC4US;
+ dbg_reg->dbcr2 = DBCR2_DAC1US | DBCR2_DAC2US;
+#endif
+
+ if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP))
+ return 0;
+
+ for (n = 0; n < (KVMPPC_BOOKE_IAC_NUM + KVMPPC_BOOKE_DAC_NUM); n++) {
+ uint64_t addr = dbg->arch.bp[n].addr;
+ uint32_t type = dbg->arch.bp[n].type;
+
+ if (type == KVMPPC_DEBUG_NONE)
+ continue;
+
+ if (type & !(KVMPPC_DEBUG_WATCH_READ |
+ KVMPPC_DEBUG_WATCH_WRITE |
+ KVMPPC_DEBUG_BREAKPOINT))
+ return -EINVAL;
+
+ if (type & KVMPPC_DEBUG_BREAKPOINT) {
+ /* Setting H/W breakpoint */
+ if (kvmppc_booke_add_breakpoint(dbg_reg, addr, b++))
+ return -EINVAL;
+ } else {
+ /* Setting H/W watchpoint */
+ if (kvmppc_booke_add_watchpoint(dbg_reg, addr,
+ type, w++))
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
void kvmppc_booke_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
{
vcpu->cpu = smp_processor_id();
@@ -1678,6 +1878,9 @@ void kvmppc_booke_vcpu_put(struct kvm_vcpu *vcpu)
{
current->thread.kvm_vcpu = NULL;
vcpu->cpu = -1;
+
+ /* Clear pending debug event in DBSR */
+ kvmppc_clear_dbsr();
}
int __init kvmppc_booke_init(void)
diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
index 5fd1ba6..a1ff67d 100644
--- a/arch/powerpc/kvm/booke.h
+++ b/arch/powerpc/kvm/booke.h
@@ -129,4 +129,9 @@ static inline void kvmppc_save_guest_fp(struct kvm_vcpu *vcpu)
giveup_fpu(current);
#endif
}
+
+static inline void kvmppc_clear_dbsr(void)
+{
+ mtspr(SPRN_DBSR, mfspr(SPRN_DBSR));
+}
#endif /* __KVM_BOOKE_H__ */
--
1.7.0.4
^ permalink raw reply related
* [PATCH] powerpc/vfio: Add missing locks for take/release TCE table ownership
From: Alexey Kardashevskiy @ 2013-06-26 5:39 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
Anton Blanchard, David Gibson
VFIO IOMMU driver for sPAPR TCE locks the whole DMA window by setting
ones to iommu_table.it_map. However this was not protected by the locks
which other clients of iommu_table use.
The patch fixes this.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/kernel/iommu.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index b20ff17..9f34742 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1076,25 +1076,41 @@ EXPORT_SYMBOL_GPL(iommu_put_tce_user_mode);
int iommu_take_ownership(struct iommu_table *tbl)
{
unsigned long sz = (tbl->it_size + 7) >> 3;
+ int i, ret = 0;
+
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_lock(&tbl->pools[i].lock);
+ spin_lock(&tbl->large_pool.lock);
if (tbl->it_offset == 0)
clear_bit(0, tbl->it_map);
if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
pr_err("iommu_tce: it_map is not empty");
- return -EBUSY;
+ ret = -EBUSY;
+ goto unlock_exit;
}
memset(tbl->it_map, 0xff, sz);
iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
- return 0;
+unlock_exit:
+ spin_unlock(&tbl->large_pool.lock);
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_unlock(&tbl->pools[i].lock);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(iommu_take_ownership);
void iommu_release_ownership(struct iommu_table *tbl)
{
unsigned long sz = (tbl->it_size + 7) >> 3;
+ int i;
+
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_lock(&tbl->pools[i].lock);
+ spin_lock(&tbl->large_pool.lock);
iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
memset(tbl->it_map, 0, sz);
@@ -1102,6 +1118,11 @@ void iommu_release_ownership(struct iommu_table *tbl)
/* Restore bit#0 set by iommu_init_table() */
if (tbl->it_offset == 0)
set_bit(0, tbl->it_map);
+
+ spin_unlock(&tbl->large_pool.lock);
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_unlock(&tbl->pools[i].lock);
+
}
EXPORT_SYMBOL_GPL(iommu_release_ownership);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] powerpc/vfio: Add missing locks for take/release TCE table ownership
From: Benjamin Herrenschmidt @ 2013-06-26 5:49 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Paul Mackerras, linuxppc-dev, Anton Blanchard, linux-kernel,
David Gibson
In-Reply-To: <1372225159-30187-1-git-send-email-aik@ozlabs.ru>
On Wed, 2013-06-26 at 15:39 +1000, Alexey Kardashevskiy wrote:
> VFIO IOMMU driver for sPAPR TCE locks the whole DMA window by setting
> ones to iommu_table.it_map. However this was not protected by the locks
> which other clients of iommu_table use.
>
> The patch fixes this.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/kernel/iommu.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index b20ff17..9f34742 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1076,25 +1076,41 @@ EXPORT_SYMBOL_GPL(iommu_put_tce_user_mode);
> int iommu_take_ownership(struct iommu_table *tbl)
> {
> unsigned long sz = (tbl->it_size + 7) >> 3;
> + int i, ret = 0;
> +
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_lock(&tbl->pools[i].lock);
> + spin_lock(&tbl->large_pool.lock);
IOMMUs can be used at interrupt time so the above will trigger lockdep
warnings unless you also bracket the whole function in
local_irq_save/restore.
Now you *shouldn't* be racing since afaik you only take the iommu
ownership after no more drivers are hooked up, but better safe than
sorry.
Cheers,
Ben.
> if (tbl->it_offset == 0)
> clear_bit(0, tbl->it_map);
>
> if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
> pr_err("iommu_tce: it_map is not empty");
> - return -EBUSY;
> + ret = -EBUSY;
> + goto unlock_exit;
> }
>
> memset(tbl->it_map, 0xff, sz);
> iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
>
> - return 0;
> +unlock_exit:
> + spin_unlock(&tbl->large_pool.lock);
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_unlock(&tbl->pools[i].lock);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(iommu_take_ownership);
>
> void iommu_release_ownership(struct iommu_table *tbl)
> {
> unsigned long sz = (tbl->it_size + 7) >> 3;
> + int i;
> +
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_lock(&tbl->pools[i].lock);
> + spin_lock(&tbl->large_pool.lock);
>
> iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
> memset(tbl->it_map, 0, sz);
> @@ -1102,6 +1118,11 @@ void iommu_release_ownership(struct iommu_table *tbl)
> /* Restore bit#0 set by iommu_init_table() */
> if (tbl->it_offset == 0)
> set_bit(0, tbl->it_map);
> +
> + spin_unlock(&tbl->large_pool.lock);
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_unlock(&tbl->pools[i].lock);
> +
> }
> EXPORT_SYMBOL_GPL(iommu_release_ownership);
>
^ permalink raw reply
* [PATCH v2] powerpc/vfio: Add missing locks for take/release TCE table ownership
From: Alexey Kardashevskiy @ 2013-06-26 6:21 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
Anton Blanchard, David Gibson
VFIO IOMMU driver for sPAPR TCE locks the whole DMA window by setting
ones to iommu_table.it_map. However this was not protected by the locks
which other clients of iommu_table use.
The patch fixes this.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
v1->v2:
* Fixed a potential warning from lockdep.
---
arch/powerpc/kernel/iommu.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index b20ff17..5c1fc89 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1076,25 +1076,42 @@ EXPORT_SYMBOL_GPL(iommu_put_tce_user_mode);
int iommu_take_ownership(struct iommu_table *tbl)
{
unsigned long sz = (tbl->it_size + 7) >> 3;
+ unsigned long i, flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&tbl->large_pool.lock, flags);
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_lock(&tbl->pools[i].lock);
if (tbl->it_offset == 0)
clear_bit(0, tbl->it_map);
if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
pr_err("iommu_tce: it_map is not empty");
- return -EBUSY;
+ ret = -EBUSY;
+ goto unlock_exit;
}
memset(tbl->it_map, 0xff, sz);
iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
- return 0;
+unlock_exit:
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_unlock(&tbl->pools[i].lock);
+ spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(iommu_take_ownership);
void iommu_release_ownership(struct iommu_table *tbl)
{
unsigned long sz = (tbl->it_size + 7) >> 3;
+ unsigned long i, flags;
+
+ spin_lock_irqsave(&tbl->large_pool.lock, flags);
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_lock(&tbl->pools[i].lock);
iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
memset(tbl->it_map, 0, sz);
@@ -1102,6 +1119,10 @@ void iommu_release_ownership(struct iommu_table *tbl)
/* Restore bit#0 set by iommu_init_table() */
if (tbl->it_offset == 0)
set_bit(0, tbl->it_map);
+
+ for (i = 0; i < tbl->nr_pools; i++)
+ spin_unlock(&tbl->pools[i].lock);
+ spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
}
EXPORT_SYMBOL_GPL(iommu_release_ownership);
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH 3/3 v16] iommu/fsl: Freescale PAMU driver and iommu implementation.
From: Sethi Varun-B16395 @ 2013-06-26 6:24 UTC (permalink / raw)
To: Alex Williamson
Cc: Wood Scott-B07421, Timur Tabi, joro@8bytes.org,
linux-kernel@vger.kernel.org, Yoder Stuart-B08248,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1372136211.30572.377.camel@ul30vt.home>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQWxleCBXaWxsaWFtc29u
IFttYWlsdG86YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb21dDQo+IFNlbnQ6IFR1ZXNkYXksIEp1
bmUgMjUsIDIwMTMgMTA6MjcgQU0NCj4gVG86IFNldGhpIFZhcnVuLUIxNjM5NQ0KPiBDYzogam9y
b0A4Ynl0ZXMub3JnOyBpb21tdUBsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZzsgbGludXhwcGMt
DQo+IGRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOw0K
PiBiZW5oQGtlcm5lbC5jcmFzaGluZy5vcmc7IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IFlv
ZGVyIFN0dWFydC1CMDgyNDg7DQo+IFdvb2QgU2NvdHQtQjA3NDIxOyBUaW11ciBUYWJpDQo+IFN1
YmplY3Q6IFJlOiBbUEFUQ0ggMy8zIHYxNl0gaW9tbXUvZnNsOiBGcmVlc2NhbGUgUEFNVSBkcml2
ZXIgYW5kIGlvbW11DQo+IGltcGxlbWVudGF0aW9uLg0KPiANCj4gT24gVGh1LCAyMDEzLTA2LTIw
IGF0IDIxOjMxICswNTMwLCBWYXJ1biBTZXRoaSB3cm90ZToNCj4gDQo+ID4gKyNkZWZpbmUgUkVR
X0FDU19GTEFHUwkoUENJX0FDU19TViB8IFBDSV9BQ1NfUlIgfCBQQ0lfQUNTX0NSIHwNCj4gUENJ
X0FDU19VRikNCj4gPiArDQo+ID4gK3N0YXRpYyBzdHJ1Y3QgaW9tbXVfZ3JvdXAgKmdldF9kZXZp
Y2VfaW9tbXVfZ3JvdXAoc3RydWN0IGRldmljZSAqZGV2KQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3Qg
aW9tbXVfZ3JvdXAgKmdyb3VwOw0KPiA+ICsNCj4gPiArCWdyb3VwID0gaW9tbXVfZ3JvdXBfZ2V0
KGRldik7DQo+ID4gKwlpZiAoIWdyb3VwKQ0KPiA+ICsJCWdyb3VwID0gaW9tbXVfZ3JvdXBfYWxs
b2MoKTsNCj4gPiArDQo+ID4gKwlyZXR1cm4gZ3JvdXA7DQo+ID4gK30NCj4gPiArDQo+IFtzbmlw
XQ0KPiA+ICsNCj4gDQo+IFRoaXMgcmVhbGx5IGdldHMgcGFyZW50IG9yIHBlZXIsIHJpZ2h0Pw0K
PiANCj4gPiArc3RhdGljIHN0cnVjdCBpb21tdV9ncm91cCAqZ2V0X3BlZXJfcGNpX2RldmljZV9n
cm91cChzdHJ1Y3QgcGNpX2Rldg0KPiA+ICsqcGRldikgew0KPiA+ICsJc3RydWN0IGlvbW11X2dy
b3VwICpncm91cCA9IE5VTEw7DQo+ID4gKw0KPiA+ICsJLyogY2hlY2sgaWYgdGhpcyBpcyB0aGUg
Zmlyc3QgZGV2aWNlIG9uIHRoZSBidXMqLw0KPiA+ICsJaWYgKHBkZXYtPmJ1c19saXN0Lm5leHQg
PT0gcGRldi0+YnVzX2xpc3QucHJldikgew0KPiANCj4gSXQncyBhIGxpc3RfaGVhZCwgdXNlIGxp
c3QgZnVuY3Rpb25zLiAgVGhlIGxpc3QgaW1wbGVtZW50YXRpb24gc2hvdWxkIGJlDQo+IHRyZWF0
ZWQgYXMgb3BhcXVlLg0KPiANCj4gaWYgKGxpc3RfaXNfc2luZ3VsYXIoJnBkZXYtPmJ1c19saXN0
KSkNCj4gDQo+ID4gKwkJc3RydWN0IHBjaV9idXMgKmJ1cyA9IHBkZXYtPmJ1cy0+cGFyZW50Ow0K
PiA+ICsJCS8qIFRyYXZlcmVzZSB0aGUgcGFyZW50IGJ1cyBsaXN0IHRvIGdldA0KPiA+ICsJCSAq
IHBkZXYgJiBkZXYgZm9yIHRoZSBzaWJsaW5nIGRldmljZS4NCj4gPiArCQkgKi8NCj4gPiArCQl3
aGlsZSAoYnVzKSB7DQo+ID4gKwkJCWlmICghbGlzdF9lbXB0eSgmYnVzLT5kZXZpY2VzKSkgew0K
PiA+ICsJCQkJcGRldiA9IGNvbnRhaW5lcl9vZihidXMtPmRldmljZXMubmV4dCwNCj4gPiArCQkJ
CQkgICAgICAgICAgICBzdHJ1Y3QgcGNpX2RldiwgYnVzX2xpc3QpOw0KPiANCj4gcGRldiA9IGxp
c3RfZmlyc3RfZW50cnkoJmJ1cy0+ZGV2aWNlcywgc3RydWN0IHBjaV9kZXYsIGJ1c19saXN0KTsN
Cj4gDQo+ID4gKwkJCQlncm91cCA9IGlvbW11X2dyb3VwX2dldCgmcGRldi0+ZGV2KTsNCj4gPiAr
CQkJCWJyZWFrOw0KPiA+ICsJCQl9IGVsc2UNCj4gPiArCQkJCWJ1cyA9IGJ1cy0+cGFyZW50Ow0K
PiANCj4gSXMgdGhpcyBldmVyIHJlYWNoZWQ/ICBEb24ndCB5b3UgYWx3YXlzIGhhdmUgYnVzLT5z
ZWxmPw0KPiANCltTZXRoaSBWYXJ1bi1CMTYzOTVdIE5vdCBzdXJlIEkgdW5kZXJzdGFuZC4gVHJ5
aW5nIHRvIGdldCB0aGUgZ3JvdXAgaW5mb3JtYXRpb24gZnJvbSB0aGUgcGFyZW50IGJ1cywgaWYg
dGhlcmUgYXJlIG5vIHNpYmxpbmcgZGV2aWNlcyBvbiB0aGUgY3VycmVudCBidXMuDQoNCj4gPiAr
CQl9DQo+ID4gKwl9IGVsc2Ugew0KPiA+ICsJCS8qDQo+ID4gKwkJICogR2V0IHRoZSBwZGV2ICYg
ZGV2IGZvciB0aGUgc2libGluZyBkZXZpY2UNCj4gPiArCQkgKi8NCj4gPiArCQlwZGV2ID0gY29u
dGFpbmVyX29mKHBkZXYtPmJ1c19saXN0LnByZXYsDQo+ID4gKwkJCQkgICAgc3RydWN0IHBjaV9k
ZXYsIGJ1c19saXN0KTsNCj4gDQo+IEhvdyBkbyB5b3Uga25vdyBpZiB5b3UncmUgYXQgdGhlIGhl
YWQgb3IgdGFpbCBvZiB0aGUgbGlzdD8NCj4gDQo+IHN0cnVjdCBwY2lfZGV2ICp0bXA7DQo+IGxp
c3RfZm9yX2VhY2hfZW50cnkodG1wLCAmcGRldi0+YnVzX2xpc3QsIGJ1c19saXN0KSB7DQo+IAlp
ZiAodG1wID09IHBkZXYpDQo+IAkJY29udGludWU7DQo+IA0KPiAJZ3JvdXAgPSBpb21tdV9ncm91
cF9nZXQoJnRtcC0+ZGV2KTsNCj4gCWJyZWFrOw0KPiB9DQo+IA0KPiA+ICsJCWdyb3VwID0gaW9t
bXVfZ3JvdXBfZ2V0KCZwZGV2LT5kZXYpOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCXJldHVybiBn
cm91cDsNCj4gPiArfQ0KPiA+ICsNCj4gPiArc3RhdGljIHN0cnVjdCBpb21tdV9ncm91cCAqZ2V0
X3BjaV9kZXZpY2VfZ3JvdXAoc3RydWN0IHBjaV9kZXYgKnBkZXYpDQo+ID4gK3sNCj4gPiArCXN0
cnVjdCBpb21tdV9ncm91cCAqZ3JvdXAgPSBOVUxMOw0KPiA+ICsJc3RydWN0IHBjaV9kZXYgKmJy
aWRnZSwgKmRtYV9wZGV2ID0gTlVMTDsNCj4gPiArCXN0cnVjdCBwY2lfY29udHJvbGxlciAqcGNp
X2N0bDsNCj4gPiArCWJvb2wgcGNpX2VuZHB0X3BhcnRpb25pbmc7DQo+ID4gKw0KPiA+ICsJcGNp
X2N0bCA9IHBjaV9idXNfdG9faG9zdChwZGV2LT5idXMpOw0KPiA+ICsJcGNpX2VuZHB0X3BhcnRp
b25pbmcgPSBjaGVja19wY2lfY3RsX2VuZHB0X3BhcnQocGNpX2N0bCk7DQo+ID4gKwkvKiBXZSBj
YW4gcGFydGl0aW9uIFBDSWUgZGV2aWNlcyBzbyBhc3NpZ24gZGV2aWNlIGdyb3VwIHRvIHRoZQ0K
PiBkZXZpY2UgKi8NCj4gPiArCWlmIChwY2lfZW5kcHRfcGFydGlvbmluZykgew0KPiA+ICsJCWJy
aWRnZSA9IHBjaV9maW5kX3Vwc3RyZWFtX3BjaWVfYnJpZGdlKHBkZXYpOw0KPiA+ICsJCWlmIChi
cmlkZ2UpIHsNCj4gPiArCQkJaWYgKHBjaV9pc19wY2llKGJyaWRnZSkpDQo+ID4gKwkJCQlkbWFf
cGRldiA9IHBjaV9nZXRfZG9tYWluX2J1c19hbmRfc2xvdCgNCj4gPiArCQkJCQkJcGNpX2RvbWFp
bl9ucihwZGV2LT5idXMpLA0KPiA+ICsJCQkJCQlicmlkZ2UtPnN1Ym9yZGluYXRlLT5udW1iZXIs
IDApOw0KPiA+ICsJCQlpZiAoIWRtYV9wZGV2KQ0KPiA+ICsJCQkJZG1hX3BkZXYgPSBwY2lfZGV2
X2dldChicmlkZ2UpOw0KPiA+ICsJCX0gZWxzZQ0KPiA+ICsJCQlkbWFfcGRldiA9IHBjaV9kZXZf
Z2V0KHBkZXYpOw0KPiA+ICsNCj4gPiArCQkvKiBBY2NvdW50IGZvciBxdWlya2VkIGRldmljZXMg
Ki8NCj4gPiArCQlzd2FwX3BjaV9yZWYoJmRtYV9wZGV2LCBwY2lfZ2V0X2RtYV9zb3VyY2UoZG1h
X3BkZXYpKTsNCj4gPiArDQo+ID4gKwkJLyoNCj4gPiArCQkgKiBJZiBpdCdzIGEgbXVsdGlmdW5j
dGlvbiBkZXZpY2UgdGhhdCBkb2VzIG5vdCBzdXBwb3J0IG91cg0KPiA+ICsJCSAqIHJlcXVpcmVk
IEFDUyBmbGFncywgYWRkIHRvIHRoZSBzYW1lIGdyb3VwIGFzIGZ1bmN0aW9uIDAuDQo+ID4gKwkJ
ICovDQo+IA0KPiBTZWUgYzE0ZDI2OTAgaW4gSm9lcmcncyBuZXh0IHRyZWUsIHVzaW5nIGZ1bmN0
aW9uIDAgd2FzIGEgcG9vcg0KPiBhc3N1bXB0aW9uLg0KW1NldGhpIFZhcnVuLUIxNjM5NV0gb2su
DQoNCj4gDQo+ID4gKwkJaWYgKGRtYV9wZGV2LT5tdWx0aWZ1bmN0aW9uICYmDQo+ID4gKwkJICAg
ICFwY2lfYWNzX2VuYWJsZWQoZG1hX3BkZXYsIFJFUV9BQ1NfRkxBR1MpKQ0KPiA+ICsJCQlzd2Fw
X3BjaV9yZWYoJmRtYV9wZGV2LA0KPiA+ICsJCQkJICAgICBwY2lfZ2V0X3Nsb3QoZG1hX3BkZXYt
PmJ1cywNCj4gPiArCQkJCQkJICBQQ0lfREVWRk4oUENJX1NMT1QoZG1hX3BkZXYtDQo+ID5kZXZm
biksDQo+ID4gKwkJCQkJCSAgMCkpKTsNCj4gPiArDQo+ID4gKwkJZ3JvdXAgPSBnZXRfZGV2aWNl
X2lvbW11X2dyb3VwKCZwZGV2LT5kZXYpOw0KPiA+ICsJCXBjaV9kZXZfcHV0KHBkZXYpOw0KPiAN
Cj4gV2hhdCB3YXMgdGhlIHBvaW50IG9mIGFsbCB0aGUgYWJvdmUgaWYgd2UgdXNlIHBkZXYgaGVy
ZSBpbnN0ZWFkIG9mDQo+IGRtYV9wZGV2PyAgV3JvbmcgZGV2aWNlIGFuZCBicm9rZW4gcmVmZXJl
bmNlIGNvdW50aW5nLg0KW1NldGhpIFZhcnVuLUIxNjM5NV0gV2lsbCBmaXggdGhpcw0KDQogIFRo
aXMgYWxzbyBpc24ndA0KPiB0ZXN0aW5nIEFDUyBhbGwgdGhlIHdheSB1cCB0byB0aGUgcm9vdCBj
b21wbGV4IG9yIGNvbnRyb2xsZXIuDQpbU2V0aGkgVmFydW4tQjE2Mzk1XSBJbiBvdXIgY2FzZSB0
aGUgSU9NTVUgY2FuIGRpZmZlcmVudGlhdGUgdHJhbnNhY3Rpb25zIGJhc2VkIG9uIHRoZSBMSU9E
Ti4gVGhlIFBDSWUgY29udHJvbGxlciBjYW4gZ2VuZXJhdGUgYSB1bmlxdWUgTElPRE4gYmFzZWQg
b24gdGhlIGJ1cyxkZXZpY2UsZnVuY3Rpb24gbnVtYmVyLiBJIGJlbGlldmUgdGhpcyB3b3VsZCBl
dmVuIGJlIHRydWUgZm9yIGRldmljZXMgY29ubmVjdGVkIHRvIGEgUENJZSBicmlkZ2UgKGFuZCBu
b3Qgb24gdGhlIHJvb3QgYnVzKS4gU28sIGRvIHdlIHN0aWxsIG5lZWQgdG8gY2hlY2sgZm9yIEFD
UyB1cCB0byB0aGUgcm9vdCBub2RlPw0KDQotVmFydW4NCg==
^ permalink raw reply
* Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction
From: tiejun.chen @ 2013-06-26 6:54 UTC (permalink / raw)
To: Bharat Bhushan
Cc: mikey, kvm, agraf, kvm-ppc, linux-kernel, Bharat Bhushan,
scottwood, linuxppc-dev
In-Reply-To: <1372225346-5029-5-git-send-email-Bharat.Bhushan@freescale.com>
On 06/26/2013 01:42 PM, Bharat Bhushan wrote:
> "ehpriv" instruction is used for setting software breakpoints
> by user space. This patch adds support to exit to user space
> with "run->debug" have relevant information.
>
> As this is the first point we are using run->debug, also defined
> the run->debug structure.
>
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> arch/powerpc/include/asm/disassemble.h | 4 ++++
> arch/powerpc/include/uapi/asm/kvm.h | 21 +++++++++++++++++----
> arch/powerpc/kvm/e500_emulate.c | 27 +++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/disassemble.h b/arch/powerpc/include/asm/disassemble.h
> index 9b198d1..856f8de 100644
> --- a/arch/powerpc/include/asm/disassemble.h
> +++ b/arch/powerpc/include/asm/disassemble.h
> @@ -77,4 +77,8 @@ static inline unsigned int get_d(u32 inst)
> return inst & 0xffff;
> }
>
> +static inline unsigned int get_oc(u32 inst)
> +{
> + return (inst >> 11) & 0x7fff;
> +}
> #endif /* __ASM_PPC_DISASSEMBLE_H__ */
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 0fb1a6e..ded0607 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -269,7 +269,24 @@ struct kvm_fpu {
> __u64 fpr[32];
> };
>
> +/*
> + * Defines for h/w breakpoint, watchpoint (read, write or both) and
> + * software breakpoint.
> + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status"
> + * for KVM_DEBUG_EXIT.
> + */
> +#define KVMPPC_DEBUG_NONE 0x0
> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
> +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
> struct kvm_debug_exit_arch {
> + __u64 address;
> + /*
> + * exiting to userspace because of h/w breakpoint, watchpoint
> + * (read, write or both) and software breakpoint.
> + */
> + __u32 status;
> + __u32 reserved;
> };
>
> /* for KVM_SET_GUEST_DEBUG */
> @@ -281,10 +298,6 @@ struct kvm_guest_debug_arch {
> * Type denotes h/w breakpoint, read watchpoint, write
> * watchpoint or watchpoint (both read and write).
> */
> -#define KVMPPC_DEBUG_NONE 0x0
> -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
> -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
> -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
> __u32 type;
> __u32 reserved;
> } bp[16];
> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
> index b10a012..dab9d07 100644
> --- a/arch/powerpc/kvm/e500_emulate.c
> +++ b/arch/powerpc/kvm/e500_emulate.c
> @@ -26,6 +26,8 @@
> #define XOP_TLBRE 946
> #define XOP_TLBWE 978
> #define XOP_TLBILX 18
> +#define XOP_EHPRIV 270
> +#define EHPRIV_OC_DEBUG 0
As I think the case, "OC = 0", is a bit specific since IIRC, if the OC
operand is omitted, its equal 0 by default. So I think we should start this OC
value from 1 or other magic number.
And if possible, we'd better add some comments to describe this to make the OC
definition readable.
Tiejun
>
> #ifdef CONFIG_KVM_E500MC
> static int dbell2prio(ulong param)
> @@ -82,6 +84,26 @@ static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu, int rb)
> }
> #endif
>
> +static int kvmppc_e500_emul_ehpriv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + unsigned int inst, int *advance)
> +{
> + int emulated = EMULATE_DONE;
> +
> + switch (get_oc(inst)) {
> + case EHPRIV_OC_DEBUG:
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.address = vcpu->arch.pc;
> + run->debug.arch.status = 0;
> + kvmppc_account_exit(vcpu, DEBUG_EXITS);
> + emulated = EMULATE_EXIT_USER;
> + *advance = 0;
> + break;
> + default:
> + emulated = EMULATE_FAIL;
> + }
> + return emulated;
> +}
> +
> int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
> unsigned int inst, int *advance)
> {
> @@ -130,6 +152,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
> emulated = kvmppc_e500_emul_tlbivax(vcpu, ea);
> break;
>
> + case XOP_EHPRIV:
> + emulated = kvmppc_e500_emul_ehpriv(run, vcpu, inst,
> + advance);
> + break;
> +
> default:
> emulated = EMULATE_FAIL;
> }
>
^ permalink raw reply
* linux-next: manual merge of the akpm tree with the powerpc tree
From: Stephen Rothwell @ 2013-06-26 6:56 UTC (permalink / raw)
To: Andrew Morton
Cc: Michael Neuling, linux-kernel, Oleg Nesterov, linux-next,
linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
Hi Andrew,
Today's linux-next merge of the akpm tree got a conflict in
arch/powerpc/kernel/ptrace.c between commit b0b0aa9c7faf
("powerpc/hw_brk: Fix setting of length for exact mode breakpoints") from
the powerpc tree and commit "ptrace/powerpc: revert "hw_breakpoints: Fix
racy access to ptrace breakpoints"" from the akpm tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc arch/powerpc/kernel/ptrace.c
index 64f7bd5,6645e57..0000000
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@@ -1449,10 -1437,7 +1437,9 @@@ static long ppc_set_hwdebug(struct task
*/
if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE) {
len = bp_info->addr2 - bp_info->addr;
- } else if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT) {
+ } else if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_EXACT)
+ len = 1;
+ else {
- ptrace_put_breakpoints(child);
return -EINVAL;
}
bp = thread->ptrace_bps[0];
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [RFC][PATCH 2/3][v2] perf/Power7: Export MDTLB_SRC fields to userspace
From: Sukadev Bhattiprolu @ 2013-06-26 7:41 UTC (permalink / raw)
To: eranian, Paul Mackerras, Anton Blanchard, ellerman,
Anshuman Khandual
Cc: sukadev, linuxppc-dev, linux-kernel
In-Reply-To: <20130626074106.GA3741@us.ibm.com>
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Tue, 25 Jun 2013 15:50:18 -0700
Subject: [RFC][PATCH 2/3][v2] perf/Power7: Export MDTLB_SRC fields to userspace
Power7 saves the "perf-event vector" information in the mmcra register.
Included in this event vector is a "marked-data-TLB source", MDTLB_SRC,
field which identifies where in the memory-hierarchy the data for a TLB
miss was eventually found.
Use the 'struct perf_mem_data_src' to export the MDTLB_SRC field to
user space.
The mapping between the Power7 hierarchy levels and the arch-neutral levels
is, unfortunately, not trivial. Some existing arch-neutral levels are unused
in Power (eg: TLB_L1, TLB_WK, TLB_OS). But, Power7 provides several other
levels for the MDTLB_SRC, so this patch proposes adding new arch-neutral
levels.
Arch-neutral levels Power7 levels
-----------------------------------------------------------------------
local TLB_L2 local (same core) L2 (FROM_L2)
local TLB_L3 local (same core) L3 (FROM_L3)
1-hop TLB_REM_L2_CCE1* different core on same chip (FROM_L2.1)
1-hop TLB_REM_L3_CCE1* different core on same chip (FROM_L3.1)
2-hops TLB_REM_CCE2* remote (different chip, same node) (FROM_RL2L3)
3-hops TLB_REM_CCE3* distant (different node) (FROM_DL2L3)
1-hop TLB_REM_RAM1* unused
2-hops TLB_REM_RAM2* remote (different chip, same node) (FROM_RMEM)
3-hops TLB_REM_RAM3* distant (different node) (FROM_DMEM)
* proposed new levels.
As shown above, Power7 supports one extra level in the cache-hierarchy (i.e
total of 3-hops). To maintain consistency in terminology (i.e 2-hops = remote,
3-hops = distant), we propose leaving the REM_RAM1 unused in Power7 and adding
another level, REM_RAM3.
Further, in the above REM_CCE1 case, Power7 can also identify if the data came
from the L2 or L3 cache of another core on the same chip. To describe this
add the levels:
PERF_MEM_TLB_REM_L2_CCE1
PERF_MEM_TLB_REM_L3_CCE1
Finally, in the REM_CCE1 and REM_CCE2 cases, Power7 also indicates whether
the entry found in the remote cache was modified (dirty). So we add a new
state
PERF_MEM_TLB_CCE_DIRTY
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
Changelog[v2]:
- Address the MDTLB_SRC field before addressing the DCACHE_SRC field
since we can then keep the new ->mem_dtlb bits contigious.
(DCACHE_SRC needs a field, ->mem_xlvl in struct perf_mem_data_src
and will be added in the next patch)
arch/powerpc/include/asm/perf_event_server.h | 2 +
arch/powerpc/perf/core-book3s.c | 4 ++
arch/powerpc/perf/power7-pmu.c | 64 ++++++++++++++++++++++++++
include/uapi/linux/perf_event.h | 14 +++++-
4 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
index f265049..30488f5 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -37,6 +37,8 @@ struct power_pmu {
void (*config_bhrb)(u64 pmu_bhrb_filter);
void (*disable_pmc)(unsigned int pmc, unsigned long mmcr[]);
int (*limited_pmc_event)(u64 event_id);
+ void (*get_mem_data_src)(union perf_mem_data_src *dsrc,
+ struct pt_regs *regs);
u32 flags;
const struct attribute_group **attr_groups;
int n_generic;
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 29c6482..e0e0848 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1627,6 +1627,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
data.br_stack = &cpuhw->bhrb_stack;
}
+ if (event->attr.sample_type & PERF_SAMPLE_DATA_SRC &&
+ ppmu->get_mem_data_src)
+ ppmu->get_mem_data_src(&data.data_src, regs);
+
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
}
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 3c475d6..c1cac96 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -209,6 +209,69 @@ static int power7_get_alternatives(u64 event, unsigned int flags, u64 alt[])
return nalt;
}
+#define POWER7_MMCRA_MDTLB_MISS (0x1LL << 50)
+#define POWER7_MMCRA_MDTLB_SRC_SHIFT 46
+#define POWER7_MMCRA_MDTLB_SRC_MASK (0xFLL << POWER7_MMCRA_MDTLB_SRC_SHIFT)
+
+/*
+ * Map MDTLB_SRC fields to the Linux memory hierarchy levels.
+ *
+ * Bits 14..17 in the MMCRA indicate the source of a marked-data-TLB miss,
+ * with of the 16 possible values referring to a specific source. Eg: if
+ * the 4-bits have the value 1 (0b0001), the mdtlb entry was found in the
+ * local L3 cache.
+ *
+ * We use the table, mdtlb_src_map, to map the value in this field, to
+ * PERF_MEM_TLB_L3, the arch-neutral representation of TLB L3 cache.
+ *
+ * Architecture neutral to Power7 hierarchy levels:
+ * 1-hop = different core on same chip (L2.1 or L3.1)
+ * 2-hops = remote (different chip on same node, RL2L3, RMEM)
+ * 3-hops = distant (different node, DL2L3, DMEM)
+ */
+#define P(a, b) PERF_MEM_S(a, b)
+#define TD(a, b) (P(TLB, CCE_DIRTY) | P(a, b))
+
+static u64 mdtlb_src_map[] = {
+ P(TLB, L2), /* 00: FROM_L2 */
+ P(TLB, L3), /* 01: FROM_L3 */
+
+ P(TLB, NA), /* 02: Reserved */
+ P(TLB, NA), /* 03: Reserved */
+
+ P(TLB, REM_L2_CCE1), /* 04: FROM_L2.1_SHR */
+ TD(TLB, REM_L2_CCE1), /* 05: FROM_L2.1_MOD */
+
+ P(TLB, REM_L3_CCE1), /* 06: FROM_L3.1_SHR */
+ TD(TLB, REM_L3_CCE1), /* 07: FROM_L3.1_MOD */
+
+ P(TLB, REM_CCE2), /* 08: FROM_RL2L3_SHR */
+ TD(TLB, REM_CCE2), /* 09: FROM_RL2L3_MOD */
+
+ P(TLB, REM_CCE3), /* 10: FROM_DL2L3_SHR */
+ TD(TLB, REM_CCE3), /* 11: FROM_DL2L3_MOD */
+
+ P(TLB, LOC_RAM), /* 12: FROM_LMEM */
+ P(TLB, REM_RAM2), /* 13: FROM_RMEM */
+ P(TLB, REM_RAM3), /* 14: FROM_DMEM */
+
+ P(TLB, NA), /* 15: Reserved */
+};
+
+static void power7_get_mem_data_src(union perf_mem_data_src *dsrc,
+ struct pt_regs *regs)
+{
+ u64 idx;
+ u64 mmcra = regs->dsisr;
+
+ if (mmcra & POWER7_MMCRA_MDTLB_MISS) {
+ idx = mmcra & POWER7_MMCRA_MDTLB_SRC_MASK;
+ idx >>= POWER7_MMCRA_MDTLB_SRC_SHIFT;
+
+ dsrc->val |= mdtlb_src_map[idx];
+ }
+}
+
/*
* Returns 1 if event counts things relating to marked instructions
* and thus needs the MMCRA_SAMPLE_ENABLE bit set, or 0 if not.
@@ -447,6 +510,7 @@ static struct power_pmu power7_pmu = {
.compute_mmcr = power7_compute_mmcr,
.get_constraint = power7_get_constraint,
.get_alternatives = power7_get_alternatives,
+ .get_mem_data_src = power7_get_mem_data_src,
.disable_pmc = power7_disable_pmc,
.flags = PPMU_ALT_SIPR,
.attr_groups = power7_pmu_attr_groups,
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 52697a3..815ee12 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -626,8 +626,8 @@ union perf_mem_data_src {
mem_lvl:14, /* memory hierarchy level */
mem_snoop:5, /* snoop mode */
mem_lock:2, /* lock instr */
- mem_dtlb:7, /* tlb access */
- mem_rsvd:31;
+ mem_dtlb:17, /* tlb access */
+ mem_rsvd:21;
};
};
@@ -678,6 +678,16 @@ union perf_mem_data_src {
#define PERF_MEM_TLB_L2 0x10 /* L2 */
#define PERF_MEM_TLB_WK 0x20 /* Hardware Walker*/
#define PERF_MEM_TLB_OS 0x40 /* OS fault handler */
+#define PERF_MEM_TLB_L3 0x80
+#define PERF_MEM_TLB_REM_L2_CCE1 0x100 /* Remote L2 cache (1 hop) */
+#define PERF_MEM_TLB_REM_L3_CCE1 0x200 /* Remote L3 cache (1 hop) */
+#define PERF_MEM_TLB_REM_CCE2 0x400 /* Remote cache (2 hops) */
+#define PERF_MEM_TLB_REM_CCE3 0x800 /* Remote cache (3 hops) */
+#define PERF_MEM_TLB_LOC_RAM 0x1000 /* Local DRAM */
+#define PERF_MEM_TLB_REM_RAM1 0x2000 /* Remote DRAM (1 hop) */
+#define PERF_MEM_TLB_REM_RAM2 0x4000 /* Remote DRAM (2 hops) */
+#define PERF_MEM_TLB_REM_RAM3 0x8000 /* Remote DRAM (3 hops) */
+#define PERF_MEM_TLB_CCE_DIRTY 0x10000 /* Remote cache entry hit, but dirty */
#define PERF_MEM_S(a, s) \
(((u64)PERF_MEM_##a##_##s) << PERF_MEM_##a##_SHIFT)
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v2 29/45] kvm/vmx: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Paolo Bonzini @ 2013-06-26 7:46 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Gleb Natapov, peterz, fweisbec, linux-kernel, H. Peter Anvin,
walken, mingo, linux-arch, vincent.guittot, kvm, x86,
xiaoguangrong, Ingo Molnar, wangyun, paulmck, nikunj, linux-pm,
rusty, rostedt, namhyung, tglx, laijs, zhong, netdev, oleg, sbw,
tj, akpm, linuxppc-dev
In-Reply-To: <20130625203043.16593.1600.stgit@srivatsabhat.in.ibm.com>
Il 25/06/2013 22:30, Srivatsa S. Bhat ha scritto:
> - cpu = get_cpu();
> + cpu = get_online_cpus_atomic();
> vmx_vcpu_load(&vmx->vcpu, cpu);
> vmx->vcpu.cpu = cpu;
> err = vmx_vcpu_setup(vmx);
> vmx_vcpu_put(&vmx->vcpu);
> - put_cpu();
> + put_online_cpus_atomic();
The new API has a weird name. Why are you adding new functions instead
of just modifying get/put_cpu?
Paolo
^ permalink raw reply
* Re: [PATCH v2 29/45] kvm/vmx: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-26 8:06 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Gleb Natapov, peterz, fweisbec, linux-kernel, H. Peter Anvin,
walken, mingo, linux-arch, vincent.guittot, kvm, x86,
xiaoguangrong, Ingo Molnar, wangyun, paulmck, nikunj, linux-pm,
rusty, rostedt, namhyung, tglx, laijs, zhong, netdev, oleg, sbw,
tj, akpm, linuxppc-dev
In-Reply-To: <51CA9C5E.1030609@redhat.com>
On 06/26/2013 01:16 PM, Paolo Bonzini wrote:
> Il 25/06/2013 22:30, Srivatsa S. Bhat ha scritto:
>> - cpu = get_cpu();
>> + cpu = get_online_cpus_atomic();
>> vmx_vcpu_load(&vmx->vcpu, cpu);
>> vmx->vcpu.cpu = cpu;
>> err = vmx_vcpu_setup(vmx);
>> vmx_vcpu_put(&vmx->vcpu);
>> - put_cpu();
>> + put_online_cpus_atomic();
>
> The new API has a weird name. Why are you adding new functions instead
> of just modifying get/put_cpu?
>
Because the purpose of those two functions are distinctly different
from each other.
get/put_cpu() is used to disable preemption on the local CPU. (Which
also disables offlining the local CPU during that critical section).
What this patchset deals with is synchronizing with offline of *any*
CPU. Typically, we use get_online_cpus()/put_online_cpus() for that
purpose. But they can't be used in atomic context, because they take
mutex locks and hence can sleep.
So the code that executes in atomic context and which wants to prevent
*any* CPU from going offline, used to disable preemption around its
critical section. Disabling preemption prevents stop_machine(), and
CPU offline (of *any* CPU) was done via stop_machine(). So disabling
preemption disabled any CPU from going offline, as a *side-effect*.
And this patchset prepares the ground for getting rid of stop_machine()
in the CPU offline path. Which means, disabling preemption only prevents
the *local* CPU from going offline. So if code in atomic context wants
to prevent any CPU from going offline, we need a new set of APIs, like
get/put_online_cpus(), but which can be invoked from atomic context.
That's why I named it as get/put_online_cpus_atomic().
One of the key points here is that we want to preserve get/put_cpu()
as it is, since its purpose is different - disable preemption and
offline of the local CPU. There is no reason to change that API, its
useful as it is.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: linux-next: manual merge of the akpm tree with the powerpc tree
From: Benjamin Herrenschmidt @ 2013-06-26 8:10 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Michael Neuling, linux-kernel, Oleg Nesterov, linux-next,
Andrew Morton, linuxppc-dev
In-Reply-To: <20130626165642.8740ecb6d5d5882b06f173c8@canb.auug.org.au>
On Wed, 2013-06-26 at 16:56 +1000, Stephen Rothwell wrote:
> Today's linux-next merge of the akpm tree got a conflict in
> arch/powerpc/kernel/ptrace.c between commit b0b0aa9c7faf
> ("powerpc/hw_brk: Fix setting of length for exact mode breakpoints") from
> the powerpc tree and commit "ptrace/powerpc: revert "hw_breakpoints: Fix
> racy access to ptrace breakpoints"" from the akpm tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
Where does the latter come from ? Why didn't it go through the powerpc tree ?
Cheers,
Ben.
^ permalink raw reply
* Re: Regression in RCU subsystem in latest mainline kernel
From: Michael Ellerman @ 2013-06-26 8:10 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Rojhalat Ibrahim, linuxppc-dev, linux-kernel, Steven Rostedt
In-Reply-To: <20130625160332.GA3828@linux.vnet.ibm.com>
On Tue, Jun 25, 2013 at 09:03:32AM -0700, Paul E. McKenney wrote:
> On Tue, Jun 25, 2013 at 05:44:23PM +1000, Michael Ellerman wrote:
> > On Tue, Jun 25, 2013 at 05:19:14PM +1000, Michael Ellerman wrote:
> > >
> > > Here's another trace from 3.10-rc7 plus a few local patches.
> >
> > And here's another with CONFIG_RCU_CPU_STALL_INFO=y in case that's useful:
> >
> > PASS running test_pmc5_6_overuse()
> > INFO: rcu_sched self-detected stall on CPU
> > 8: (1 GPs behind) idle=8eb/140000000000002/0 softirq=215/220
>
> So this CPU has been out of action since before the beginning of the
> current grace period ("1 GPs behind"). It is not idle, having taken
> a pair of nested interrupts from process context (matching the stack
> below). This CPU has take five softirqs since the last grace period
> that it noticed, which makes it likely that the loop is within the
> softirq handler.
>
> > (t=2100 jiffies g=18446744073709551583 c=18446744073709551582 q=13)
>
> Assuming HZ=100, this stall has been going on for 21 seconds. There
> is a grace period in progress according to RCU's global state (which
> this CPU is not yet aware of). There are a total of 13 RCU callbacks
> queued across the entire system.
>
> If the system is at all responsive, I suggest using ftrace (either from
> the boot command line or at runtime) to trace __do_softirq() and
> hrtimer_interrupt().
Thanks for decoding it Paul.
I've narrowed down the test case and I think this is probably just a
case of too many perf interrupts. If I reduce the sampling period by
half the test runs fine.
There is logic in perf to detect an interrupt storm, but for some reason
it's not saving us. I'll dig in there, but I don't think it's an RCU
problem.
cheers
^ permalink raw reply
* Re: [PATCH v2 28/45] KVM: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Paolo Bonzini @ 2013-06-26 8:20 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Gleb Natapov, peterz, fweisbec, linux-kernel, walken, mingo,
linux-arch, vincent.guittot, kvm, xiaoguangrong, wangyun, paulmck,
nikunj, linux-pm, rusty, rostedt, namhyung, tglx, laijs, zhong,
netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <20130625203036.16593.50377.stgit@srivatsabhat.in.ibm.com>
Il 25/06/2013 22:30, Srivatsa S. Bhat ha scritto:
> Once stop_machine() is gone from the CPU offline path, we won't be able
> to depend on disabling preemption to prevent CPUs from going offline
> from under us.
>
> Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
> offline, while invoking from atomic context.
>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> ---
>
> virt/kvm/kvm_main.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 302681c..5bbfa30 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -174,7 +174,7 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
>
> zalloc_cpumask_var(&cpus, GFP_ATOMIC);
>
> - me = get_cpu();
> + me = get_online_cpus_atomic();
> kvm_for_each_vcpu(i, vcpu, kvm) {
> kvm_make_request(req, vcpu);
> cpu = vcpu->cpu;
> @@ -192,7 +192,7 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
> smp_call_function_many(cpus, ack_flush, NULL, 1);
> else
> called = false;
> - put_cpu();
> + put_online_cpus_atomic();
> free_cpumask_var(cpus);
> return called;
> }
> @@ -1707,11 +1707,11 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
> ++vcpu->stat.halt_wakeup;
> }
>
> - me = get_cpu();
> + me = get_online_cpus_atomic();
> if (cpu != me && (unsigned)cpu < nr_cpu_ids && cpu_online(cpu))
> if (kvm_arch_vcpu_should_kick(vcpu))
> smp_send_reschedule(cpu);
> - put_cpu();
> + put_online_cpus_atomic();
> }
> EXPORT_SYMBOL_GPL(kvm_vcpu_kick);
> #endif /* !CONFIG_S390 */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 29/45] kvm/vmx: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Paolo Bonzini @ 2013-06-26 8:23 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Gleb Natapov, peterz, fweisbec, linux-kernel, H. Peter Anvin,
walken, mingo, linux-arch, vincent.guittot, kvm, x86,
xiaoguangrong, Ingo Molnar, wangyun, paulmck, nikunj, linux-pm,
rusty, rostedt, namhyung, tglx, laijs, zhong, netdev, oleg, sbw,
tj, akpm, linuxppc-dev
In-Reply-To: <51CAA116.2060906@linux.vnet.ibm.com>
Il 26/06/2013 10:06, Srivatsa S. Bhat ha scritto:
> On 06/26/2013 01:16 PM, Paolo Bonzini wrote:
>> Il 25/06/2013 22:30, Srivatsa S. Bhat ha scritto:
>>> - cpu = get_cpu();
>>> + cpu = get_online_cpus_atomic();
>>> vmx_vcpu_load(&vmx->vcpu, cpu);
>>> vmx->vcpu.cpu = cpu;
>>> err = vmx_vcpu_setup(vmx);
>>> vmx_vcpu_put(&vmx->vcpu);
>>> - put_cpu();
>>> + put_online_cpus_atomic();
>>
>> The new API has a weird name. Why are you adding new functions instead
>> of just modifying get/put_cpu?
>>
>
> Because the purpose of those two functions are distinctly different
> from each other.
>
> get/put_cpu() is used to disable preemption on the local CPU. (Which
> also disables offlining the local CPU during that critical section).
Ok, then I understood correctly... and I acked the other KVM patch.
However, keeping the code on the local CPU is exactly the point of this
particular use of get_cpu()/put_cpu(). Why does it need to synchronize
with offlining of other CPUs?
Paolo
> What this patchset deals with is synchronizing with offline of *any*
> CPU. Typically, we use get_online_cpus()/put_online_cpus() for that
> purpose. But they can't be used in atomic context, because they take
> mutex locks and hence can sleep.
>
> So the code that executes in atomic context and which wants to prevent
> *any* CPU from going offline, used to disable preemption around its
> critical section. Disabling preemption prevents stop_machine(), and
> CPU offline (of *any* CPU) was done via stop_machine(). So disabling
> preemption disabled any CPU from going offline, as a *side-effect*.
>
> And this patchset prepares the ground for getting rid of stop_machine()
> in the CPU offline path. Which means, disabling preemption only prevents
> the *local* CPU from going offline. So if code in atomic context wants
> to prevent any CPU from going offline, we need a new set of APIs, like
> get/put_online_cpus(), but which can be invoked from atomic context.
> That's why I named it as get/put_online_cpus_atomic().
>
> One of the key points here is that we want to preserve get/put_cpu()
> as it is, since its purpose is different - disable preemption and
> offline of the local CPU. There is no reason to change that API, its
> useful as it is.
^ permalink raw reply
* Re: [PATCH 7/8] powerpc/perf: Core EBB support for 64-bit book3s
From: Anshuman Khandual @ 2013-06-26 8:38 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, sukadev, Paul Mackerras
In-Reply-To: <1372073336-8189-7-git-send-email-michael@ellerman.id.au>
On 06/24/2013 04:58 PM, Michael Ellerman wrote:
> Add support for EBB (Event Based Branches) on 64-bit book3s. See the
> included documentation for more details.
>
> EBBs are a feature which allows the hardware to branch directly to a
> specified user space address when a PMU event overflows. This can be
> used by programs for self-monitoring with no kernel involvement in the
> inner loop.
>
> Most of the logic is in the generic book3s code, primarily to avoid a
> proliferation of PMU callbacks.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> Documentation/powerpc/00-INDEX | 2 +
> Documentation/powerpc/pmu-ebb.txt | 122 +++++++++++++++++++
> arch/powerpc/include/asm/perf_event_server.h | 6 +
> arch/powerpc/include/asm/processor.h | 3 +-
> arch/powerpc/include/asm/reg.h | 8 ++
> arch/powerpc/include/asm/switch_to.h | 14 +++
> arch/powerpc/kernel/process.c | 4 +
> arch/powerpc/perf/core-book3s.c | 161 +++++++++++++++++++++++---
> 8 files changed, 306 insertions(+), 14 deletions(-)
> create mode 100644 Documentation/powerpc/pmu-ebb.txt
>
> diff --git a/Documentation/powerpc/00-INDEX b/Documentation/powerpc/00-INDEX
> index dd9e928..05026ce 100644
> --- a/Documentation/powerpc/00-INDEX
> +++ b/Documentation/powerpc/00-INDEX
> @@ -14,6 +14,8 @@ hvcs.txt
> - IBM "Hypervisor Virtual Console Server" Installation Guide
> mpc52xx.txt
> - Linux 2.6.x on MPC52xx family
> +pmu-ebb.txt
> + - Description of the API for using the PMU with Event Based Branches.
> qe_firmware.txt
> - describes the layout of firmware binaries for the Freescale QUICC
> Engine and the code that parses and uploads the microcode therein.
> diff --git a/Documentation/powerpc/pmu-ebb.txt b/Documentation/powerpc/pmu-ebb.txt
> new file mode 100644
> index 0000000..65b6989
> --- /dev/null
> +++ b/Documentation/powerpc/pmu-ebb.txt
> @@ -0,0 +1,122 @@
> +PMU Event Based Branches
> +========================
> +
> +Event Based Branches (EBBs) are a feature which allows the hardware to
> +branch directly to a specified user space address when certain events occur.
> +
> +The full specification is available in Power ISA v2.07:
> +
> + https://www.power.org/documentation/power-isa-version-2-07/
> +
> +One type of event for which EBBs can be configured is PMU exceptions. This
> +document describes the API for configuring the Power PMU to generate EBBs,
> +using the Linux perf_events API.
> +
> +
> +Terminology
> +-----------
> +
> +Throughout this document we will refer to an "EBB event" or "EBB events". This
> +just refers to a struct perf_event which has set the "EBB" flag in its
> +attr.config. All events which can be configured on the hardware PMU are
> +possible "EBB events".
> +
Then why we have a condition like this where the event code must have the PMC
value inside it (in the next patch) ?
+ if (!pmc && ebb)
+ /* EBB events must specify the PMC */
+ return -1;
> +
> +Background
> +----------
> +
> +When a PMU EBB occurs it is delivered to the currently running process. As such
> +EBBs can only sensibly be used by programs for self-monitoring.
> +
> +It is a feature of the perf_events API that events can be created on other
> +processes, subject to standard permission checks. This is also true of EBB
> +events, however unless the target process enables EBBs (via mtspr(BESCR)) no
> +EBBs will ever be delivered.
> +
> +This makes it possible for a process to enable EBBs for itself, but not
> +actually configure any events. At a later time another process can come along
> +and attach an EBB event to the process, which will then cause EBBs to be
> +delivered to the first process. It's not clear if this is actually useful.
> +
May be useful when a "master process" wants each of the thread to collect statistics
(about the same thread) on various dynamically configured events (as and when it
wishes) and report it back to the master process. Just a thought about a case where
it can be useful.
> +When the PMU is configured for EBBs, all PMU interrupts are delivered to the
> +user process. This means once an EBB event is scheduled on the PMU, no non-EBB
> +events can be configured. This means that EBB events can not be run
> +concurrently with regular 'perf' commands.
> +
> +It is however safe to run 'perf' commands on a process which is using EBBs. In
> +general the EBB event will take priority, though it depends on the exact
> +options used on the perf_event_open() and the timing.
> +
This is confusing. If a process A is using EBB for itself on event "p" and gets scheduled
on CPU X. Process B has started perf session on process A for event "q". Now the PMU of
CPU X would to be programmed for both the events "p" and "q" on different PMCs at the same
point of time (with a condition checking that they dont collide on the same PMC though).
What you are saying is that when the event "p" overflows, the PMU interrupt (CPU X) would
be delivered to the process A user space and when the event "q" overflows, the PMU interrupt
(CPU X) would be delivered inside the perf kernel component "perf_event_interrupt()" and would
be processed for the perf session initiated by the second process B on process A.
But again this contradicts your previous statement that when PMU is configured for EBB, *all*
PMU interrupts would be delivered to the user space. Could you please kindly clarify this
scenario.
> +
> +Creating an EBB event
> +---------------------
> +
> +To request that an event is counted using EBB, the event code should have bit
> +63 set.
> +
This macro (defined in arch header) identifies any event as an EBB event
+/*
+ * We use the event config bit 63 as a flag to request EBB.
+ */
+#define EVENT_CONFIG_EBB_SHIFT 63
+
So any user program would have to include the arch header to be able to set EBB bit.
Numeric 63 will not be a clean ABI.
> +EBB events must be created with a particular, and restrictive, set of
> +attributes - this is so that they interoperate correctly with the rest of the
> +perf_events subsystem.
> +
> +An EBB event must be created with the "pinned" and "exclusive" attributes set.
> +Note that if you are creating a group of EBB events, only the leader can have
> +these attributes set.
> +
> +An EBB event must NOT set any of the "inherit", "sample_period", "freq" or
> +"enable_on_exec" attributes.
> +
> +An EBB event must be attached to a task. This is specified to perf_event_open()
> +by passing a pid value, typically 0 indicating the current task.
> +
> +All events in a group must agree on whether they want EBB. That is all events
> +must request EBB, or none may request EBB.
> +
> +
> +Enabling an EBB event
> +---------------------
> +
> +Once an EBB event has been successfully opened, it must be enabled with the
> +perf_events API. This can be achieved either via the ioctl() interface, or the
> +prctl() interface.
> +
> +However, due to the design of the perf_events API, enabling an event does not
> +guarantee that it has been scheduled on the PMU. To ensure that the EBB event
> +has been scheduled on the PMU, you must perform a read() on the event. If the
> +read() returns EOF, then the event has not been scheduled and EBBs are not
> +enabled.
> +
> +
> +Reading an EBB event
> +--------------------
> +
> +It is possible to read() from an EBB event. However the results are
> +meaningless. Because interrupts are being delivered to the user process the
> +kernel is not able to count the event, and so will return a junk value.
> +
> +
> +Closing an EBB event
> +--------------------
> +
> +When an EBB event is finished with, you can close it using close() as for any
> +regular event. If this is the last EBB event the PMU will be deconfigured and
> +no further PMU EBBs will be delivered.
> +
> +
> +EBB Handler
> +-----------
> +
> +The EBB handler is just regular userspace code, however it must be written in
> +the style of an interrupt handler. When the handler is entered all registers
> +are live (possibly) and so must be saved somehow before the handler can invoke
> +other code.
> +
> +It's up to the program how to handle this. For C programs a relatively simple
> +option is to create an interrupt frame on the stack and save registers there.
> +
Would be a great if you could give sample framework here on how to save and restore
registers. Moreover we could actually put the various essential parts of the EBB
handler construct in the perf_event_server.h file, so that the user would be able
to user them directly and only focus on core part of the event handling.
> +Fork
> +----
> +
> +EBB events are not inherited across fork. If the child process wishes to use
> +EBBs it should open a new event for itself. Similarly the EBB state in
> +BESCR/EBBHR/EBBRR is cleared across fork().
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index f265049..2dd7bfc 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -60,6 +60,7 @@ struct power_pmu {
> #define PPMU_HAS_SSLOT 0x00000020 /* Has sampled slot in MMCRA */
> #define PPMU_HAS_SIER 0x00000040 /* Has SIER */
> #define PPMU_BHRB 0x00000080 /* has BHRB feature enabled */
> +#define PPMU_EBB 0x00000100 /* supports event based branch */
>
> /*
> * Values for flags to get_alternatives()
> @@ -68,6 +69,11 @@ struct power_pmu {
> #define PPMU_LIMITED_PMC_REQD 2 /* have to put this on a limited PMC */
> #define PPMU_ONLY_COUNT_RUN 4 /* only counting in run state */
>
> +/*
> + * We use the event config bit 63 as a flag to request EBB.
> + */
> +#define EVENT_CONFIG_EBB_SHIFT 63
> +
> extern int register_power_pmu(struct power_pmu *);
>
> struct pt_regs;
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 48af5d7..f9a4cdc 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -287,8 +287,9 @@ struct thread_struct {
> unsigned long siar;
> unsigned long sdar;
> unsigned long sier;
> - unsigned long mmcr0;
> unsigned long mmcr2;
> + unsigned mmcr0;
> + unsigned used_ebb;
> #endif
> };
>
Why mmrc0 has to change position here.
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 362142b..5d7d9c2 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -621,6 +621,9 @@
> #define MMCR0_PMXE 0x04000000UL /* performance monitor exception enable */
> #define MMCR0_FCECE 0x02000000UL /* freeze ctrs on enabled cond or event */
> #define MMCR0_TBEE 0x00400000UL /* time base exception enable */
> +#define MMCR0_EBE 0x00100000UL /* Event based branch enable */
> +#define MMCR0_PMCC 0x000c0000UL /* PMC control */
> +#define MMCR0_PMCC_U6 0x00080000UL /* PMC1-6 are R/W by user (PR) */
> #define MMCR0_PMC1CE 0x00008000UL /* PMC1 count enable*/
> #define MMCR0_PMCjCE 0x00004000UL /* PMCj count enable*/
> #define MMCR0_TRIGGER 0x00002000UL /* TRIGGER enable */
> @@ -674,6 +677,11 @@
> #define SIER_SIAR_VALID 0x0400000 /* SIAR contents valid */
> #define SIER_SDAR_VALID 0x0200000 /* SDAR contents valid */
>
> +/* When EBB is enabled, some of MMCR0/MMCR2/SIER are user accessible */
> +#define MMCR0_USER_MASK (MMCR0_FC | MMCR0_PMXE | MMCR0_PMAO)
> +#define MMCR2_USER_MASK 0x4020100804020000UL /* (FC1P|FC2P|FC3P|FC4P|FC5P|FC6P) */
> +#define SIER_USER_MASK 0x7fffffUL
> +
Ohh these are the bits in SPR which are available in user space to read and write as well ?
Better to have macros instead of hex codes here.
> #define SPRN_PA6T_MMCR0 795
> #define PA6T_MMCR0_EN0 0x0000000000000001UL
> #define PA6T_MMCR0_EN1 0x0000000000000002UL
> diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
> index 200d763..49a13e0 100644
> --- a/arch/powerpc/include/asm/switch_to.h
> +++ b/arch/powerpc/include/asm/switch_to.h
> @@ -67,4 +67,18 @@ static inline void flush_spe_to_thread(struct task_struct *t)
> }
> #endif
>
> +static inline void clear_task_ebb(struct task_struct *t)
> +{
> +#ifdef CONFIG_PPC_BOOK3S_64
> + /* EBB perf events are not inherited, so clear all EBB state. */
> + t->thread.bescr = 0;
> + t->thread.mmcr2 = 0;
> + t->thread.mmcr0 = 0;
> + t->thread.siar = 0;
> + t->thread.sdar = 0;
> + t->thread.sier = 0;
> + t->thread.used_ebb = 0;
> +#endif
> +}
> +
> #endif /* _ASM_POWERPC_SWITCH_TO_H */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 076d124..c517dbe 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -916,7 +916,11 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
> flush_altivec_to_thread(src);
> flush_vsx_to_thread(src);
> flush_spe_to_thread(src);
> +
> *dst = *src;
> +
> + clear_task_ebb(dst);
> +
> return 0;
> }
Blank lines are not necessary here.
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index d3ee2e5..c6bbbf9 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -77,6 +77,9 @@ static unsigned int freeze_events_kernel = MMCR0_FCS;
> #define MMCR0_PMCjCE MMCR0_PMCnCE
> #define MMCR0_FC56 0
> #define MMCR0_PMAO 0
> +#define MMCR0_EBE 0
> +#define MMCR0_PMCC 0
> +#define MMCR0_PMCC_U6 0
>
> #define SPRN_MMCRA SPRN_MMCR2
> #define MMCRA_SAMPLE_ENABLE 0
> @@ -104,6 +107,15 @@ static inline int siar_valid(struct pt_regs *regs)
> return 1;
> }
>
> +static bool is_ebb_event(struct perf_event *event) { return false; }
> +static int ebb_event_check(struct perf_event *event) { return 0; }
> +static void ebb_event_add(struct perf_event *event) { }
> +static void ebb_switch_out(unsigned long mmcr0) { }
> +static unsigned long ebb_switch_in(bool ebb, unsigned long mmcr0)
> +{
> + return mmcr0;
> +}
> +
> static inline void power_pmu_bhrb_enable(struct perf_event *event) {}
> static inline void power_pmu_bhrb_disable(struct perf_event *event) {}
> void power_pmu_flush_branch_stack(void) {}
> @@ -464,6 +476,89 @@ void power_pmu_bhrb_read(struct cpu_hw_events *cpuhw)
> return;
> }
>
> +static bool is_ebb_event(struct perf_event *event)
> +{
> + /*
> + * This could be a per-PMU callback, but we'd rather avoid the cost. We
> + * check that the PMU supports EBB, meaning those that don't can still
> + * use bit 63 of the event code for something else if they wish.
> + */
> + return (ppmu->flags & PPMU_EBB) &&
> + ((event->attr.config >> EVENT_CONFIG_EBB_SHIFT) & 1);
> +}
> +
> +static int ebb_event_check(struct perf_event *event)
> +{
> + struct perf_event *leader = event->group_leader;
> +
> + /* Event and group leader must agree on EBB */
> + if (is_ebb_event(leader) != is_ebb_event(event))
> + return -EINVAL;
> +
> + if (is_ebb_event(event)) {
> + if (!(event->attach_state & PERF_ATTACH_TASK))
> + return -EINVAL;
> +
> + if (!leader->attr.pinned || !leader->attr.exclusive)
> + return -EINVAL;
> +
> + if (event->attr.inherit || event->attr.sample_period ||
> + event->attr.enable_on_exec || event->attr.freq)
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void ebb_event_add(struct perf_event *event)
> +{
> + if (!is_ebb_event(event) || current->thread.used_ebb)
> + return;
> +
> + /*
> + * IFF this is the first time we've added an EBB event, set
> + * PMXE in the user MMCR0 so we can detect when it's cleared by
> + * userspace. We need this so that we can context switch while
> + * userspace is in the EBB handler (where PMXE is 0).
> + */
> + current->thread.used_ebb = 1;
> + current->thread.mmcr0 |= MMCR0_PMXE;
> +}
> +
> +static void ebb_switch_out(unsigned long mmcr0)
> +{
> + if (!(mmcr0 & MMCR0_EBE))
> + return;
> +
> + current->thread.siar = mfspr(SPRN_SIAR);
> + current->thread.sier = mfspr(SPRN_SIER);
> + current->thread.sdar = mfspr(SPRN_SDAR);
> + current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
> + current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
> +}
> +
We also need to filter sier value for SIER_USER_MASK, right ?
> +static unsigned long ebb_switch_in(bool ebb, unsigned long mmcr0)
> +{
> + if (!ebb)
> + goto out;
> +
> + /* Enable EBB and read/write to all 6 PMCs for userspace */
> + mmcr0 |= MMCR0_EBE | MMCR0_PMCC_U6;
> +
> + /* Add any bits from the user reg, FC or PMAO */
> + mmcr0 |= current->thread.mmcr0;
> +
> + /* Be careful not to set PMXE if userspace had it cleared */
> + if (!(current->thread.mmcr0 & MMCR0_PMXE))
> + mmcr0 &= ~MMCR0_PMXE;
> +
> + mtspr(SPRN_SIAR, current->thread.siar);
> + mtspr(SPRN_SIER, current->thread.sier);
> + mtspr(SPRN_SDAR, current->thread.sdar);
> + mtspr(SPRN_MMCR2, current->thread.mmcr2);
> +out:
> + return mmcr0;
> +}
> #endif /* CONFIG_PPC64 */
>
> static void perf_event_interrupt(struct pt_regs *regs);
> @@ -734,6 +829,13 @@ static void power_pmu_read(struct perf_event *event)
>
> if (!event->hw.idx)
> return;
> +
> + if (is_ebb_event(event)) {
> + val = read_pmc(event->hw.idx);
> + local64_set(&event->hw.prev_count, val);
> + return;
> + }
> +
> /*
> * Performance monitor interrupts come even when interrupts
> * are soft-disabled, as long as interrupts are hard-enabled.
> @@ -854,7 +956,7 @@ static void write_mmcr0(struct cpu_hw_events *cpuhw, unsigned long mmcr0)
> static void power_pmu_disable(struct pmu *pmu)
> {
> struct cpu_hw_events *cpuhw;
> - unsigned long flags, val;
> + unsigned long flags, mmcr0, val;
>
> if (!ppmu)
> return;
> @@ -871,11 +973,11 @@ static void power_pmu_disable(struct pmu *pmu)
> }
>
> /*
> - * Set the 'freeze counters' bit, clear PMAO/FC56.
> + * Set the 'freeze counters' bit, clear EBE/PMCC/PMAO/FC56.
> */
> - val = mfspr(SPRN_MMCR0);
> + val = mmcr0 = mfspr(SPRN_MMCR0);
> val |= MMCR0_FC;
> - val &= ~(MMCR0_PMAO | MMCR0_FC56);
> + val &= ~(MMCR0_EBE | MMCR0_PMCC | MMCR0_PMAO | MMCR0_FC56);
>
> /*
> * The barrier is to make sure the mtspr has been
> @@ -896,7 +998,10 @@ static void power_pmu_disable(struct pmu *pmu)
>
> cpuhw->disabled = 1;
> cpuhw->n_added = 0;
> +
> + ebb_switch_out(mmcr0);
> }
> +
> local_irq_restore(flags);
> }
>
> @@ -911,15 +1016,15 @@ static void power_pmu_enable(struct pmu *pmu)
> struct cpu_hw_events *cpuhw;
> unsigned long flags;
> long i;
> - unsigned long val;
> + unsigned long val, mmcr0;
> s64 left;
> unsigned int hwc_index[MAX_HWEVENTS];
> int n_lim;
> int idx;
> + bool ebb;
>
> if (!ppmu)
> return;
> -
> local_irq_save(flags);
>
> cpuhw = &__get_cpu_var(cpu_hw_events);
> @@ -934,6 +1039,13 @@ static void power_pmu_enable(struct pmu *pmu)
> cpuhw->disabled = 0;
>
> /*
> + * EBB requires an exclusive group and all events must have the EBB
> + * flag set, or not set, so we can just check a single event. Also we
> + * know we have at least one event.
> + */
> + ebb = is_ebb_event(cpuhw->event[0]);
> +
> + /*
> * If we didn't change anything, or only removed events,
> * no need to recalculate MMCR* settings and reset the PMCs.
> * Just reenable the PMU with the current MMCR* settings
> @@ -1008,25 +1120,34 @@ static void power_pmu_enable(struct pmu *pmu)
> ++n_lim;
> continue;
> }
> - val = 0;
> - if (event->hw.sample_period) {
> - left = local64_read(&event->hw.period_left);
> - if (left < 0x80000000L)
> - val = 0x80000000L - left;
> +
> + if (ebb)
> + val = local64_read(&event->hw.prev_count);
> + else {
> + val = 0;
> + if (event->hw.sample_period) {
> + left = local64_read(&event->hw.period_left);
> + if (left < 0x80000000L)
> + val = 0x80000000L - left;
> + }
> + local64_set(&event->hw.prev_count, val);
> }
> - local64_set(&event->hw.prev_count, val);
> +
> event->hw.idx = idx;
> if (event->hw.state & PERF_HES_STOPPED)
> val = 0;
> write_pmc(idx, val);
> +
> perf_event_update_userpage(event);
> }
> cpuhw->n_limited = n_lim;
> cpuhw->mmcr[0] |= MMCR0_PMXE | MMCR0_FCECE;
>
> out_enable:
> + mmcr0 = ebb_switch_in(ebb, cpuhw->mmcr[0]);
> +
> mb();
> - write_mmcr0(cpuhw, cpuhw->mmcr[0]);
> + write_mmcr0(cpuhw, mmcr0);
>
> /*
> * Enable instruction sampling if necessary
> @@ -1124,6 +1245,8 @@ static int power_pmu_add(struct perf_event *event, int ef_flags)
> event->hw.config = cpuhw->events[n0];
>
> nocheck:
> + ebb_event_add(event);
> +
> ++cpuhw->n_events;
> ++cpuhw->n_added;
>
> @@ -1484,6 +1607,11 @@ static int power_pmu_event_init(struct perf_event *event)
> }
> }
>
> + /* Extra checks for EBB */
> + err = ebb_event_check(event);
> + if (err)
> + return err;
> +
> /*
> * If this is in a group, check if it can go on with all the
> * other hardware events in the group. We assume the event
> @@ -1523,6 +1651,13 @@ static int power_pmu_event_init(struct perf_event *event)
> local64_set(&event->hw.period_left, event->hw.last_period);
>
> /*
> + * For EBB events we just context switch the PMC value, we don't do any
> + * of the sample_period logic. We use hw.prev_count for this.
> + */
> + if (is_ebb_event(event))
> + local64_set(&event->hw.prev_count, 0);
> +
> + /*
> * See if we need to reserve the PMU.
> * If no events are currently in use, then we have to take a
> * mutex to ensure that we don't race with another task doing
>
^ permalink raw reply
* RE: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction
From: Bhushan Bharat-R65777 @ 2013-06-26 8:44 UTC (permalink / raw)
To: tiejun.chen
Cc: Wood Scott-B07421, mikey@neuling.org, kvm@vger.kernel.org,
agraf@suse.de, kvm-ppc@vger.kernel.org,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <51CA903A.4070809@windriver.com>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogdGllanVuLmNoZW4gW21h
aWx0bzp0aWVqdW4uY2hlbkB3aW5kcml2ZXIuY29tXQ0KPiBTZW50OiBXZWRuZXNkYXksIEp1bmUg
MjYsIDIwMTMgMTI6MjUgUE0NCj4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0KPiBDYzoga3Zt
LXBwY0B2Z2VyLmtlcm5lbC5vcmc7IGt2bUB2Z2VyLmtlcm5lbC5vcmc7IGFncmFmQHN1c2UuZGU7
IFdvb2QgU2NvdHQtDQo+IEIwNzQyMTsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOyBsaW51eHBw
Yy1kZXZAbGlzdHMub3psYWJzLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7
IG1pa2V5QG5ldWxpbmcub3JnOyBCaHVzaGFuIEJoYXJhdC1SNjU3NzcNCj4gU3ViamVjdDogUmU6
IFtQQVRDSCA0LzYgdjVdIEtWTTogUFBDOiBleGl0IHRvIHVzZXIgc3BhY2Ugb24gImVocHJpdiIg
aW5zdHJ1Y3Rpb24NCj4gDQo+IE9uIDA2LzI2LzIwMTMgMDE6NDIgUE0sIEJoYXJhdCBCaHVzaGFu
IHdyb3RlOg0KPiA+ICJlaHByaXYiIGluc3RydWN0aW9uIGlzIHVzZWQgZm9yIHNldHRpbmcgc29m
dHdhcmUgYnJlYWtwb2ludHMNCj4gPiBieSB1c2VyIHNwYWNlLiBUaGlzIHBhdGNoIGFkZHMgc3Vw
cG9ydCB0byBleGl0IHRvIHVzZXIgc3BhY2UNCj4gPiB3aXRoICJydW4tPmRlYnVnIiBoYXZlIHJl
bGV2YW50IGluZm9ybWF0aW9uLg0KPiA+DQo+ID4gQXMgdGhpcyBpcyB0aGUgZmlyc3QgcG9pbnQg
d2UgYXJlIHVzaW5nIHJ1bi0+ZGVidWcsIGFsc28gZGVmaW5lZA0KPiA+IHRoZSBydW4tPmRlYnVn
IHN0cnVjdHVyZS4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6IEJoYXJhdCBCaHVzaGFuIDxiaGFy
YXQuYmh1c2hhbkBmcmVlc2NhbGUuY29tPg0KPiA+IC0tLQ0KPiA+ICAgYXJjaC9wb3dlcnBjL2lu
Y2x1ZGUvYXNtL2Rpc2Fzc2VtYmxlLmggfCAgICA0ICsrKysNCj4gPiAgIGFyY2gvcG93ZXJwYy9p
bmNsdWRlL3VhcGkvYXNtL2t2bS5oICAgIHwgICAyMSArKysrKysrKysrKysrKysrKy0tLS0NCj4g
PiAgIGFyY2gvcG93ZXJwYy9rdm0vZTUwMF9lbXVsYXRlLmMgICAgICAgIHwgICAyNyArKysrKysr
KysrKysrKysrKysrKysrKysrKysNCj4gPiAgIDMgZmlsZXMgY2hhbmdlZCwgNDggaW5zZXJ0aW9u
cygrKSwgNCBkZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMv
aW5jbHVkZS9hc20vZGlzYXNzZW1ibGUuaA0KPiBiL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9k
aXNhc3NlbWJsZS5oDQo+ID4gaW5kZXggOWIxOThkMS4uODU2ZjhkZSAxMDA2NDQNCj4gPiAtLS0g
YS9hcmNoL3Bvd2VycGMvaW5jbHVkZS9hc20vZGlzYXNzZW1ibGUuaA0KPiA+ICsrKyBiL2FyY2gv
cG93ZXJwYy9pbmNsdWRlL2FzbS9kaXNhc3NlbWJsZS5oDQo+ID4gQEAgLTc3LDQgKzc3LDggQEAg
c3RhdGljIGlubGluZSB1bnNpZ25lZCBpbnQgZ2V0X2QodTMyIGluc3QpDQo+ID4gICAJcmV0dXJu
IGluc3QgJiAweGZmZmY7DQo+ID4gICB9DQo+ID4NCj4gPiArc3RhdGljIGlubGluZSB1bnNpZ25l
ZCBpbnQgZ2V0X29jKHUzMiBpbnN0KQ0KPiA+ICt7DQo+ID4gKwlyZXR1cm4gKGluc3QgPj4gMTEp
ICYgMHg3ZmZmOw0KPiA+ICt9DQo+ID4gICAjZW5kaWYgLyogX19BU01fUFBDX0RJU0FTU0VNQkxF
X0hfXyAqLw0KPiA+IGRpZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMvaW5jbHVkZS91YXBpL2FzbS9r
dm0uaA0KPiBiL2FyY2gvcG93ZXJwYy9pbmNsdWRlL3VhcGkvYXNtL2t2bS5oDQo+ID4gaW5kZXgg
MGZiMWE2ZS4uZGVkMDYwNyAxMDA2NDQNCj4gPiAtLS0gYS9hcmNoL3Bvd2VycGMvaW5jbHVkZS91
YXBpL2FzbS9rdm0uaA0KPiA+ICsrKyBiL2FyY2gvcG93ZXJwYy9pbmNsdWRlL3VhcGkvYXNtL2t2
bS5oDQo+ID4gQEAgLTI2OSw3ICsyNjksMjQgQEAgc3RydWN0IGt2bV9mcHUgew0KPiA+ICAgCV9f
dTY0IGZwclszMl07DQo+ID4gICB9Ow0KPiA+DQo+ID4gKy8qDQo+ID4gKyAqIERlZmluZXMgZm9y
IGgvdyBicmVha3BvaW50LCB3YXRjaHBvaW50IChyZWFkLCB3cml0ZSBvciBib3RoKSBhbmQNCj4g
PiArICogc29mdHdhcmUgYnJlYWtwb2ludC4NCj4gPiArICogVGhlc2UgYXJlIHVzZWQgYXMgInR5
cGUiIGluIEtWTV9TRVRfR1VFU1RfREVCVUcgaW9jdGwgYW5kICJzdGF0dXMiDQo+ID4gKyAqIGZv
ciBLVk1fREVCVUdfRVhJVC4NCj4gPiArICovDQo+ID4gKyNkZWZpbmUgS1ZNUFBDX0RFQlVHX05P
TkUJCTB4MA0KPiA+ICsjZGVmaW5lIEtWTVBQQ19ERUJVR19CUkVBS1BPSU5UCQkoMVVMIDw8IDEp
DQo+ID4gKyNkZWZpbmUgS1ZNUFBDX0RFQlVHX1dBVENIX1dSSVRFCSgxVUwgPDwgMikNCj4gPiAr
I2RlZmluZSBLVk1QUENfREVCVUdfV0FUQ0hfUkVBRAkJKDFVTCA8PCAzKQ0KPiA+ICAgc3RydWN0
IGt2bV9kZWJ1Z19leGl0X2FyY2ggew0KPiA+ICsJX191NjQgYWRkcmVzczsNCj4gPiArCS8qDQo+
ID4gKwkgKiBleGl0aW5nIHRvIHVzZXJzcGFjZSBiZWNhdXNlIG9mIGgvdyBicmVha3BvaW50LCB3
YXRjaHBvaW50DQo+ID4gKwkgKiAocmVhZCwgd3JpdGUgb3IgYm90aCkgYW5kIHNvZnR3YXJlIGJy
ZWFrcG9pbnQuDQo+ID4gKwkgKi8NCj4gPiArCV9fdTMyIHN0YXR1czsNCj4gPiArCV9fdTMyIHJl
c2VydmVkOw0KPiA+ICAgfTsNCj4gPg0KPiA+ICAgLyogZm9yIEtWTV9TRVRfR1VFU1RfREVCVUcg
Ki8NCj4gPiBAQCAtMjgxLDEwICsyOTgsNiBAQCBzdHJ1Y3Qga3ZtX2d1ZXN0X2RlYnVnX2FyY2gg
ew0KPiA+ICAgCQkgKiBUeXBlIGRlbm90ZXMgaC93IGJyZWFrcG9pbnQsIHJlYWQgd2F0Y2hwb2lu
dCwgd3JpdGUNCj4gPiAgIAkJICogd2F0Y2hwb2ludCBvciB3YXRjaHBvaW50IChib3RoIHJlYWQg
YW5kIHdyaXRlKS4NCj4gPiAgIAkJICovDQo+ID4gLSNkZWZpbmUgS1ZNUFBDX0RFQlVHX05PTkUJ
CTB4MA0KPiA+IC0jZGVmaW5lIEtWTVBQQ19ERUJVR19CUkVBS1BPSU5UCQkoMVVMIDw8IDEpDQo+
ID4gLSNkZWZpbmUgS1ZNUFBDX0RFQlVHX1dBVENIX1dSSVRFCSgxVUwgPDwgMikNCj4gPiAtI2Rl
ZmluZSBLVk1QUENfREVCVUdfV0FUQ0hfUkVBRAkJKDFVTCA8PCAzKQ0KPiA+ICAgCQlfX3UzMiB0
eXBlOw0KPiA+ICAgCQlfX3UzMiByZXNlcnZlZDsNCj4gPiAgIAl9IGJwWzE2XTsNCj4gPiBkaWZm
IC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2t2bS9lNTAwX2VtdWxhdGUuYyBiL2FyY2gvcG93ZXJwYy9r
dm0vZTUwMF9lbXVsYXRlLmMNCj4gPiBpbmRleCBiMTBhMDEyLi5kYWI5ZDA3IDEwMDY0NA0KPiA+
IC0tLSBhL2FyY2gvcG93ZXJwYy9rdm0vZTUwMF9lbXVsYXRlLmMNCj4gPiArKysgYi9hcmNoL3Bv
d2VycGMva3ZtL2U1MDBfZW11bGF0ZS5jDQo+ID4gQEAgLTI2LDYgKzI2LDggQEANCj4gPiAgICNk
ZWZpbmUgWE9QX1RMQlJFICAgOTQ2DQo+ID4gICAjZGVmaW5lIFhPUF9UTEJXRSAgIDk3OA0KPiA+
ICAgI2RlZmluZSBYT1BfVExCSUxYICAxOA0KPiA+ICsjZGVmaW5lIFhPUF9FSFBSSVYgIDI3MA0K
PiA+ICsjZGVmaW5lIEVIUFJJVl9PQ19ERUJVRyAwDQo+IA0KPiBBcyBJIHRoaW5rIHRoZSBjYXNl
LCAiT0MgPSAwIiwgaXMgYSBiaXQgc3BlY2lmaWMgc2luY2UgSUlSQywgaWYgdGhlIE9DDQo+IG9w
ZXJhbmQgaXMgb21pdHRlZCwgaXRzIGVxdWFsIDAgYnkgZGVmYXVsdC4gU28gSSB0aGluayB3ZSBz
aG91bGQgc3RhcnQgdGhpcyBPQw0KPiB2YWx1ZSBmcm9tIDEgb3Igb3RoZXIgbWFnaWMgbnVtYmVy
Lg0KDQplaHByaXYgaW5zdHJ1Y3Rpb24gaXMgZGVmaW5lZCB0byBiZSB1c2VkIGFzOg0KCWVocHJp
diBPQyAvLyB3aGVyZSBPQyBjYW4gYmUgMCwxLCAuLi4gbg0KYW5kIGluIGV4dGVuZGVkIGZvciBp
dCBjYW4gYmUgdXNlZCBhcw0KCWVocHJpdiAvLyBXaXRoIG5vIE9DLCBhbmQgaGVyZSBpdCBhc3N1
bWVzIE9DID0gMA0KU28gT0MgPSAwIGlzIG5vdCBzcGVjaWZpYyBidXQgImVocHJpdiIgaXMgc2Ft
ZSBhcyAiZWhwcml2IDAiLg0KDQpJIGRvIG5vdCB0aGluayBvZiBhbnkgc3BlY2lhbCByZWFzb24g
dG8gcmVzZXJ2ZSAiZWhwcml2IiBhbmQgImVocHJpdiAwIi4NCg0KVGhhbmtzDQotQmhhcmF0DQoN
Cj4gDQo+IEFuZCBpZiBwb3NzaWJsZSwgd2UnZCBiZXR0ZXIgYWRkIHNvbWUgY29tbWVudHMgdG8g
ZGVzY3JpYmUgdGhpcyB0byBtYWtlIHRoZSBPQw0KPiBkZWZpbml0aW9uIHJlYWRhYmxlLg0KPiAN
Cj4gVGllanVuDQo+IA0KPiA+DQo+ID4gICAjaWZkZWYgQ09ORklHX0tWTV9FNTAwTUMNCj4gPiAg
IHN0YXRpYyBpbnQgZGJlbGwycHJpbyh1bG9uZyBwYXJhbSkNCj4gPiBAQCAtODIsNiArODQsMjYg
QEAgc3RhdGljIGludCBrdm1wcGNfZTUwMF9lbXVsX21zZ3NuZChzdHJ1Y3Qga3ZtX3ZjcHUgKnZj
cHUsDQo+IGludCByYikNCj4gPiAgIH0NCj4gPiAgICNlbmRpZg0KPiA+DQo+ID4gK3N0YXRpYyBp
bnQga3ZtcHBjX2U1MDBfZW11bF9laHByaXYoc3RydWN0IGt2bV9ydW4gKnJ1biwgc3RydWN0IGt2
bV92Y3B1DQo+ICp2Y3B1LA0KPiA+ICsJCQkJICAgdW5zaWduZWQgaW50IGluc3QsIGludCAqYWR2
YW5jZSkNCj4gPiArew0KPiA+ICsJaW50IGVtdWxhdGVkID0gRU1VTEFURV9ET05FOw0KPiA+ICsN
Cj4gPiArCXN3aXRjaCAoZ2V0X29jKGluc3QpKSB7DQo+ID4gKwljYXNlIEVIUFJJVl9PQ19ERUJV
RzoNCj4gPiArCQlydW4tPmV4aXRfcmVhc29uID0gS1ZNX0VYSVRfREVCVUc7DQo+ID4gKwkJcnVu
LT5kZWJ1Zy5hcmNoLmFkZHJlc3MgPSB2Y3B1LT5hcmNoLnBjOw0KPiA+ICsJCXJ1bi0+ZGVidWcu
YXJjaC5zdGF0dXMgPSAwOw0KPiA+ICsJCWt2bXBwY19hY2NvdW50X2V4aXQodmNwdSwgREVCVUdf
RVhJVFMpOw0KPiA+ICsJCWVtdWxhdGVkID0gRU1VTEFURV9FWElUX1VTRVI7DQo+ID4gKwkJKmFk
dmFuY2UgPSAwOw0KPiA+ICsJCWJyZWFrOw0KPiA+ICsJZGVmYXVsdDoNCj4gPiArCQllbXVsYXRl
ZCA9IEVNVUxBVEVfRkFJTDsNCj4gPiArCX0NCj4gPiArCXJldHVybiBlbXVsYXRlZDsNCj4gPiAr
fQ0KPiA+ICsNCj4gPiAgIGludCBrdm1wcGNfY29yZV9lbXVsYXRlX29wKHN0cnVjdCBrdm1fcnVu
ICpydW4sIHN0cnVjdCBrdm1fdmNwdSAqdmNwdSwNCj4gPiAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIHVuc2lnbmVkIGludCBpbnN0LCBpbnQgKmFkdmFuY2UpDQo+ID4gICB7DQo+ID4gQEAg
LTEzMCw2ICsxNTIsMTEgQEAgaW50IGt2bXBwY19jb3JlX2VtdWxhdGVfb3Aoc3RydWN0IGt2bV9y
dW4gKnJ1biwgc3RydWN0DQo+IGt2bV92Y3B1ICp2Y3B1LA0KPiA+ICAgCQkJZW11bGF0ZWQgPSBr
dm1wcGNfZTUwMF9lbXVsX3RsYml2YXgodmNwdSwgZWEpOw0KPiA+ICAgCQkJYnJlYWs7DQo+ID4N
Cj4gPiArCQljYXNlIFhPUF9FSFBSSVY6DQo+ID4gKwkJCWVtdWxhdGVkID0ga3ZtcHBjX2U1MDBf
ZW11bF9laHByaXYocnVuLCB2Y3B1LCBpbnN0LA0KPiA+ICsJCQkJCQkJICAgYWR2YW5jZSk7DQo+
ID4gKwkJCWJyZWFrOw0KPiA+ICsNCj4gPiAgIAkJZGVmYXVsdDoNCj4gPiAgIAkJCWVtdWxhdGVk
ID0gRU1VTEFURV9GQUlMOw0KPiA+ICAgCQl9DQo+ID4NCj4gDQoNCg==
^ permalink raw reply
* Re: [PATCH v2 29/45] kvm/vmx: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-26 8:41 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, peterz, fweisbec, oleg, H. Peter Anvin, walken, mingo,
linux-arch, vincent.guittot, Gleb Natapov, x86, xiaoguangrong,
Ingo Molnar, wangyun, paulmck, nikunj, linux-pm, rusty, rostedt,
namhyung, tglx, laijs, zhong, netdev, linux-kernel, sbw, tj, akpm,
linuxppc-dev
In-Reply-To: <51CAA514.8050609@redhat.com>
On 06/26/2013 01:53 PM, Paolo Bonzini wrote:
> Il 26/06/2013 10:06, Srivatsa S. Bhat ha scritto:
>> On 06/26/2013 01:16 PM, Paolo Bonzini wrote:
>>> Il 25/06/2013 22:30, Srivatsa S. Bhat ha scritto:
>>>> - cpu = get_cpu();
>>>> + cpu = get_online_cpus_atomic();
>>>> vmx_vcpu_load(&vmx->vcpu, cpu);
>>>> vmx->vcpu.cpu = cpu;
>>>> err = vmx_vcpu_setup(vmx);
>>>> vmx_vcpu_put(&vmx->vcpu);
>>>> - put_cpu();
>>>> + put_online_cpus_atomic();
>>>
>>> The new API has a weird name. Why are you adding new functions instead
>>> of just modifying get/put_cpu?
>>>
>>
>> Because the purpose of those two functions are distinctly different
>> from each other.
>>
>> get/put_cpu() is used to disable preemption on the local CPU. (Which
>> also disables offlining the local CPU during that critical section).
>
> Ok, then I understood correctly... and I acked the other KVM patch.
>
Thank you!
> However, keeping the code on the local CPU is exactly the point of this
> particular use of get_cpu()/put_cpu(). Why does it need to synchronize
> with offlining of other CPUs?
>
Now that I looked at it again, I think you are right, get/put_cpu() is
good enough here.
But let me explain why I initially thought we needed full synchronization
with CPU offline. In short, I wanted to synchronize the calls to
__loaded_vmcs_clear(). We have the scenario shown below:
CPU offline:
CPU_DYING:
hardware_disable();
->vmclear_local_loaded_vmcss();
->__loaded_vmcs_clear(v);
And vmx_vcpu_load() (among others) can do:
vmx_vcpu_load();
-> loaded_vmcs_clear();
-> __loaded_vmcs_clear();
So I wanted to avoid this race-condition and hence wrapped the code with
get/put_online_cpus_atomic().
But the point I missed earlier is that loaded_vmcs_clear() calls
__loaded_vmcs_clear() using smp_call_function_single(), which itself
synchronizes properly with CPU hotplug. So there is no need to add full
hotplug synchronization in the vmx code, as you noted above.
So, please ignore this patch, and sorry for the noise!
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH v2 29/45] kvm/vmx: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Paolo Bonzini @ 2013-06-26 8:57 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: kvm, peterz, fweisbec, oleg, H. Peter Anvin, walken, mingo,
linux-arch, vincent.guittot, Gleb Natapov, x86, xiaoguangrong,
Ingo Molnar, wangyun, paulmck, nikunj, linux-pm, rusty, rostedt,
namhyung, tglx, laijs, zhong, netdev, linux-kernel, sbw, tj, akpm,
linuxppc-dev
In-Reply-To: <51CAA921.7030906@linux.vnet.ibm.com>
Il 26/06/2013 10:41, Srivatsa S. Bhat ha scritto:
> On 06/26/2013 01:53 PM, Paolo Bonzini wrote:
>> Il 26/06/2013 10:06, Srivatsa S. Bhat ha scritto:
>>> On 06/26/2013 01:16 PM, Paolo Bonzini wrote:
>>>> Il 25/06/2013 22:30, Srivatsa S. Bhat ha scritto:
>>>>> - cpu = get_cpu();
>>>>> + cpu = get_online_cpus_atomic();
>>>>> vmx_vcpu_load(&vmx->vcpu, cpu);
>>>>> vmx->vcpu.cpu = cpu;
>>>>> err = vmx_vcpu_setup(vmx);
>>>>> vmx_vcpu_put(&vmx->vcpu);
>>>>> - put_cpu();
>>>>> + put_online_cpus_atomic();
>>>>
>>>> The new API has a weird name. Why are you adding new functions instead
>>>> of just modifying get/put_cpu?
>>>>
>>>
>>> Because the purpose of those two functions are distinctly different
>>> from each other.
>>>
>>> get/put_cpu() is used to disable preemption on the local CPU. (Which
>>> also disables offlining the local CPU during that critical section).
>>
>> Ok, then I understood correctly... and I acked the other KVM patch.
>>
>
> Thank you!
>
>> However, keeping the code on the local CPU is exactly the point of this
>> particular use of get_cpu()/put_cpu(). Why does it need to synchronize
>> with offlining of other CPUs?
>
> Now that I looked at it again, I think you are right, get/put_cpu() is
> good enough here.
>
> But let me explain why I initially thought we needed full synchronization
> with CPU offline. In short, I wanted to synchronize the calls to
> __loaded_vmcs_clear(). We have the scenario shown below:
>
> CPU offline:
> CPU_DYING:
> hardware_disable();
> ->vmclear_local_loaded_vmcss();
> ->__loaded_vmcs_clear(v);
>
>
>
> And vmx_vcpu_load() (among others) can do:
> vmx_vcpu_load();
> -> loaded_vmcs_clear();
> -> __loaded_vmcs_clear();
>
>
> So I wanted to avoid this race-condition and hence wrapped the code with
> get/put_online_cpus_atomic().
>
> But the point I missed earlier is that loaded_vmcs_clear() calls
> __loaded_vmcs_clear() using smp_call_function_single(), which itself
> synchronizes properly with CPU hotplug. So there is no need to add full
> hotplug synchronization in the vmx code, as you noted above.
Makes sense, and I see now that it's patch 9 in this series.
In general, I would rather add an extra get_online_cpus_atomic pair
where it it actually needed (i.e. closer to where cpu_online is actually
used), and leave get_cpu/put_cpu as is in the caller... which is exactly
what happens in this case, since "where it is actually needed" is "in
smp_call_function_single()".
> So, please ignore this patch, and sorry for the noise!
No problem, thanks for the heads-up.
Paolo
^ permalink raw reply
* RE: [PATCH 3/6] powerpc/powernv: Replace variables with flags
From: David Laight @ 2013-06-26 9:12 UTC (permalink / raw)
To: Gavin Shan, linuxppc-dev
In-Reply-To: <1372210688-12214-4-git-send-email-shangw@linux.vnet.ibm.com>
> We have 2 fields in "struct pnv_phb" to trace the states. The patch
> replace the fields with one and introduces flags for that. The patch
> doesn't impact the logic.
What is the benefit of this change?
...
> +
> +#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */
> +#define PNV_EEH_STATE_REMOVED (1 << 1) /* PHB removed */
> +
> #endif /* CONFIG_EEH */
>=20
> struct pnv_phb {
> @@ -92,8 +96,7 @@ struct pnv_phb {
>=20
> #ifdef CONFIG_EEH
> struct pnv_eeh_ops *eeh_ops;
> - int eeh_enabled;
> - int removed;
> + int eeh_state;
> #endif
>=20
> #ifdef CONFIG_DEBUG_FS
All I can see is that it possibly reduces a structure by 4 bytes
while adding extra code.
(On 64 bit systems there might be a 4 byte pad added.)
David
^ permalink raw reply
* Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction
From: tiejun.chen @ 2013-06-26 9:17 UTC (permalink / raw)
To: Bhushan Bharat-R65777
Cc: Wood Scott-B07421, mikey@neuling.org, kvm@vger.kernel.org,
agraf@suse.de, kvm-ppc@vger.kernel.org,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07086C2F@039-SN2MPN1-013.039d.mgd.msft.net>
On 06/26/2013 04:44 PM, Bhushan Bharat-R65777 wrote:
>
>
>> -----Original Message-----
>> From: tiejun.chen [mailto:tiejun.chen@windriver.com]
>> Sent: Wednesday, June 26, 2013 12:25 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Wood Scott-
>> B07421; benh@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; linux-
>> kernel@vger.kernel.org; mikey@neuling.org; Bhushan Bharat-R65777
>> Subject: Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction
>>
>> On 06/26/2013 01:42 PM, Bharat Bhushan wrote:
>>> "ehpriv" instruction is used for setting software breakpoints
>>> by user space. This patch adds support to exit to user space
>>> with "run->debug" have relevant information.
>>>
>>> As this is the first point we are using run->debug, also defined
>>> the run->debug structure.
>>>
>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
>>> ---
>>> arch/powerpc/include/asm/disassemble.h | 4 ++++
>>> arch/powerpc/include/uapi/asm/kvm.h | 21 +++++++++++++++++----
>>> arch/powerpc/kvm/e500_emulate.c | 27 +++++++++++++++++++++++++++
>>> 3 files changed, 48 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/disassemble.h
>> b/arch/powerpc/include/asm/disassemble.h
>>> index 9b198d1..856f8de 100644
>>> --- a/arch/powerpc/include/asm/disassemble.h
>>> +++ b/arch/powerpc/include/asm/disassemble.h
>>> @@ -77,4 +77,8 @@ static inline unsigned int get_d(u32 inst)
>>> return inst & 0xffff;
>>> }
>>>
>>> +static inline unsigned int get_oc(u32 inst)
>>> +{
>>> + return (inst >> 11) & 0x7fff;
>>> +}
>>> #endif /* __ASM_PPC_DISASSEMBLE_H__ */
>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h
>> b/arch/powerpc/include/uapi/asm/kvm.h
>>> index 0fb1a6e..ded0607 100644
>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>>> @@ -269,7 +269,24 @@ struct kvm_fpu {
>>> __u64 fpr[32];
>>> };
>>>
>>> +/*
>>> + * Defines for h/w breakpoint, watchpoint (read, write or both) and
>>> + * software breakpoint.
>>> + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status"
>>> + * for KVM_DEBUG_EXIT.
>>> + */
>>> +#define KVMPPC_DEBUG_NONE 0x0
>>> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
>>> +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
>>> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
>>> struct kvm_debug_exit_arch {
>>> + __u64 address;
>>> + /*
>>> + * exiting to userspace because of h/w breakpoint, watchpoint
>>> + * (read, write or both) and software breakpoint.
>>> + */
>>> + __u32 status;
>>> + __u32 reserved;
>>> };
>>>
>>> /* for KVM_SET_GUEST_DEBUG */
>>> @@ -281,10 +298,6 @@ struct kvm_guest_debug_arch {
>>> * Type denotes h/w breakpoint, read watchpoint, write
>>> * watchpoint or watchpoint (both read and write).
>>> */
>>> -#define KVMPPC_DEBUG_NONE 0x0
>>> -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
>>> -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
>>> -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
>>> __u32 type;
>>> __u32 reserved;
>>> } bp[16];
>>> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
>>> index b10a012..dab9d07 100644
>>> --- a/arch/powerpc/kvm/e500_emulate.c
>>> +++ b/arch/powerpc/kvm/e500_emulate.c
>>> @@ -26,6 +26,8 @@
>>> #define XOP_TLBRE 946
>>> #define XOP_TLBWE 978
>>> #define XOP_TLBILX 18
>>> +#define XOP_EHPRIV 270
>>> +#define EHPRIV_OC_DEBUG 0
>>
>> As I think the case, "OC = 0", is a bit specific since IIRC, if the OC
>> operand is omitted, its equal 0 by default. So I think we should start this OC
>> value from 1 or other magic number.
>
> ehpriv instruction is defined to be used as:
> ehpriv OC // where OC can be 0,1, ... n
> and in extended for it can be used as
> ehpriv // With no OC, and here it assumes OC = 0
> So OC = 0 is not specific but "ehpriv" is same as "ehpriv 0".
Yes, this is just what I mean.
>
> I do not think of any special reason to reserve "ehpriv" and "ehpriv 0".
So I still prefer we can reserve the 'ehpriv' without OC operand as one simple
approach to test or develop something for KVM quickly because its really
convenient to trap into the hypervisor only with one 'ehpriv' instruction easily.
But I have no further objection if you guys are fine to this ;-)
Tiejun
>
> Thanks
> -Bharat
>
>>
>> And if possible, we'd better add some comments to describe this to make the OC
>> definition readable.
>>
>> Tiejun
>>
>>>
>>> #ifdef CONFIG_KVM_E500MC
>>> static int dbell2prio(ulong param)
>>> @@ -82,6 +84,26 @@ static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu,
>> int rb)
>>> }
>>> #endif
>>>
>>> +static int kvmppc_e500_emul_ehpriv(struct kvm_run *run, struct kvm_vcpu
>> *vcpu,
>>> + unsigned int inst, int *advance)
>>> +{
>>> + int emulated = EMULATE_DONE;
>>> +
>>> + switch (get_oc(inst)) {
>>> + case EHPRIV_OC_DEBUG:
>>> + run->exit_reason = KVM_EXIT_DEBUG;
>>> + run->debug.arch.address = vcpu->arch.pc;
>>> + run->debug.arch.status = 0;
>>> + kvmppc_account_exit(vcpu, DEBUG_EXITS);
>>> + emulated = EMULATE_EXIT_USER;
>>> + *advance = 0;
>>> + break;
>>> + default:
>>> + emulated = EMULATE_FAIL;
>>> + }
>>> + return emulated;
>>> +}
>>> +
>>> int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>> unsigned int inst, int *advance)
>>> {
>>> @@ -130,6 +152,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct
>> kvm_vcpu *vcpu,
>>> emulated = kvmppc_e500_emul_tlbivax(vcpu, ea);
>>> break;
>>>
>>> + case XOP_EHPRIV:
>>> + emulated = kvmppc_e500_emul_ehpriv(run, vcpu, inst,
>>> + advance);
>>> + break;
>>> +
>>> default:
>>> emulated = EMULATE_FAIL;
>>> }
>>>
>>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox