| Message ID | 20200604132126.750999-6-clg@kaod.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xive/p9: various small cleanups | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch master (6bf21350da32776aac8ba75bf48933854647bd7e) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
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@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@linux.ibm.com> Best regards, Gustavo
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@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@linux.ibm.com> > > > Best regards, > Gustavo
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@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
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])
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@kaod.org> --- hw/xive.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)