On Thu, 21 Nov 2019 10:22:38 +0100 Cédric Le Goater <c...@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 <c...@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 <c...@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 <c...@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 <c...@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 <gr...@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)); > >>>>> > >>>> > >>> > >> > > >