[Skiboot] [PATCH v2 17/17] xive/p9: introduce the ESB magic MMIO offsets

[Cédric Le Goater]

On 24/09/2019 08:52, Oliver O'Halloran wrote:
> On Thu, 2019-09-12 at 19:22 +0200, Cédric Le Goater wrote:
>> The following offsets into the ESB MMIO allow to read or manipulate
>> the PQ bits. They must be used with an 8-byte load instruction. They
>> all return the previous state of the interrupt (atomically).
>>
>> Additionally, some ESB pages support doing an EOI via a store and
>> some ESBs support doing a trigger via a separate trigger page.
>>
>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>> ---
>>  include/xive-regs.h | 18 ++++++++++++++++++
>>  hw/xive.c           | 14 +++++++-------
>>  2 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/xive-regs.h b/include/xive-regs.h
>> index f7d9fd4e0a3e..e395e9ab702b 100644
>> --- a/include/xive-regs.h
>> +++ b/include/xive-regs.h
>> @@ -94,4 +94,22 @@
>>  #define TM_QW3_NSR_I		PPC_BIT8(2)
>>  #define TM_QW3_NSR_GRP_LVL	PPC_BIT8(3,7)
>>  
>> +/*
>> + * "magic" Event State Buffer (ESB) MMIO offsets.
>> + *
>> + * The following offsets into the ESB MMIO allow to read or manipulate
>> + * the PQ bits. They must be used with an 8-byte load instruction.
>> + * They all return the previous state of the interrupt (atomically).
>> + *
>> + * Additionally, some ESB pages support doing an EOI via a store and
>> + * some ESBs support doing a trigger via a separate trigger page.
>> + */
>> +#define XIVE_ESB_STORE_EOI      0x400 /* Store */
>> +#define XIVE_ESB_LOAD_EOI       0x000 /* Load */
>> +#define XIVE_ESB_GET            0x800 /* Load */
>> +#define XIVE_ESB_SET_PQ_00      0xc00 /* Load */
>> +#define XIVE_ESB_SET_PQ_01      0xd00 /* Load */
>> +#define XIVE_ESB_SET_PQ_10      0xe00 /* Load */
>> +#define XIVE_ESB_SET_PQ_11      0xf00 /* Load */
> 
> Might as well include the XIVE_ESB_STORE_TRIGGER when storing to 0x000-
> 0x3FF. 

ok. That's a zero offset but it would clarify things.

> Looks like xive_ipi_trigger() and xive_ipi_eoi() use it.

Ah I missed those because they are part of the XICS emulation, which I tend
to ignore ...

Thanks,

C. 

> 
>> +
>>  #endif /* XIVE_REGS_H__ */
>> diff --git a/hw/xive.c b/hw/xive.c
>> index b0e3e0c77276..2f6816aa4b61 100644
>> --- a/hw/xive.c
>> +++ b/hw/xive.c
>> @@ -1064,7 +1064,7 @@ static void xive_scrub_workaround_eq(struct xive *x, uint32_t block __unused, ui
>>  	/* Ensure the above has returned before we do anything else
>>  	 * the XIVE store queue is completely empty
>>  	 */
>> -	load_wait(in_be64(mmio + 0x800));
>> +	load_wait(in_be64(mmio + XIVE_ESB_GET));
>>  }
>>  
>>  static int64_t __xive_cache_scrub(struct xive *x, enum xive_cache_type ctype,
>> @@ -2219,9 +2219,9 @@ static void xive_update_irq_mask(struct xive_src *s, uint32_t idx, bool masked)
>>  	if (s->flags & XIVE_SRC_EOI_PAGE1)
>>  		mmio_base += 1ull << (s->esb_shift - 1);
>>  	if (masked)
>> -		offset = 0xd00; /* PQ = 01 */
>> +		offset = XIVE_ESB_SET_PQ_01;
>>  	else
>> -		offset = 0xc00; /* PQ = 00 */
>> +		offset = XIVE_ESB_SET_PQ_00;
>>  
>>  	in_be64(mmio_base + offset);
>>  }
>> @@ -2234,7 +2234,7 @@ static int64_t xive_sync(struct xive *x)
>>  	lock(&x->lock);
>>  
>>  	/* Second 2K range of second page */
>> -	p = x->ic_base + (1 << x->ic_shift) + 0x800;
>> +	p = x->ic_base + (1 << x->ic_shift) + XIVE_ESB_GET;
>>  
>>  	/* TODO: Make this more fine grained */
>>  	out_be64(p + (10 << 7), 0); /* Sync OS escalations */
>> @@ -2387,7 +2387,7 @@ static void __xive_source_eoi(struct irq_source *is, uint32_t isn)
>>  
>>  	/* If the XIVE supports the new "store EOI facility, use it */
>>  	if (s->flags & XIVE_SRC_STORE_EOI)
>> -		out_be64(mmio_base + 0x400, 0);
>> +		out_be64(mmio_base + XIVE_ESB_STORE_EOI, 0);
>>  	else {
>>  		uint64_t offset;
>>  
>> @@ -2404,7 +2404,7 @@ static void __xive_source_eoi(struct irq_source *is, uint32_t isn)
>>  		if (s->flags & XIVE_SRC_LSI)
>>  			in_be64(mmio_base);
>>  		else {
>> -			offset = 0xc00;
>> +			offset = XIVE_ESB_SET_PQ_00;
>>  			eoi_val = in_be64(mmio_base + offset);
>>  			xive_vdbg(s->xive, "ISN: %08x EOI=%llx\n",
>>  				  isn, eoi_val);
>> @@ -3247,7 +3247,7 @@ static int64_t opal_xive_eoi(uint32_t xirr)
>>  		 * Note: We aren't doing an actual EOI. Instead we are clearing
>>  		 * both P and Q and will re-check the queue if Q was set.
>>  		 */
>> -		eoi_val = in_8(xs->eqmmio + 0xc00);
>> +		eoi_val = in_8(xs->eqmmio + XIVE_ESB_SET_PQ_00);
>>  		xive_cpu_vdbg(c, "  isn %08x, eoi_val=%02x\n", xirr, eoi_val);
>>  
>>  		/* Q was set ? Check EQ again after doing a sync to ensure
> 
> Do the weird MMIOs in xive_ipi_eoi() and __opal_xive_dump_emu() also
> need updating?
> 
>