On Wed, May 29, 2019 at 07:22:19PM -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. Comment/question below: > --- 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 In the !CONFIG_BOOKE case, this will fall through to code which will return 0 for HV KVM or 1 for PR KVM. Is that what was intended? If so, then why do we need the CONFIG_BOOKE case? Isn't hv_enabled always 0 on Book E? In any case, I think this needs at least a /* fall through */ comment in the code, and something explicit in the patch description to say that we intend to return 1 on PR KVM. Paul.
