capi: Keep the current mmio windows in the mbt cache table.

When the phb is used as a CAPI interface, the current mmio windows list
is cleaned before adding the capi and the prefetchable memory (M64)
windows, which implies that the non-prefetchable BAR is no more
configured.
This patch allows to set only the mbt bar to pass capi mmio window and
to keep, as defined, the other mmio values (M32 and M64).

Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
 hw/phb4.c | 61 ++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

Le 03/04/2018 à 16:20, Christophe Lombard a écrit :
> When the phb is used as a CAPI interface, the current mmio windows list
> is cleaned before adding the capi and the prefetchable memory (M64)
> windows, which implies that the non-prefetchable BAR is no more
> configured.
> This patch allows to set only the mbt bar to pass capi mmio window and
> to keep, as defined, the other mmio values (M32 and M64).
> 
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> ---
>   hw/phb4.c | 61 ++++++++++++++++++++++++++++++++-----------------------------
>   1 file changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/phb4.c b/hw/phb4.c
> index e45be01..2f6ca32 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -3829,7 +3829,8 @@ static int64_t enable_capi_mode(struct phb4 *p, uint64_t pe_number,
>   				uint32_t capp_eng)
>   {
>   	uint64_t reg, start_addr, end_addr, stq_eng, dma_eng;
> -	int i;
> +	uint64_t mbt_0, mbt_1;
> +	int i, entc = -1, entf = -1;
> 
>   	/* CAPP Control Register */
>   	xscom_read(p->chip_id, p->pe_xscom + XPEC_NEST_CAPP_CNTL, &reg);
> @@ -3975,41 +3976,43 @@ static int64_t enable_capi_mode(struct phb4 *p, uint64_t pe_number,
>   	for (i = 0; i < p->tvt_size; i++)
>   		out_be64(p->regs + PHB_IODA_DATA0, p->tve_cache[i]);
> 
> -	/* set mbt bar to pass capi mmio window. First applied cleared
> -	 * values to HW
> -	 */
> -	for (i = 0; i < p->mbt_size; i++) {
> -		p->mbt_cache[i][0] = 0;
> -		p->mbt_cache[i][1] = 0;
> -	}
> -	phb4_ioda_sel(p, IODA3_TBL_MBT, 0, true);
> -	for (i = 0; i < p->mbt_size; i++) {
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][0]);
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][1]);
> -	}
> -
> -	p->mbt_cache[0][0] = IODA3_MBT0_ENABLE |
> -			     IODA3_MBT0_TYPE_M64 |
> -		SETFIELD(IODA3_MBT0_MODE, 0ull, IODA3_MBT0_MODE_SINGLE_PE) |
> -		SETFIELD(IODA3_MBT0_MDT_COLUMN, 0ull, 0) |
> -		(p->mm0_base & IODA3_MBT0_BASE_ADDR);
> -	p->mbt_cache[0][1] = IODA3_MBT1_ENABLE |
> -		((~(p->mm0_size - 1)) & IODA3_MBT1_MASK) |
> -		SETFIELD(IODA3_MBT1_SINGLE_PE_NUM, 0ull, pe_number);
> -
> -	p->mbt_cache[1][0] = IODA3_MBT0_ENABLE |
> -			     IODA3_MBT0_TYPE_M64 |
> +	/* set mbt bar to pass capi mmio window and keep the other
> +	 * mmio values */
> +	mbt_0 = IODA3_MBT0_ENABLE | IODA3_MBT0_TYPE_M64 |
>   		SETFIELD(IODA3_MBT0_MODE, 0ull, IODA3_MBT0_MODE_SINGLE_PE) |
>   		SETFIELD(IODA3_MBT0_MDT_COLUMN, 0ull, 0) |
>   		(0x0002000000000000ULL & IODA3_MBT0_BASE_ADDR);
> -	p->mbt_cache[1][1] = IODA3_MBT1_ENABLE |
> +
> +	mbt_1 = IODA3_MBT1_ENABLE |
>   		(0x00ff000000000000ULL & IODA3_MBT1_MASK) |
>   		SETFIELD(IODA3_MBT1_SINGLE_PE_NUM, 0ull, pe_number);
> 
> -	phb4_ioda_sel(p, IODA3_TBL_MBT, 0, true);
>   	for (i = 0; i < p->mbt_size; i++) {
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][0]);
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][1]);
> +		/* search if the capi mmio window is already present */
> +		if ((p->mbt_cache[i][0] == mbt_0) &&
> +		    (p->mbt_cache[i][1] == mbt_1)) {
> +			 entc = i;
> +			 break;
> +		}
> +		/* search a free entry */
> +		if ((p->mbt_cache[i][0] == 0) &&
> +		    (p->mbt_cache[i][1] == 0)) {
> +			if (entf == -1)
> +				entf = i;
> +		}


I discussed it with Christophe, 0 is not a good guarantee that the entry 
is not in use, it is initialized with phb4_default_mbt0().
Though not perfect, adding the capi window in a disabled entry may be 
better.

   Fred



> +	}
> +
> +	if ((entc == -1) && (entf == -1)) {
> +		/* this case should never happen */
> +		PHBERR(p, "CAPP: Failed to add CAPI mmio window\n");
> +	} else if (entc == -1) {
> +		/* no capi mmio window found, so add it */
> +		p->mbt_cache[entf][0] = mbt_0;
> +		p->mbt_cache[entf][1] = mbt_1;
> +
> +		phb4_ioda_sel(p, IODA3_TBL_MBT, 0, true);
> +		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[entf][0]);
> +		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[entf][1]);
>   	}
> 
>   	phb4_init_capp_errors(p);
>

Hi Christophe,

The flow looks good to me but some minor comments regarding code
re-organization.

Christophe Lombard <clombard@linux.vnet.ibm.com> writes:

> --- a/hw/phb4.c
> +++ b/hw/phb4.c
<snip>
>  	for (i = 0; i < p->mbt_size; i++) {
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][0]);
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][1]);
> +		/* search if the capi mmio window is already present */
> +		if ((p->mbt_cache[i][0] == mbt_0) &&
> +		    (p->mbt_cache[i][1] == mbt_1)) {
> +			 entc = i;
> +			 break;
> +		}
> +		/* search a free entry */
> +		if ((p->mbt_cache[i][0] == 0) &&
> +		    (p->mbt_cache[i][1] == 0)) {
> +			if (entf == -1)
> +				entf = i;
> +		}
'if (entf == -1 && (p->mbt_cache[i][0] == 0) && (p->mbt_cache[i][1] == 0))'

> +	}
> +
> +	if ((entc == -1) && (entf == -1)) {
> +		/* this case should never happen */
> +		PHBERR(p, "CAPP: Failed to add CAPI mmio window\n");
> +	} else if (entc == -1) {
> +		/* no capi mmio window found, so add it */
> +		p->mbt_cache[entf][0] = mbt_0;
> +		p->mbt_cache[entf][1] = mbt_1;
> +
> +		phb4_ioda_sel(p, IODA3_TBL_MBT, 0, true);
> +		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[entf][0]);
> +		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[entf][1]);
>  	}

You can get rid of variable 'entc' entirely as its value can be drived from
loop index 'i'. This should also simplify the loop and the 'if-else' ladder
afterwards:

if (entf >= 0 && i == p->mbt_size) /* Free entry */
else if (i == p->mbt_size) /* mbt cache full */
else /* duplicate entry */

or

if (i < p->mbt_size) /* duplicate entry */
else if (entf >= 0) /* Free Entry */
else /* mbt cache full */