diff mbox series

[v2,4/8] phb4/capp: Update and re-factor phb4_set_capi_mode()

Message ID 20181209141744.4787-5-vaibhav@linux.ibm.com
State Superseded
Headers show
Series Enable fast-reboot support for CAPI-2 | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot fail Test snowpatch/job/snowpatch-skiboot on branch master

Commit Message

Vaibhav Jain Dec. 9, 2018, 2:17 p.m. UTC 
Presently phb4_set_capi_mode() performs certain CAPP checks
like, checking of CAPP ucode loaded or checks if CAPP is still in
recovery, even when the requested mode is to switch to PCI mode.

Hence this patch updates and re-factors phb4_set_capi_mode() to make
sure CAPP related checks are only performed when request to enable
CAPP is made by mode==OPAL_PHB_CAPI_MODE_CAPI/DMA_TVT1. We also update
other possible modes requests to return a more appropriate status code
based on if CAPP is activated or not.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Change-log:

v2: Using 'struct capp* instead of global 'capi_lock' to indicate that
    CAPP is now attached to a PHB [Andrew]
    Code formatting that removed the use of a confusing tertiary
    operator. [Andrew]
---
 hw/phb4.c | 93 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 57 insertions(+), 36 deletions(-)

Comments

Andrew Donnellan Dec. 10, 2018, 5:05 a.m. UTC | #1 
On 10/12/18 1:17 am, Vaibhav Jain wrote:
> Presently phb4_set_capi_mode() performs certain CAPP checks
> like, checking of CAPP ucode loaded or checks if CAPP is still in
> recovery, even when the requested mode is to switch to PCI mode.
> 
> Hence this patch updates and re-factors phb4_set_capi_mode() to make
> sure CAPP related checks are only performed when request to enable
> CAPP is made by mode==OPAL_PHB_CAPI_MODE_CAPI/DMA_TVT1. We also update
> other possible modes requests to return a more appropriate status code
> based on if CAPP is activated or not.
> 
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>

Thanks for addressing the issues I raised.

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Spelling nitpick below if you happen to respin a v3.

> ---
> Change-log:
> 
> v2: Using 'struct capp* instead of global 'capi_lock' to indicate that
>      CAPP is now attached to a PHB [Andrew]
>      Code formatting that removed the use of a confusing tertiary
>      operator. [Andrew]
> ---
>   hw/phb4.c | 93 ++++++++++++++++++++++++++++++++++---------------------
>   1 file changed, 57 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/phb4.c b/hw/phb4.c
> index 5819ad0b..df483e27 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -4436,54 +4436,75 @@ static int64_t phb4_set_capi_mode(struct phb *phb, uint64_t mode,
>   	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;
> -	}
> -
> -	/* mark the capp attached to the phb */
> -	capp->phb = phb;
> -	capp->attached_pe = pe_number;
> +	switch (mode) {
>   
> -	offset = PHB4_CAPP_REG_OFFSET(p);
> -	xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
> -	if ((reg & PPC_BIT(5))) {
> -		PHBERR(p, "CAPP: recovery failed (%016llx)\n", reg);
> -		return OPAL_HARDWARE;
> -	} else if ((reg & PPC_BIT(0)) && (!(reg & PPC_BIT(1)))) {
> -		PHBDBG(p, "CAPP: recovery in progress\n");
> -		return OPAL_BUSY;
> -	}
> +	case OPAL_PHB_CAPI_MODE_DMA: /* Enabled by default on p9 */
> +	case OPAL_PHB_CAPI_MODE_SNOOP_ON:
> +		/* nothing to do on P9 if CAPP is alreay enabled */

already :)

