[Skiboot] [PATCH 11/16] [PATCH 11/16] opencapi5: hmi scom dump

[Frederic Barrat]

On 20/08/2021 11:45, Christophe Lombard wrote:
> This patch add a new function to dump PAU registers when a HMI has been
> raised and an OpenCAPI link has been hit by an error.
> 
> For each register, the scom address and the register value are printed.
> 
> The hmi.c has been redesigned in order to support the new PHB/PCIEX
> type (PAU OpenCapi). Now, the *npu* functions support NPU and PAU units of
> P8, P9 and P10 chips.
> 
> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
> ---


A few comments below, but I like the cleanup done by this patch, 
unifying the code patch between all generations of NPUs...


>   core/hmi.c               | 263 ++++++++++++++++++---------------------
>   hw/npu2-common.c         |  30 ++---
>   hw/pau.c                 |  50 ++++++++
>   include/npu2-regs.h      |   5 +
>   include/npu2.h           |   2 +-
>   include/pau-regs.h       |  24 ++++
>   include/pau.h            |   2 +
>   include/xscom-p10-regs.h |   3 +
>   8 files changed, 213 insertions(+), 166 deletions(-)
> 
> diff --git a/core/hmi.c b/core/hmi.c
> index 9363cc5f..8d287cb3 100644
> --- a/core/hmi.c
> +++ b/core/hmi.c

> @@ -758,114 +754,112 @@ static void encode_npu2_xstop_reason(uint32_t *xstop_reason,
>   		bit = ilog2(fir);
>   		reason = fir_number << 6;
>   		reason |= (63 - bit); // IBM numbering
> -		add_npu2_xstop_reason(xstop_reason, reason);
> +		add_npu_xstop_reason(xstop_reason, reason);
>   		fir ^= 1ULL << bit;
>   	}
>   }
> 
> -static void find_npu2_checkstop_reason(int flat_chip_id,
> -				      struct OpalHMIEvent *hmi_evt,
> -				      uint64_t *out_flags)
> +static bool npu_fir_errors(struct phb *phb, int flat_chip_id,
> +			   uint32_t *xstop_reason)
>   {
> -	struct phb *phb;
> -	int i;
> -	bool npu2_hmi_verbose = false, found = false;
> -	uint64_t npu2_fir;
> -	uint64_t npu2_fir_mask;
> -	uint64_t npu2_fir_action0;
> -	uint64_t npu2_fir_action1;
> -	uint64_t npu2_fir_addr;
> -	uint64_t npu2_fir_mask_addr;
> -	uint64_t npu2_fir_action0_addr;
> -	uint64_t npu2_fir_action1_addr;
> +	uint64_t fir, fir_mask;
> +	uint64_t fir_action0, fir_action1;
> +	uint64_t fir_reg, fir_mask_reg;
> +	uint64_t fir_action0_reg, fir_action1_reg;
>   	uint64_t fatal_errors;
> -	uint32_t xstop_reason = 0;
> -	int total_errors = 0;
> +	uint64_t xscom_base;
> +	bool fir_errors = false;
> +	int fir_regs;
>   	const char *loc;
> -
> -	/* NPU2 only */
> -	if (PVR_TYPE(mfspr(SPR_PVR)) != PVR_TYPE_P9)
> -		return;
> -
> -	/* Find the NPU on the chip associated with the HMI. */
> -	for_each_phb(phb) {
> -		/* NOTE: if a chip ever has >1 NPU this will need adjusting */
> -		if (phb_is_npu2(phb->dt_node) &&
> -		    (dt_get_chip_id(phb->dt_node) == flat_chip_id)) {
> -			found = true;
> -			break;
> +	struct npu *npu;
> +
> +	fir_regs = (phb->phb_type == phb_type_pcie_v3) ? 1 : 3;
> +
> +	for (uint32_t i = 0; i < fir_regs; i++) {
> +		switch (phb->phb_type) {
> +		case phb_type_pcie_v3:
> +			fir_reg = NX_FIR;
> +			fir_mask_reg = NX_FIR_MASK;
> +			fir_action0_reg = NX_FIR_ACTION0;
> +			fir_action1_reg = NX_FIR_ACTION1;
> +
> +			npu = phb_to_npu(phb);
> +			if (!(npu == NULL))


That's unusual!


> +				xscom_base = npu->at_xscom;
> +			else
> +				continue;
> +		break;
> +		case phb_type_npu_v2:
> +		case phb_type_npu_v2_opencapi:
> +			fir_reg = NPU2_FIR(i);
> +			fir_mask_reg = NPU2_FIR_MASK(i);
> +			fir_action0_reg = NPU2_FIR_ACTION0(i);
> +			fir_action1_reg = NPU2_FIR_ACTION1(i);
> +			xscom_base = dt_prop_get_u32(phb->dt_node, "ibm,xscom-base");

We should avoid going back to the device tree to read the scom base, 
it's pretty expensive and we already have the information in the npu 
structure that we can reach easily from the phb struct.


> +		break;
> +		case phb_type_pau_opencapi:
> +			fir_reg = PAU_FIR(i);
> +			fir_mask_reg = PAU_FIR_MASK(i);
> +			fir_action0_reg = PAU_FIR_ACTION0(i);
> +			fir_action1_reg = PAU_FIR_ACTION1(i);
> +			xscom_base = dt_prop_get_u32(phb->dt_node, "ibm,xscom-base");

Same comment as above.

   Fred