Paul Mackerras <paulus@xxxxxxxxxx> writes: > 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? Yes. The intention is to return 0 for HV and 1 for everything else. > If so, then why do we need the CONFIG_BOOKE case? Isn't hv_enabled > always 0 on Book E? Good point. I made a mistake there indeed. > 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. I'll send another version. Thanks
