Re: [PATCH][v4] PPC: add paravirt idle loop for 64-bit book E

2013-03-13 Thread Kumar Gala

On Feb 8, 2013, at 1:22 PM, Stuart Yoder wrote:

 From: Stuart Yoder stuart.yo...@freescale.com
 
 Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
 ---
 
 -removed KVM prefix to patch subject, patch is not KVM specific
 
 arch/powerpc/kernel/epapr_hcalls.S |2 ++
 arch/powerpc/kernel/idle_book3e.S  |   32 ++--
 2 files changed, 32 insertions(+), 2 deletions(-)

applied to next

- k
--
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: [STRAWMAN PATCH] KVM: PPC: Add ioctl to specify interrupt controller architecture to emulate

2013-03-13 Thread Scott Wood

On 03/08/2013 05:04:30 AM, Alexander Graf wrote:



Am 08.03.2013 um 11:37 schrieb Paul Mackerras pau...@samba.org:

 On Thu, Mar 07, 2013 at 03:00:52PM +0100, Alexander Graf wrote:

 Could you please (in a quick and drafty way) try and see if  
setting the IRQ arch (using enable_cap) after the vcpu got created  
would work for you?


 That enable_cap would then have to loop through all devices and  
notify irq controllers that a new cpu got spawned.
 All vcpu local payloads would have to get allocated and  
initialized outside of vcpu_create too then.


 So, the first thing I noticed is that KVM_ENABLE_CAP is a vcpu  
ioctl,
 not a vm ioctl.  Apparently qemu calls it once for every vcpu when  
it

 calls it on ppc targets.  That means that it doesn't have to loop
 through all vcpus; it just needs to connect up the one it's called
 for, which simplifies things.

That's the point, yes :). And if for some weird reason one vcpu isn't  
connected to the interrupt controller (or to a different one), we can  
model that too ;).


 I'm coding it up now and porting my XICS emulation to the kvm device
 API proposed by Scott.  It looks like it's going to be OK.

Awesome! Scott is going to prototype whether using fds as tokens  
makes sense. But even if we change it to an fd model, there should be  
very little work to do to move xics to it too if it's already modeled  
for create_device.


It looks like the fd approach will be workable.  Paul, do you want to  
post what you have in terms of the capability approach, so I can base  
an fd version of the device control patchset on it, or should I fd-ize  
the current patchset without it, and then rework mpic on top of the  
capability stuff once you've posted your device-control-using patchset?



 I have
 used the first argument (cap-args[0]) to specify which interrupt
 controller you want to connect the vcpu to.

Ah, nice idea. So you basically make the vcpu connection explicit.  
Perfect! Then just pass the interrupt controller pin id in  
cap-args[1] so we don't need to guess which vcpu we're talking about  
and all is well :). No implicit assumptions left in the kernel.


Is the IRQ architecture now implicit based on what sort of irqchip you  
point at, or is there a separate capability for each IRQ architecture?   
The latter may make more sense -- you can test for specific  
architectures, provide architecture-specific arguments, some  
architectures may not require pointing at a device (e.g. the LAPIC in  
kernel, IO-APIC in userspace model), etc.


-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [STRAWMAN PATCH] KVM: PPC: Add ioctl to specify interrupt controller architecture to emulate

2013-03-13 Thread Alexander Graf

On 14.03.2013, at 01:14, Scott Wood wrote:

 On 03/08/2013 05:04:30 AM, Alexander Graf wrote:
 Am 08.03.2013 um 11:37 schrieb Paul Mackerras pau...@samba.org:
  On Thu, Mar 07, 2013 at 03:00:52PM +0100, Alexander Graf wrote:
 
  Could you please (in a quick and drafty way) try and see if setting the 
  IRQ arch (using enable_cap) after the vcpu got created would work for you?
 
  That enable_cap would then have to loop through all devices and notify 
  irq controllers that a new cpu got spawned.
  All vcpu local payloads would have to get allocated and initialized 
  outside of vcpu_create too then.
 
  So, the first thing I noticed is that KVM_ENABLE_CAP is a vcpu ioctl,
  not a vm ioctl.  Apparently qemu calls it once for every vcpu when it
  calls it on ppc targets.  That means that it doesn't have to loop
  through all vcpus; it just needs to connect up the one it's called
  for, which simplifies things.
 That's the point, yes :). And if for some weird reason one vcpu isn't 
 connected to the interrupt controller (or to a different one), we can model 
 that too ;).
  I'm coding it up now and porting my XICS emulation to the kvm device
  API proposed by Scott.  It looks like it's going to be OK.
 Awesome! Scott is going to prototype whether using fds as tokens makes 
 sense. But even if we change it to an fd model, there should be very little 
 work to do to move xics to it too if it's already modeled for create_device.
 
 It looks like the fd approach will be workable.  

