On Fri, Jun 05, 2015 at 06:09:38PM +1000, Alexey Kardashevskiy wrote:
> On 06/05/2015 02:25 PM, Bharata B Rao wrote:
> >XICS is setup for each CPU during initialization. Provide a routine
> >to undo the same when CPU is unplugged. Also ensure xics reset doesn't set
> >irq for CPUs that are already unplugged.
> >
> >This allows reboot of a VM that has undergone CPU hotplug and unplug
> >to work correctly.
> >
> >Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
> >---
> > hw/intc/xics.c | 14 ++++++++++++++
> > hw/intc/xics_kvm.c | 15 +++++++++++++--
> > include/hw/ppc/xics.h | 2 ++
> > 3 files changed, 29 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> >index 924b1ae..3f87f82 100644
> >--- a/hw/intc/xics.c
> >+++ b/hw/intc/xics.c
> >@@ -44,6 +44,20 @@ static int get_cpu_index_by_dt_id(int cpu_dt_id)
> > return -1;
> > }
> >
> >+void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu)
>
>
> xics_cpu_destroy() is not used by anything, may be push it later with the
> stuff which needs it?
Yeah it is not used in this patchset, will leave to David/agraf
to see if they want this dropped from this series.
>
>
> >+{
> >+ CPUState *cs = CPU(cpu);
> >+ XICSStateClass *info = XICS_COMMON_GET_CLASS(icp);
> >+ ICPState *ss = &icp->ss[cs->cpu_index];
> >+
> >+ assert(cs->cpu_index < icp->nr_servers);
> >+
> >+ ss->output = NULL;
> >+ if (info->cpu_destroy) {
> >+ info->cpu_destroy(icp, cpu);
> >+ }
> >+}
> >+
> > void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
> > {
> > CPUState *cs = CPU(cpu);
> >diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> >index d58729c..e35f319 100644
> >--- a/hw/intc/xics_kvm.c
> >+++ b/hw/intc/xics_kvm.c
> >@@ -109,8 +109,10 @@ static void icp_kvm_reset(DeviceState *dev)
> > icp->pending_priority = 0xff;
> > icp->mfrr = 0xff;
> >
> >- /* Make all outputs are deasserted */
> >- qemu_set_irq(icp->output, 0);
> >+ /* Make all outputs are deasserted only if the CPU thread is in use */
> >+ if (icp->output) {
> >+ qemu_set_irq(icp->output, 0);
> >+ }
>
>
> This feels unrelated to what the patch claims that it does. Or
> xics_cpu_destroy() somehow indirectly calls icp_kvm_reset()?
When a VM is rebooted, device_reset of icp device will result in
icp_kvm_reset() which will do qemu_set_irq(). This shouldn't happen for
a CPU that is already removed. So above check ensures that we
don't try to desssert irq for a removed CPU.
Regards,
Bharata.
