| Message ID | 20191115162436.30548-16-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | ppc/pnv: add XIVE support for KVM guests | expand |
On Fri, 15 Nov 2019 17:24:28 +0100 Cédric Le Goater <clg@kaod.org> wrote: > Now that the machines have handlers implementing the XiveFabric and > XivePresenter interfaces, remove xive_presenter_match() and make use > of the 'match_nvt' handler of the machine. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/intc/xive.c | 48 +++++++++++++++++------------------------------- > 1 file changed, 17 insertions(+), 31 deletions(-) > Nice diffstat :) > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index 1c9e58f8deac..ab62bda85788 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, > return -1; > } > > -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > - uint8_t nvt_blk, uint32_t nvt_idx, > - bool cam_ignore, uint8_t priority, > - uint32_t logic_serv, XiveTCTXMatch *match) > -{ > - XivePresenter *xptr = XIVE_PRESENTER(xrtr); > - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); > - int count; > - > - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, > - priority, logic_serv, match); > - if (count < 0) { > - return false; > - } > - > - if (!match->tctx) { > - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", > - nvt_blk, nvt_idx); Maybe keep this trace... > - return false; > - } > - > - return true; > -} > - > /* > * This is our simple Xive Presenter Engine model. It is merged in the > * Router as it does not require an extra object. > @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > * > * The parameters represent what is sent on the PowerBus > */ > -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, > +static bool xive_presenter_notify(uint8_t format, > uint8_t nvt_blk, uint32_t nvt_idx, > bool cam_ignore, uint8_t priority, > uint32_t logic_serv) > { > + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); > + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); > XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; > - bool found; > + int count; > > - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, > - priority, logic_serv, &match); > - if (found) { > + /* > + * Ask the machine to scan the interrupt controllers for a match > + */ > + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, > + priority, logic_serv, &match); > + if (count < 0) { > + return false; > + } > + > + /* handle CPU exception delivery */ > + if (count) { > ipb_update(&match.tctx->regs[match.ring], priority); > xive_tctx_notify(match.tctx, match.ring); > } ... in an else block here ^^ ? > > - return found; > + return count; Implicit cast is ok I guess, but !!count would ensure no paranoid compiler ever complains. > } > > /* > @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, > return; > } > > - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, > + found = xive_presenter_notify(format, nvt_blk, nvt_idx, > xive_get_field32(END_W7_F0_IGNORE, end.w7), > priority, > xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7));
On 20/11/2019 19:30, Greg Kurz wrote: > On Fri, 15 Nov 2019 17:24:28 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> Now that the machines have handlers implementing the XiveFabric and >> XivePresenter interfaces, remove xive_presenter_match() and make use >> of the 'match_nvt' handler of the machine. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/intc/xive.c | 48 +++++++++++++++++------------------------------- >> 1 file changed, 17 insertions(+), 31 deletions(-) >> > > Nice diffstat :) > >> diff --git a/hw/intc/xive.c b/hw/intc/xive.c >> index 1c9e58f8deac..ab62bda85788 100644 >> --- a/hw/intc/xive.c >> +++ b/hw/intc/xive.c >> @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, >> return -1; >> } >> >> -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >> - uint8_t nvt_blk, uint32_t nvt_idx, >> - bool cam_ignore, uint8_t priority, >> - uint32_t logic_serv, XiveTCTXMatch *match) >> -{ >> - XivePresenter *xptr = XIVE_PRESENTER(xrtr); >> - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); >> - int count; >> - >> - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, >> - priority, logic_serv, match); >> - if (count < 0) { >> - return false; >> - } >> - >> - if (!match->tctx) { >> - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", >> - nvt_blk, nvt_idx); > > Maybe keep this trace... It's in spapr_xive_match_nvt() now. > >> - return false; >> - } >> - >> - return true; >> -} >> - >> /* >> * This is our simple Xive Presenter Engine model. It is merged in the >> * Router as it does not require an extra object. >> @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >> * >> * The parameters represent what is sent on the PowerBus >> */ >> -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, >> +static bool xive_presenter_notify(uint8_t format, >> uint8_t nvt_blk, uint32_t nvt_idx, >> bool cam_ignore, uint8_t priority, >> uint32_t logic_serv) >> { >> + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); >> + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); >> XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; >> - bool found; >> + int count; >> >> - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, >> - priority, logic_serv, &match); >> - if (found) { >> + /* >> + * Ask the machine to scan the interrupt controllers for a match >> + */ >> + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, >> + priority, logic_serv, &match); >> + if (count < 0) { >> + return false; >> + } >> + >> + /* handle CPU exception delivery */ >> + if (count) { >> ipb_update(&match.tctx->regs[match.ring], priority); >> xive_tctx_notify(match.tctx, match.ring); >> } > > ... in an else block here ^^ ? > >> >> - return found; >> + return count; > > Implicit cast is ok I guess, but !!count would ensure no paranoid > compiler ever complains. yes. Thanks, C. > >> } >> >> /* >> @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, >> return; >> } >> >> - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, >> + found = xive_presenter_notify(format, nvt_blk, nvt_idx, >> xive_get_field32(END_W7_F0_IGNORE, end.w7), >> priority, >> xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7)); >
On Thu, 21 Nov 2019 08:01:44 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 20/11/2019 19:30, Greg Kurz wrote: > > On Fri, 15 Nov 2019 17:24:28 +0100 > > Cédric Le Goater <clg@kaod.org> wrote: > > > >> Now that the machines have handlers implementing the XiveFabric and > >> XivePresenter interfaces, remove xive_presenter_match() and make use > >> of the 'match_nvt' handler of the machine. > >> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >> --- > >> hw/intc/xive.c | 48 +++++++++++++++++------------------------------- > >> 1 file changed, 17 insertions(+), 31 deletions(-) > >> > > > > Nice diffstat :) > > > >> diff --git a/hw/intc/xive.c b/hw/intc/xive.c > >> index 1c9e58f8deac..ab62bda85788 100644 > >> --- a/hw/intc/xive.c > >> +++ b/hw/intc/xive.c > >> @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, > >> return -1; > >> } > >> > >> -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > >> - uint8_t nvt_blk, uint32_t nvt_idx, > >> - bool cam_ignore, uint8_t priority, > >> - uint32_t logic_serv, XiveTCTXMatch *match) > >> -{ > >> - XivePresenter *xptr = XIVE_PRESENTER(xrtr); > >> - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); > >> - int count; > >> - > >> - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, > >> - priority, logic_serv, match); > >> - if (count < 0) { > >> - return false; > >> - } > >> - > >> - if (!match->tctx) { > >> - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", > >> - nvt_blk, nvt_idx); > > > > Maybe keep this trace... > > It's in spapr_xive_match_nvt() now. > Not really... spapr_xive_match_nvt() has a trace for the opposite case of duplicate matches: if (match->tctx) { qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread " "context NVT %x/%x\n", nvt_blk, nvt_idx); return -1; } > > > >> - return false; > >> - } > >> - > >> - return true; > >> -} > >> - > >> /* > >> * This is our simple Xive Presenter Engine model. It is merged in the > >> * Router as it does not require an extra object. > >> @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > >> * > >> * The parameters represent what is sent on the PowerBus > >> */ > >> -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, > >> +static bool xive_presenter_notify(uint8_t format, > >> uint8_t nvt_blk, uint32_t nvt_idx, > >> bool cam_ignore, uint8_t priority, > >> uint32_t logic_serv) > >> { > >> + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); > >> + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); > >> XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; > >> - bool found; > >> + int count; > >> > >> - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, > >> - priority, logic_serv, &match); > >> - if (found) { > >> + /* > >> + * Ask the machine to scan the interrupt controllers for a match > >> + */ > >> + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, > >> + priority, logic_serv, &match); > >> + if (count < 0) { > >> + return false; > >> + } > >> + > >> + /* handle CPU exception delivery */ > >> + if (count) { > >> ipb_update(&match.tctx->regs[match.ring], priority); > >> xive_tctx_notify(match.tctx, match.ring); > >> } > > > > ... in an else block here ^^ ? > > > >> > >> - return found; > >> + return count; > > > > Implicit cast is ok I guess, but !!count would ensure no paranoid > > compiler ever complains. > > yes. > > Thanks, > > C. > > > > > >> } > >> > >> /* > >> @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, > >> return; > >> } > >> > >> - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, > >> + found = xive_presenter_notify(format, nvt_blk, nvt_idx, > >> xive_get_field32(END_W7_F0_IGNORE, end.w7), > >> priority, > >> xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7)); > > >
On 21/11/2019 08:30, Greg Kurz wrote: > On Thu, 21 Nov 2019 08:01:44 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> On 20/11/2019 19:30, Greg Kurz wrote: >>> On Fri, 15 Nov 2019 17:24:28 +0100 >>> Cédric Le Goater <clg@kaod.org> wrote: >>> >>>> Now that the machines have handlers implementing the XiveFabric and >>>> XivePresenter interfaces, remove xive_presenter_match() and make use >>>> of the 'match_nvt' handler of the machine. >>>> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> hw/intc/xive.c | 48 +++++++++++++++++------------------------------- >>>> 1 file changed, 17 insertions(+), 31 deletions(-) >>>> >>> >>> Nice diffstat :) >>> >>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c >>>> index 1c9e58f8deac..ab62bda85788 100644 >>>> --- a/hw/intc/xive.c >>>> +++ b/hw/intc/xive.c >>>> @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, >>>> return -1; >>>> } >>>> >>>> -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >>>> - uint8_t nvt_blk, uint32_t nvt_idx, >>>> - bool cam_ignore, uint8_t priority, >>>> - uint32_t logic_serv, XiveTCTXMatch *match) >>>> -{ >>>> - XivePresenter *xptr = XIVE_PRESENTER(xrtr); >>>> - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); >>>> - int count; >>>> - >>>> - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, >>>> - priority, logic_serv, match); >>>> - if (count < 0) { >>>> - return false; >>>> - } >>>> - >>>> - if (!match->tctx) { >>>> - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", >>>> - nvt_blk, nvt_idx); >>> >>> Maybe keep this trace... >> >> It's in spapr_xive_match_nvt() now. >> > > Not really... spapr_xive_match_nvt() has a trace for the opposite case of duplicate > matches: not that one. The one in spapr.c ... Yes I need to change the name. C. > > if (match->tctx) { > qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread " > "context NVT %x/%x\n", nvt_blk, nvt_idx); > return -1; > } > >>> >>>> - return false; >>>> - } >>>> - >>>> - return true; >>>> -} >>>> - >>>> /* >>>> * This is our simple Xive Presenter Engine model. It is merged in the >>>> * Router as it does not require an extra object. >>>> @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >>>> * >>>> * The parameters represent what is sent on the PowerBus >>>> */ >>>> -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, >>>> +static bool xive_presenter_notify(uint8_t format, >>>> uint8_t nvt_blk, uint32_t nvt_idx, >>>> bool cam_ignore, uint8_t priority, >>>> uint32_t logic_serv) >>>> { >>>> + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); >>>> + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); >>>> XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; >>>> - bool found; >>>> + int count; >>>> >>>> - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, >>>> - priority, logic_serv, &match); >>>> - if (found) { >>>> + /* >>>> + * Ask the machine to scan the interrupt controllers for a match >>>> + */ >>>> + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, >>>> + priority, logic_serv, &match); >>>> + if (count < 0) { >>>> + return false; >>>> + } >>>> + >>>> + /* handle CPU exception delivery */ >>>> + if (count) { >>>> ipb_update(&match.tctx->regs[match.ring], priority); >>>> xive_tctx_notify(match.tctx, match.ring); >>>> } >>> >>> ... in an else block here ^^ ? >>> >>>> >>>> - return found; >>>> + return count; >>> >>> Implicit cast is ok I guess, but !!count would ensure no paranoid >>> compiler ever complains. >> >> yes. >> >> Thanks, >> >> C. >> >> >>> >>>> } >>>> >>>> /* >>>> @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, >>>> return; >>>> } >>>> >>>> - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, >>>> + found = xive_presenter_notify(format, nvt_blk, nvt_idx, >>>> xive_get_field32(END_W7_F0_IGNORE, end.w7), >>>> priority, >>>> xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7)); >>> >> >
On Thu, 21 Nov 2019 08:40:32 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 21/11/2019 08:30, Greg Kurz wrote: > > On Thu, 21 Nov 2019 08:01:44 +0100 > > Cédric Le Goater <clg@kaod.org> wrote: > > > >> On 20/11/2019 19:30, Greg Kurz wrote: > >>> On Fri, 15 Nov 2019 17:24:28 +0100 > >>> Cédric Le Goater <clg@kaod.org> wrote: > >>> > >>>> Now that the machines have handlers implementing the XiveFabric and > >>>> XivePresenter interfaces, remove xive_presenter_match() and make use > >>>> of the 'match_nvt' handler of the machine. > >>>> > >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >>>> --- > >>>> hw/intc/xive.c | 48 +++++++++++++++++------------------------------- > >>>> 1 file changed, 17 insertions(+), 31 deletions(-) > >>>> > >>> > >>> Nice diffstat :) > >>> > >>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c > >>>> index 1c9e58f8deac..ab62bda85788 100644 > >>>> --- a/hw/intc/xive.c > >>>> +++ b/hw/intc/xive.c > >>>> @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, > >>>> return -1; > >>>> } > >>>> > >>>> -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > >>>> - uint8_t nvt_blk, uint32_t nvt_idx, > >>>> - bool cam_ignore, uint8_t priority, > >>>> - uint32_t logic_serv, XiveTCTXMatch *match) > >>>> -{ > >>>> - XivePresenter *xptr = XIVE_PRESENTER(xrtr); > >>>> - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); > >>>> - int count; > >>>> - > >>>> - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, > >>>> - priority, logic_serv, match); > >>>> - if (count < 0) { > >>>> - return false; > >>>> - } > >>>> - > >>>> - if (!match->tctx) { > >>>> - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", > >>>> - nvt_blk, nvt_idx); > >>> > >>> Maybe keep this trace... > >> > >> It's in spapr_xive_match_nvt() now. > >> > > > > Not really... spapr_xive_match_nvt() has a trace for the opposite case of duplicate > > matches: > > not that one. The one in spapr.c ... Yes I need to change the name. > ... and it seems I cannot memorize a change that was made by the previous patch :-\ Sorry for the noise. With or without the !!count change: Reviewed-by: Greg Kurz <groug@kaod.org> > C. > > > > > if (match->tctx) { > > qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread " > > "context NVT %x/%x\n", nvt_blk, nvt_idx); > > return -1; > > } > > > >>> > >>>> - return false; > >>>> - } > >>>> - > >>>> - return true; > >>>> -} > >>>> - > >>>> /* > >>>> * This is our simple Xive Presenter Engine model. It is merged in the > >>>> * Router as it does not require an extra object. > >>>> @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > >>>> * > >>>> * The parameters represent what is sent on the PowerBus > >>>> */ > >>>> -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, > >>>> +static bool xive_presenter_notify(uint8_t format, > >>>> uint8_t nvt_blk, uint32_t nvt_idx, > >>>> bool cam_ignore, uint8_t priority, > >>>> uint32_t logic_serv) > >>>> { > >>>> + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); > >>>> + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); > >>>> XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; > >>>> - bool found; > >>>> + int count; > >>>> > >>>> - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, > >>>> - priority, logic_serv, &match); > >>>> - if (found) { > >>>> + /* > >>>> + * Ask the machine to scan the interrupt controllers for a match > >>>> + */ > >>>> + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, > >>>> + priority, logic_serv, &match); > >>>> + if (count < 0) { > >>>> + return false; > >>>> + } > >>>> + > >>>> + /* handle CPU exception delivery */ > >>>> + if (count) { > >>>> ipb_update(&match.tctx->regs[match.ring], priority); > >>>> xive_tctx_notify(match.tctx, match.ring); > >>>> } > >>> > >>> ... in an else block here ^^ ? > >>> > >>>> > >>>> - return found; > >>>> + return count; > >>> > >>> Implicit cast is ok I guess, but !!count would ensure no paranoid > >>> compiler ever complains. > >> > >> yes. > >> > >> Thanks, > >> > >> C. > >> > >> > >>> > >>>> } > >>>> > >>>> /* > >>>> @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, > >>>> return; > >>>> } > >>>> > >>>> - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, > >>>> + found = xive_presenter_notify(format, nvt_blk, nvt_idx, > >>>> xive_get_field32(END_W7_F0_IGNORE, end.w7), > >>>> priority, > >>>> xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7)); > >>> > >> > > >
On 21/11/2019 09:08, Greg Kurz wrote: > On Thu, 21 Nov 2019 08:40:32 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> On 21/11/2019 08:30, Greg Kurz wrote: >>> On Thu, 21 Nov 2019 08:01:44 +0100 >>> Cédric Le Goater <clg@kaod.org> wrote: >>> >>>> On 20/11/2019 19:30, Greg Kurz wrote: >>>>> On Fri, 15 Nov 2019 17:24:28 +0100 >>>>> Cédric Le Goater <clg@kaod.org> wrote: >>>>> >>>>>> Now that the machines have handlers implementing the XiveFabric and >>>>>> XivePresenter interfaces, remove xive_presenter_match() and make use >>>>>> of the 'match_nvt' handler of the machine. >>>>>> >>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>>>> --- >>>>>> hw/intc/xive.c | 48 +++++++++++++++++------------------------------- >>>>>> 1 file changed, 17 insertions(+), 31 deletions(-) >>>>>> >>>>> >>>>> Nice diffstat :) >>>>> >>>>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c >>>>>> index 1c9e58f8deac..ab62bda85788 100644 >>>>>> --- a/hw/intc/xive.c >>>>>> +++ b/hw/intc/xive.c >>>>>> @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, >>>>>> return -1; >>>>>> } >>>>>> >>>>>> -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >>>>>> - uint8_t nvt_blk, uint32_t nvt_idx, >>>>>> - bool cam_ignore, uint8_t priority, >>>>>> - uint32_t logic_serv, XiveTCTXMatch *match) >>>>>> -{ >>>>>> - XivePresenter *xptr = XIVE_PRESENTER(xrtr); >>>>>> - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); >>>>>> - int count; >>>>>> - >>>>>> - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, >>>>>> - priority, logic_serv, match); >>>>>> - if (count < 0) { >>>>>> - return false; >>>>>> - } >>>>>> - >>>>>> - if (!match->tctx) { >>>>>> - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", >>>>>> - nvt_blk, nvt_idx); >>>>> >>>>> Maybe keep this trace... >>>> >>>> It's in spapr_xive_match_nvt() now. >>>> >>> >>> Not really... spapr_xive_match_nvt() has a trace for the opposite case of duplicate >>> matches: >> >> not that one. The one in spapr.c ... Yes I need to change the name. >> > > ... and it seems I cannot memorize a change that was made by the > previous patch :-\ Sorry for the noise. np but this is problem for gdb ! Any suggestion on the name : spapr_match_nvt() ? > With or without the !!count change: I will add the !! C. > > Reviewed-by: Greg Kurz <groug@kaod.org> > >> C. >> >>> >>> if (match->tctx) { >>> qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread " >>> "context NVT %x/%x\n", nvt_blk, nvt_idx); >>> return -1; >>> } >>> >>>>> >>>>>> - return false; >>>>>> - } >>>>>> - >>>>>> - return true; >>>>>> -} >>>>>> - >>>>>> /* >>>>>> * This is our simple Xive Presenter Engine model. It is merged in the >>>>>> * Router as it does not require an extra object. >>>>>> @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, >>>>>> * >>>>>> * The parameters represent what is sent on the PowerBus >>>>>> */ >>>>>> -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, >>>>>> +static bool xive_presenter_notify(uint8_t format, >>>>>> uint8_t nvt_blk, uint32_t nvt_idx, >>>>>> bool cam_ignore, uint8_t priority, >>>>>> uint32_t logic_serv) >>>>>> { >>>>>> + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); >>>>>> + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); >>>>>> XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; >>>>>> - bool found; >>>>>> + int count; >>>>>> >>>>>> - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, >>>>>> - priority, logic_serv, &match); >>>>>> - if (found) { >>>>>> + /* >>>>>> + * Ask the machine to scan the interrupt controllers for a match >>>>>> + */ >>>>>> + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, >>>>>> + priority, logic_serv, &match); >>>>>> + if (count < 0) { >>>>>> + return false; >>>>>> + } >>>>>> + >>>>>> + /* handle CPU exception delivery */ >>>>>> + if (count) { >>>>>> ipb_update(&match.tctx->regs[match.ring], priority); >>>>>> xive_tctx_notify(match.tctx, match.ring); >>>>>> } >>>>> >>>>> ... in an else block here ^^ ? >>>>> >>>>>> >>>>>> - return found; >>>>>> + return count; >>>>> >>>>> Implicit cast is ok I guess, but !!count would ensure no paranoid >>>>> compiler ever complains. >>>> >>>> yes. >>>> >>>> Thanks, >>>> >>>> C. >>>> >>>> >>>>> >>>>>> } >>>>>> >>>>>> /* >>>>>> @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, >>>>>> return; >>>>>> } >>>>>> >>>>>> - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, >>>>>> + found = xive_presenter_notify(format, nvt_blk, nvt_idx, >>>>>> xive_get_field32(END_W7_F0_IGNORE, end.w7), >>>>>> priority, >>>>>> xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7)); >>>>> >>>> >>> >> >
On Thu, 21 Nov 2019 10:22:38 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 21/11/2019 09:08, Greg Kurz wrote: > > On Thu, 21 Nov 2019 08:40:32 +0100 > > Cédric Le Goater <clg@kaod.org> wrote: > > > >> On 21/11/2019 08:30, Greg Kurz wrote: > >>> On Thu, 21 Nov 2019 08:01:44 +0100 > >>> Cédric Le Goater <clg@kaod.org> wrote: > >>> > >>>> On 20/11/2019 19:30, Greg Kurz wrote: > >>>>> On Fri, 15 Nov 2019 17:24:28 +0100 > >>>>> Cédric Le Goater <clg@kaod.org> wrote: > >>>>> > >>>>>> Now that the machines have handlers implementing the XiveFabric and > >>>>>> XivePresenter interfaces, remove xive_presenter_match() and make use > >>>>>> of the 'match_nvt' handler of the machine. > >>>>>> > >>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >>>>>> --- > >>>>>> hw/intc/xive.c | 48 +++++++++++++++++------------------------------- > >>>>>> 1 file changed, 17 insertions(+), 31 deletions(-) > >>>>>> > >>>>> > >>>>> Nice diffstat :) > >>>>> > >>>>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c > >>>>>> index 1c9e58f8deac..ab62bda85788 100644 > >>>>>> --- a/hw/intc/xive.c > >>>>>> +++ b/hw/intc/xive.c > >>>>>> @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, > >>>>>> return -1; > >>>>>> } > >>>>>> > >>>>>> -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > >>>>>> - uint8_t nvt_blk, uint32_t nvt_idx, > >>>>>> - bool cam_ignore, uint8_t priority, > >>>>>> - uint32_t logic_serv, XiveTCTXMatch *match) > >>>>>> -{ > >>>>>> - XivePresenter *xptr = XIVE_PRESENTER(xrtr); > >>>>>> - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); > >>>>>> - int count; > >>>>>> - > >>>>>> - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, > >>>>>> - priority, logic_serv, match); > >>>>>> - if (count < 0) { > >>>>>> - return false; > >>>>>> - } > >>>>>> - > >>>>>> - if (!match->tctx) { > >>>>>> - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", > >>>>>> - nvt_blk, nvt_idx); > >>>>> > >>>>> Maybe keep this trace... > >>>> > >>>> It's in spapr_xive_match_nvt() now. > >>>> > >>> > >>> Not really... spapr_xive_match_nvt() has a trace for the opposite case of duplicate > >>> matches: > >> > >> not that one. The one in spapr.c ... Yes I need to change the name. > >> > > > > ... and it seems I cannot memorize a change that was made by the > > previous patch :-\ Sorry for the noise. > > np but this is problem for gdb ! Any suggestion on the name : > > spapr_match_nvt() > I guess having nvt in the name is enough to identify this as a XIVE related function. It's ok for me. BTW, the same problem exists in powernv: $ git grep pnv_xive_match_nvt hw/intc/pnv_xive.c:static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format, hw/intc/pnv_xive.c: xpc->match_nvt = pnv_xive_match_nvt; hw/ppc/pnv.c:static int pnv_xive_match_nvt(XiveFabric *xfb, uint8_t format, hw/ppc/pnv.c: xfc->match_nvt = pnv_xive_match_nvt; > ? > > > With or without the !!count change: > > I will add the !! > > C. > > > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > >> C. > >> > >>> > >>> if (match->tctx) { > >>> qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread " > >>> "context NVT %x/%x\n", nvt_blk, nvt_idx); > >>> return -1; > >>> } > >>> > >>>>> > >>>>>> - return false; > >>>>>> - } > >>>>>> - > >>>>>> - return true; > >>>>>> -} > >>>>>> - > >>>>>> /* > >>>>>> * This is our simple Xive Presenter Engine model. It is merged in the > >>>>>> * Router as it does not require an extra object. > >>>>>> @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, > >>>>>> * > >>>>>> * The parameters represent what is sent on the PowerBus > >>>>>> */ > >>>>>> -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, > >>>>>> +static bool xive_presenter_notify(uint8_t format, > >>>>>> uint8_t nvt_blk, uint32_t nvt_idx, > >>>>>> bool cam_ignore, uint8_t priority, > >>>>>> uint32_t logic_serv) > >>>>>> { > >>>>>> + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); > >>>>>> + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); > >>>>>> XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; > >>>>>> - bool found; > >>>>>> + int count; > >>>>>> > >>>>>> - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, > >>>>>> - priority, logic_serv, &match); > >>>>>> - if (found) { > >>>>>> + /* > >>>>>> + * Ask the machine to scan the interrupt controllers for a match > >>>>>> + */ > >>>>>> + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, > >>>>>> + priority, logic_serv, &match); > >>>>>> + if (count < 0) { > >>>>>> + return false; > >>>>>> + } > >>>>>> + > >>>>>> + /* handle CPU exception delivery */ > >>>>>> + if (count) { > >>>>>> ipb_update(&match.tctx->regs[match.ring], priority); > >>>>>> xive_tctx_notify(match.tctx, match.ring); > >>>>>> } > >>>>> > >>>>> ... in an else block here ^^ ? > >>>>> > >>>>>> > >>>>>> - return found; > >>>>>> + return count; > >>>>> > >>>>> Implicit cast is ok I guess, but !!count would ensure no paranoid > >>>>> compiler ever complains. > >>>> > >>>> yes. > >>>> > >>>> Thanks, > >>>> > >>>> C. > >>>> > >>>> > >>>>> > >>>>>> } > >>>>>> > >>>>>> /* > >>>>>> @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, > >>>>>> return; > >>>>>> } > >>>>>> > >>>>>> - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, > >>>>>> + found = xive_presenter_notify(format, nvt_blk, nvt_idx, > >>>>>> xive_get_field32(END_W7_F0_IGNORE, end.w7), > >>>>>> priority, > >>>>>> xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7)); > >>>>> > >>>> > >>> > >> > > >
diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 1c9e58f8deac..ab62bda85788 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -1423,30 +1423,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx, return -1; } -static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, - uint8_t nvt_blk, uint32_t nvt_idx, - bool cam_ignore, uint8_t priority, - uint32_t logic_serv, XiveTCTXMatch *match) -{ - XivePresenter *xptr = XIVE_PRESENTER(xrtr); - XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); - int count; - - count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore, - priority, logic_serv, match); - if (count < 0) { - return false; - } - - if (!match->tctx) { - qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n", - nvt_blk, nvt_idx); - return false; - } - - return true; -} - /* * This is our simple Xive Presenter Engine model. It is merged in the * Router as it does not require an extra object. @@ -1462,22 +1438,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format, * * The parameters represent what is sent on the PowerBus */ -static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format, +static bool xive_presenter_notify(uint8_t format, uint8_t nvt_blk, uint32_t nvt_idx, bool cam_ignore, uint8_t priority, uint32_t logic_serv) { + XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine()); + XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb); XiveTCTXMatch match = { .tctx = NULL, .ring = 0 }; - bool found; + int count; - found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore, - priority, logic_serv, &match); - if (found) { + /* + * Ask the machine to scan the interrupt controllers for a match + */ + count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore, + priority, logic_serv, &match); + if (count < 0) { + return false; + } + + /* handle CPU exception delivery */ + if (count) { ipb_update(&match.tctx->regs[match.ring], priority); xive_tctx_notify(match.tctx, match.ring); } - return found; + return count; } /* @@ -1590,7 +1576,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, return; } - found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx, + found = xive_presenter_notify(format, nvt_blk, nvt_idx, xive_get_field32(END_W7_F0_IGNORE, end.w7), priority, xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7));
Now that the machines have handlers implementing the XiveFabric and XivePresenter interfaces, remove xive_presenter_match() and make use of the 'match_nvt' handler of the machine. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/intc/xive.c | 48 +++++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 31 deletions(-)