[Skiboot] [PATCH 05/11] xive/p9: Clarify the escalation IRQ encoding

[Gustavo Romero]

On 6/8/20 10:59 AM, Cédric Le Goater wrote:
> On 6/8/20 5:31 AM, Gustavo Romero wrote:
>> On 6/4/20 10:21 AM, Cédric Le Goater wrote:
>>> When an interrupt can not be delivered, an escalation interrupt can be
>>> triggered. The EQ descriptor of the pending interrupt should be
>>> configured to generate an escalation event, 'e' bit, and words 4 and 5
>>> of the EQ descriptor should contain an IVE pointing to the escalation
>>> EQ to trigger. This is why EQs are considered as interrupt sources and
>>> registered as such when initializing the interrupt controller.
>>>
>>> These interrupts are identified as escalations by setting a special
>>> bit in their global interrupt number. Clarify that and check that the
>>> number of EQDs is not overflowing the global interrupt encoding.
>>>
>>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>>> ---
>>>    hw/xive.c | 9 +++++++--
>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/xive.c b/hw/xive.c
>>> index 2a132ce87f3b..4a029c3e97db 100644
>>> --- a/hw/xive.c
>>> +++ b/hw/xive.c
>>> @@ -503,16 +503,21 @@ static uint32_t xive_chip_to_block(uint32_t chip_id)
>>>     * the top 8 bits are reserved for the CPPR value.
>>>     */
>>>    #define INT_SHIFT        20
>>> +#define INT_ESC_SHIFT        (INT_SHIFT + 4) /* 4bits block id */
>>>      #if XIVE_INT_ORDER > INT_SHIFT
>>>    #error "Too many ESBs for IRQ encoding"
>>>    #endif
>>>    +#if XIVE_EQ_ORDER > INT_SHIFT
>>> +#error "Too many EQs for escalation IRQ number encoding"
>>> +#endif
>>> +
>>>    #define GIRQ_TO_BLK(__g)    (((__g) >> INT_SHIFT) & 0xf)
>>>    #define GIRQ_TO_IDX(__g)    ((__g) & ((1 << INT_SHIFT) - 1))
>>>    #define BLKIDX_TO_GIRQ(__b,__i)    (((uint32_t)(__b)) << INT_SHIFT | (__i))
>>> -#define GIRQ_IS_ESCALATION(__g)    ((__g) & 0x01000000)
>>> -#define MAKE_ESCALATION_GIRQ(__b,__i)(BLKIDX_TO_GIRQ(__b,__i) | 0x01000000)
>>> +#define GIRQ_IS_ESCALATION(__g)    ((__g) & (1 << INT_ESC_SHIFT))
>>> +#define MAKE_ESCALATION_GIRQ(__b,__i)(BLKIDX_TO_GIRQ(__b,__i) | (1 << INT_ESC_SHIFT))
>>>      /* Block/IRQ to chip# conversions */
>>>    #define PC_BLK_TO_CHIP(__b)    (xive_block_to_chip[__b])
>>
>> I think this commit message is too good to be left outside the source.
>> So how about changing:
>>
>>   502  * the E bit indicates that this is an escalation interrupt, in
>>   503  * that case, the BLOC/INDEX represents the EQ containig the
>>   504  * corresponding escalation descriptor.
>>
>> by something like:
>>       When an interrupt can not be delivered, an escalation interrupt can be
>>   triggered. The EQ descriptor of the pending interrupt should be
>>   configured to generate an escalation event, 'E' bit, and words 4 and 5
> 
> There is an 'e' bit in the XIVE EQ descriptor and an 'E' bit in the global
> IRQ number. These are different notions.

Does a 'e' set implies a 'E' set, or there isn't such a kind of relation?


> I can extend the comments in v2.

Thanks.


Kind regards,
Gustavo