[Skiboot] [PATCH V2 5/5] capi: Handle HMI events

Andrew Donnellan andrew.donnellan at au1.ibm.com
Fri Apr 7 16:35:37 AEST 2017


On 01/04/17 03:52, Christophe Lombard wrote:
> Find the CAPP on the chip associated with the HMI event for PHB4.
> If a recoverable error has been detected the capp is re-initialize and
> functional operations are resumed.
>
> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>

This mostly looks alright, but a few things below.

> ---
>  core/hmi.c          | 117 +++++++++++++++-------------------------------------
>  hw/capp.c           |   9 ++++
>  hw/phb3.c           |  31 ++++++++++++++
>  hw/phb4.c           |  29 +++++++++++++
>  include/capp.h      |  18 ++++++++
>  include/phb4-capp.h |   3 ++
>  6 files changed, 124 insertions(+), 83 deletions(-)
>
> diff --git a/core/hmi.c b/core/hmi.c
> index bb99e59..431f1d3 100644
> --- a/core/hmi.c
> +++ b/core/hmi.c
> @@ -19,7 +19,6 @@
>  #include <processor.h>
>  #include <chiptod.h>
>  #include <xscom.h>
> -#include <phb3-capp.h>
>  #include <pci.h>
>  #include <cpu.h>
>  #include <chip.h>
> @@ -245,58 +244,6 @@ static int queue_hmi_event(struct OpalHMIEvent *hmi_evt, int recover)
>  				num_params, (uint64_t *)hmi_evt);
>  }
>
> -static int is_capp_recoverable(int chip_id, int capp_index)
> -{
> -	uint64_t reg;
> -	uint32_t reg_offset = capp_index ? CAPP1_REG_OFFSET : 0x0;
> -
> -	xscom_read(chip_id, CAPP_ERR_STATUS_CTRL + reg_offset, &reg);
> -	return (reg & PPC_BIT(0)) != 0;
> -}
> -
> -#define CAPP_PHB3_ATTACHED(chip, phb_index) \
> -	((chip)->capp_phb3_attached_mask & (1 << (phb_index)))
> -
> -#define CHIP_IS_NAPLES(chip) ((chip)->type == PROC_CHIP_P8_NAPLES)
> -
> -static int handle_capp_recoverable(int chip_id, int capp_index)
> -{
> -	struct dt_node *np;
> -	u64 phb_id;
> -	u32 dt_chip_id;
> -	struct phb *phb;
> -	u32 phb_index;
> -	struct proc_chip *chip = get_chip(chip_id);
> -
> -	dt_for_each_compatible(dt_root, np, "ibm,power8-pciex") {
> -		dt_chip_id = dt_prop_get_u32(np, "ibm,chip-id");
> -		phb_index = dt_prop_get_u32(np, "ibm,phb-index");
> -		phb_id = dt_prop_get_u64(np, "ibm,opal-phbid");
> -
> -		/*
> -		 * Murano/Venice have a single capp (capp0) per chip,
> -		 * that can be attached to phb0, phb1 or phb2.
> -		 * The capp is identified as being attached to the chip,
> -		 * regardless of the phb index.
> -		 *
> -		 * Naples has two capps per chip: capp0 attached to phb0,
> -		 * and capp1 attached to phb1.
> -		 * Once we know that the capp is attached to the chip,
> -		 * we must also check that capp/phb indices are equal.
> -		 */
> -		if ((chip_id == dt_chip_id) &&
> -		    CAPP_PHB3_ATTACHED(chip, phb_index) &&
> -		    (!CHIP_IS_NAPLES(chip) || phb_index == capp_index)) {
> -			phb = pci_get_phb(phb_id);
> -			phb_lock(phb);
> -			phb->ops->set_capp_recovery(phb);
> -			phb_unlock(phb);
> -			return 1;
> -		}
> -	}
> -	return 0;
> -}
> -
>  static bool decode_core_fir(struct cpu_thread *cpu,
>  				struct OpalHMIEvent *hmi_evt)
>  {
> @@ -385,47 +332,51 @@ static void find_capp_checkstop_reason(int flat_chip_id,
>  				       struct OpalHMIEvent *hmi_evt,
>  				       bool *event_generated)
>  {
> -	int capp_index;
> -	struct proc_chip *chip = get_chip(flat_chip_id);
> -	int capp_num = CHIP_IS_NAPLES(chip) ? 2 : 1;
> -	uint32_t reg_offset;
> +	struct capp_info info;
> +	struct phb *phb;
>  	uint64_t capp_fir;
>  	uint64_t capp_fir_mask;
>  	uint64_t capp_fir_action0;
>  	uint64_t capp_fir_action1;
> +	uint64_t rc, reg;
> +
> +	/* Find the CAPP on the chip associated with the HMI. */
> +	for_each_phb(phb) {
> +		/* get the CAPP info */
> +		rc = capp_get_info(phb, &info);
> +		if (rc == OPAL_PARAMETER)
> +			continue;
>
> -	for (capp_index = 0; capp_index < capp_num; capp_index++) {
> -		reg_offset = capp_index ? CAPP1_REG_OFFSET : 0x0;
> -
> -		if (xscom_read(flat_chip_id,
> -			       CAPP_FIR + reg_offset, &capp_fir) ||
> -		    xscom_read(flat_chip_id,
> -			       CAPP_FIR_MASK + reg_offset, &capp_fir_mask) ||
> -		    xscom_read(flat_chip_id,
> -			       CAPP_FIR_ACTION0 + reg_offset, &capp_fir_action0) ||
> -		    xscom_read(flat_chip_id,
> -			       CAPP_FIR_ACTION1 + reg_offset, &capp_fir_action1)) {
> -			prerror("CAPP: Couldn't read CAPP#%d FIR registers by XSCOM!\n",
> -				capp_index);
> +		if (xscom_read(flat_chip_id, info.capp_fir_reg, &capp_fir) ||
> +		    xscom_read(flat_chip_id, info.capp_fir_mask_reg, &capp_fir_mask) ||
> +		    xscom_read(flat_chip_id, info.capp_fir_action0_reg, &capp_fir_action0) ||
> +		    xscom_read(flat_chip_id, info.capp_fir_action1_reg, &capp_fir_action1)) {
> +			prerror("CAPP: Couldn't read CAPP#%d (PHB:#%d) FIR registers by XSCOM!\n",
> +				info.capp_index, info.phb_index);

looks good

>  			continue;
>  		}
>
>  		if (!(capp_fir & ~capp_fir_mask))
>  			continue;
>
> -		prlog(PR_DEBUG, "HMI: CAPP#%d: FIR 0x%016llx mask 0x%016llx\n",
> -		      capp_index, capp_fir, capp_fir_mask);
> -		prlog(PR_DEBUG, "HMI: CAPP#%d: ACTION0 0x%016llx, ACTION1 0x%016llx\n",
> -		      capp_index, capp_fir_action0, capp_fir_action1);
> -
> -		if (is_capp_recoverable(flat_chip_id, capp_index)) {
> -			if (handle_capp_recoverable(flat_chip_id, capp_index)) {
> -				hmi_evt->severity = OpalHMI_SEV_NO_ERROR;
> -				hmi_evt->type = OpalHMI_ERROR_CAPP_RECOVERY;
> -				queue_hmi_event(hmi_evt, 1);
> -				*event_generated = true;
> -				return;
> -			}
> +		prlog(PR_DEBUG, "HMI: CAPP#%d (PHB:#%d): FIR 0x%016llx mask 0x%016llx\n",
> +		      info.capp_index, info.phb_index, capp_fir, capp_fir_mask);
> +		prlog(PR_DEBUG, "HMI: CAPP#%d (PHB:#%d): ACTION0 0x%016llx, ACTION1 0x%016llx\n",
> +		      info.capp_index, info.phb_index, capp_fir_action0, capp_fir_action1);

looks good

> +
> +		/* If this bit is set (=1) a Recoverable Error has been detected */
> +		xscom_read(flat_chip_id, info.capp_err_status_ctrl_reg, &reg);

wooo, nice simplification

> +		if ((reg & PPC_BIT(0)) != 0) {
> +			phb_lock(phb);
> +			phb->ops->set_capp_recovery(phb);
> +			phb_unlock(phb);
> +
> +			hmi_evt->severity = OpalHMI_SEV_NO_ERROR;
> +			hmi_evt->type = OpalHMI_ERROR_CAPP_RECOVERY;
> +			queue_hmi_event(hmi_evt, 1);
> +			*event_generated = true;
> +
> +			return;
>  		}
>  	}
>  }
> diff --git a/hw/capp.c b/hw/capp.c
> index f3b5850..a93a6d7 100644
> --- a/hw/capp.c
> +++ b/hw/capp.c
> @@ -35,6 +35,7 @@ static struct {
>  #define CAPP_UCODE_MAX_SIZE 0x20000
>
>  struct lock capi_lock = LOCK_UNLOCKED;
> +struct capp_ops capi_ops = { NULL };
>
>  bool capp_ucode_loaded(struct proc_chip *chip, unsigned int index)
>  {
> @@ -227,3 +228,11 @@ int64_t capp_load_ucode(unsigned int chip_id, uint32_t opal_id,
>
>  	return OPAL_SUCCESS;
>  }
> +
> +uint64_t capp_get_info(struct phb *phb, struct capp_info *info)
> +{
> +	if (capi_ops.get_capp_info)
> +		return capi_ops.get_capp_info(phb, info);
> +
> +	return OPAL_PARAMETER;

Function needs to be signed

> +}
> diff --git a/hw/phb3.c b/hw/phb3.c
> index a8f5890..d851ac3 100644
> --- a/hw/phb3.c
> +++ b/hw/phb3.c
> @@ -3401,6 +3401,34 @@ static int64_t phb3_get_diag_data(struct phb *phb,
>  	return OPAL_SUCCESS;
>  }
>
> +static uint64_t phb3_get_capp_info(struct phb *phb, struct capp_info *info)
> +{
> +	struct phb3 *p = phb_to_phb3(phb);
> +	struct proc_chip *chip = get_chip(p->chip_id);
> +	uint32_t offset;
> +
> +	if (!((1 << p->index) & chip->capp_phb3_attached_mask))
> +		return OPAL_PARAMETER;

Function needs to be signed

> +
> +	offset = PHB3_CAPP_REG_OFFSET(p);
> +
> +	if (PHB3_IS_NAPLES(p))
> +		if (p->index == 0)
> +			info->capp_index = 0;
> +		else
> +			info->capp_index = 1;
> +	else
> +		info->capp_index = 0;

Since you're nesting ifs, please please please use braces.

> +	info->phb_index = p->index;
> +	info->capp_fir_reg = CAPP_FIR + offset;
> +	info->capp_fir_mask_reg = CAPP_FIR_MASK + offset;
> +	info->capp_fir_action0_reg = CAPP_FIR_ACTION0 + offset;
> +	info->capp_fir_action1_reg = CAPP_FIR_ACTION1 + offset;
> +	info->capp_err_status_ctrl_reg = CAPP_ERR_STATUS_CTRL + offset;
> +
> +	return OPAL_SUCCESS;
> +}
> +
>  static void phb3_init_capp_regs(struct phb3 *p, bool dma_mode)
>  {
>  	uint64_t reg;
> @@ -3612,6 +3640,9 @@ static int64_t enable_capi_mode(struct phb3 *p, uint64_t pe_number, bool dma_mod
>  		return OPAL_HARDWARE;
>  	}
>
> +	/* set callbacks to handle HMI events */
> +	capi_ops.get_capp_info = &phb3_get_capp_info;
> +
>  	return OPAL_SUCCESS;
>  }
>
> diff --git a/hw/phb4.c b/hw/phb4.c
> index ab1785b..0b1d987 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -2495,6 +2495,31 @@ static int64_t phb4_get_diag_data(struct phb *phb,
>  	return OPAL_SUCCESS;
>  }
>
> +static uint64_t phb4_get_capp_info(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 (!((1 << p->index) & chip->capp_phb4_attached_mask))
> +		return OPAL_PARAMETER;

Function needs to be signed

> +
> +	offset = PHB4_CAPP_REG_OFFSET(p);
> +
> +	if (p->index == CAPP0_PHB_INDEX)
> +		info->capp_index = 0;
> +	if (p->index == CAPP1_PHB_INDEX)
> +		info->capp_index = 1;
> +	info->phb_index = p->index;
> +	info->capp_fir_reg = CAPP_FIR + offset;
> +	info->capp_fir_mask_reg = CAPP_FIR_MASK + offset;
> +	info->capp_fir_action0_reg = CAPP_FIR_ACTION0 + offset;
> +	info->capp_fir_action1_reg = CAPP_FIR_ACTION1 + offset;
> +	info->capp_err_status_ctrl_reg = CAPP_ERR_STATUS_CTRL + offset;
> +
> +	return OPAL_SUCCESS;
> +}
> +
>  static void phb4_init_capp_regs(struct phb4 *p)
>  {
>  	uint64_t reg;
> @@ -2712,6 +2737,10 @@ static int64_t enable_capi_mode(struct phb4 *p, uint64_t pe_number)
>  			return OPAL_HARDWARE;
>  		}
>  	}
> +
> +	/* set callbacks to handle HMI events */
> +	capi_ops.get_capp_info = &phb4_get_capp_info;
> +
>  	return OPAL_SUCCESS;
>  }
>
> diff --git a/include/capp.h b/include/capp.h
> index c1aa8d2..4043a30 100644
> --- a/include/capp.h
> +++ b/include/capp.h
> @@ -65,8 +65,24 @@ enum capp_reg {
>  	apc_master_powerbus_ctrl	= 0xB
>  };
>
> +struct capp_info {
> +	unsigned int capp_index;
> +	unsigned int phb_index;
> +	uint64_t capp_fir_reg;
> +	uint64_t capp_fir_mask_reg;
> +	uint64_t capp_fir_action0_reg;
> +	uint64_t capp_fir_action1_reg;
> +	uint64_t capp_err_status_ctrl_reg;
> +};
> +
> +struct capp_ops {
> +	uint64_t (*get_capp_info)(struct phb *, struct capp_info *);
> +};
> +
>  struct proc_chip;
>  extern struct lock capi_lock;
> +extern struct capp_ops capi_ops;
> +
>  extern bool capp_ucode_loaded(struct proc_chip *chip, unsigned int index);
>
>  extern int64_t capp_load_ucode(unsigned int chip_id, uint32_t opal_id,
> @@ -74,4 +90,6 @@ extern int64_t capp_load_ucode(unsigned int chip_id, uint32_t opal_id,
>  			       uint64_t apc_master_addr, uint64_t apc_master_write,
>  			       uint64_t snp_array_addr, uint64_t snp_array_write);
>
> +extern uint64_t capp_get_info(struct phb *phb, struct capp_info *info);
> +
>  #endif /* __CAPP_H */
> diff --git a/include/phb4-capp.h b/include/phb4-capp.h
> index 418838d..d6128c7 100644
> --- a/include/phb4-capp.h
> +++ b/include/phb4-capp.h
> @@ -23,6 +23,9 @@
>  #define CAPP_APC_MASTER_ARRAY_WRITE_REG 	0x2010842  /* Satellite 2 */
>
>  #define CAPP_FIR				0x2010800
> +#define CAPP_FIR_MASK				0x2010803
> +#define CAPP_FIR_ACTION0			0x2010806
> +#define CAPP_FIR_ACTION1			0x2010807
>  #define CAPP_ERR_RPT_CLR			0x2010813
>  #define APC_MASTER_PB_CTRL			0x2010818
>  #define APC_MASTER_CAPI_CTRL			0x2010819
>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited



More information about the Skiboot mailing list