diff mbox series

Revert "FSP: Disable PSI link whenever FSP tells OPAL about impending R/R"

Message ID 20200312124145.21279-1-hegdevasant@linux.vnet.ibm.com
State Accepted
Headers show
Series Revert "FSP: Disable PSI link whenever FSP tells OPAL about impending R/R" | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (2700092e09adc8f3c2d578878c1a98cb44b9863d)
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present

Commit Message

Vasant Hegde March 12, 2020, 12:41 p.m. UTC
This reverts commit a4788a49f004a91bb8ca015336abf9ae119fbc52.

Above patch was added to handle host power down with FSP in R/R state.
But FSP is not liking OPAL giving up PSI link early in R/R process. For
FSP initiated R/R OPAL should wait until we get PSI interrupt. Hence
reverting above commit.

Also partially reverting commit e04a34af to make fsp_dpo_pending as
global variable.

We have made several improvement in the way we handle FSP communication
and also in power down path. Now if host sends powerdown message when
FSP in RR, OPAL return OPAL_BUSY_EVENT. Kernel will run poller() and
retry power down message after sometime. So I think this patch will not
have any side effect on power down path.

Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
Oliver,
  I have tested this patch with FSP RR followed by host power down
  scenario. It worked fine. Once FSP booted it detected power down
  command and moved host to standby state.

-Vasant

 hw/fsp/fsp-dpo.c |  2 +-
 hw/fsp/fsp.c     | 25 +++++++++++++++++--------
 include/fsp.h    |  1 +
 3 files changed, 19 insertions(+), 9 deletions(-)

Comments

Ananth N Mavinakayanahalli March 13, 2020, 8:41 a.m. UTC | #1
On 3/12/20 6:11 PM, Vasant Hegde wrote:
> This reverts commit a4788a49f004a91bb8ca015336abf9ae119fbc52.
> 
> Above patch was added to handle host power down with FSP in R/R state.
> But FSP is not liking OPAL giving up PSI link early in R/R process. For
> FSP initiated R/R OPAL should wait until we get PSI interrupt. Hence
> reverting above commit.
> 
> Also partially reverting commit e04a34af to make fsp_dpo_pending as
> global variable.
> 
> We have made several improvement in the way we handle FSP communication
> and also in power down path. Now if host sends powerdown message when
> FSP in RR, OPAL return OPAL_BUSY_EVENT. Kernel will run poller() and
> retry power down message after sometime. So I think this patch will not
> have any side effect on power down path.
> 
> Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> ---
> Oliver,
>    I have tested this patch with FSP RR followed by host power down
>    scenario. It worked fine. Once FSP booted it detected power down
>    command and moved host to standby state.
> 
> -Vasant
> 
>   hw/fsp/fsp-dpo.c |  2 +-
>   hw/fsp/fsp.c     | 25 +++++++++++++++++--------
>   include/fsp.h    |  1 +
>   3 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/fsp/fsp-dpo.c b/hw/fsp/fsp-dpo.c
> index da83a93d8..8f0861ed4 100644
> --- a/hw/fsp/fsp-dpo.c
> +++ b/hw/fsp/fsp-dpo.c
> @@ -18,7 +18,7 @@
>   #define DPO_CMD_SGN_BYTE1	0x20 /* Byte[1] signature */
>   #define DPO_TIMEOUT		2700 /* 45 minutes in seconds */
> 
> -static bool fsp_dpo_pending;
> +bool fsp_dpo_pending;
>   static unsigned long fsp_dpo_init_tb;
> 
>   /*
> diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c
> index 392cf5d36..117810bf8 100644
> --- a/hw/fsp/fsp.c
> +++ b/hw/fsp/fsp.c
> @@ -726,14 +726,23 @@ static void fsp_handle_errors(struct fsp *fsp)
>   		if (fsp->state == fsp_mbx_rr)
>   			return;
> 
> -		/*
> -		 * Its possible that the host has gone down, and OPAL is on
> -		 * its way down and hence will not see the subsequent PSI
> -		 * interrupt. So, just give up the link here.
> -		 */
> -		psi_disable_link(psi);
> -		prlog(PR_NOTICE, "FSP #%d: FSP in reset."
> -		      " Giving up PSI link\n", fsp->index);
> +		if (fsp_dpo_pending) {
> +			/*
> +			 * If we are about to process a reset when DPO
> +			 * is pending, its possible that the host has
> +			 * gone down, and OPAL is on its way down and
> +			 * hence will not see the subsequent PSI interrupt.
> +			 * So, just give up the link here.
> +			 */
> +			prlog(PR_NOTICE, "FSP #%d: FSP reset with DPO pending."
> +					" Giving up PSI link\n",
> +					fsp->index);
> +			psi_disable_link(psi);
> +		} else {
> +			prlog(PR_NOTICE, "FSP #%d: FSP in Reset."
> +				" Waiting for PSI interrupt\n",
> +				fsp->index);
> +		}
>   		fsp_start_rr(fsp);
>   	}
> 
> diff --git a/include/fsp.h b/include/fsp.h
> index ec079a274..472ce62ff 100644
> --- a/include/fsp.h
> +++ b/include/fsp.h
> @@ -842,6 +842,7 @@ extern void fsp_epow_init(void);
> 
>   /* DPO */
>   extern void fsp_dpo_init(void);
> +extern bool fsp_dpo_pending;
> 
>   /* Chiptod */
>   extern void fsp_chiptod_init(void);
> 

