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.
C.