> +		ret = phb->capp->phb ? OPAL_SUCCESS : OPAL_UNSUPPORTED;
> +		break;
>   
> -	switch (mode) {
> -	case OPAL_PHB_CAPI_MODE_CAPI:
> -		ret = enable_capi_mode(p, pe_number,
> -					CAPP_MAX_STQ_ENGINES |
> -					CAPP_MIN_DMA_READ_ENGINES);
> -		disable_fast_reboot("CAPP being enabled");
> +	case OPAL_PHB_CAPI_MODE_SNOOP_OFF:
> +	case OPAL_PHB_CAPI_MODE_PCIE: /* Not supported at the moment */
> +		ret = phb->capp->phb ? OPAL_UNSUPPORTED : OPAL_SUCCESS;
>   		break;
> +
> +	case OPAL_PHB_CAPI_MODE_CAPI: /* Fall Through */
>   	case OPAL_PHB_CAPI_MODE_DMA_TVT1:
> -		ret = enable_capi_mode(p, pe_number,
> -					CAPP_MIN_STQ_ENGINES |
> -					CAPP_MAX_DMA_READ_ENGINES);
> -		disable_fast_reboot("CAPP being enabled");
> -		break;
> -	case OPAL_PHB_CAPI_MODE_SNOOP_ON:
> -		/* nothing to do P9 if CAPP is alreay enabled */
> -		ret = OPAL_SUCCESS;
> +		/* Check if ucode is available */
> +		if (!capp_ucode_loaded(chip, p->index)) {
> +			PHBERR(p, "CAPP: ucode not loaded\n");
> +			ret = OPAL_RESOURCE;
> +			break;
> +		}
> +
> +		xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
> +		if ((reg & PPC_BIT(5))) {
> +			PHBERR(p, "CAPP: recovery failed (%016llx)\n", reg);
> +			ret = OPAL_HARDWARE;
> +			break;
> +		} else if ((reg & PPC_BIT(0)) && (!(reg & PPC_BIT(1)))) {
> +			PHBDBG(p, "CAPP: recovery in progress\n");
> +			ret = OPAL_BUSY;
> +			break;
> +		}
> +
> +		/*
> +		 * Mark the CAPP attached to the PHB right away so that
> +		 * if a MCE happens during CAPP init we can handle it.
> +		 * In case of an error in CAPP init we remove the PHB
> +		 * from the attached_mask later.
> +		 */
> +		capp->phb = phb;
> +		capp->attached_pe = pe_number;
> +
> +		if (mode == OPAL_PHB_CAPI_MODE_DMA_TVT1)
> +			ret = enable_capi_mode(p, pe_number,
> +					       CAPP_MIN_STQ_ENGINES |
> +					       CAPP_MAX_DMA_READ_ENGINES);
> +
> +		else
> +			ret = enable_capi_mode(p, pe_number,
> +					       CAPP_MAX_STQ_ENGINES |
> +					       CAPP_MIN_DMA_READ_ENGINES);
> +		if (ret == OPAL_SUCCESS) {
> +			/* Disable fast reboot for CAPP */
> +			disable_fast_reboot("CAPP being enabled");
> +		} else {
> +			/* In case of an error mark the PHB detached */
> +			capp->phb = NULL;
> +			capp->attached_pe = phb4_get_reserved_pe_number(phb);
> +		}
>   		break;
>   
> -	case OPAL_PHB_CAPI_MODE_PCIE: /* shouldn't be called on p9*/
> -	case OPAL_PHB_CAPI_MODE_DMA: /* Enabled by default on p9 */
> -	case OPAL_PHB_CAPI_MODE_SNOOP_OFF: /* shouldn't be called on p9*/
>   	default:
>   		ret = OPAL_UNSUPPORTED;
> -	}
> +		break;
> +	};
>   
>   	return ret;
>   }
>
Christophe Lombard Dec. 10, 2018, 5:03 p.m. UTC | #2 
Le 09/12/2018 à 15:17, Vaibhav Jain a écrit :
> Presently phb4_set_capi_mode() performs certain CAPP checks
> like, checking of CAPP ucode loaded or checks if CAPP is still in
> recovery, even when the requested mode is to switch to PCI mode.
> 
> Hence this patch updates and re-factors phb4_set_capi_mode() to make
> sure CAPP related checks are only performed when request to enable
> CAPP is made by mode==OPAL_PHB_CAPI_MODE_CAPI/DMA_TVT1. We also update
> other possible modes requests to return a more appropriate status code
> based on if CAPP is activated or not.
> 
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Change-log:
> 
> v2: Using 'struct capp* instead of global 'capi_lock' to indicate that
>      CAPP is now attached to a PHB [Andrew]
>      Code formatting that removed the use of a confusing tertiary
>      operator. [Andrew]
> ---
>   hw/phb4.c | 93 ++++++++++++++++++++++++++++++++++---------------------
>   1 file changed, 57 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/phb4.c b/hw/phb4.c
> index 5819ad0b..df483e27 100644
> --- a/hw/phb4.c
> +++ b/hw/phb4.c
> @@ -4436,54 +4436,75 @@ static int64_t phb4_set_capi_mode(struct phb *phb, uint64_t mode,
>   	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;
> -	}
> -
> -	/* mark the capp attached to the phb */
> -	capp->phb = phb;
> -	capp->attached_pe = pe_number;
> +	switch (mode) {
> 

If I am not wrong, I think, that a window could exist where we
could have some impacts in cxllib.c and pci.c and specially
in the recovery case, because the checks are only performed
with mode==OPAL_PHB_CAPI_MODE_CAPI/DMA_TVT1 and not in all
cases as previously.

> -	offset = PHB4_CAPP_REG_OFFSET(p);
> -	xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
> -	if ((reg & PPC_BIT(5))) {
> -		PHBERR(p, "CAPP: recovery failed (%016llx)\n", reg);
> -		return OPAL_HARDWARE;
> -	} else if ((reg & PPC_BIT(0)) && (!(reg & PPC_BIT(1)))) {
> -		PHBDBG(p, "CAPP: recovery in progress\n");
> -		return OPAL_BUSY;
> -	}
> +	case OPAL_PHB_CAPI_MODE_DMA: /* Enabled by default on p9 */
> +	case OPAL_PHB_CAPI_MODE_SNOOP_ON:
> +		/* nothing to do on P9 if CAPP is alreay enabled */
> +		ret = phb->capp->phb ? OPAL_SUCCESS : OPAL_UNSUPPORTED;
> +		break;
> 
> -	switch (mode) {
> -	case OPAL_PHB_CAPI_MODE_CAPI:
> -		ret = enable_capi_mode(p, pe_number,
> -					CAPP_MAX_STQ_ENGINES |
> -					CAPP_MIN_DMA_READ_ENGINES);
> -		disable_fast_reboot("CAPP being enabled");
> +	case OPAL_PHB_CAPI_MODE_SNOOP_OFF:
> +	case OPAL_PHB_CAPI_MODE_PCIE: /* Not supported at the moment */
> +		ret = phb->capp->phb ? OPAL_UNSUPPORTED : OPAL_SUCCESS;
>   		break;
> +
> +	case OPAL_PHB_CAPI_MODE_CAPI: /* Fall Through */
>   	case OPAL_PHB_CAPI_MODE_DMA_TVT1:
> -		ret = enable_capi_mode(p, pe_number,
> -					CAPP_MIN_STQ_ENGINES |
> -					CAPP_MAX_DMA_READ_ENGINES);
> -		disable_fast_reboot("CAPP being enabled");
> -		break;
> -	case OPAL_PHB_CAPI_MODE_SNOOP_ON:
> -		/* nothing to do P9 if CAPP is alreay enabled */
> -		ret = OPAL_SUCCESS;
> +		/* Check if ucode is available */
> +		if (!capp_ucode_loaded(chip, p->index)) {
> +			PHBERR(p, "CAPP: ucode not loaded\n");
> +			ret = OPAL_RESOURCE;
> +			break;
> +		}
> +
> +		xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
> +		if ((reg & PPC_BIT(5))) {
> +			PHBERR(p, "CAPP: recovery failed (%016llx)\n", reg);
> +			ret = OPAL_HARDWARE;
> +			break;
> +		} else if ((reg & PPC_BIT(0)) && (!(reg & PPC_BIT(1)))) {
> +			PHBDBG(p, "CAPP: recovery in progress\n");
> +			ret = OPAL_BUSY;
> +			break;
> +		}
> +
> +		/*
> +		 * Mark the CAPP attached to the PHB right away so that
> +		 * if a MCE happens during CAPP init we can handle it.
> +		 * In case of an error in CAPP init we remove the PHB
> +		 * from the attached_mask later.
> +		 */
> +		capp->phb = phb;
> +		capp->attached_pe = pe_number;
> +
> +		if (mode == OPAL_PHB_CAPI_MODE_DMA_TVT1)
> +			ret = enable_capi_mode(p, pe_number,
> +					       CAPP_MIN_STQ_ENGINES |
> +					       CAPP_MAX_DMA_READ_ENGINES);
> +
> +		else
> +			ret = enable_capi_mode(p, pe_number,
> +					       CAPP_MAX_STQ_ENGINES |
> +					       CAPP_MIN_DMA_READ_ENGINES);
> +		if (ret == OPAL_SUCCESS) {
> +			/* Disable fast reboot for CAPP */
> +			disable_fast_reboot("CAPP being enabled");
> +		} else {
> +			/* In case of an error mark the PHB detached */
> +			capp->phb = NULL;
> +			capp->attached_pe = phb4_get_reserved_pe_number(phb);
> +		}
>   		break;
> 
> -	case OPAL_PHB_CAPI_MODE_PCIE: /* shouldn't be called on p9*/
> -	case OPAL_PHB_CAPI_MODE_DMA: /* Enabled by default on p9 */
> -	case OPAL_PHB_CAPI_MODE_SNOOP_OFF: /* shouldn't be called on p9*/
>   	default:
>   		ret = OPAL_UNSUPPORTED;
> -	}
> +		break;
> +	};
> 
>   	return ret;
>   }
>
diff mbox series

