| Message ID | 20190320183951.29537-2-farosas@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | KVM: PPC: Inform userspace about singlestep support | expand |
On 21/03/2019 05:39, Fabiano Rosas wrote: > When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request > the next instruction to be single stepped via the > KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure. > > We currently don't have support for guest single stepping implemented > in Book3S HV. > > This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order > to inform userspace about the state of single stepping support. > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > arch/powerpc/kvm/powerpc.c | 5 +++++ > include/uapi/linux/kvm.h | 1 + > 2 files changed, 6 insertions(+) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 8885377ec3e0..5ba990b0ec74 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -538,6 +538,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > case KVM_CAP_IMMEDIATE_EXIT: > r = 1; > break; > + case KVM_CAP_PPC_GUEST_DEBUG_SSTEP: > +#ifdef CONFIG_BOOKE In the cover letter (which is not really required for a single patch) you say the capability will be present for BookE and PR KVM (which Book3s) but here it is BookE only, is that intentional? Also, you need to update Documentation/virtual/kvm/api.txt for the new capability. After reading which I started wondering could not we just use existing KVM_CAP_GUEST_DEBUG_HW_BPS? > + r = 1; > + break; > +#endif > case KVM_CAP_PPC_PAIRED_SINGLES: > case KVM_CAP_PPC_OSI: > case KVM_CAP_PPC_GET_PVINFO: > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 6d4ea4b6c922..33e8a4db867e 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_ARM_VM_IPA_SIZE 165 > #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166 > #define KVM_CAP_HYPERV_CPUID 167 > +#define KVM_CAP_PPC_GUEST_DEBUG_SSTEP 168 > > #ifdef KVM_CAP_IRQ_ROUTING > >
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > In the cover letter (which is not really required for a single patch) > you say the capability will be present for BookE and PR KVM (which > Book3s) but here it is BookE only, is that intentional? A few lines below (falling through) we have: /* We support this only for PR */ r = !hv_enabled; > Also, you need to update Documentation/virtual/kvm/api.txt for the new > capability. After reading which I started wondering could not we just > use existing KVM_CAP_GUEST_DEBUG_HW_BPS? We _could_, but I think that would conflate two different concepts. Single stepping does not necessarily makes use of hardware breakpoints (e.g. Trace Interrupt on Book3s PR). I also think we should use KVM_CAP_GUEST_DEBUG_HW_BPS in the future to let QEMU know about: i) the lack of hardware breakpoints in Book3s and ii) BookE's hardware breakpoints (Instruction Address Compare) that are currently not being reported via HW_BPS.
On Wed, Mar 20, 2019 at 03:39:50PM -0300, Fabiano Rosas wrote: > When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request > the next instruction to be single stepped via the > KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure. > > We currently don't have support for guest single stepping implemented > in Book3S HV. > > This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order > to inform userspace about the state of single stepping support. > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > arch/powerpc/kvm/powerpc.c | 5 +++++ > include/uapi/linux/kvm.h | 1 + > 2 files changed, 6 insertions(+) I assume since this is [RFC] that you're not expecting it to go upstream as-is. If it were to go upstream it would need to include an update to Documentation/virtual/kvm/api.txt. Paul.
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 8885377ec3e0..5ba990b0ec74 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -538,6 +538,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_IMMEDIATE_EXIT: r = 1; break; + case KVM_CAP_PPC_GUEST_DEBUG_SSTEP: +#ifdef CONFIG_BOOKE + r = 1; + break; +#endif case KVM_CAP_PPC_PAIRED_SINGLES: case KVM_CAP_PPC_OSI: case KVM_CAP_PPC_GET_PVINFO: diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 6d4ea4b6c922..33e8a4db867e 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_ARM_VM_IPA_SIZE 165 #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166 #define KVM_CAP_HYPERV_CPUID 167 +#define KVM_CAP_PPC_GUEST_DEBUG_SSTEP 168 #ifdef KVM_CAP_IRQ_ROUTING
When calling the KVM_SET_GUEST_DEBUG ioctl, userspace might request the next instruction to be single stepped via the KVM_GUESTDBG_SINGLESTEP control bit of the kvm_guest_debug structure. We currently don't have support for guest single stepping implemented in Book3S HV. This patch adds the KVM_CAP_PPC_GUEST_DEBUG_SSTEP capability in order to inform userspace about the state of single stepping support. Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- arch/powerpc/kvm/powerpc.c | 5 +++++ include/uapi/linux/kvm.h | 1 + 2 files changed, 6 insertions(+)