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

[Cédric Le Goater]

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.

>  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). Hence,
>  the E bit indicates that this is an escalation interrupt and, in that
>  case, the BLOC/INDEX represents the EQ containing the corresponding
>  escalation descriptor.


I can extend the comments in v2.

Thanks,

C.


> 
> Reviewed-by: Gustavo Romero <gromero at linux.ibm.com>
> 
> 
> Best regards,
> Gustavo