[PATCH 2/2 v6] KVM: PPC: booke: Add watchdog emulation
This patch adds the watchdog emulation in KVM. The watchdog
emulation is enabled by KVM_ENABLE_CAP(KVM_CAP_PPC_BOOKE_WATCHDOG) ioctl.
The kernel timer are used for watchdog emulation and emulates
h/w watchdog state machine. On watchdog timer expiry, it exit to QEMU
if TCR.WRC is non ZERO. QEMU can reset/shutdown etc depending upon how
it is configured.
Signed-off-by: Liu Yu
Signed-off-by: Scott Wood
[bharat.bhus...@freescale.com: reworked patch]
Signed-off-by: Bharat Bhushan
---
v6:
- Added kvmppc_subarch_vcpu_unit/uninit() for subarch specific initialization
- Using spin_lock_irqsave()
- Using del_timer_sync().
v5:
- Checking that TSR_ENW/TSR_WIS are still set on KVM_EXIT_WATCHDOG.
- Moved watchdog_next_timeout() in lock.
arch/powerpc/include/asm/kvm_host.h |3 +
arch/powerpc/include/asm/kvm_ppc.h |4 +
arch/powerpc/include/asm/reg_booke.h |7 ++
arch/powerpc/kvm/book3s.c|9 ++
arch/powerpc/kvm/booke.c | 156 ++
arch/powerpc/kvm/booke_emulate.c |8 ++
arch/powerpc/kvm/powerpc.c | 14 +++-
include/linux/kvm.h |2 +
include/linux/kvm_host.h |1 +
9 files changed, 202 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 50ea12f..43cac48 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -467,6 +467,8 @@ struct kvm_vcpu_arch {
ulong fault_esr;
ulong queued_dear;
ulong queued_esr;
+ spinlock_t wdt_lock;
+ struct timer_list wdt_timer;
u32 tlbcfg[4];
u32 mmucfg;
u32 epr;
@@ -482,6 +484,7 @@ struct kvm_vcpu_arch {
u8 osi_needed;
u8 osi_enabled;
u8 papr_enabled;
+ u8 watchdog_enabled;
u8 sane;
u8 cpu_type;
u8 hcall_needed;
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 0124937..823d563 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -68,6 +68,8 @@ extern void kvmppc_emulate_dec(struct kvm_vcpu *vcpu);
extern u32 kvmppc_get_dec(struct kvm_vcpu *vcpu, u64 tb);
extern void kvmppc_decrementer_func(unsigned long data);
extern int kvmppc_sanity_check(struct kvm_vcpu *vcpu);
+extern int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu);
+extern void kvmppc_subarch_vcpu_uninit(struct kvm_vcpu *vcpu);
/* Core-specific hooks */
@@ -104,6 +106,8 @@ extern void kvmppc_core_queue_external(struct kvm_vcpu
*vcpu,
struct kvm_interrupt *irq);
extern void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu,
struct kvm_interrupt *irq);
+extern void kvmppc_core_queue_watchdog(struct kvm_vcpu *vcpu);
+extern void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu);
extern int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
unsigned int op, int *advance);
diff --git a/arch/powerpc/include/asm/reg_booke.h
b/arch/powerpc/include/asm/reg_booke.h
index 2d916c4..e07e6af 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -539,6 +539,13 @@
#define TCR_FIE0x0080 /* FIT Interrupt Enable */
#define TCR_ARE0x0040 /* Auto Reload Enable */
+#ifdef CONFIG_E500
+#define TCR_GET_WP(tcr) tcr) & 0xC000) >> 30) | \
+ (((tcr) & 0x1E) >> 15))
+#else
+#define TCR_GET_WP(tcr) (((tcr) & 0xC000) >> 30)
+#endif
+
/* Bit definitions for the TSR. */
#define TSR_ENW0x8000 /* Enable Next Watchdog */
#define TSR_WIS0x4000 /* WDT Interrupt Status */
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 3f2a836..e946665 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -411,6 +411,15 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
return 0;
}
+int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu)
+{
+ return 0;
+}
+
+void kvmppc_subarch_vcpu_uninit(struct kvm_vcpu *vcpu)
+{
+}
+
int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
{
int i;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index d25a097..eadd86c 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -206,6 +206,16 @@ void kvmppc_core_dequeue_external(struct kvm_vcpu *vcpu,
clear_bit(BOOKE_IRQPRIO_EXTERNAL_LEVEL, &vcpu->arch.pending_exceptions);
}
+void kvmppc_core_queue_watchdog(struct kvm_vcpu *vcpu)
+{
+ kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_WATCHDOG);
+}
+
+void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu)
+{
+ clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
+}
+
static void set_guest_srr(struct kvm_vcpu *vcpu, unsigned lon
[PATCH] KVM: PPC: BookE: HV: Fix compile
After merging the register type check patches from Ben's tree, the hv enabled booke implementation ceased to compile. This patch fixes things up so everyone's happy again. Signed-off-by: Alexander Graf --- arch/powerpc/kvm/bookehv_interrupts.S | 77 + 1 files changed, 39 insertions(+), 38 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index d28c2d4..099fe82 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -50,8 +50,9 @@ #define HOST_R2 (3 * LONGBYTES) #define HOST_CR (4 * LONGBYTES) #define HOST_NV_GPRS(5 * LONGBYTES) -#define HOST_NV_GPR(n) (HOST_NV_GPRS + ((n - 14) * LONGBYTES)) -#define HOST_MIN_STACK_SIZE (HOST_NV_GPR(31) + LONGBYTES) +#define __HOST_NV_GPR(n) (HOST_NV_GPRS + ((n - 14) * LONGBYTES)) +#define HOST_NV_GPR(n) __HOST_NV_GPR(__REG_##n) +#define HOST_MIN_STACK_SIZE (HOST_NV_GPR(R31) + LONGBYTES) #define HOST_STACK_SIZE ((HOST_MIN_STACK_SIZE + 15) & ~15) /* Align. */ #define HOST_STACK_LR (HOST_STACK_SIZE + LONGBYTES) /* In caller stack frame. */ @@ -410,24 +411,24 @@ heavyweight_exit: PPC_STL r31, VCPU_GPR(R31)(r4) /* Load host non-volatile register state from host stack. */ - PPC_LL r14, HOST_NV_GPR(r14)(r1) - PPC_LL r15, HOST_NV_GPR(r15)(r1) - PPC_LL r16, HOST_NV_GPR(r16)(r1) - PPC_LL r17, HOST_NV_GPR(r17)(r1) - PPC_LL r18, HOST_NV_GPR(r18)(r1) - PPC_LL r19, HOST_NV_GPR(r19)(r1) - PPC_LL r20, HOST_NV_GPR(r20)(r1) - PPC_LL r21, HOST_NV_GPR(r21)(r1) - PPC_LL r22, HOST_NV_GPR(r22)(r1) - PPC_LL r23, HOST_NV_GPR(r23)(r1) - PPC_LL r24, HOST_NV_GPR(r24)(r1) - PPC_LL r25, HOST_NV_GPR(r25)(r1) - PPC_LL r26, HOST_NV_GPR(r26)(r1) - PPC_LL r27, HOST_NV_GPR(r27)(r1) - PPC_LL r28, HOST_NV_GPR(r28)(r1) - PPC_LL r29, HOST_NV_GPR(r29)(r1) - PPC_LL r30, HOST_NV_GPR(r30)(r1) - PPC_LL r31, HOST_NV_GPR(r31)(r1) + PPC_LL r14, HOST_NV_GPR(R14)(r1) + PPC_LL r15, HOST_NV_GPR(R15)(r1) + PPC_LL r16, HOST_NV_GPR(R16)(r1) + PPC_LL r17, HOST_NV_GPR(R17)(r1) + PPC_LL r18, HOST_NV_GPR(R18)(r1) + PPC_LL r19, HOST_NV_GPR(R19)(r1) + PPC_LL r20, HOST_NV_GPR(R20)(r1) + PPC_LL r21, HOST_NV_GPR(R21)(r1) + PPC_LL r22, HOST_NV_GPR(R22)(r1) + PPC_LL r23, HOST_NV_GPR(R23)(r1) + PPC_LL r24, HOST_NV_GPR(R24)(r1) + PPC_LL r25, HOST_NV_GPR(R25)(r1) + PPC_LL r26, HOST_NV_GPR(R26)(r1) + PPC_LL r27, HOST_NV_GPR(R27)(r1) + PPC_LL r28, HOST_NV_GPR(R28)(r1) + PPC_LL r29, HOST_NV_GPR(R29)(r1) + PPC_LL r30, HOST_NV_GPR(R30)(r1) + PPC_LL r31, HOST_NV_GPR(R31)(r1) /* Return to kvm_vcpu_run(). */ mtlrr5 @@ -453,24 +454,24 @@ _GLOBAL(__kvmppc_vcpu_run) stw r5, HOST_CR(r1) /* Save host non-volatile register state to stack. */ - PPC_STL r14, HOST_NV_GPR(r14)(r1) - PPC_STL r15, HOST_NV_GPR(r15)(r1) - PPC_STL r16, HOST_NV_GPR(r16)(r1) - PPC_STL r17, HOST_NV_GPR(r17)(r1) - PPC_STL r18, HOST_NV_GPR(r18)(r1) - PPC_STL r19, HOST_NV_GPR(r19)(r1) - PPC_STL r20, HOST_NV_GPR(r20)(r1) - PPC_STL r21, HOST_NV_GPR(r21)(r1) - PPC_STL r22, HOST_NV_GPR(r22)(r1) - PPC_STL r23, HOST_NV_GPR(r23)(r1) - PPC_STL r24, HOST_NV_GPR(r24)(r1) - PPC_STL r25, HOST_NV_GPR(r25)(r1) - PPC_STL r26, HOST_NV_GPR(r26)(r1) - PPC_STL r27, HOST_NV_GPR(r27)(r1) - PPC_STL r28, HOST_NV_GPR(r28)(r1) - PPC_STL r29, HOST_NV_GPR(r29)(r1) - PPC_STL r30, HOST_NV_GPR(r30)(r1) - PPC_STL r31, HOST_NV_GPR(r31)(r1) + PPC_STL r14, HOST_NV_GPR(R14)(r1) + PPC_STL r15, HOST_NV_GPR(R15)(r1) + PPC_STL r16, HOST_NV_GPR(R16)(r1) + PPC_STL r17, HOST_NV_GPR(R17)(r1) + PPC_STL r18, HOST_NV_GPR(R18)(r1) + PPC_STL r19, HOST_NV_GPR(R19)(r1) + PPC_STL r20, HOST_NV_GPR(R20)(r1) + PPC_STL r21, HOST_NV_GPR(R21)(r1) + PPC_STL r22, HOST_NV_GPR(R22)(r1) + PPC_STL r23, HOST_NV_GPR(R23)(r1) + PPC_STL r24, HOST_NV_GPR(R24)(r1) + PPC_STL r25, HOST_NV_GPR(R25)(r1) + PPC_STL r26, HOST_NV_GPR(R26)(r1) + PPC_STL r27, HOST_NV_GPR(R27)(r1) + PPC_STL r28, HOST_NV_GPR(R28)(r1) + PPC_STL r29, HOST_NV_GPR(R29)(r1) + PPC_STL r30, HOST_NV_GPR(R30)(r1) + PPC_STL r31, HOST_NV_GPR(R31)(r1) /* Load guest non-volatiles. */ PPC_LL r14, VCPU_GPR(R14)(r4) -- 1.6.0.2 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
On 07/24/2012 02:45 AM, Bhushan Bharat-R65777 wrote: > > >> -Original Message- >> From: Wood Scott-B07421 >> Sent: Monday, July 23, 2012 10:00 PM >> To: Bhushan Bharat-R65777 >> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; >> ag...@suse.de >> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation >> >> On 07/23/2012 11:04 AM, Bhushan Bharat-R65777 wrote: >>> >>> -Original Message- From: Wood Scott-B07421 Sent: Monday, July 23, 2012 9:31 PM To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation On 07/23/2012 10:43 AM, Bhushan Bharat-R65777 wrote: > > >> -Original Message- >> From: Wood Scott-B07421 >> Sent: Monday, July 23, 2012 9:02 PM >> To: Bhushan Bharat-R65777 >> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; >> k...@vger.kernel.org; ag...@suse.de >> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation >> >> On 07/22/2012 11:10 PM, Bhushan Bharat-R65777 wrote: > @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu > *vcpu) #ifdef CONFIG_KVM_EXIT_TIMING > mutex_init(&vcpu->arch.exit_timing_lock); > #endif > - > +#ifdef CONFIG_BOOKE > + spin_lock_init(&vcpu->arch.wdt_lock); > + /* setup watchdog timer once */ > + setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func, > + (unsigned long)vcpu); > +#endif > return 0; > } Can you do this in kvmppc_booke_init()? >>> >>> I do not think we can do this in kvmppc_booke_init(). Watchdog >>> have association with vcpu, while there is no vcpu at >>> kvmppc_booke_init(). >> >> Sorry, I meant kvm_arch_vcpu_setup() in booke.c. > > Any specific reason to move this in above mentioned function? Want > to avoid #ifdef config_booke ? Yes, to avoid the ifdef. We already have a (poorly named) place for booke- specific vcpu init. >>> >>> Where we want to delete watchdog timer? >>> >>> Kvm_arch_vcpu_setup() is in flow of CREATE_VCPU ioctl, I do not see >> DESTROY_VCPU type of code? >>> >>> So is it ok to let del_timer_sync() as is with #ifdef config_booke ? >> >> You could add such a function. I suggest correcting the naming issue at the >> same time -- have kvm_subarch_vcpu_init() and kvm_subarch_vcpu_free(). > > I am sorry Scott but I did not get which functions you want to > rename. What I understood from current code for powerpc is: > > kvm_vm_ioctl_create_vcpu() > | > |-> kvm_arch_vcpu_create() > |-> kvm_arch_vcpu_setup() - This is where you want me to call > watchdog timer setup. We cannot (require more efforts) rename this as > this is called from virt/kvm/kvm_main.c. Say if we add, now let us > figure out where to uninit what is done in this function: Sigh, I didn't realize this came from generic code. What, from generic code's perspective, is the difference between kvm_arch_vcpu_init() and kvm_arch_vcpu_setup() supposed to be? They're synonyms! And as always in Linux, there's no documentation (that I can find) on what's expected of functions that have multiple implementations. Inferring from what the ppc code does, apparently kvm_arch_vcpu_create() is supposed to call kvm_vcpu_init(), which then calls back into arch code with kvm_arch_vcpu_init(). Then, after the preempt notifier is initialized (but not actually activated), generic code goes back to arch code with kvm_arch_vcpu_setup(). So basically the "setup" version comes later. And PPC KVM arbitrarily decides to use one of these for PPC-wide init, and the other for subarch init. This could really use some untangling (or documentation if there really is a method to the madness). In any case, we can still put this init code in kvmppc_arch_vcpu_setup() (i.e. the subarch init), and add a kvmppc_subarch_vcpu_uninit() that we call from kvm_arch_vcpu_uninit(). Or just move kvm_arch_vcpu_uninit() into subarch code -- the only thing it does is call kvmppc_mmu_destroy, which is a subarch hook anyway. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] booke: Added ONE_REG interface for IAC/DAC debug registers
IAC/DAC are defined as 32 bit while they are 64 bit wide. So ONE_REG
interface is added to set/get them.
Signed-off-by: Bharat Bhushan
---
v3:
- IAC3/4 defined on non-fsl booke
- kvmppc_debug_reg renamed to kvmppc_booke_debug_reg
v2:
- Using copy_to/from_user() apis.
arch/powerpc/include/asm/kvm.h | 12 ++
arch/powerpc/include/asm/kvm_host.h | 24 +++-
arch/powerpc/kvm/booke.c| 66 +-
arch/powerpc/kvm/booke_emulate.c|8 ++--
4 files changed, 102 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index 1bea4d8..3c14202 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -221,6 +221,12 @@ struct kvm_sregs {
__u32 dbsr; /* KVM_SREGS_E_UPDATE_DBSR */
__u32 dbcr[3];
+ /*
+* iac/dac registers are 64bit wide, while this API
+* interface provides only lower 32 bits on 64 bit
+* processors. ONE_REG interface is added for 64bit
+* iac/dac registers.
+*/
__u32 iac[4];
__u32 dac[2];
__u32 dvc[2];
@@ -326,5 +332,11 @@ struct kvm_book3e_206_tlb_params {
};
#define KVM_REG_PPC_HIOR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x1)
+#define KVM_REG_PPC_IAC1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x2)
+#define KVM_REG_PPC_IAC2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x3)
+#define KVM_REG_PPC_IAC3 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x4)
+#define KVM_REG_PPC_IAC4 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x5)
+#define KVM_REG_PPC_DAC1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x6)
+#define KVM_REG_PPC_DAC2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x7)
#endif /* __LINUX_KVM_POWERPC_H */
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 43cac48..7a45194 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -342,6 +342,27 @@ struct kvmppc_slb {
bool class : 1;
};
+# ifdef CONFIG_PPC_FSL_BOOK3E
+#define KVMPPC_BOOKE_IAC_NUM 2
+#define KVMPPC_BOOKE_DAC_NUM 2
+# else
+#define KVMPPC_BOOKE_IAC_NUM 4
+#define KVMPPC_BOOKE_DAC_NUM 2
+# endif
+#define KVMPPC_BOOKE_MAX_IAC 4
+#define KVMPPC_BOOKE_MAX_DAC 2
+
+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];
+};
+
struct kvm_vcpu_arch {
ulong host_stack;
u32 host_pid;
@@ -436,9 +457,8 @@ struct kvm_vcpu_arch {
u32 ccr0;
u32 ccr1;
- u32 dbcr0;
- u32 dbcr1;
u32 dbsr;
+ struct kvmppc_booke_debug_reg dbg_reg;
u64 mmcr[3];
u32 pmc[8];
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index be71b8e..2d41f78 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1353,12 +1353,74 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
{
- return -EINVAL;
+ int r = -EINVAL;
+
+ switch(reg->id) {
+ case KVM_REG_PPC_IAC1:
+ r = copy_to_user((u64 __user *)(long)reg->addr,
+&vcpu->arch.dbg_reg.iac[0], sizeof(u64));
+ break;
+ case KVM_REG_PPC_IAC2:
+ r = copy_to_user((u64 __user *)(long)reg->addr,
+&vcpu->arch.dbg_reg.iac[1], sizeof(u64));
+ break;
+#ifndef CONFIG_PPC_FSL_BOOK3E
+ case KVM_REG_PPC_IAC3:
+ r = copy_to_user((u64 __user *)(long)reg->addr,
+&vcpu->arch.dbg_reg.iac[2], sizeof(u64));
+ break;
+ case KVM_REG_PPC_IAC4:
+ r = copy_to_user((u64 __user *)(long)reg->addr,
+&vcpu->arch.dbg_reg.iac[3], sizeof(u64));
+ break;
+#endif
+ case KVM_REG_PPC_DAC1:
+ r = copy_to_user((u64 __user *)(long)reg->addr,
+&vcpu->arch.dbg_reg.dac[0], sizeof(u64));
+ break;
+ case KVM_REG_PPC_DAC2:
+ r = copy_to_user((u64 __user *)(long)reg->addr,
+&vcpu->arch.dbg_reg.dac[1], sizeof(u64));
+ break;
+ default:
+ break;
+ }
+ return r;
}
int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
{
- return -EINVAL;
+ int r = -EINVAL;
+
+ switch(reg->id) {
+ case KVM_REG_PPC_IAC1:
+ r = copy_from_user(&vcpu->arch.dbg_reg.iac[0],
+(u64 __user *)(long)reg->addr, sizeof(u64));
+
Re: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC debug registers
On 07/24/2012 03:26 PM, Bhushan Bharat-R65777 wrote:
+struct kvmppc_debug_reg {
+#ifdef CONFIG_BOOKE
+ u32 dbcr0;
+ u32 dbcr1;
+ u32 dbcr2;
+#ifdef CONFIG_KVM_E500MC
+ u32 dbcr4;
+#endif
+ u64 iac[KVMPPC_MAX_IAC];
+ u64 dac[KVMPPC_MAX_DAC];
+#endif
+};
Is there any reason for this to be a separate struct?
No specific reason, The rest of debug ( which will follow sometime
soon) uses
this structure and looks to make code look easy.
So why not use an fsl / booke specific struct for the debug patches
then? Debug registers are really nothing common between book3s and
booke, so we shouldn't treat them as such by using the same struct
definition.
All elements of struct are under #ifdef CONFIG_BOOKE? So for book3s it is as
good as void, only struct type if declared. Do you want the struct type also
under config_booke ?
struct kvmppc_booke_debug_reg {
};
struct kvmppc_book3s_debug_reg {
};
void booke_foo() {
struct kvmppc_booke_debug_reg r;
kvmppc_booke_debug_reg or kvmppc_book3s_debug_reg ?
In booke_foo() you certainly will never want to use struct
kvmppc_book3s_debug_reg, no?
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC debug registers
> > +struct kvmppc_debug_reg {
> > +#ifdef CONFIG_BOOKE
> > + u32 dbcr0;
> > + u32 dbcr1;
> > + u32 dbcr2;
> > +#ifdef CONFIG_KVM_E500MC
> > + u32 dbcr4;
> > +#endif
> > + u64 iac[KVMPPC_MAX_IAC];
> > + u64 dac[KVMPPC_MAX_DAC];
> > +#endif
> > +};
> Is there any reason for this to be a separate struct?
> >>> No specific reason, The rest of debug ( which will follow sometime
> >>> soon) uses
> >> this structure and looks to make code look easy.
> >>
> >> So why not use an fsl / booke specific struct for the debug patches
> >> then? Debug registers are really nothing common between book3s and
> >> booke, so we shouldn't treat them as such by using the same struct
> definition.
> >>
> > All elements of struct are under #ifdef CONFIG_BOOKE? So for book3s it is as
> good as void, only struct type if declared. Do you want the struct type also
> under config_booke ?
>
> struct kvmppc_booke_debug_reg {
>
> };
>
> struct kvmppc_book3s_debug_reg {
>
> };
>
> void booke_foo() {
>struct kvmppc_booke_debug_reg r;
kvmppc_booke_debug_reg or kvmppc_book3s_debug_reg ?
Thanks
-Bharat
>r.dbcr0 = 0;
> }
>
> vs
>
> struct kvmppc_debug_reg {
> #ifdef CONFIG_BOOKE
>
> #else
>
> #endif
> };
>
> void booke_foo() {
>struct kvmppc_debug_reg r;
>r.dbcr0 = 0;
> }
>
> Which one has less #ifdefs? Keep in mind that the less #ifdefs you have, the
> more readable and maintainable your code becomes, because config options have
> less effect on your syntax.
>
>
> Alex
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body
> of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC debug registers
On 07/24/2012 03:04 AM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Monday, July 23, 2012 11:12 PM
To: Bhushan Bharat-R65777
Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org
Subject: Re: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC debug
registers
On 07/23/2012 05:48 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Wood Scott-B07421
Sent: Monday, July 23, 2012 9:12 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@vger.kernel.org; ag...@suse.de; k...@vger.kernel.org;
Bhushan Bharat-
R65777
Subject: Re: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC
debug registers
On 07/23/2012 06:19 AM, Bharat Bhushan wrote:
IAC/DAC are defined as 32 bit while they are 64 bit wide. So ONE_REG
interface is added to set/get them.
Signed-off-by: Bharat Bhushan
---
v2:
- Using copy_to/from_user() apis.
arch/powerpc/include/asm/kvm.h | 12 ++
arch/powerpc/include/asm/kvm_host.h | 28 ++-
arch/powerpc/kvm/booke.c| 64
+-
arch/powerpc/kvm/booke_emulate.c|8 ++--
4 files changed, 104 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm.h
b/arch/powerpc/include/asm/kvm.h index 1bea4d8..3c14202 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -221,6 +221,12 @@ struct kvm_sregs {
__u32 dbsr; /* KVM_SREGS_E_UPDATE_DBSR */
__u32 dbcr[3];
+ /*
+* iac/dac registers are 64bit wide, while this API
+* interface provides only lower 32 bits on 64 bit
+* processors. ONE_REG interface is added for 64bit
+* iac/dac registers.
+*/
__u32 iac[4];
__u32 dac[2];
__u32 dvc[2];
@@ -326,5 +332,11 @@ struct kvm_book3e_206_tlb_params { };
#define KVM_REG_PPC_HIOR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x1)
+#define KVM_REG_PPC_IAC1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x2)
+#define KVM_REG_PPC_IAC2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x3)
+#define KVM_REG_PPC_IAC3 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x4)
+#define KVM_REG_PPC_IAC4 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x5)
+#define KVM_REG_PPC_DAC1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x6)
+#define KVM_REG_PPC_DAC2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x7)
#endif /* __LINUX_KVM_POWERPC_H */ diff --git
a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 43cac48..2c0f015 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -342,6 +342,31 @@ struct kvmppc_slb {
bool class : 1;
};
+#ifdef CONFIG_BOOKE
+# ifdef CONFIG_PPC_FSL_BOOK3E
+#define KVMPPC_IAC_NUM 2
+#define KVMPPC_DAC_NUM 2
+# else
+#define KVMPPC_IAC_NUM 4
+#define KVMPPC_DAC_NUM 2
+# endif
+#define KVMPPC_MAX_IAC 4
+#define KVMPPC_MAX_DAC 2
+#endif
+
+struct kvmppc_debug_reg {
+#ifdef CONFIG_BOOKE
+ u32 dbcr0;
+ u32 dbcr1;
+ u32 dbcr2;
+#ifdef CONFIG_KVM_E500MC
+ u32 dbcr4;
+#endif
+ u64 iac[KVMPPC_MAX_IAC];
+ u64 dac[KVMPPC_MAX_DAC];
+#endif
+};
Is there any reason for this to be a separate struct?
No specific reason, The rest of debug ( which will follow sometime soon) uses
this structure and looks to make code look easy.
So why not use an fsl / booke specific struct for the debug patches then? Debug
registers are really nothing common between book3s and booke, so we shouldn't
treat them as such by using the same struct definition.
All elements of struct are under #ifdef CONFIG_BOOKE? So for book3s it is as
good as void, only struct type if declared. Do you want the struct type also
under config_booke ?
struct kvmppc_booke_debug_reg {
};
struct kvmppc_book3s_debug_reg {
};
void booke_foo() {
struct kvmppc_booke_debug_reg r;
r.dbcr0 = 0;
}
vs
struct kvmppc_debug_reg {
#ifdef CONFIG_BOOKE
#else
#endif
};
void booke_foo() {
struct kvmppc_debug_reg r;
r.dbcr0 = 0;
}
Which one has less #ifdefs? Keep in mind that the less #ifdefs you have,
the more readable and maintainable your code becomes, because config
options have less effect on your syntax.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
> -Original Message-
> From: Wood Scott-B07421
> Sent: Monday, July 23, 2012 10:00 PM
> To: Bhushan Bharat-R65777
> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
> ag...@suse.de
> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>
> On 07/23/2012 11:04 AM, Bhushan Bharat-R65777 wrote:
> >
> >
> >> -Original Message-
> >> From: Wood Scott-B07421
> >> Sent: Monday, July 23, 2012 9:31 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
> >> ag...@suse.de
> >> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
> >>
> >> On 07/23/2012 10:43 AM, Bhushan Bharat-R65777 wrote:
> >>>
> >>>
> -Original Message-
> From: Wood Scott-B07421
> Sent: Monday, July 23, 2012 9:02 PM
> To: Bhushan Bharat-R65777
> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org;
> k...@vger.kernel.org; ag...@suse.de
> Subject: Re: [PATCH 2/2 v5] KVM: PPC: booke: Add watchdog emulation
>
> On 07/22/2012 11:10 PM, Bhushan Bharat-R65777 wrote:
> >>> @@ -386,13 +387,23 @@ int kvm_arch_vcpu_init(struct kvm_vcpu
> >>> *vcpu) #ifdef CONFIG_KVM_EXIT_TIMING
> >>> mutex_init(&vcpu->arch.exit_timing_lock);
> >>> #endif
> >>> -
> >>> +#ifdef CONFIG_BOOKE
> >>> + spin_lock_init(&vcpu->arch.wdt_lock);
> >>> + /* setup watchdog timer once */
> >>> + setup_timer(&vcpu->arch.wdt_timer, kvmppc_watchdog_func,
> >>> + (unsigned long)vcpu);
> >>> +#endif
> >>> return 0;
> >>> }
> >>
> >> Can you do this in kvmppc_booke_init()?
> >
> > I do not think we can do this in kvmppc_booke_init(). Watchdog
> > have association with vcpu, while there is no vcpu at
> > kvmppc_booke_init().
>
> Sorry, I meant kvm_arch_vcpu_setup() in booke.c.
> >>>
> >>> Any specific reason to move this in above mentioned function? Want
> >>> to avoid
> >> #ifdef config_booke ?
> >>
> >> Yes, to avoid the ifdef. We already have a (poorly named) place for
> >> booke- specific vcpu init.
> >
> > Where we want to delete watchdog timer?
> >
> > Kvm_arch_vcpu_setup() is in flow of CREATE_VCPU ioctl, I do not see
> DESTROY_VCPU type of code?
> >
> > So is it ok to let del_timer_sync() as is with #ifdef config_booke ?
>
> You could add such a function. I suggest correcting the naming issue at the
> same time -- have kvm_subarch_vcpu_init() and kvm_subarch_vcpu_free().
I am sorry Scott but I did not get which functions you want to rename. What I
understood from current code for powerpc is:
kvm_vm_ioctl_create_vcpu()
|
|-> kvm_arch_vcpu_create()
|-> kvm_arch_vcpu_setup() - This is where you want me to call watchdog timer
setup. We cannot (require more efforts) rename this as this is called from
virt/kvm/kvm_main.c. Say if we add, now let us figure out where to uninit what
is done in this function:
1)
kvm_arch_vcpu_destroy()
|
|-> kvm_arch_vcpu_free() -> this undo what is done by 1) kvm_arch_vcpu_init()
2) kvm_arch_vcpu_create().
This also does not undo what is done in
kvm_arch_vcpu_setup()
Also kvm_arch_vcpu_destroy() is called as error fallback of
kvm_vm_ioctl_create_vcpu(), so this is not normal cleaup function call.
2)
The other function to undo is kvm_arch_destroy_vm() (which is called from
kvm_vm_release(), kvm_vcpu_release() and error fallback in
kvm_dev_ioctl_create_vm()/kvm_vm_ioctl_create_vcpu(). So this function is
normal cleanup function and what it does is:
kvm_arch_destroy_vm()
|
|-> kvm_for_each_vcpu()
| |
| |->kvm_arch_vcpu_free() -> this undo what is done by 1)
kvm_arch_vcpu_init() 2) kvm_arch_vcpu_create().
| This also does not undo what is done in
kvm_arch_vcpu_setup()
|
|-> kvmppc_core_destroy_vm() -> this undo what was done by kvm_arch_init_vm()
in flow of kvm_create_vm()
So what I understood from this is:
1) Move the kvm_vcpu_arch_setup() into powerpc.c
2) define kvm_subarch_vcpu_init() in (booke/book3s) to do subarch specific work
and call this from kvm_vcpu_arch_setup()
3) define kvm_subarch_vcpu_free() to be called from kvm_arch_vcpu_free(). This
will undo what is done by kvm_subarch_vcpu_init().
4) Also kvm_arch_vcpu_free() should do what is done by kvm_vcpu_srch_setup() in
powerpc.c
Thanks
-Bharat
>
> -Scott
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ&�)ߡ�a���� �G���h��j:+v���w��٥
