[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 3/7] ppc/pnv: Introduce a PnvCore reset handler
From: |
David Gibson |
Subject: |
Re: [PATCH v5 3/7] ppc/pnv: Introduce a PnvCore reset handler |
Date: |
Thu, 24 Oct 2019 13:33:22 +1100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Wed, Oct 23, 2019 at 01:18:06PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
>
> On 10/22/19 6:38 PM, Cédric Le Goater wrote:
> > in which individual CPUs are reset. It will ease the introduction of
> > future change reseting the interrupt presenter from the CPU reset
> > handler.
> >
> > Signed-off-by: Cédric Le Goater <address@hidden>
> > Reviewed-by: Greg Kurz <address@hidden>
> > ---
> > hw/ppc/pnv_core.c | 19 +++++++++++++++----
> > 1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> > index b1a7489e7abf..9f981a4940e6 100644
> > --- a/hw/ppc/pnv_core.c
> > +++ b/hw/ppc/pnv_core.c
> > @@ -40,9 +40,8 @@ static const char *pnv_core_cpu_typename(PnvCore *pc)
> > return cpu_type;
> > }
> > -static void pnv_cpu_reset(void *opaque)
> > +static void pnv_core_cpu_reset(PowerPCCPU *cpu)
> > {
> > - PowerPCCPU *cpu = opaque;
> > CPUState *cs = CPU(cpu);
> > CPUPPCState *env = &cpu->env;
> > @@ -192,8 +191,17 @@ static void pnv_realize_vcpu(PowerPCCPU *cpu, PnvChip
> > *chip, Error **errp)
> > /* Set time-base frequency to 512 MHz */
> > cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
> > +}
> > +
> > +static void pnv_core_reset(void *dev)
>
> Here the opaque pointer is a 'PnvCore *pc'.
> If you don't want to call it 'opaque', maybe 'pc' is better.
>
> > +{
> > + CPUCore *cc = CPU_CORE(dev);
> > + PnvCore *pc = PNV_CORE(dev);
>
> This type conversion is not necessary.
>
> What about:
>
> static void pnv_core_reset(void *opaque)
> {
> PnvCore *pc = opaque;
> CPUCore *cc = CPU_CORE(pc);
Those suggestions look good to me, but they can be done as a follow up.
>
> > + int i;
> > - qemu_register_reset(pnv_cpu_reset, cpu);
> > + for (i = 0; i < cc->nr_threads; i++) {
> > + pnv_core_cpu_reset(pc->threads[i]);
> > + }
> > }
> > static void pnv_core_realize(DeviceState *dev, Error **errp)
> > @@ -244,6 +252,8 @@ static void pnv_core_realize(DeviceState *dev, Error
> > **errp)
> > snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id);
> > pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), pcc->xscom_ops,
> > pc, name, PNV_XSCOM_EX_SIZE);
> > +
> > + qemu_register_reset(pnv_core_reset, pc);
> > return;
> > err:
> > @@ -259,7 +269,6 @@ static void pnv_unrealize_vcpu(PowerPCCPU *cpu)
> > {
> > PnvCPUState *pnv_cpu = pnv_cpu_state(cpu);
> > - qemu_unregister_reset(pnv_cpu_reset, cpu);
> > object_unparent(OBJECT(pnv_cpu_state(cpu)->intc));
> > cpu_remove_sync(CPU(cpu));
> > cpu->machine_data = NULL;
> > @@ -273,6 +282,8 @@ static void pnv_core_unrealize(DeviceState *dev, Error
> > **errp)
> > CPUCore *cc = CPU_CORE(dev);
> > int i;
> > + qemu_unregister_reset(pnv_core_reset, pc);
> > +
> > for (i = 0; i < cc->nr_threads; i++) {
> > pnv_unrealize_vcpu(pc->threads[i]);
> > }
> >
>
--
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
signature.asc
Description: PGP signature
- [PATCH v5 0/7] ppc: reset the interrupt presenter from the CPU reset handler, Cédric Le Goater, 2019/10/22
- [PATCH v5 1/7] spapr: move CPU reset after presenter creation, Cédric Le Goater, 2019/10/22
- [PATCH v5 2/7] spapr_cpu_core: Implement DeviceClass::reset, Cédric Le Goater, 2019/10/22
- [PATCH v5 3/7] ppc/pnv: Introduce a PnvCore reset handler, Cédric Le Goater, 2019/10/22
- [PATCH v5 4/7] ppc/pnv: Add a PnvChip pointer to PnvCore, Cédric Le Goater, 2019/10/22
- Re: [PATCH v5 4/7] ppc/pnv: Add a PnvChip pointer to PnvCore, Greg Kurz, 2019/10/24
- Re: [PATCH v5 4/7] ppc/pnv: Add a PnvChip pointer to PnvCore, David Gibson, 2019/10/27
[PATCH v5 5/7] ppc: Reset the interrupt presenter from the CPU reset handler, Cédric Le Goater, 2019/10/22