Very nice :).

 Paul, do you want to post what you have in terms of the capability approach, 
 so I can base an fd version of the device control patchset on it, or should I 
 fd-ize the current patchset without it, and then rework mpic on top of the 
 capability stuff once you've posted your device-control-using patchset?
 
  I have
  used the first argument (cap-args[0]) to specify which interrupt
  controller you want to connect the vcpu to.
 Ah, nice idea. So you basically make the vcpu connection explicit. Perfect! 
 Then just pass the interrupt controller pin id in cap-args[1] so we don't 
 need to guess which vcpu we're talking about and all is well :). No implicit 
 assumptions left in the kernel.
 
 Is the IRQ architecture now implicit based on what sort of irqchip you point 
 at, or is there a separate capability for each IRQ architecture?  The latter 
 may make more sense -- you can test for specific architectures, provide 
 architecture-specific arguments, some architectures may not require pointing 
 at a device (e.g. the LAPIC in kernel, IO-APIC in userspace model), etc.

I tend to be quite indifferent whether we should make it implicit or explicit. 
But so far, explicit usually ended up more flexible and a better interface. So 
yes, let's go with different CAPs for different interrupt controllers. However, 
let's make sure the parameters stay compatible for now.


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: [STRAWMAN PATCH] KVM: PPC: Add ioctl to specify interrupt controller architecture to emulate

2013-03-13 Thread Paul Mackerras
On Wed, Mar 13, 2013 at 07:14:48PM -0500, Scott Wood wrote:
 On 03/08/2013 05:04:30 AM, Alexander Graf wrote:
 
 
 Am 08.03.2013 um 11:37 schrieb Paul Mackerras pau...@samba.org:
 
  On Thu, Mar 07, 2013 at 03:00:52PM +0100, Alexander Graf wrote:
 
  Could you please (in a quick and drafty way) try and see if
 setting the IRQ arch (using enable_cap) after the vcpu got created
 would work for you?
 
  That enable_cap would then have to loop through all devices and
 notify irq controllers that a new cpu got spawned.
  All vcpu local payloads would have to get allocated and
 initialized outside of vcpu_create too then.
 
  So, the first thing I noticed is that KVM_ENABLE_CAP is a vcpu
 ioctl,
  not a vm ioctl.  Apparently qemu calls it once for every vcpu
 when it
  calls it on ppc targets.  That means that it doesn't have to loop
  through all vcpus; it just needs to connect up the one it's called
  for, which simplifies things.
 
 That's the point, yes :). And if for some weird reason one vcpu
 isn't connected to the interrupt controller (or to a different
 one), we can model that too ;).
 
  I'm coding it up now and porting my XICS emulation to the kvm device
  API proposed by Scott.  It looks like it's going to be OK.
 
 Awesome! Scott is going to prototype whether using fds as tokens
 makes sense. But even if we change it to an fd model, there should
 be very little work to do to move xics to it too if it's already
 modeled for create_device.
 
 It looks like the fd approach will be workable.  Paul, do you want
 to post what you have in terms of the capability approach, so I can
 base an fd version of the device control patchset on it, or should I
 fd-ize the current patchset without it, and then rework mpic on top
 of the capability stuff once you've posted your device-control-using
 patchset?

I have a complete patchset based on your kvm: add device control API
patch, tested and ready to go. :)  I just posted the first patch of
that series, the one that adds the KVM_CAP_IRQ_ARCH capability.  If
you're going to change the device API then I'll hold off posting the
rest of the series for now.

  I have
  used the first argument (cap-args[0]) to specify which interrupt
  controller you want to connect the vcpu to.
 
 Ah, nice idea. So you basically make the vcpu connection explicit.
 Perfect! Then just pass the interrupt controller pin id in
 cap-args[1] so we don't need to guess which vcpu we're talking
 about and all is well :). No implicit assumptions left in the
 kernel.
 
 Is the IRQ architecture now implicit based on what sort of irqchip
 you point at, or is there a separate capability for each IRQ
 architecture?  The latter may make more sense -- you can test for
 specific architectures, provide architecture-specific arguments,
 some architectures may not require pointing at a device (e.g. the
 LAPIC in kernel, IO-APIC in userspace model), etc.

The way I have done it, there is one capability, and args[0] is a
token for the IRQ architecture (not a device ID).  I arbitrarily
assigned 0x58494353 for KVM_CAP_IRQ_XICS as the args[0] value to
indicate XICS.  I think it would be better if we don't have to get a
new capability number assigned every time we want to add a new type of
interrupt controller.

Paul.
--
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/7] Added ONE_REG interface for debug instruction

2013-03-13 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, March 07, 2013 6:38 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 2/7] Added ONE_REG interface for debug instruction
 
 
 On 28.02.2013, at 05:13, Bharat Bhushan wrote:
 
  This patch adds the one_reg interface to get the special instruction
  to be used for setting software breakpoint from userspace.
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  Documentation/virtual/kvm/api.txt |1 +
  arch/powerpc/include/asm/kvm_book3s.h |1 +
  arch/powerpc/include/asm/kvm_booke.h  |2 ++
  arch/powerpc/include/uapi/asm/kvm.h   |4 
  arch/powerpc/kvm/book3s.c |6 ++
  arch/powerpc/kvm/booke.c  |6 ++
  6 files changed, 20 insertions(+), 0 deletions(-)
 
  diff --git a/Documentation/virtual/kvm/api.txt
  b/Documentation/virtual/kvm/api.txt
  index cce500a..dbfcc04 100644
  --- a/Documentation/virtual/kvm/api.txt
  +++ b/Documentation/virtual/kvm/api.txt
  @@ -1766,6 +1766,7 @@ registers, find a list below:
PPC   | KVM_REG_PPC_TSR   | 32
PPC   | KVM_REG_PPC_OR_TSR| 32
PPC   | KVM_REG_PPC_CLEAR_TSR | 32
  +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
 
  4.69 KVM_GET_ONE_REG
 
  diff --git a/arch/powerpc/include/asm/kvm_book3s.h
  b/arch/powerpc/include/asm/kvm_book3s.h
  index 5a56e1c..36164cc 100644
  --- a/arch/powerpc/include/asm/kvm_book3s.h
  +++ b/arch/powerpc/include/asm/kvm_book3s.h
  @@ -458,6 +458,7 @@ static inline bool kvmppc_critical_section(struct 
  kvm_vcpu
 *vcpu)
  #define OSI_SC_MAGIC_R4 0x77810F9B
 
  #define INS_DCBZ0x7c0007ec
  +#define INS_TW 0x7c08
 
 This one should be trap, so TO needs to be 31. The instruction as it's here 
 is
 a nop if I read the spec correctly.

Yes I missed this.
BTW rather than setting TO = 31, what if we set TO = 2 as RA and RB is same 
here.

