| Message ID | 1429858066-12088-19-git-send-email-bharata@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Apr 24, 2015 at 12:17:40PM +0530, Bharata B Rao wrote: > When supporting CPU hot removal by parking the vCPU fd and reusing > it during hotplug again, there can be cases where we try to reenable > KVM_CAP_IRQ_XICS CAP for the vCPU for which it was already enabled. > Introduce a boolean member in ICPState to track this and don't > reenable the CAP if it was already enabled earlier. > > This change allows CPU hot removal to work for sPAPR. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> Do you actually need this? Is there any harm in setting the capability multiple times, or could you just ignore the "already set" error? > --- > hw/intc/xics_kvm.c | 10 ++++++++++ > include/hw/ppc/xics.h | 1 + > 2 files changed, 11 insertions(+) > > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index c15453f..5b27bf8 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -331,6 +331,15 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu) > abort(); > } > > + /* > + * If we are reusing a parked vCPU fd corresponding to the CPU > + * which was hot-removed earlier we don't have to renable > + * KVM_CAP_IRQ_XICS capability again. > + */ > + if (ss->cap_irq_xics_enabled) { > + return; > + } > + > if (icpkvm->kernel_xics_fd != -1) { > int ret; > > @@ -343,6 +352,7 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu) > kvm_arch_vcpu_id(cs), strerror(errno)); > exit(1); > } > + ss->cap_irq_xics_enabled = true; > } > } > > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index a214dd7..355a966 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -109,6 +109,7 @@ struct ICPState { > uint8_t pending_priority; > uint8_t mfrr; > qemu_irq output; > + bool cap_irq_xics_enabled; > }; > > #define TYPE_ICS "ics"
On Tue, May 05, 2015 at 05:22:52PM +1000, David Gibson wrote: > On Fri, Apr 24, 2015 at 12:17:40PM +0530, Bharata B Rao wrote: > > When supporting CPU hot removal by parking the vCPU fd and reusing > > it during hotplug again, there can be cases where we try to reenable > > KVM_CAP_IRQ_XICS CAP for the vCPU for which it was already enabled. > > Introduce a boolean member in ICPState to track this and don't > > reenable the CAP if it was already enabled earlier. > > > > This change allows CPU hot removal to work for sPAPR. > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > Do you actually need this? Is there any harm in setting the > capability multiple times, or could you just ignore the "already set" > error? We discussed this last time and concluded that this patch is needed. Ref: http://lists.nongnu.org/archive/html/qemu-devel/2015-03/msg05361.html Regards, Bharata.
On Wed, May 06, 2015 at 11:12:00AM +0530, Bharata B Rao wrote: > On Tue, May 05, 2015 at 05:22:52PM +1000, David Gibson wrote: > > On Fri, Apr 24, 2015 at 12:17:40PM +0530, Bharata B Rao wrote: > > > When supporting CPU hot removal by parking the vCPU fd and reusing > > > it during hotplug again, there can be cases where we try to reenable > > > KVM_CAP_IRQ_XICS CAP for the vCPU for which it was already enabled. > > > Introduce a boolean member in ICPState to track this and don't > > > reenable the CAP if it was already enabled earlier. > > > > > > This change allows CPU hot removal to work for sPAPR. > > > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > > > Do you actually need this? Is there any harm in setting the > > capability multiple times, or could you just ignore the "already set" > > error? > > We discussed this last time and concluded that this patch is needed. > > Ref: http://lists.nongnu.org/archive/html/qemu-devel/2015-03/msg05361.html Ah, ok. Can you include the explanation from that message into the commit message so I don't forget next time (and for the benefit of other reviewers).
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index c15453f..5b27bf8 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -331,6 +331,15 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu) abort(); } + /* + * If we are reusing a parked vCPU fd corresponding to the CPU + * which was hot-removed earlier we don't have to renable + * KVM_CAP_IRQ_XICS capability again. + */ + if (ss->cap_irq_xics_enabled) { + return; + } + if (icpkvm->kernel_xics_fd != -1) { int ret; @@ -343,6 +352,7 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu) kvm_arch_vcpu_id(cs), strerror(errno)); exit(1); } + ss->cap_irq_xics_enabled = true; } } diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index a214dd7..355a966 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -109,6 +109,7 @@ struct ICPState { uint8_t pending_priority; uint8_t mfrr; qemu_irq output; + bool cap_irq_xics_enabled; }; #define TYPE_ICS "ics"
When supporting CPU hot removal by parking the vCPU fd and reusing it during hotplug again, there can be cases where we try to reenable KVM_CAP_IRQ_XICS CAP for the vCPU for which it was already enabled. Introduce a boolean member in ICPState to track this and don't reenable the CAP if it was already enabled earlier. This change allows CPU hot removal to work for sPAPR. Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> --- hw/intc/xics_kvm.c | 10 ++++++++++ include/hw/ppc/xics.h | 1 + 2 files changed, 11 insertions(+)