| Message ID | 20150320155127.GA19984@in.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20.03.15 16:51, Bharata B Rao wrote: > On Fri, Mar 20, 2015 at 12:34:18PM +0100, Alexander Graf wrote: >> >> >> On 20.03.15 12:26, Paul Mackerras wrote: >>> On Fri, Mar 20, 2015 at 12:01:32PM +0100, Alexander Graf wrote: >>>> >>>> >>>> On 20.03.15 10:39, Paul Mackerras wrote: >>>>> From: Bharata B Rao <bharata@linux.vnet.ibm.com> >>>>> >>>>> Since KVM isn't equipped to handle closure of vcpu fd from userspace(QEMU) >>>>> correctly, certain work arounds have to be employed to allow reuse of >>>>> vcpu array slot in KVM during cpu hot plug/unplug from guest. One such >>>>> proposed workaround is to park the vcpu fd in userspace during cpu unplug >>>>> and reuse it later during next hotplug. >>>>> >>>>> More details can be found here: >>>>> KVM: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html >>>>> QEMU: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00859.html >>>>> >>>>> In order to support this workaround with PowerPC KVM, don't create or >>>>> initialize ICP if the vCPU is found to be already associated with an ICP. >>>>> >>>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >>>>> Signed-off-by: Paul Mackerras <paulus@samba.org> >>>> >>>> This probably makes some sense, but please make sure that user space has >>>> some way to figure out whether hotplug works at all. >>> >>> Bharata is working on the qemu side of all this, so I assume he has >>> that covered. >> >> Well, so far the kernel doesn't expose anything he can query, so I >> suppose he just blindly assumes that older host kernels will randomly >> break and nobody cares. I'd rather prefer to see a CAP exposed that qemu >> can check on. > > I see that you have already taken this into your tree. I have an updated > patch to expose a CAP. If the below patch looks ok, then let me know how > you would prefer to take this patch in. > > Regards, > Bharata. > > KVM: PPC: BOOK3S: Allow reuse of vCPU object > > From: Bharata B Rao <bharata@linux.vnet.ibm.com> > > Since KVM isn't equipped to handle closure of vcpu fd from userspace(QEMU) > correctly, certain work arounds have to be employed to allow reuse of > vcpu array slot in KVM during cpu hot plug/unplug from guest. One such > proposed workaround is to park the vcpu fd in userspace during cpu unplug > and reuse it later during next hotplug. > > More details can be found here: > KVM: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html > QEMU: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00859.html > > In order to support this workaround with PowerPC KVM, don't create or > initialize ICP if the vCPU is found to be already associated with an ICP. > User space (QEMU) can reuse the vCPU after checking for the availability > of KVM_CAP_SPAPR_REUSE_VCPU capability. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > --- > arch/powerpc/kvm/book3s_xics.c | 9 +++++++-- > arch/powerpc/kvm/powerpc.c | 12 ++++++++++++ > include/uapi/linux/kvm.h | 1 + > 3 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c > index a4a8d9f..ead3a35 100644 > --- a/arch/powerpc/kvm/book3s_xics.c > +++ b/arch/powerpc/kvm/book3s_xics.c > @@ -1313,8 +1313,13 @@ int kvmppc_xics_connect_vcpu(struct kvm_device *dev, struct kvm_vcpu *vcpu, > return -EPERM; > if (xics->kvm != vcpu->kvm) > return -EPERM; > - if (vcpu->arch.irq_type) > - return -EBUSY; > + > + /* > + * If irq_type is already set, don't reinialize but > + * return success allowing this vcpu to be reused. > + */ > + if (vcpu->arch.irq_type != KVMPPC_IRQ_DEFAULT) > + return 0; > > r = kvmppc_xics_create_icp(vcpu, xcpu); > if (!r) > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 27c0fac..5b7007c 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -564,6 +564,18 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > r = 1; > break; > #endif > + case KVM_CAP_SPAPR_REUSE_VCPU: > + /* > + * Kernel currently doesn't support closing of vCPU fd from > + * user space (QEMU) correctly. Hence the option available > + * is to park the vCPU fd in user space whenever a guest > + * CPU is hot removed and reuse the same later when another > + * guest CPU is hotplugged. This capability determines whether > + * it is safe to assume if parking of vCPU fd and reuse from > + * user space works for sPAPR guests. I don't see how the code you're changing here has anything to do with parking vcpus. It's all about being able to call connect on an already connected vcpu and not erroring out. Please reflect this in the cap name and description. You also need to update Documentation/virtual/kvm/api.txt. Furthermore, thinking about this a bit more, I might still miss the exact case why you need this. Why is QEMU issuing a connect again? Could it maybe just not do it? Also please just send a patch on top of this one for the CAP. It doesn't hurt to separate them. Unless we end up concluding that you don't need it at all, then I'll just remove your patch from my queue again ;). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Mar 21, 2015 at 8:28 PM, Alexander Graf <agraf@suse.de> wrote: > > > On 20.03.15 16:51, Bharata B Rao wrote: >> On Fri, Mar 20, 2015 at 12:34:18PM +0100, Alexander Graf wrote: >>> >>> >>> On 20.03.15 12:26, Paul Mackerras wrote: >>>> On Fri, Mar 20, 2015 at 12:01:32PM +0100, Alexander Graf wrote: >>>>> >>>>> >>>>> On 20.03.15 10:39, Paul Mackerras wrote: >>>>>> From: Bharata B Rao <bharata@linux.vnet.ibm.com> >>>>>> >>>>>> Since KVM isn't equipped to handle closure of vcpu fd from userspace(QEMU) >>>>>> correctly, certain work arounds have to be employed to allow reuse of >>>>>> vcpu array slot in KVM during cpu hot plug/unplug from guest. One such >>>>>> proposed workaround is to park the vcpu fd in userspace during cpu unplug >>>>>> and reuse it later during next hotplug. >>>>>> >>>>>> More details can be found here: >>>>>> KVM: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html >>>>>> QEMU: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00859.html >>>>>> >>>>>> In order to support this workaround with PowerPC KVM, don't create or >>>>>> initialize ICP if the vCPU is found to be already associated with an ICP. >>>>>> >>>>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >>>>>> Signed-off-by: Paul Mackerras <paulus@samba.org> >>>>> >>>>> This probably makes some sense, but please make sure that user space has >>>>> some way to figure out whether hotplug works at all. >>>> >>>> Bharata is working on the qemu side of all this, so I assume he has >>>> that covered. >>> >>> Well, so far the kernel doesn't expose anything he can query, so I >>> suppose he just blindly assumes that older host kernels will randomly >>> break and nobody cares. I'd rather prefer to see a CAP exposed that qemu >>> can check on. >> >> I see that you have already taken this into your tree. I have an updated >> patch to expose a CAP. If the below patch looks ok, then let me know how >> you would prefer to take this patch in. >> >> Regards, >> Bharata. >> >> KVM: PPC: BOOK3S: Allow reuse of vCPU object >> >> From: Bharata B Rao <bharata@linux.vnet.ibm.com> >> >> Since KVM isn't equipped to handle closure of vcpu fd from userspace(QEMU) >> correctly, certain work arounds have to be employed to allow reuse of >> vcpu array slot in KVM during cpu hot plug/unplug from guest. One such >> proposed workaround is to park the vcpu fd in userspace during cpu unplug >> and reuse it later during next hotplug. >> >> More details can be found here: >> KVM: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html >> QEMU: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00859.html >> >> In order to support this workaround with PowerPC KVM, don't create or >> initialize ICP if the vCPU is found to be already associated with an ICP. >> User space (QEMU) can reuse the vCPU after checking for the availability >> of KVM_CAP_SPAPR_REUSE_VCPU capability. >> >> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >> --- >> arch/powerpc/kvm/book3s_xics.c | 9 +++++++-- >> arch/powerpc/kvm/powerpc.c | 12 ++++++++++++ >> include/uapi/linux/kvm.h | 1 + >> 3 files changed, 20 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c >> index a4a8d9f..ead3a35 100644 >> --- a/arch/powerpc/kvm/book3s_xics.c >> +++ b/arch/powerpc/kvm/book3s_xics.c >> @@ -1313,8 +1313,13 @@ int kvmppc_xics_connect_vcpu(struct kvm_device *dev, struct kvm_vcpu *vcpu, >> return -EPERM; >> if (xics->kvm != vcpu->kvm) >> return -EPERM; >> - if (vcpu->arch.irq_type) >> - return -EBUSY; >> + >> + /* >> + * If irq_type is already set, don't reinialize but >> + * return success allowing this vcpu to be reused. >> + */ >> + if (vcpu->arch.irq_type != KVMPPC_IRQ_DEFAULT) >> + return 0; >> >> r = kvmppc_xics_create_icp(vcpu, xcpu); >> if (!r) >> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >> index 27c0fac..5b7007c 100644 >> --- a/arch/powerpc/kvm/powerpc.c >> +++ b/arch/powerpc/kvm/powerpc.c >> @@ -564,6 +564,18 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) >> r = 1; >> break; >> #endif >> + case KVM_CAP_SPAPR_REUSE_VCPU: >> + /* >> + * Kernel currently doesn't support closing of vCPU fd from >> + * user space (QEMU) correctly. Hence the option available >> + * is to park the vCPU fd in user space whenever a guest >> + * CPU is hot removed and reuse the same later when another >> + * guest CPU is hotplugged. This capability determines whether >> + * it is safe to assume if parking of vCPU fd and reuse from >> + * user space works for sPAPR guests. > > I don't see how the code you're changing here has anything to do with > parking vcpus. It's all about being able to call connect on an already > connected vcpu and not erroring out. Please reflect this in the cap name > and description. > > You also need to update Documentation/virtual/kvm/api.txt. > > Furthermore, thinking about this a bit more, I might still miss the > exact case why you need this. Why is QEMU issuing a connect again? Could > it maybe just not do it? Thinking more, I see that I can handle this entirely in user space. So no need for this patch. Sorry for the noise :( Regards, Bharata. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23.03.15 08:50, Bharata B Rao wrote: > On Sat, Mar 21, 2015 at 8:28 PM, Alexander Graf <agraf@suse.de> wrote: >> >> >> On 20.03.15 16:51, Bharata B Rao wrote: >>> On Fri, Mar 20, 2015 at 12:34:18PM +0100, Alexander Graf wrote: >>>> >>>> >>>> On 20.03.15 12:26, Paul Mackerras wrote: >>>>> On Fri, Mar 20, 2015 at 12:01:32PM +0100, Alexander Graf wrote: >>>>>> >>>>>> >>>>>> On 20.03.15 10:39, Paul Mackerras wrote: >>>>>>> From: Bharata B Rao <bharata@linux.vnet.ibm.com> >>>>>>> >>>>>>> Since KVM isn't equipped to handle closure of vcpu fd from userspace(QEMU) >>>>>>> correctly, certain work arounds have to be employed to allow reuse of >>>>>>> vcpu array slot in KVM during cpu hot plug/unplug from guest. One such >>>>>>> proposed workaround is to park the vcpu fd in userspace during cpu unplug >>>>>>> and reuse it later during next hotplug. >>>>>>> >>>>>>> More details can be found here: >>>>>>> KVM: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html >>>>>>> QEMU: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00859.html >>>>>>> >>>>>>> In order to support this workaround with PowerPC KVM, don't create or >>>>>>> initialize ICP if the vCPU is found to be already associated with an ICP. >>>>>>> >>>>>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >>>>>>> Signed-off-by: Paul Mackerras <paulus@samba.org> >>>>>> >>>>>> This probably makes some sense, but please make sure that user space has >>>>>> some way to figure out whether hotplug works at all. >>>>> >>>>> Bharata is working on the qemu side of all this, so I assume he has >>>>> that covered. >>>> >>>> Well, so far the kernel doesn't expose anything he can query, so I >>>> suppose he just blindly assumes that older host kernels will randomly >>>> break and nobody cares. I'd rather prefer to see a CAP exposed that qemu >>>> can check on. >>> >>> I see that you have already taken this into your tree. I have an updated >>> patch to expose a CAP. If the below patch looks ok, then let me know how >>> you would prefer to take this patch in. >>> >>> Regards, >>> Bharata. >>> >>> KVM: PPC: BOOK3S: Allow reuse of vCPU object >>> >>> From: Bharata B Rao <bharata@linux.vnet.ibm.com> >>> >>> Since KVM isn't equipped to handle closure of vcpu fd from userspace(QEMU) >>> correctly, certain work arounds have to be employed to allow reuse of >>> vcpu array slot in KVM during cpu hot plug/unplug from guest. One such >>> proposed workaround is to park the vcpu fd in userspace during cpu unplug >>> and reuse it later during next hotplug. >>> >>> More details can be found here: >>> KVM: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html >>> QEMU: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00859.html >>> >>> In order to support this workaround with PowerPC KVM, don't create or >>> initialize ICP if the vCPU is found to be already associated with an ICP. >>> User space (QEMU) can reuse the vCPU after checking for the availability >>> of KVM_CAP_SPAPR_REUSE_VCPU capability. >>> >>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >>> --- >>> arch/powerpc/kvm/book3s_xics.c | 9 +++++++-- >>> arch/powerpc/kvm/powerpc.c | 12 ++++++++++++ >>> include/uapi/linux/kvm.h | 1 + >>> 3 files changed, 20 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c >>> index a4a8d9f..ead3a35 100644 >>> --- a/arch/powerpc/kvm/book3s_xics.c >>> +++ b/arch/powerpc/kvm/book3s_xics.c >>> @@ -1313,8 +1313,13 @@ int kvmppc_xics_connect_vcpu(struct kvm_device *dev, struct kvm_vcpu *vcpu, >>> return -EPERM; >>> if (xics->kvm != vcpu->kvm) >>> return -EPERM; >>> - if (vcpu->arch.irq_type) >>> - return -EBUSY; >>> + >>> + /* >>> + * If irq_type is already set, don't reinialize but >>> + * return success allowing this vcpu to be reused. >>> + */ >>> + if (vcpu->arch.irq_type != KVMPPC_IRQ_DEFAULT) >>> + return 0; >>> >>> r = kvmppc_xics_create_icp(vcpu, xcpu); >>> if (!r) >>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >>> index 27c0fac..5b7007c 100644 >>> --- a/arch/powerpc/kvm/powerpc.c >>> +++ b/arch/powerpc/kvm/powerpc.c >>> @@ -564,6 +564,18 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) >>> r = 1; >>> break; >>> #endif >>> + case KVM_CAP_SPAPR_REUSE_VCPU: >>> + /* >>> + * Kernel currently doesn't support closing of vCPU fd from >>> + * user space (QEMU) correctly. Hence the option available >>> + * is to park the vCPU fd in user space whenever a guest >>> + * CPU is hot removed and reuse the same later when another >>> + * guest CPU is hotplugged. This capability determines whether >>> + * it is safe to assume if parking of vCPU fd and reuse from >>> + * user space works for sPAPR guests. >> >> I don't see how the code you're changing here has anything to do with >> parking vcpus. It's all about being able to call connect on an already >> connected vcpu and not erroring out. Please reflect this in the cap name >> and description. >> >> You also need to update Documentation/virtual/kvm/api.txt. >> >> Furthermore, thinking about this a bit more, I might still miss the >> exact case why you need this. Why is QEMU issuing a connect again? Could >> it maybe just not do it? > > Thinking more, I see that I can handle this entirely in user space. So > no need for this patch. > > Sorry for the noise :( Perfect, removed this patch from my queue again :). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c index a4a8d9f..ead3a35 100644 --- a/arch/powerpc/kvm/book3s_xics.c +++ b/arch/powerpc/kvm/book3s_xics.c @@ -1313,8 +1313,13 @@ int kvmppc_xics_connect_vcpu(struct kvm_device *dev, struct kvm_vcpu *vcpu, return -EPERM; if (xics->kvm != vcpu->kvm) return -EPERM; - if (vcpu->arch.irq_type) - return -EBUSY; + + /* + * If irq_type is already set, don't reinialize but + * return success allowing this vcpu to be reused. + */ + if (vcpu->arch.irq_type != KVMPPC_IRQ_DEFAULT) + return 0; r = kvmppc_xics_create_icp(vcpu, xcpu); if (!r) diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 27c0fac..5b7007c 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -564,6 +564,18 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r = 1; break; #endif + case KVM_CAP_SPAPR_REUSE_VCPU: + /* + * Kernel currently doesn't support closing of vCPU fd from + * user space (QEMU) correctly. Hence the option available + * is to park the vCPU fd in user space whenever a guest + * CPU is hot removed and reuse the same later when another + * guest CPU is hotplugged. This capability determines whether + * it is safe to assume if parking of vCPU fd and reuse from + * user space works for sPAPR guests. + */ + r = 1; + break; default: r = 0; break; diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 8055706..8464755 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -760,6 +760,7 @@ struct kvm_ppc_smmu_info { #define KVM_CAP_PPC_ENABLE_HCALL 104 #define KVM_CAP_CHECK_EXTENSION_VM 105 #define KVM_CAP_S390_USER_SIGP 106 +#define KVM_CAP_SPAPR_REUSE_VCPU 107 #ifdef KVM_CAP_IRQ_ROUTING