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.
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));
>>>
>>
>