-Bharat

 
 Alex
 
 
  /* LPIDs we support with this build -- runtime limit may be lower */
  #define KVMPPC_NR_LPIDS (LPID_RSVD + 1)
  diff --git a/arch/powerpc/include/asm/kvm_booke.h
  b/arch/powerpc/include/asm/kvm_booke.h
  index b7cd335..d3c1eb3 100644
  --- a/arch/powerpc/include/asm/kvm_booke.h
  +++ b/arch/powerpc/include/asm/kvm_booke.h
  @@ -26,6 +26,8 @@
  /* LPIDs we support with this build -- runtime limit may be lower */
  #define KVMPPC_NR_LPIDS64
 
  +#define KVMPPC_INST_EHPRIV 0x7c00021c
  +
  static inline void kvmppc_set_gpr(struct kvm_vcpu *vcpu, int num,
  ulong val) {
  vcpu-arch.gpr[num] = val;
  diff --git a/arch/powerpc/include/uapi/asm/kvm.h
  b/arch/powerpc/include/uapi/asm/kvm.h
  index ef072b1..c2ff99c 100644
  --- a/arch/powerpc/include/uapi/asm/kvm.h
  +++ b/arch/powerpc/include/uapi/asm/kvm.h
  @@ -422,4 +422,8 @@ struct kvm_get_htab_header {
  #define KVM_REG_PPC_CLEAR_TSR   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x88)
  #define KVM_REG_PPC_TCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x89)
  #define KVM_REG_PPC_TSR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x8a)
  +
  +/* Debugging: Special instruction for software breakpoint */
  +#define KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x8b)
  +
  #endif /* __LINUX_KVM_POWERPC_H */
  diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
  index a4b6452..975a401 100644
  --- a/arch/powerpc/kvm/book3s.c
  +++ b/arch/powerpc/kvm/book3s.c
  @@ -530,6 +530,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
 struct kvm_one_reg *reg)
  val = get_reg_val(reg-id, vcpu-arch.vscr.u[3]);
  break;
  #endif /* CONFIG_ALTIVEC */
  +   case KVM_REG_PPC_DEBUG_INST: {
  +   u32 opcode = INS_TW;
  +   r = copy_to_user((u32 __user *)(long)reg-addr,
  +opcode, sizeof(u32));
  +   break;
  +   }
  default:
  r = -EINVAL;
  break;
  diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
  8b553c0..a41cd6d 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
  @@ -1448,6 +1448,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
 struct kvm_one_reg *reg)
  case KVM_REG_PPC_TSR:
  r = put_user(vcpu-arch.tsr, (u32 __user *)(long)reg-addr);
  break;
  +   case KVM_REG_PPC_DEBUG_INST: {
  +   u32 opcode = KVMPPC_INST_EHPRIV;
  +   r = copy_to_user((u32 __user *)(long)reg-addr,
  +opcode, sizeof(u32));
  +   break;
  +   }
  default:
  break;
  }
  --
  1.7.0.4
 
 
  --
  To unsubscribe from this list: send the line unsubscribe kvm-ppc in
  the body of a message to majord...@vger.kernel.org More 

RE: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined

2013-03-13 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, March 07, 2013 6:51 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/7] KVM: PPC: debug stub interface parameter defined
 
 
 On 28.02.2013, at 05:13, Bharat Bhushan wrote:
 
  This patch defines the interface parameter for KVM_SET_GUEST_DEBUG
  ioctl support. Follow up patches will use this for setting up hardware
  breakpoints, watchpoints and software breakpoints.
 
  Also kvm_arch_vcpu_ioctl_set_guest_debug() is brought one level below.
  This is because I am not sure what is required for book3s. So this
  ioctl behaviour will not change for book3s.
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/uapi/asm/kvm.h |   23 +++
  arch/powerpc/kvm/book3s.c   |6 ++
  arch/powerpc/kvm/booke.c|6 ++
  arch/powerpc/kvm/powerpc.c  |6 --
  4 files changed, 35 insertions(+), 6 deletions(-)
 
  diff --git a/arch/powerpc/include/uapi/asm/kvm.h
  b/arch/powerpc/include/uapi/asm/kvm.h
  index c2ff99c..15f9a00 100644
  --- a/arch/powerpc/include/uapi/asm/kvm.h
  +++ b/arch/powerpc/include/uapi/asm/kvm.h
  @@ -272,8 +272,31 @@ struct kvm_debug_exit_arch {
 
  /* for KVM_SET_GUEST_DEBUG */
  struct kvm_guest_debug_arch {
  +   struct {
  +   /* H/W breakpoint/watchpoint address */
  +   __u64 addr;
  +   /*
  +* Type denotes h/w breakpoint, read watchpoint, write
  +* watchpoint or watchpoint (both read and write).
  +*/
  +#define KVMPPC_DEBUG_NOTYPE0x0
  +#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];
  };
 
  +/* Debug related defines */
  +/*
  + * kvm_guest_debug-control is a 32 bit field. The lower 16 bits are
  +generic
  + * and upper 16 bits are architecture specific. Architecture specific
  +defines
  + * that ioctl is for setting hardware breakpoint or software breakpoint.
  + */
  +#define KVM_GUESTDBG_USE_SW_BP 0x0001
  +#define KVM_GUESTDBG_USE_HW_BP 0x0002
 
 You only need
 
 #define KVM_GUESTDBG_HW_BP 0x0001
 
 In absence of the flag, it's a SW breakpoint.

We kept this for 2 reasons; 1) Same logic is applied for i386, so trying to 
keep consistent 2) better clarity.

If you want than I can code this as you described.

