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

[Gustavo Romero]

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


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


Best regards,
Gustavo