From: "Cédric Le Goater" <clg@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org, Greg Kurz <groug@kaod.org>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 09/17] ppc/xive: Extend XiveTCTX with a XiveRouter pointer
Date: Mon, 29 Jul 2019 09:34:35 +0200 [thread overview]
Message-ID: <756c3df9-6e8f-3ef1-6e6a-7786ccf48270@kaod.org> (raw)
In-Reply-To: <20190729061131.GA8687@umbus>
On 29/07/2019 08:11, David Gibson wrote:
> On Sun, Jul 28, 2019 at 11:06:27AM +0200, Cédric Le Goater wrote:
>> On 28/07/2019 09:46, David Gibson wrote:
>>> On Thu, Jul 18, 2019 at 01:54:12PM +0200, Cédric Le Goater wrote:
>>>> This is to perform lookups in the NVT table when a vCPU is dispatched
>>>> and possibily resend interrupts.
>>>>
>>>> Future XIVE chip will use a different class for the model of the
>>>> interrupt controller and we might need to change the type of
>>>> 'XiveRouter *' to 'Object *'
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>
>>> Hrm. This still bothers me.
>>
>> Your feeling is right. There should be a good reason to link two objects
>> together as it can be an issue later on (such P10). It should not be an
>> hidden parameter to function calls. this is more or less the case.
>>
>> See below for more explanation.
>>
>>> AIUI there can be multiple XiveRouters in the system, yes?
>>
>> yes and it works relatively well with 4 chips. I say relatively because
>> the presenter model is taking a shortcut we should fix.
>>
>>> And at least theoretically can present irqs from multiple routers?
>>
>> Yes. the console being the most simple example. We only have one device
>> per system on the LPC bus of chip 0.
>>
>>> In which case what's the rule for which one should be associated with
>>> a specific.
>>> I guess it's the one on the same chip, but that needs to be explained
>>> up front, with some justification of why that's the relevant one.
>>
>> Yes. we try to minimize the traffic on the PowerBUS so generally CPU
>> targets are on the same IC. The EAT on POWER10 should be on the same
>> HW chip.
>>
>>
>> I think we can address the proposed changes from another perspective,
>> from the presenter one. this is cleaner and reflects better the HW design.
>>
>> The thread contexts are owned by the presenter. It can scan its list
>> when doing CAM matching and when the thread context registers are being
>> accessed by a CPU. Adding a XiveRouter parameter to all the TIMA
>> operations seems like a better option and solves the problem.
>>
>>
>> The thread context registers are modeled under the CPU object today
>> because it was practical for sPAPR but on HW, these are SRAM entries,
>> one for each HW thread of the chip. So may be, we should have introduced
>> an other table under the XiveRouter to model the contexts but there
>> was no real need for the XIVE POWER9 IC of the pseries machine. This
>> design might be reaching its limits with PowerNV and POWER10.
>>
>>
>> Looking at :
>>
>> [PATCH v2 15/17] ppc/pnv: Grab the XiveRouter object from XiveTCTX in pnv_xive_get_tctx()
>>
>> we see that the code adds an extra check on the THREAD_ENABLE registers
>> and for that, its needs the IC to which belongs the thread context. This
>> code is wrong. We should not be need to find a XiveRouter object from a
>> XiveRouter handler.
>>
>> This is because the xive_presenter_match() routine does:
>>
>> CPU_FOREACH(cs) {
>> XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
>>
>> we should be, instead, looping on the different IC of the system
>> looking for a match. Something else to fix. I think I will use the
>> PIR to match the CPU of a chip.
>>
>>
>> Looking at POWER10, XIVE internal structures have changed and we will
>> need to introduce new IC models, one for PowerNV obviously but surely
>> also one for pseries. A third one ... yes, sorry about that. If we go
>> in that direction, it would be good to have a common XiveTCTX and not
>> link it to a specific XiveRouter (P9 or P10). Another good reason not
>> to use that link.
>>
>>
>> So I will rework the end of that patchset. Thanks for having given me
>> time to think more about it before merging. I did more experiments and
>> the models are now more precise, specially with guest and multichip
>> support.
>
> Ok, good to hear. I will wait for the respin.
>
Could you check the patch adding the powernv{8,9} machines ? I am
rebasing the respin on top of this patch and introducing a new handler
to the machine to perform the CAM line matching.
Thanks,
C.
next prev parent reply other threads:[~2019-07-29 7:35 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-18 11:54 [Qemu-devel] [PATCH v2 00/17] ppc/pnv: add XIVE support for KVM guests Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 01/17] ppc/xive: use an abstract type for XiveNotifier Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 02/17] ppc/pnv: add more dummy XSCOM addresses for the P9 CAPP Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 03/17] ppc/xive: Implement TM_PULL_OS_CTX special command Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 04/17] ppc/xive: Provide backlog support Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 05/17] ppc/xive: Provide escalation support Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 06/17] ppc/xive: Provide unconditional " Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 07/17] ppc/xive: Provide silent " Cédric Le Goater
2019-07-22 8:27 ` David Gibson
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 08/17] ppc/xive: Improve 'info pic' support Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 09/17] ppc/xive: Extend XiveTCTX with a XiveRouter pointer Cédric Le Goater
2019-07-28 7:46 ` David Gibson
2019-07-28 9:06 ` Cédric Le Goater
2019-07-29 6:11 ` David Gibson
2019-07-29 7:34 ` Cédric Le Goater [this message]
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 10/17] ppc/xive: Introduce xive_tctx_ipb_update() Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 11/17] ppc/xive: Synthesize interrupt from the saved IPB in the NVT Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 12/17] ppc/pnv: Remove pnv_xive_vst_size() routine Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 13/17] ppc/pnv: Dump the XIVE NVT table Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 14/17] ppc/pnv: Skip empty slots of " Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 15/17] ppc/pnv: Grab the XiveRouter object from XiveTCTX in pnv_xive_get_tctx() Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 16/17] ppc/pnv: Introduce a pnv_xive_get_block_id() interface to XiveRouter Cédric Le Goater
2019-07-18 11:54 ` [Qemu-devel] [PATCH v2 17/17] ppc/pnv: quiesce some XIVE errors Cédric Le Goater
2019-07-18 19:55 ` [Qemu-devel] [PATCH v2 00/17] ppc/pnv: add XIVE support for KVM guests no-reply
2019-07-22 8:29 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=756c3df9-6e8f-3ef1-6e6a-7786ccf48270@kaod.org \
--to=clg@kaod.org \
--cc=david@gibson.dropbear.id.au \
--cc=groug@kaod.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).