On Fri, Nov 30, 2018 at 07:41:33AM +0100, Cédric Le Goater wrote:
> On 11/30/18 2:04 AM, David Gibson wrote:
> > On Thu, Nov 29, 2018 at 11:06:13PM +0100, Cédric Le Goater wrote:
> >> On 11/22/18 6:13 AM, David Gibson wrote:
> >>> On Fri, Nov 16, 2018 at 11:56:59AM +0100, Cédric Le Goater wrote:
> >>>> The Event Notification Descriptor also contains two Event State
> >>>> Buffers providing further coalescing of interrupts, one for the
> >>>> notification event (ESn) and one for the escalation events (ESe). A
> >>>> MMIO page is assigned for each to control the EOI through loads
> >>>> only. Stores are not allowed.
> >>>>
> >>>> The END ESBs are modeled through an object resembling the 'XiveSource'
> >>>> It is stateless as the END state bits are backed into the XiveEND
> >>>> structure under the XiveRouter and the MMIO accesses follow the same
> >>>> rules as for the standard source ESBs.
> >>>>
> >>>> END ESBs are not supported by the Linux drivers neither on OPAL nor on
> >>>> sPAPR. Nevetherless, it provides a mean to study the question in the
> >>>> future and validates a bit more the XIVE model.
> >>>>
> >>>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> >>>> ---
> >>>> include/hw/ppc/xive.h | 20 ++++++
> >>>> hw/intc/xive.c | 160 +++++++++++++++++++++++++++++++++++++++++-
> >>>> 2 files changed, 178 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> >>>> index ce62aaf28343..24301bf2076d 100644
> >>>> --- a/include/hw/ppc/xive.h
> >>>> +++ b/include/hw/ppc/xive.h
> >>>> @@ -208,6 +208,26 @@ int xive_router_get_end(XiveRouter *xrtr, uint8_t
> >>>> end_blk, uint32_t end_idx,
> >>>> int xive_router_set_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t
> >>>> end_idx,
> >>>> XiveEND *end);
> >>>>
> >>>> +/*
> >>>> + * XIVE END ESBs
> >>>> + */
> >>>> +
> >>>> +#define TYPE_XIVE_END_SOURCE "xive-end-source"
> >>>> +#define XIVE_END_SOURCE(obj) \
> >>>> + OBJECT_CHECK(XiveENDSource, (obj), TYPE_XIVE_END_SOURCE)
> >>>
> >>> Is there a particular reason to make this a full QOM object, rather
> >>> than just embedding it in the XiveRouter?
> >>
> >> Coming back on this question because removing the chip_id from the
> >> router is a problem for the END triggering. At least with the current
> >> design. See below for the comment.
> >>
> >>>> +typedef struct XiveENDSource {
> >>>> + SysBusDevice parent;
> >>>> +
> >>>> + uint32_t nr_ends;
> >>>> +
> >>>> + /* ESB memory region */
> >>>> + uint32_t esb_shift;
> >>>> + MemoryRegion esb_mmio;
> >>>> +
> >>>> + XiveRouter *xrtr;
> >>>> +} XiveENDSource;
> >>>> +
> >>>> /*
> >>>> * For legacy compatibility, the exceptions define up to 256 different
> >>>> * priorities. P9 implements only 9 levels : 8 active levels [0 - 7]
> >>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> >>>> index 9cb001e7b540..5a8882d47a98 100644
> >>>> --- a/hw/intc/xive.c
> >>>> +++ b/hw/intc/xive.c
> >>>> @@ -622,8 +622,18 @@ static void xive_router_end_notify(XiveRouter
> >>>> *xrtr, uint8_t end_blk,
> >>>> * even futher coalescing in the Router
> >>>> */
> >>>> if (!(end.w0 & END_W0_UCOND_NOTIFY)) {
> >>>> - qemu_log_mask(LOG_UNIMP, "XIVE: !UCOND_NOTIFY not
> >>>> implemented\n");
> >>>> - return;
> >>>> + uint8_t pq = GETFIELD(END_W1_ESn, end.w1);
> >>>> + bool notify = xive_esb_trigger(&pq);
> >>>> +
> >>>> + if (pq != GETFIELD(END_W1_ESn, end.w1)) {
> >>>> + end.w1 = SETFIELD(END_W1_ESn, end.w1, pq);
> >>>> + xive_router_set_end(xrtr, end_blk, end_idx, &end);
> >>>> + }
> >>>> +
> >>>> + /* ESn[Q]=1 : end of notification */
> >>>> + if (!notify) {
> >>>> + return;
> >>>> + }
> >>>> }
> >>>>
> >>>> /*
> >>>> @@ -706,6 +716,151 @@ void xive_eas_pic_print_info(XiveEAS *eas,
> >>>> uint32_t lisn, Monitor *mon)
> >>>> (uint32_t) GETFIELD(EAS_END_DATA, eas->w));
> >>>> }
> >>>>
> >>>> +/*
> >>>> + * END ESB MMIO loads
> >>>> + */
> >>>> +static uint64_t xive_end_source_read(void *opaque, hwaddr addr,
> >>>> unsigned size)
> >>>> +{
> >>>> + XiveENDSource *xsrc = XIVE_END_SOURCE(opaque);
> >>>> + XiveRouter *xrtr = xsrc->xrtr;
> >>>> + uint32_t offset = addr & 0xFFF;
> >>>> + uint8_t end_blk;
> >>>> + uint32_t end_idx;
> >>>> + XiveEND end;
> >>>> + uint32_t end_esmask;
> >>>> + uint8_t pq;
> >>>> + uint64_t ret = -1;
> >>>> +
> >>>> + end_blk = xrtr->chip_id;
> >>>> + end_idx = addr >> (xsrc->esb_shift + 1);
> >>>> + if (xive_router_get_end(xrtr, end_blk, end_idx, &end)) {
> >>
> >> The current END accessors require a block identifier, hence xrtr->chip_id,
> >> but in this case, we don't really need it because we are using the ENDT
> >> local to the router/chip.
> >
> >> I don't know how to handle simply this case without keeping chip_id :/
> >
> > I don't really follow how chip_id is relevant here. AFAICT the END
> > accessors take a block id and the back end is responsible for
> > interpreting them. The ponwernv one will map it to chip id, but the
> > PAPR one can just ignore it or only use block 0.
>
> Yes. But the block value comes from the xrtr->chip_id today, on PAPR and
> PowerNV, even if it's block 0.
>
> What I could do is add a "chip-id" property to XiveENDSource possibly.
This still seems wrong for the PAPR model. Why can't you configure
the end_block value directly in the Xive components, then just set it
equal to the chip_id when you build the powernv machine?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