-Bharat

 
 
 Alex
 
  +
  /* definition of registers in kvm_run */ struct kvm_sync_regs { };
  diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
  index 975a401..cb85d73 100644
  --- a/arch/powerpc/kvm/book3s.c
  +++ b/arch/powerpc/kvm/book3s.c
  @@ -613,6 +613,12 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu 
  *vcpu,
  return 0;
  }
 
  +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
  +   struct kvm_guest_debug *dbg)
  +{
  +   return -EINVAL;
  +}
  +
  void kvmppc_decrementer_func(unsigned long data) {
  struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data; diff --git
  a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
  a41cd6d..1de93a8 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
  @@ -1527,6 +1527,12 @@ 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;
  diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
  index 934413c..4c94ca9 100644
  --- a/arch/powerpc/kvm/powerpc.c
  +++ b/arch/powerpc/kvm/powerpc.c
  @@ -532,12 +532,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
  #endif }
 
  -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
  -struct kvm_guest_debug *dbg)
  -{
  -   return -EINVAL;
  -}
  -
  static void kvmppc_complete_dcr_load(struct kvm_vcpu *vcpu,
   struct kvm_run *run) {
  --
  1.7.0.4
 
 
 


--
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 4/7] booke: Save and restore debug registers on guest entry and exit

2013-03-13 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, March 07, 2013 6:56 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 4/7] booke: Save and restore debug registers on guest 
 entry
 and exit
 
 
 On 28.02.2013, at 05:13, Bharat Bhushan wrote:
 
  On Guest entry: if guest is wants to use the debug register then save
  h/w debug register in host_dbg_reg and load the debug registers with
  shadow_dbg_reg. Otherwise leave h/w debug registers as is.
 
 Why can't we switch the majority of registers on vcpu_put/get and only enable 
 or
 disable debugging on guest entry/exit?


One of the reason for not doing this is that the KVM is a host kernel module 
and let this be debugged by host (I do not this how much useful this is :)) 
So I am not able to recall the specific reason, maybe we have just coded this 
like this and tried to keep overhead as low as possible by switching registers 
only when they are used.

As we discussed before, we can keep this option open for future.

