[Skiboot] [PATCH v2 3/8] capp/phb: Introduce 'struct capp' to hold capp related info in 'struct phb'

[christophe lombard]

Le 09/12/2018 à 15:17, Vaibhav Jain a écrit :
> Previously struct proc_chip member 'capp_phb3_attached_mask' was used for
> Power-8 to keep track of PHB attached to the single CAPP on the
> chip. CAPP on that chip supported a flexible PHB assignment
> scheme. However since then new chips only support a static assignment
> i.e a CAPP can only be attached to a specific PEC.
> 
> Hence instead of using 'proc_chip.capp_phb4_attached_mask' to manage
> CAPP <-> PEC assignments which needs a global lock (capi_lock) to be
> updated, we introduce a new struct named 'capp' a pointer to which
> resides inside struct 'phb'. Since updates to struct 'phb' already
> happen in context of phb_lock; this eliminates the
> need to use mutex 'capi_lock' while updating
> 'capp_phb4_attached_mask'.
> 
> Also this struct is used to hold CAPP specific variables such as
> pointer to the 'struct phb' to which the CAPP is attached,
> 'capp_xscom_offset' which is the xscom offset to be added to CAPP
> registers in case there are more than 1 on the chip, 'capp_index'
> which is the index of the CAPP on the chip, and 'attached_pe' which is
> the process endpoint index to which CAPP is attached.
> 
> Signed-off-by: Vaibhav Jain <vaibhav at linux.ibm.com>
> ---
>   hw/phb4.c      | 49 ++++++++++++++++++++++++++++++++++++++++++-------
>   include/capp.h |  7 +++++++
>   include/chip.h |  1 -
>   include/pci.h  |  3 +++
>   4 files changed, 52 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/phb4.c b/hw/phb4.c
> index c0797647..5819ad0b 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -3880,13 +3880,13 @@ static int64_t phb4_get_capp_info(int chip_id, struct phb *phb,
>   				  struct capp_info *info)
>   {
>   	struct phb4 *p = phb_to_phb4(phb);
> -	struct proc_chip *chip = get_chip(p->chip_id);
>   	uint32_t offset;
> 
>   	if (chip_id != p->chip_id)
>   		return OPAL_PARAMETER;
> 
> -	if (!((1 << p->index) & chip->capp_phb4_attached_mask))
> +	/* Check is CAPP is attached to the PHB */
> +	if (phb->capp == NULL || phb->capp->phb != phb)
>   		return OPAL_PARAMETER;
> 
>   	offset = PHB4_CAPP_REG_OFFSET(p);
> @@ -4397,23 +4397,58 @@ static int64_t enable_capi_mode(struct phb4 *p, uint64_t pe_number,
>   	return OPAL_SUCCESS;
>   }
> 
> +
> +static int64_t phb4_init_capp(struct phb4 *p)
> +{
> +	struct capp *capp;
> +
> +	if (p->index != CAPP0_PHB_INDEX &&
> +	    p->index != CAPP1_PHB_INDEX)
> +		return OPAL_UNSUPPORTED;
> +
> +	capp = zalloc(sizeof(struct capp));
> +	if (capp == NULL)
> +		return OPAL_NO_MEM;
> +
> +	if (p->index == CAPP0_PHB_INDEX) {
> +		capp->capp_index = 0;
> +		capp->capp_xscom_offset = 0;
> +
> +	} else if (p->index == CAPP1_PHB_INDEX) {
> +		capp->capp_index = 1;
> +		capp->capp_xscom_offset = CAPP1_REG_OFFSET;
> +	}
> +
> +	capp->attached_pe = phb4_get_reserved_pe_number(phb);
> +
> +	/* Load capp microcode into the capp unit */
> +	load_capp_ucode(p);
> +
> +	p->phb.capp = capp;
> +
> +	return OPAL_SUCCESS;
> +}
> +
>   static int64_t phb4_set_capi_mode(struct phb *phb, uint64_t mode,
>   				  uint64_t pe_number)
>   {
>   	struct phb4 *p = phb_to_phb4(phb);
>   	struct proc_chip *chip = get_chip(p->chip_id);
> +	struct capp *capp = phb->capp;
>   	uint64_t reg, ret;
>   	uint32_t offset;
> 
> +	if (capp == NULL)
> +		return OPAL_UNSUPPORTED;
> 
>   	if (!capp_ucode_loaded(chip, p->index)) {
>   		PHBERR(p, "CAPP: ucode not loaded\n");
>   		return OPAL_RESOURCE;
>   	}
> 
> -	lock(&capi_lock);
> -	chip->capp_phb4_attached_mask |= 1 << p->index;
> -	unlock(&capi_lock);
> +	/* mark the capp attached to the phb */
> +	capp->phb = phb;
> +	capp->attached_pe = pe_number;
> 
>   	offset = PHB4_CAPP_REG_OFFSET(p);
>   	xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
> @@ -5595,8 +5630,8 @@ static void phb4_create(struct dt_node *np)
>   	/* Get the HW up and running */
>   	phb4_init_hw(p);
> 
> -	/* Load capp microcode into capp unit */
> -	load_capp_ucode(p);
> +	/* init capp that might get attached to the phb */
> +	phb4_init_capp(p);
> 
>   	/* Compute XIVE source flags depending on PHB revision */
>   	irq_flags = 0;
> diff --git a/include/capp.h b/include/capp.h
> index 6ec3f7fe..43a5916d 100644
> --- a/include/capp.h
> +++ b/include/capp.h
> @@ -79,6 +79,13 @@ struct capp_ops {
>   	int64_t (*get_capp_info)(int, struct phb *, struct capp_info *);
>   };
> 
> +struct capp {
> +	struct phb *phb;
> +	unsigned int capp_index;
> +	uint64_t capp_xscom_offset;
> +	uint64_t attached_pe;
> +};
> +
>   struct proc_chip;
>   extern struct lock capi_lock;
>   extern struct capp_ops capi_ops;
> diff --git a/include/chip.h b/include/chip.h
> index 2fb8126d..c759d0a0 100644
> --- a/include/chip.h
> +++ b/include/chip.h
> @@ -197,7 +197,6 @@ struct proc_chip {
> 
>   	/* Must hold capi_lock to change */
>   	uint8_t			capp_phb3_attached_mask;
> -	uint8_t			capp_phb4_attached_mask;
>   	uint8_t			capp_ucode_loaded;
> 
>   	/* Used by hw/centaur.c */
> diff --git a/include/pci.h b/include/pci.h
> index 05165614..946f40b2 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -401,6 +401,9 @@ struct phb {
> 
>   	/* Additional data the platform might need to attach */
>   	void			*platform_data;
> +
> +	/* Any capp instance attached to the PHB */
> +	struct capp		*capp;
>   };
> 

hum, if your solution sounds good, don't forget that CAPP is
only linked to phb3/4. The current code phb4_get_capp_info()
was based on the fact that we did not want to add information
specific to the CAPP on the phb structure.

>   static inline void phb_lock(struct phb *phb)
>