[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [RFC PATCH v2.1 11/12] xics, xics_kvm: Handle CPU unplug
From: |
Michael Roth |
Subject: |
Re: [Qemu-ppc] [RFC PATCH v2.1 11/12] xics, xics_kvm: Handle CPU unplug correctly |
Date: |
Fri, 08 Apr 2016 18:40:29 -0500 |
User-agent: |
alot/0.3.6 |
Quoting David Gibson (2016-04-05 19:43:46)
> On Tue, Apr 05, 2016 at 07:24:39PM -0500, Michael Roth wrote:
> > Quoting Bharata B Rao (2016-03-31 03:39:20)
> > > XICS is setup for each CPU during initialization. Provide a routine
> > > to undo the same when CPU is unplugged. While here, move ss->cs management
> > > into xics from xics_kvm since there is nothing KVM specific in it.
> > > 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 <address@hidden>
> > > Reviewed-by: David Gibson <address@hidden>
> > > ---
> > > hw/intc/xics.c | 14 ++++++++++++++
> > > hw/intc/xics_kvm.c | 8 ++++----
> > > include/hw/ppc/xics.h | 1 +
> > > 3 files changed, 19 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> > > index 213a370..9fdb551 100644
> > > --- a/hw/intc/xics.c
> > > +++ b/hw/intc/xics.c
> > > @@ -45,6 +45,18 @@ static int get_cpu_index_by_dt_id(int cpu_dt_id)
> > > return -1;
> > > }
> > >
> > > +void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu)
> > > +{
> > > + CPUState *cs = CPU(cpu);
> > > + ICPState *ss = &icp->ss[cs->cpu_index];
> > > +
> >
> > If the following assertion is false, I think you'd get an OOB before you
> > reached it due to the assignment above.
>
> It should be ok. The assignment above will compute an out of bounds
> address, but it doesn't dereference it.
Ahh, right. I was thinking there was an actual access for some reason.
I might still prefer moving the assignment after the assert from a
stylistic standpoint, in case other variables get introduced in the
future that rely on *ss. Just a nit though.
>
> > > + assert(cs->cpu_index < icp->nr_servers);
> > > + assert(cs == ss->cs);
> > > +
> > > + ss->output = NULL;
> > > + ss->cs = NULL;
> > > +}
> > > +
> >
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson