On Thu, Oct 24, 2019 at 11:28:45AM +0200, Greg Kurz wrote: > On Thu, 24 Oct 2019 14:02:31 +1100 > David Gibson wrote: > > > On Wed, Oct 23, 2019 at 04:52:21PM +0200, Greg Kurz wrote: > > > Now that presenter objects are parented to the interrupt controller, stop > > > relying on CPU_FOREACH() which can race with CPU hotplug and crash QEMU. > > > > > > Signed-off-by: Greg Kurz > > > > So.. we might be able to go further than this. Having the > > TYPE_INTERRUPT_STATS_PROVIDER interrupt on the machine is actually an > > spapr and pnv oddity. In most cases that interface is on the various > > components of the interrupt controller directly. hmp_info_irq() scans > > the whole QOM tree looking for everything with the interface to > > produce the info pic output. > > > > It would be nice if we can do the same for xics and xive. The tricky > > bit is that we do have the possibility of both, in which case the > > individual components would need to know if they're currently "active" > > and minimize their output if so. > > > > Yes but this looks like 4.3 material. If we want to fix this for 4.2, > I'm now thinking it might be safer to keep CPU_FOREACH() and check the > state. Yeah, for 4.2 that sounds like a good idea. > > > > --- > > > hw/intc/spapr_xive.c | 8 +------- > > > hw/intc/xics.c | 12 ++++++++++++ > > > hw/intc/xics_spapr.c | 8 +------- > > > hw/intc/xive.c | 12 ++++++++++++ > > > include/hw/ppc/xics.h | 1 + > > > include/hw/ppc/xive.h | 2 ++ > > > 6 files changed, 29 insertions(+), 14 deletions(-) > > > > > > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > > > index d74ee71e76b4..05763a58cf5d 100644 > > > --- a/hw/intc/spapr_xive.c > > > +++ b/hw/intc/spapr_xive.c > > > @@ -579,14 +579,8 @@ static void spapr_xive_set_irq(SpaprInterruptController *intc, int irq, int val) > > > static void spapr_xive_print_info(SpaprInterruptController *intc, Monitor *mon) > > > { > > > SpaprXive *xive = SPAPR_XIVE(intc); > > > - CPUState *cs; > > > - > > > - CPU_FOREACH(cs) { > > > - PowerPCCPU *cpu = POWERPC_CPU(cs); > > > - > > > - xive_tctx_pic_print_info(spapr_cpu_state(cpu)->tctx, mon); > > > - } > > > > > > + xive_presenter_print_info(XIVE_ROUTER(intc), mon); > > > spapr_xive_pic_print_info(xive, mon); > > > } > > > > > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > > index d5e4db668a4b..6e820c4851f3 100644 > > > --- a/hw/intc/xics.c > > > +++ b/hw/intc/xics.c > > > @@ -88,6 +88,18 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon) > > > } > > > } > > > > > > +static int do_ics_pic_print_icp_infos(Object *child, void *opaque) > > > +{ > > > + icp_pic_print_info(ICP(child), opaque); > > > + return 0; > > > +} > > > + > > > +void ics_pic_print_icp_infos(ICSState *ics, const char *type, Monitor *mon) > > > +{ > > > + object_child_foreach_type(OBJECT(ics), type, do_ics_pic_print_icp_infos, > > > + mon); > > > +} > > > + > > > /* > > > * ICP: Presentation layer > > > */ > > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > > > index 080ed73aad64..7624d693c8da 100644 > > > --- a/hw/intc/xics_spapr.c > > > +++ b/hw/intc/xics_spapr.c > > > @@ -400,14 +400,8 @@ static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) > > > static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) > > > { > > > ICSState *ics = ICS_SPAPR(intc); > > > - CPUState *cs; > > > - > > > - CPU_FOREACH(cs) { > > > - PowerPCCPU *cpu = POWERPC_CPU(cs); > > > - > > > - icp_pic_print_info(spapr_cpu_state(cpu)->icp, mon); > > > - } > > > > > > + ics_pic_print_icp_infos(ics, TYPE_ICP, mon); > > > ics_pic_print_info(ics, mon); > > > } > > > > > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > > > index 8d2da4a11163..40ce43152456 100644 > > > --- a/hw/intc/xive.c > > > +++ b/hw/intc/xive.c > > > @@ -547,6 +547,18 @@ void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon) > > > } > > > } > > > > > > +static int do_xive_presenter_print_info(Object *child, void *opaque) > > > +{ > > > + xive_tctx_pic_print_info(XIVE_TCTX(child), opaque); > > > + return 0; > > > +} > > > + > > > +void xive_presenter_print_info(XiveRouter *xrtr, Monitor *mon) > > > +{ > > > + object_child_foreach_type(OBJECT(xrtr), TYPE_XIVE_TCTX, > > > + do_xive_presenter_print_info, mon); > > > +} > > > + > > > void xive_tctx_reset(XiveTCTX *tctx) > > > { > > > memset(tctx->regs, 0, sizeof(tctx->regs)); > > > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > > > index f4827e748fd8..4de1f421c997 100644 > > > --- a/include/hw/ppc/xics.h > > > +++ b/include/hw/ppc/xics.h > > > @@ -175,6 +175,7 @@ static inline bool ics_irq_free(ICSState *ics, uint32_t srcno) > > > void ics_set_irq_type(ICSState *ics, int srcno, bool lsi); > > > void icp_pic_print_info(ICPState *icp, Monitor *mon); > > > void ics_pic_print_info(ICSState *ics, Monitor *mon); > > > +void ics_pic_print_icp_infos(ICSState *ics, const char *type, Monitor *mon); > > > > > > void ics_resend(ICSState *ics); > > > void icp_resend(ICPState *ss); > > > diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h > > > index 8fd439ec9bba..14690428a0aa 100644 > > > --- a/include/hw/ppc/xive.h > > > +++ b/include/hw/ppc/xive.h > > > @@ -367,6 +367,8 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, > > > XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs); > > > void xive_router_notify(XiveNotifier *xn, uint32_t lisn); > > > > > > +void xive_presenter_print_info(XiveRouter *xrtr, Monitor *mon); > > > + > > > /* > > > * XIVE END ESBs > > > */ > > > > > > -- 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