| Message ID | 20190128094625.4428-4-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc: support for the baremetal XIVE interrupt controller (POWER9) | expand |
On Mon, Jan 28, 2019 at 10:46:09AM +0100, Cédric Le Goater wrote: > The PowerNV machine can perform indirect loads and stores on the TIMA > on behalf of another CPU. The PIR of the CPU is controlled by a set of > 4 registers, one per TIMA page. To know which page is being accessed, > we need to inform the controller model of the operation offset. This doesn't seem like the right place to handle this. Shouldn't you instead be passing a different CPUState in for "foreign" TCTX accesses? > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > include/hw/ppc/xive.h | 4 ++-- > hw/intc/spapr_xive.c | 3 ++- > hw/intc/xive.c | 12 +++++++----- > 3 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h > index 04d54e8315f7..a1f5ea2d9143 100644 > --- a/include/hw/ppc/xive.h > +++ b/include/hw/ppc/xive.h > @@ -352,7 +352,7 @@ typedef struct XiveRouterClass { > XiveNVT *nvt); > int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, > XiveNVT *nvt, uint8_t word_number); > - XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs); > + XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset); > } XiveRouterClass; > > void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon); > @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, > XiveNVT *nvt); > int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, > XiveNVT *nvt, uint8_t word_number); > -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs); > +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset); > > /* > * XIVE END ESBs > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > index a0f5ff929447..c41ee96c4c84 100644 > --- a/hw/intc/spapr_xive.c > +++ b/hw/intc/spapr_xive.c > @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, > g_assert_not_reached(); > } > > -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs) > +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs, > + hwaddr offset) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index f5642f2338de..39dff557fadc 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write) > static void xive_tm_write(void *opaque, hwaddr offset, > uint64_t value, unsigned size) > { > - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); > + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, > + offset); > const XiveTmOp *xto; > > /* > @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset, > > static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size) > { > - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); > + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, > + offset); > const XiveTmOp *xto; > > /* > @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, > return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number); > } > > -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs) > +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset) > { > XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr); > > - return xrc->get_tctx(xrtr, cs); > + return xrc->get_tctx(xrtr, cs, offset); > } > > /* > @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > */ > > CPU_FOREACH(cs) { > - XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs); > + XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0); > int ring; > > /*
On 2/12/19 5:34 AM, David Gibson wrote: > On Mon, Jan 28, 2019 at 10:46:09AM +0100, Cédric Le Goater wrote: >> The PowerNV machine can perform indirect loads and stores on the TIMA >> on behalf of another CPU. The PIR of the CPU is controlled by a set of >> 4 registers, one per TIMA page. To know which page is being accessed, >> we need to inform the controller model of the operation offset. > > This doesn't seem like the right place to handle this. Shouldn't you > instead be passing a different CPUState in for "foreign" TCTX > accesses? This patch only adds the page offset required to pick the correct register from the XIVE IC. May be I should have introduced this page offset extension in the baremetal patch. I might reorganize the patchset in the next round. Anyhow, we pass a different XiveTCTX instead. See the routines in PnvXive, xive_tm_indirect_read() and xive_tm_indirect_write(). We don't pass a CPUState because we depend on the machine data to retrieve the XiveTCTX and that would be problematic to do in the common XIVE routines. C. > >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> include/hw/ppc/xive.h | 4 ++-- >> hw/intc/spapr_xive.c | 3 ++- >> hw/intc/xive.c | 12 +++++++----- >> 3 files changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h >> index 04d54e8315f7..a1f5ea2d9143 100644 >> --- a/include/hw/ppc/xive.h >> +++ b/include/hw/ppc/xive.h >> @@ -352,7 +352,7 @@ typedef struct XiveRouterClass { >> XiveNVT *nvt); >> int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >> XiveNVT *nvt, uint8_t word_number); >> - XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs); >> + XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset); >> } XiveRouterClass; >> >> void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon); >> @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >> XiveNVT *nvt); >> int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >> XiveNVT *nvt, uint8_t word_number); >> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs); >> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset); >> >> /* >> * XIVE END ESBs >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c >> index a0f5ff929447..c41ee96c4c84 100644 >> --- a/hw/intc/spapr_xive.c >> +++ b/hw/intc/spapr_xive.c >> @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, >> g_assert_not_reached(); >> } >> >> -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs) >> +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs, >> + hwaddr offset) >> { >> PowerPCCPU *cpu = POWERPC_CPU(cs); >> >> diff --git a/hw/intc/xive.c b/hw/intc/xive.c >> index f5642f2338de..39dff557fadc 100644 >> --- a/hw/intc/xive.c >> +++ b/hw/intc/xive.c >> @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write) >> static void xive_tm_write(void *opaque, hwaddr offset, >> uint64_t value, unsigned size) >> { >> - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); >> + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, >> + offset); >> const XiveTmOp *xto; >> >> /* >> @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset, >> >> static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size) >> { >> - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); >> + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, >> + offset); >> const XiveTmOp *xto; >> >> /* >> @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >> return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number); >> } >> >> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs) >> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset) >> { >> XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr); >> >> - return xrc->get_tctx(xrtr, cs); >> + return xrc->get_tctx(xrtr, cs, offset); >> } >> >> /* >> @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >> */ >> >> CPU_FOREACH(cs) { >> - XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs); >> + XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0); >> int ring; >> >> /* >
On 2/12/19 9:25 AM, Cédric Le Goater wrote: > On 2/12/19 5:34 AM, David Gibson wrote: >> On Mon, Jan 28, 2019 at 10:46:09AM +0100, Cédric Le Goater wrote: >>> The PowerNV machine can perform indirect loads and stores on the TIMA >>> on behalf of another CPU. The PIR of the CPU is controlled by a set of >>> 4 registers, one per TIMA page. To know which page is being accessed, >>> we need to inform the controller model of the operation offset. >> >> This doesn't seem like the right place to handle this. Shouldn't you >> instead be passing a different CPUState in for "foreign" TCTX >> accesses? > > This patch only adds the page offset required to pick the correct > register from the XIVE IC. May be I should have introduced this > page offset extension in the baremetal patch. I might reorganize > the patchset in the next round. I had a chat with our hardware designer and the four indirect access registers of the XIVE IC have no relation with the four pages exposing the TIMA. It's just four different engines. We can drop this change. next patch "ppc/pnv: xive: export the TIMA memory accessors" is still needed though. C. > Anyhow, we pass a different XiveTCTX instead. See the routines in > PnvXive, xive_tm_indirect_read() and xive_tm_indirect_write(). > > We don't pass a CPUState because we depend on the machine data to > retrieve the XiveTCTX and that would be problematic to do in the > common XIVE routines. > > C. > > >> >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> --- >>> include/hw/ppc/xive.h | 4 ++-- >>> hw/intc/spapr_xive.c | 3 ++- >>> hw/intc/xive.c | 12 +++++++----- >>> 3 files changed, 11 insertions(+), 8 deletions(-) >>> >>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h >>> index 04d54e8315f7..a1f5ea2d9143 100644 >>> --- a/include/hw/ppc/xive.h >>> +++ b/include/hw/ppc/xive.h >>> @@ -352,7 +352,7 @@ typedef struct XiveRouterClass { >>> XiveNVT *nvt); >>> int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >>> XiveNVT *nvt, uint8_t word_number); >>> - XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs); >>> + XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset); >>> } XiveRouterClass; >>> >>> void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon); >>> @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >>> XiveNVT *nvt); >>> int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >>> XiveNVT *nvt, uint8_t word_number); >>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs); >>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset); >>> >>> /* >>> * XIVE END ESBs >>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c >>> index a0f5ff929447..c41ee96c4c84 100644 >>> --- a/hw/intc/spapr_xive.c >>> +++ b/hw/intc/spapr_xive.c >>> @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, >>> g_assert_not_reached(); >>> } >>> >>> -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs) >>> +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs, >>> + hwaddr offset) >>> { >>> PowerPCCPU *cpu = POWERPC_CPU(cs); >>> >>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c >>> index f5642f2338de..39dff557fadc 100644 >>> --- a/hw/intc/xive.c >>> +++ b/hw/intc/xive.c >>> @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write) >>> static void xive_tm_write(void *opaque, hwaddr offset, >>> uint64_t value, unsigned size) >>> { >>> - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); >>> + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, >>> + offset); >>> const XiveTmOp *xto; >>> >>> /* >>> @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset, >>> >>> static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size) >>> { >>> - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); >>> + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, >>> + offset); >>> const XiveTmOp *xto; >>> >>> /* >>> @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, >>> return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number); >>> } >>> >>> -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs) >>> +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset) >>> { >>> XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr); >>> >>> - return xrc->get_tctx(xrtr, cs); >>> + return xrc->get_tctx(xrtr, cs, offset); >>> } >>> >>> /* >>> @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >>> */ >>> >>> CPU_FOREACH(cs) { >>> - XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs); >>> + XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0); >>> int ring; >>> >>> /* >> >
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h index 04d54e8315f7..a1f5ea2d9143 100644 --- a/include/hw/ppc/xive.h +++ b/include/hw/ppc/xive.h @@ -352,7 +352,7 @@ typedef struct XiveRouterClass { XiveNVT *nvt); int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, XiveNVT *nvt, uint8_t word_number); - XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs); + XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs, hwaddr offset); } XiveRouterClass; void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon); @@ -367,7 +367,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, XiveNVT *nvt); int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, XiveNVT *nvt, uint8_t word_number); -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs); +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset); /* * XIVE END ESBs diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index a0f5ff929447..c41ee96c4c84 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -391,7 +391,8 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, g_assert_not_reached(); } -static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs) +static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs, + hwaddr offset) { PowerPCCPU *cpu = POWERPC_CPU(cs); diff --git a/hw/intc/xive.c b/hw/intc/xive.c index f5642f2338de..39dff557fadc 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -320,7 +320,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write) static void xive_tm_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, + offset); const XiveTmOp *xto; /* @@ -358,7 +359,8 @@ static void xive_tm_write(void *opaque, hwaddr offset, static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size) { - XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu); + XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu, + offset); const XiveTmOp *xto; /* @@ -1134,11 +1136,11 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx, return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number); } -XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs) +XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs, hwaddr offset) { XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr); - return xrc->get_tctx(xrtr, cs); + return xrc->get_tctx(xrtr, cs, offset); } /* @@ -1234,7 +1236,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, */ CPU_FOREACH(cs) { - XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs); + XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs, 0); int ring; /*
The PowerNV machine can perform indirect loads and stores on the TIMA on behalf of another CPU. The PIR of the CPU is controlled by a set of 4 registers, one per TIMA page. To know which page is being accessed, we need to inform the controller model of the operation offset. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- include/hw/ppc/xive.h | 4 ++-- hw/intc/spapr_xive.c | 3 ++- hw/intc/xive.c | 12 +++++++----- 3 files changed, 11 insertions(+), 8 deletions(-)