-Bharat

 
 
 Alex
 
 
  On guest exit: If guest/user-space is using the debug resource then
  restore the h/w debug register with host_dbg_reg. No need to save
  guest debug register as shadow_dbg_reg is having required values. If
  guest is not using the debug resources then no need to restore h/w 
  registers.
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |5 ++
  arch/powerpc/kernel/asm-offsets.c   |   26 
  arch/powerpc/kvm/booke_interrupts.S |  114
  +++
  3 files changed, 145 insertions(+), 0 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index f4ba881..a9feeb0 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -504,7 +504,12 @@ struct kvm_vcpu_arch {
  u32 mmucfg;
  u32 epr;
  u32 crit_save;
  +   /* guest debug registers*/
  struct kvmppc_booke_debug_reg dbg_reg;
  +   /* shadow debug registers */
  +   struct kvmppc_booke_debug_reg shadow_dbg_reg;
  +   /* host debug registers*/
  +   struct kvmppc_booke_debug_reg host_dbg_reg;
  #endif
  gpa_t paddr_accessed;
  gva_t vaddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 02048f3..22deda7 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -563,6 +563,32 @@ int main(void)
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
  DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
  +   DEFINE(VCPU_DBSR, offsetof(struct kvm_vcpu, arch.dbsr));
  +   DEFINE(VCPU_SHADOW_DBG, offsetof(struct kvm_vcpu, arch.shadow_dbg_reg));
  +   DEFINE(VCPU_HOST_DBG, offsetof(struct kvm_vcpu, arch.host_dbg_reg));
  +   DEFINE(KVMPPC_DBG_DBCR0, offsetof(struct kvmppc_booke_debug_reg,
  + dbcr0));
  +   DEFINE(KVMPPC_DBG_DBCR1, offsetof(struct kvmppc_booke_debug_reg,
  + dbcr1));
  +   DEFINE(KVMPPC_DBG_DBCR2, offsetof(struct kvmppc_booke_debug_reg,
  + dbcr2));
  +#ifdef CONFIG_KVM_E500MC
  +   DEFINE(KVMPPC_DBG_DBCR4, offsetof(struct kvmppc_booke_debug_reg,
  + dbcr4));
  +#endif
  +   DEFINE(KVMPPC_DBG_IAC1, offsetof(struct kvmppc_booke_debug_reg,
  +iac[0]));
  +   DEFINE(KVMPPC_DBG_IAC2, offsetof(struct kvmppc_booke_debug_reg,
  +iac[1]));
  +   DEFINE(KVMPPC_DBG_IAC3, offsetof(struct kvmppc_booke_debug_reg,
  +iac[2]));
  +   DEFINE(KVMPPC_DBG_IAC4, offsetof(struct kvmppc_booke_debug_reg,
  +iac[3]));
  +   DEFINE(KVMPPC_DBG_DAC1, offsetof(struct kvmppc_booke_debug_reg,
  +dac[0]));
  +   DEFINE(KVMPPC_DBG_DAC2, offsetof(struct kvmppc_booke_debug_reg,
  +dac[1]));
  +   DEFINE(VCPU_GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index 2c6deb5..6d78e01 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -39,6 +39,8 @@
  #define HOST_MIN_STACK_SIZE (HOST_NV_GPR(R31) + 4) #define
  HOST_STACK_SIZE (((HOST_MIN_STACK_SIZE + 15) / 16) * 16) /* Align. */
  #define HOST_STACK_LR   (HOST_STACK_SIZE + 4) /* In caller stack frame. */
  +#define DBCR0_AC_BITS  (DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | 
  DBCR0_IAC4 | \
  +DBCR0_DAC1R | DBCR0_DAC1W | 

RE: [PATCH 7/7] KVM: PPC: Add userspace debug stub support

2013-03-13 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, March 07, 2013 7:09 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 7/7] KVM: PPC: Add userspace debug stub support
 
 
 On 28.02.2013, at 05:13, Bharat Bhushan wrote:
 
  This patch adds the debug stub support on booke/bookehv.
  Now QEMU debug stub can use hw breakpoint, watchpoint and software
  breakpoint to debug guest.
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/uapi/asm/kvm.h |   22 +-
  arch/powerpc/kvm/booke.c|  143 
  +++---
  arch/powerpc/kvm/e500_emulate.c |6 ++
  arch/powerpc/kvm/e500mc.c   |3 +-
  4 files changed, 155 insertions(+), 19 deletions(-)
 
  diff --git a/arch/powerpc/include/uapi/asm/kvm.h
  b/arch/powerpc/include/uapi/asm/kvm.h
  index 15f9a00..d7ce449 100644
  --- a/arch/powerpc/include/uapi/asm/kvm.h
  +++ b/arch/powerpc/include/uapi/asm/kvm.h
  @@ -25,6 +25,7 @@
  /* Select powerpc specific features in linux/kvm.h */ #define
  __KVM_HAVE_SPAPR_TCE #define __KVM_HAVE_PPC_SMT
  +#define __KVM_HAVE_GUEST_DEBUG
 
  struct kvm_regs {
  __u64 pc;
  @@ -267,7 +268,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 */
  @@ -279,10 +297,6 @@ struct kvm_guest_debug_arch {
   * Type denotes h/w breakpoint, read watchpoint, write
   * watchpoint or watchpoint (both read and write).
   */
  -#define KVMPPC_DEBUG_NOTYPE0x0
  -#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/booke.c b/arch/powerpc/kvm/booke.c index
  1de93a8..21b0313 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
  @@ -133,6 +133,30 @@ 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. Do not allow guest to change MSR[DE].
  +*/
  +   vcpu-arch.shared-msr |= MSR_DE;
  +   mtspr(SPRN_MSRP, mfspr(SPRN_MSRP) | MSRP_DEP); #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 +174,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, @@
  -736,6 +761,13 @@ static int emulation_exit(struct kvm_run *run, struct
 kvm_vcpu *vcpu)
  run-exit_reason = KVM_EXIT_DCR;
  return RESUME_HOST;
 
  +   case EMULATE_EXIT_USER:
  +   run-exit_reason = KVM_EXIT_DEBUG;
  +   run-debug.arch.address = vcpu-arch.pc;
  +   run-debug.arch.status = 0;
  +   kvmppc_account_exit(vcpu, DEBUG_EXITS);
 
 As mentioned previously, this is wrong and needs to go into the instruction
 emulation code for that opcode.

ok

 
  +   return RESUME_HOST;
  +
  case EMULATE_FAIL:
  printk(KERN_CRIT %s: emulation at %lx failed (%08x)\n,
 __func__, vcpu-arch.pc, vcpu-arch.last_inst); @@ -751,6
  +783,28 @@ 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) {
  +   u32 dbsr = vcpu-arch.dbsr;
  +   run-debug.arch.status = 0;
  +