* [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge
@ 2016-04-07 6:28 Russell Currey
2016-04-07 6:28 ` [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens Russell Currey
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Russell Currey @ 2016-04-07 6:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Russell Currey
In the "ibm,configure-pe" and "ibm,configure-bridge" RTAS calls, the
spec states that values of 9900-9905 can be returned, indicating that
software should delay for 10^x (where x is the last digit, i.e. 990x)
milliseconds and attempt the call again. Currently, the kernel doesn't
know about this, and respecting it fixes some PCI failures when the
hypervisor is busy.
The delay is capped at 0.2 seconds.
Cc: <stable@vger.kernel.org> # 3.10+
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
V3 changelog:
- Refactorings and rewordings thanks to Gavin
- Treat return values >9902 as 9902 thanks to Tyrel
---
arch/powerpc/platforms/pseries/eeh_pseries.c | 51 ++++++++++++++++++++--------
1 file changed, 36 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index ac3ffd9..405baaf 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -615,29 +615,50 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
{
int config_addr;
int ret;
+ /* Waiting 0.2s maximum before skipping configuration */
+ int max_wait = 200;
/* Figure out the PE address */
config_addr = pe->config_addr;
if (pe->addr)
config_addr = pe->addr;
- /* Use new configure-pe function, if supported */
- if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) {
- ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
- config_addr, BUID_HI(pe->phb->buid),
- BUID_LO(pe->phb->buid));
- } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) {
- ret = rtas_call(ibm_configure_bridge, 3, 1, NULL,
- config_addr, BUID_HI(pe->phb->buid),
- BUID_LO(pe->phb->buid));
- } else {
- return -EFAULT;
- }
+ while (max_wait > 0) {
+ /* Use new configure-pe function, if supported */
+ if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) {
+ ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
+ config_addr, BUID_HI(pe->phb->buid),
+ BUID_LO(pe->phb->buid));
+ } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) {
+ ret = rtas_call(ibm_configure_bridge, 3, 1, NULL,
+ config_addr, BUID_HI(pe->phb->buid),
+ BUID_LO(pe->phb->buid));
+ } else {
+ return -EFAULT;
+ }
- if (ret)
- pr_warn("%s: Unable to configure bridge PHB#%d-PE#%x (%d)\n",
- __func__, pe->phb->global_number, pe->addr, ret);
+ if (!ret)
+ return ret;
+
+ /*
+ * If RTAS returns a delay value that's above 100ms, cut it
+ * down to 100ms in case firmware made a mistake. For more
+ * on how these delay values work see rtas_busy_delay_time
+ */
+ if (ret > RTAS_EXTENDED_DELAY_MIN+2 &&
+ ret <= RTAS_EXTENDED_DELAY_MAX)
+ ret = RTAS_EXTENDED_DELAY_MIN+2;
+
+ max_wait -= rtas_busy_delay_time(ret);
+
+ if (max_wait < 0)
+ break;
+
+ rtas_busy_delay(ret);
+ }
+ pr_warn("%s: Unable to configure bridge PHB#%d-PE#%x (%d)\n",
+ __func__, pe->phb->global_number, pe->addr, ret);
return ret;
}
--
2.8.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens
2016-04-07 6:28 [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge Russell Currey
@ 2016-04-07 6:28 ` Russell Currey
2016-04-07 23:43 ` Gavin Shan
2016-05-31 10:17 ` [V3,2/2] " Michael Ellerman
2016-04-07 23:45 ` [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge Gavin Shan
2016-05-31 10:17 ` [V3, " Michael Ellerman
2 siblings, 2 replies; 6+ messages in thread
From: Russell Currey @ 2016-04-07 6:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Russell Currey
The RTAS calls "ibm,configure-pe" and "ibm,configure-bridge" perform the
same actions, however the former can skip configuration if unnecessary.
The existing code treats them as different tokens even though only one
will ever be called. Refactor this by making a single token that is
assigned during init.
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
V3: Reorder commits so the previous patch doesn't depend on this
I had a look at doing the same with some other duplicated tokens but
they had slight differences in semantics so it wasn't helping clarity.
---
arch/powerpc/platforms/pseries/eeh_pseries.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 405baaf..3998e0f 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -53,7 +53,6 @@ static int ibm_read_slot_reset_state2;
static int ibm_slot_error_detail;
static int ibm_get_config_addr_info;
static int ibm_get_config_addr_info2;
-static int ibm_configure_bridge;
static int ibm_configure_pe;
/*
@@ -81,7 +80,14 @@ static int pseries_eeh_init(void)
ibm_get_config_addr_info2 = rtas_token("ibm,get-config-addr-info2");
ibm_get_config_addr_info = rtas_token("ibm,get-config-addr-info");
ibm_configure_pe = rtas_token("ibm,configure-pe");
- ibm_configure_bridge = rtas_token("ibm,configure-bridge");
+
+ /*
+ * ibm,configure-pe and ibm,configure-bridge have the same semantics,
+ * however ibm,configure-pe can be faster. If we can't find
+ * ibm,configure-pe then fall back to using ibm,configure-bridge.
+ */
+ if (ibm_configure_pe == RTAS_UNKNOWN_SERVICE)
+ ibm_configure_pe = rtas_token("ibm,configure-bridge");
/*
* Necessary sanity check. We needn't check "get-config-addr-info"
@@ -93,8 +99,7 @@ static int pseries_eeh_init(void)
(ibm_read_slot_reset_state2 == RTAS_UNKNOWN_SERVICE &&
ibm_read_slot_reset_state == RTAS_UNKNOWN_SERVICE) ||
ibm_slot_error_detail == RTAS_UNKNOWN_SERVICE ||
- (ibm_configure_pe == RTAS_UNKNOWN_SERVICE &&
- ibm_configure_bridge == RTAS_UNKNOWN_SERVICE)) {
+ ibm_configure_pe == RTAS_UNKNOWN_SERVICE) {
pr_info("EEH functionality not supported\n");
return -EINVAL;
}
@@ -624,18 +629,9 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
config_addr = pe->addr;
while (max_wait > 0) {
- /* Use new configure-pe function, if supported */
- if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) {
- ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
- config_addr, BUID_HI(pe->phb->buid),
- BUID_LO(pe->phb->buid));
- } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) {
- ret = rtas_call(ibm_configure_bridge, 3, 1, NULL,
- config_addr, BUID_HI(pe->phb->buid),
- BUID_LO(pe->phb->buid));
- } else {
- return -EFAULT;
- }
+ ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
+ config_addr, BUID_HI(pe->phb->buid),
+ BUID_LO(pe->phb->buid));
if (!ret)
return ret;
--
2.8.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens
2016-04-07 6:28 ` [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens Russell Currey
@ 2016-04-07 23:43 ` Gavin Shan
2016-05-31 10:17 ` [V3,2/2] " Michael Ellerman
1 sibling, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2016-04-07 23:43 UTC (permalink / raw)
To: Russell Currey; +Cc: linuxppc-dev
On Thu, Apr 07, 2016 at 04:28:27PM +1000, Russell Currey wrote:
>The RTAS calls "ibm,configure-pe" and "ibm,configure-bridge" perform the
>same actions, however the former can skip configuration if unnecessary.
>The existing code treats them as different tokens even though only one
>will ever be called. Refactor this by making a single token that is
>assigned during init.
>
>Signed-off-by: Russell Currey <ruscur@russell.cc>
Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>---
>V3: Reorder commits so the previous patch doesn't depend on this
>
>I had a look at doing the same with some other duplicated tokens but
>they had slight differences in semantics so it wasn't helping clarity.
>---
> arch/powerpc/platforms/pseries/eeh_pseries.c | 28 ++++++++++++----------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
>diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
>index 405baaf..3998e0f 100644
>--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
>+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
>@@ -53,7 +53,6 @@ static int ibm_read_slot_reset_state2;
> static int ibm_slot_error_detail;
> static int ibm_get_config_addr_info;
> static int ibm_get_config_addr_info2;
>-static int ibm_configure_bridge;
> static int ibm_configure_pe;
>
> /*
>@@ -81,7 +80,14 @@ static int pseries_eeh_init(void)
> ibm_get_config_addr_info2 = rtas_token("ibm,get-config-addr-info2");
> ibm_get_config_addr_info = rtas_token("ibm,get-config-addr-info");
> ibm_configure_pe = rtas_token("ibm,configure-pe");
>- ibm_configure_bridge = rtas_token("ibm,configure-bridge");
>+
>+ /*
>+ * ibm,configure-pe and ibm,configure-bridge have the same semantics,
>+ * however ibm,configure-pe can be faster. If we can't find
>+ * ibm,configure-pe then fall back to using ibm,configure-bridge.
>+ */
>+ if (ibm_configure_pe == RTAS_UNKNOWN_SERVICE)
>+ ibm_configure_pe = rtas_token("ibm,configure-bridge");
>
> /*
> * Necessary sanity check. We needn't check "get-config-addr-info"
>@@ -93,8 +99,7 @@ static int pseries_eeh_init(void)
> (ibm_read_slot_reset_state2 == RTAS_UNKNOWN_SERVICE &&
> ibm_read_slot_reset_state == RTAS_UNKNOWN_SERVICE) ||
> ibm_slot_error_detail == RTAS_UNKNOWN_SERVICE ||
>- (ibm_configure_pe == RTAS_UNKNOWN_SERVICE &&
>- ibm_configure_bridge == RTAS_UNKNOWN_SERVICE)) {
>+ ibm_configure_pe == RTAS_UNKNOWN_SERVICE) {
> pr_info("EEH functionality not supported\n");
> return -EINVAL;
> }
>@@ -624,18 +629,9 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
> config_addr = pe->addr;
>
> while (max_wait > 0) {
>- /* Use new configure-pe function, if supported */
>- if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) {
>- ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
>- config_addr, BUID_HI(pe->phb->buid),
>- BUID_LO(pe->phb->buid));
>- } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) {
>- ret = rtas_call(ibm_configure_bridge, 3, 1, NULL,
>- config_addr, BUID_HI(pe->phb->buid),
>- BUID_LO(pe->phb->buid));
>- } else {
>- return -EFAULT;
>- }
>+ ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
>+ config_addr, BUID_HI(pe->phb->buid),
>+ BUID_LO(pe->phb->buid));
>
> if (!ret)
> return ret;
>--
>2.8.0
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge
2016-04-07 6:28 [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge Russell Currey
2016-04-07 6:28 ` [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens Russell Currey
@ 2016-04-07 23:45 ` Gavin Shan
2016-05-31 10:17 ` [V3, " Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Gavin Shan @ 2016-04-07 23:45 UTC (permalink / raw)
To: Russell Currey; +Cc: linuxppc-dev
On Thu, Apr 07, 2016 at 04:28:26PM +1000, Russell Currey wrote:
>In the "ibm,configure-pe" and "ibm,configure-bridge" RTAS calls, the
>spec states that values of 9900-9905 can be returned, indicating that
>software should delay for 10^x (where x is the last digit, i.e. 990x)
>milliseconds and attempt the call again. Currently, the kernel doesn't
>know about this, and respecting it fixes some PCI failures when the
>hypervisor is busy.
>
>The delay is capped at 0.2 seconds.
>
>Cc: <stable@vger.kernel.org> # 3.10+
>Signed-off-by: Russell Currey <ruscur@russell.cc>
Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>---
>V3 changelog:
> - Refactorings and rewordings thanks to Gavin
> - Treat return values >9902 as 9902 thanks to Tyrel
>---
> arch/powerpc/platforms/pseries/eeh_pseries.c | 51 ++++++++++++++++++++--------
> 1 file changed, 36 insertions(+), 15 deletions(-)
>
>diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
>index ac3ffd9..405baaf 100644
>--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
>+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
>@@ -615,29 +615,50 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
> {
> int config_addr;
> int ret;
>+ /* Waiting 0.2s maximum before skipping configuration */
>+ int max_wait = 200;
>
> /* Figure out the PE address */
> config_addr = pe->config_addr;
> if (pe->addr)
> config_addr = pe->addr;
>
>- /* Use new configure-pe function, if supported */
>- if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) {
>- ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
>- config_addr, BUID_HI(pe->phb->buid),
>- BUID_LO(pe->phb->buid));
>- } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) {
>- ret = rtas_call(ibm_configure_bridge, 3, 1, NULL,
>- config_addr, BUID_HI(pe->phb->buid),
>- BUID_LO(pe->phb->buid));
>- } else {
>- return -EFAULT;
>- }
>+ while (max_wait > 0) {
>+ /* Use new configure-pe function, if supported */
>+ if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) {
>+ ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
>+ config_addr, BUID_HI(pe->phb->buid),
>+ BUID_LO(pe->phb->buid));
>+ } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) {
>+ ret = rtas_call(ibm_configure_bridge, 3, 1, NULL,
>+ config_addr, BUID_HI(pe->phb->buid),
>+ BUID_LO(pe->phb->buid));
>+ } else {
>+ return -EFAULT;
>+ }
>
>- if (ret)
>- pr_warn("%s: Unable to configure bridge PHB#%d-PE#%x (%d)\n",
>- __func__, pe->phb->global_number, pe->addr, ret);
>+ if (!ret)
>+ return ret;
>+
>+ /*
>+ * If RTAS returns a delay value that's above 100ms, cut it
>+ * down to 100ms in case firmware made a mistake. For more
>+ * on how these delay values work see rtas_busy_delay_time
>+ */
>+ if (ret > RTAS_EXTENDED_DELAY_MIN+2 &&
>+ ret <= RTAS_EXTENDED_DELAY_MAX)
>+ ret = RTAS_EXTENDED_DELAY_MIN+2;
>+
>+ max_wait -= rtas_busy_delay_time(ret);
>+
>+ if (max_wait < 0)
>+ break;
>+
>+ rtas_busy_delay(ret);
>+ }
>
>+ pr_warn("%s: Unable to configure bridge PHB#%d-PE#%x (%d)\n",
>+ __func__, pe->phb->global_number, pe->addr, ret);
> return ret;
> }
>
>--
>2.8.0
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [V3, 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge
2016-04-07 6:28 [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge Russell Currey
2016-04-07 6:28 ` [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens Russell Currey
2016-04-07 23:45 ` [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge Gavin Shan
@ 2016-05-31 10:17 ` Michael Ellerman
2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-05-31 10:17 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev
On Thu, 2016-07-04 at 06:28:26 UTC, Russell Currey wrote:
> In the "ibm,configure-pe" and "ibm,configure-bridge" RTAS calls, the
> spec states that values of 9900-9905 can be returned, indicating that
> software should delay for 10^x (where x is the last digit, i.e. 990x)
> milliseconds and attempt the call again. Currently, the kernel doesn't
> know about this, and respecting it fixes some PCI failures when the
> hypervisor is busy.
>
> The delay is capped at 0.2 seconds.
>
> Cc: <stable@vger.kernel.org> # 3.10+
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/871e178e0f2c4fa788f694721a
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [V3,2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens
2016-04-07 6:28 ` [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens Russell Currey
2016-04-07 23:43 ` Gavin Shan
@ 2016-05-31 10:17 ` Michael Ellerman
1 sibling, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-05-31 10:17 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev
On Thu, 2016-07-04 at 06:28:27 UTC, Russell Currey wrote:
> The RTAS calls "ibm,configure-pe" and "ibm,configure-bridge" perform the
> same actions, however the former can skip configuration if unnecessary.
> The existing code treats them as different tokens even though only one
> will ever be called. Refactor this by making a single token that is
> assigned during init.
>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/bd000b82e86503d5e8b9e6d40a
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-05-31 10:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-07 6:28 [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge Russell Currey
2016-04-07 6:28 ` [PATCH V3 2/2] pseries/eeh: Refactor the configure_bridge RTAS tokens Russell Currey
2016-04-07 23:43 ` Gavin Shan
2016-05-31 10:17 ` [V3,2/2] " Michael Ellerman
2016-04-07 23:45 ` [PATCH V3 1/2] pseries/eeh: Handle RTAS delay requests in configure_bridge Gavin Shan
2016-05-31 10:17 ` [V3, " Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).