Acked-by: Ananth N Mavinakayanahalli <ananth@linux.ibm.com>
Oliver O'Halloran April 1, 2020, 4:33 a.m. UTC | #2
On Thu, Mar 12, 2020 at 11:44 PM Vasant Hegde
<hegdevasant@linux.vnet.ibm.com> wrote:
>
> This reverts commit a4788a49f004a91bb8ca015336abf9ae119fbc52.
>
> Above patch was added to handle host power down with FSP in R/R state.
> But FSP is not liking OPAL giving up PSI link early in R/R process. For
> FSP initiated R/R OPAL should wait until we get PSI interrupt. Hence
> reverting above commit.
>
> Also partially reverting commit e04a34af to make fsp_dpo_pending as
> global variable.
>
> We have made several improvement in the way we handle FSP communication
> and also in power down path. Now if host sends powerdown message when
> FSP in RR, OPAL return OPAL_BUSY_EVENT. Kernel will run poller() and
> retry power down message after sometime. So I think this patch will not
> have any side effect on power down path.
>
> Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> ---

Thanks, Merged as of 3a4daeca7643
diff mbox series

Patch

diff --git a/hw/fsp/fsp-dpo.c b/hw/fsp/fsp-dpo.c
index da83a93d8..8f0861ed4 100644
--- a/hw/fsp/fsp-dpo.c
+++ b/hw/fsp/fsp-dpo.c
@@ -18,7 +18,7 @@ 
 #define DPO_CMD_SGN_BYTE1	0x20 /* Byte[1] signature */
 #define DPO_TIMEOUT		2700 /* 45 minutes in seconds */
 
-static bool fsp_dpo_pending;
+bool fsp_dpo_pending;
 static unsigned long fsp_dpo_init_tb;
 
 /*
diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c
index 392cf5d36..117810bf8 100644
--- a/hw/fsp/fsp.c
+++ b/hw/fsp/fsp.c
@@ -726,14 +726,23 @@  static void fsp_handle_errors(struct fsp *fsp)
 		if (fsp->state == fsp_mbx_rr)
 			return;
 
-		/*
-		 * Its possible that the host has gone down, and OPAL is on
-		 * its way down and hence will not see the subsequent PSI
-		 * interrupt. So, just give up the link here.
-		 */
-		psi_disable_link(psi);
-		prlog(PR_NOTICE, "FSP #%d: FSP in reset."
-		      " Giving up PSI link\n", fsp->index);
+		if (fsp_dpo_pending) {
+			/*
+			 * If we are about to process a reset when DPO
+			 * is pending, its possible that the host has
+			 * gone down, and OPAL is on its way down and
+			 * hence will not see the subsequent PSI interrupt.
+			 * So, just give up the link here.
+			 */
+			prlog(PR_NOTICE, "FSP #%d: FSP reset with DPO pending."
+					" Giving up PSI link\n",
+					fsp->index);
+			psi_disable_link(psi);
+		} else {
+			prlog(PR_NOTICE, "FSP #%d: FSP in Reset."
+				" Waiting for PSI interrupt\n",
+				fsp->index);
+		}
 		fsp_start_rr(fsp);
 	}
 
diff --git a/include/fsp.h b/include/fsp.h
index ec079a274..472ce62ff 100644
--- a/include/fsp.h
+++ b/include/fsp.h
@@ -842,6 +842,7 @@  extern void fsp_epow_init(void);
 
 /* DPO */
 extern void fsp_dpo_init(void);
+extern bool fsp_dpo_pending;
 
 /* Chiptod */
 extern void fsp_chiptod_init(void);