Patch

diff --git a/hw/phb4.c b/hw/phb4.c
index 5819ad0b..df483e27 100644
--- a/hw/phb4.c
+++ b/hw/phb4.c
@@ -4436,54 +4436,75 @@  static int64_t phb4_set_capi_mode(struct phb *phb, uint64_t mode,
 	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;
-	}
-
-	/* mark the capp attached to the phb */
-	capp->phb = phb;
-	capp->attached_pe = pe_number;
+	switch (mode) {
 
-	offset = PHB4_CAPP_REG_OFFSET(p);
-	xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
-	if ((reg & PPC_BIT(5))) {
-		PHBERR(p, "CAPP: recovery failed (%016llx)\n", reg);
-		return OPAL_HARDWARE;
-	} else if ((reg & PPC_BIT(0)) && (!(reg & PPC_BIT(1)))) {
-		PHBDBG(p, "CAPP: recovery in progress\n");
-		return OPAL_BUSY;
-	}
+	case OPAL_PHB_CAPI_MODE_DMA: /* Enabled by default on p9 */
+	case OPAL_PHB_CAPI_MODE_SNOOP_ON:
+		/* nothing to do on P9 if CAPP is alreay enabled */
+		ret = phb->capp->phb ? OPAL_SUCCESS : OPAL_UNSUPPORTED;
+		break;
 
-	switch (mode) {
-	case OPAL_PHB_CAPI_MODE_CAPI:
-		ret = enable_capi_mode(p, pe_number,
-					CAPP_MAX_STQ_ENGINES |
-					CAPP_MIN_DMA_READ_ENGINES);
-		disable_fast_reboot("CAPP being enabled");
+	case OPAL_PHB_CAPI_MODE_SNOOP_OFF:
+	case OPAL_PHB_CAPI_MODE_PCIE: /* Not supported at the moment */
+		ret = phb->capp->phb ? OPAL_UNSUPPORTED : OPAL_SUCCESS;
 		break;
+
+	case OPAL_PHB_CAPI_MODE_CAPI: /* Fall Through */
 	case OPAL_PHB_CAPI_MODE_DMA_TVT1:
-		ret = enable_capi_mode(p, pe_number,
-					CAPP_MIN_STQ_ENGINES |
-					CAPP_MAX_DMA_READ_ENGINES);
-		disable_fast_reboot("CAPP being enabled");
-		break;
-	case OPAL_PHB_CAPI_MODE_SNOOP_ON:
-		/* nothing to do P9 if CAPP is alreay enabled */
-		ret = OPAL_SUCCESS;
+		/* Check if ucode is available */
+		if (!capp_ucode_loaded(chip, p->index)) {
+			PHBERR(p, "CAPP: ucode not loaded\n");
+			ret = OPAL_RESOURCE;
+			break;
+		}
+
+		xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
+		if ((reg & PPC_BIT(5))) {
+			PHBERR(p, "CAPP: recovery failed (%016llx)\n", reg);
+			ret = OPAL_HARDWARE;
+			break;
+		} else if ((reg & PPC_BIT(0)) && (!(reg & PPC_BIT(1)))) {
+			PHBDBG(p, "CAPP: recovery in progress\n");
+			ret = OPAL_BUSY;
+			break;
+		}
+
+		/*
+		 * Mark the CAPP attached to the PHB right away so that
+		 * if a MCE happens during CAPP init we can handle it.
+		 * In case of an error in CAPP init we remove the PHB
+		 * from the attached_mask later.
+		 */
+		capp->phb = phb;
+		capp->attached_pe = pe_number;
+
+		if (mode == OPAL_PHB_CAPI_MODE_DMA_TVT1)
+			ret = enable_capi_mode(p, pe_number,
+					       CAPP_MIN_STQ_ENGINES |
+					       CAPP_MAX_DMA_READ_ENGINES);
+
+		else
+			ret = enable_capi_mode(p, pe_number,
+					       CAPP_MAX_STQ_ENGINES |
+					       CAPP_MIN_DMA_READ_ENGINES);
+		if (ret == OPAL_SUCCESS) {
+			/* Disable fast reboot for CAPP */
+			disable_fast_reboot("CAPP being enabled");
+		} else {
+			/* In case of an error mark the PHB detached */
+			capp->phb = NULL;
+			capp->attached_pe = phb4_get_reserved_pe_number(phb);
+		}
 		break;
 
-	case OPAL_PHB_CAPI_MODE_PCIE: /* shouldn't be called on p9*/
-	case OPAL_PHB_CAPI_MODE_DMA: /* Enabled by default on p9 */
-	case OPAL_PHB_CAPI_MODE_SNOOP_OFF: /* shouldn't be called on p9*/
 	default:
 		ret = OPAL_UNSUPPORTED;
-	}
+		break;
+	};
 
 	return ret;
 }