On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote: > On 03/07/2019 04:07, David Gibson wrote: > > On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote: > >> This is to perform lookups in the NVT table when a vCPU is dispatched > >> and possibly resend interrupts. > > > > I'm slightly confused by this one. Aren't there multiple router > > objects, each of which can deliver to any thread? In which case what > > router object is associated with a specific TCTX? > > when a vCPU is dispatched on a HW thread, the hypervisor does a store > on the CAM line to store the VP id. At that time, it checks the IPB in > the associated NVT structure and notifies the thread if an interrupt is > pending. > > We need to do a NVT lookup, just like the presenter in HW, hence the > router pointer. You should look at the following patch which clarifies > the resend sequence.
Hm, ok. > >> Future XIVE chip will use a different class for the model of the > >> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'. > > > > This seems odd to me, shouldn't it be an interface pointer or > > something in that case? > > I have duplicated most of the XIVE models for P10 because the internal > structures have changed. I managed to keep the XiveSource and XiveTCTX > but we now have a Xive10Router, this is the reason why. Right, but XiveRouter and Xive10Router must have something in common if they can both be used here. Usually that's expressed as a shared QOM interface - in which case you can use a pointer to the interface, rathe than using Object * which kind of implies *anything* can go here. > > If I was to duplicate XiveTCTX also, I will switch it back to a XiveRouter > pointer in the P9 version. > > C. > > > >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > > > >> --- > >> include/hw/ppc/xive.h | 4 +++- > >> hw/intc/xive.c | 11 ++++++++++- > >> hw/ppc/pnv.c | 2 +- > >> hw/ppc/spapr_irq.c | 2 +- > >> 4 files changed, 15 insertions(+), 4 deletions(-) > >> > >> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h > >> index d922524982d3..b764e1e4e6d4 100644 > >> --- a/include/hw/ppc/xive.h > >> +++ b/include/hw/ppc/xive.h > >> @@ -321,6 +321,8 @@ typedef struct XiveTCTX { > >> qemu_irq os_output; > >> > >> uint8_t regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE]; > >> + > >> + Object *xrtr; > >> } XiveTCTX; > >> > >> /* > >> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, > >> uint64_t value, > >> uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size); > >> > >> void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon); > >> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp); > >> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp); > >> > >> static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t > >> nvt_idx) > >> { > >> diff --git a/hw/intc/xive.c b/hw/intc/xive.c > >> index f7ba1c3b622f..56700681884f 100644 > >> --- a/hw/intc/xive.c > >> +++ b/hw/intc/xive.c > >> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error > >> **errp) > >> Object *obj; > >> Error *local_err = NULL; > >> > >> + obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err); > >> + if (!obj) { > >> + error_propagate(errp, local_err); > >> + error_prepend(errp, "required link 'xrtr' not found: "); > >> + return; > >> + } > >> + tctx->xrtr = obj; > >> + > >> obj = object_property_get_link(OBJECT(dev), "cpu", &local_err); > >> if (!obj) { > >> error_propagate(errp, local_err); > >> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = { > >> .class_init = xive_tctx_class_init, > >> }; > >> > >> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp) > >> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp) > >> { > >> Error *local_err = NULL; > >> Object *obj; > >> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter > >> *xrtr, Error **errp) > >> object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort); > >> object_unref(obj); > >> object_property_add_const_link(obj, "cpu", cpu, &error_abort); > >> + object_property_add_const_link(obj, "xrtr", xrtr, &error_abort); > >> object_property_set_bool(obj, true, "realized", &local_err); > >> if (local_err) { > >> goto error; > >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > >> index b87e01e5b925..11916dc273c2 100644 > >> --- a/hw/ppc/pnv.c > >> +++ b/hw/ppc/pnv.c > >> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, > >> PowerPCCPU *cpu, > >> * controller object is initialized afterwards. Hopefully, it's > >> * only used at runtime. > >> */ > >> - obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), > >> &local_err); > >> + obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err); > >> if (local_err) { > >> error_propagate(errp, local_err); > >> return; > >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > >> index b2b01e850de8..5b3c3c50967b 100644 > >> --- a/hw/ppc/spapr_irq.c > >> +++ b/hw/ppc/spapr_irq.c > >> @@ -353,7 +353,7 @@ static void > >> spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr, > >> Object *obj; > >> SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); > >> > >> - obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), > >> &local_err); > >> + obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err); > >> if (local_err) { > >> error_propagate(errp, local_err); > >> return; > > > -- 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