[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/6] spapr: Don't use CPU_FOREACH() in 'info pic'
From: |
David Gibson |
Subject: |
Re: [PATCH 5/6] spapr: Don't use CPU_FOREACH() in 'info pic' |
Date: |
Sun, 27 Oct 2019 18:01:49 +0100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Thu, Oct 24, 2019 at 11:28:45AM +0200, Greg Kurz wrote:
> On Thu, 24 Oct 2019 14:02:31 +1100
> David Gibson <address@hidden> 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 <address@hidden>
> >
> > 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
signature.asc
Description: PGP signature
- Re: [PATCH 3/6] ppc: Reparent presenter objects to the interrupt controller object, (continued)
[PATCH 4/6] qom: Add object_child_foreach_type() helper function, Greg Kurz, 2019/10/23
[PATCH 5/6] spapr: Don't use CPU_FOREACH() in 'info pic', Greg Kurz, 2019/10/23
[PATCH 6/6] xive: Don't use CPU_FOREACH() to perform CAM line matching, Greg Kurz, 2019/10/23