first step of standardising OPAL_BUSY handling

PATCH In review Nicholas Piggin 3 participants 15 messages Apr 9, 2018 – Apr 10, 2018

Subsystems (2)

PowerPC Real time clock (RTC)

lore

Share & tools

lore

Nicholas Piggin

author reviewer

2018-04-09 05:24 UTC #0 lore raw

v2 cover first step of standardising OPAL_BUSY handling

Since v1:
- Added console and opal-kmsg
- Fix a missing case in opal_shutdown that would delay even if OPAL_SUCCESS was returned.

Thanks,
Nick

Nicholas Piggin (9):
powerpc/powernv: define a standard delay for OPAL_BUSY type retry
loops
powerpc/powernv: OPAL RTC driver standardise OPAL_BUSY loops powerpc/powernv: opal_put_chars partial write fix powerpc/powernv: OPAL console standardise OPAL_BUSY loops powerpc/powernv: OPAL platform standardise OPAL_BUSY loops powerpc/powernv: OPAL NVRAM driver standardise OPAL_BUSY delays powerpc/powernv: OPAL dump support standardise OPAL_BUSY delays powerpc/xive: standardise OPAL_BUSY delays powerpc/powernv: opal-kmsg standardise OPAL_BUSY handling

 arch/powerpc/include/asm/opal.h             |   3 +
 arch/powerpc/platforms/powernv/opal-dump.c  |   6 +-
 arch/powerpc/platforms/powernv/opal-kmsg.c  |  24 ++-
 arch/powerpc/platforms/powernv/opal-nvram.c |   7 +-
 arch/powerpc/platforms/powernv/opal-rtc.c   |   6 +-
 arch/powerpc/platforms/powernv/opal.c       |  46 +++--
 arch/powerpc/platforms/powernv/setup.c      |  16 +-
 arch/powerpc/sysdev/xive/native.c           | 193 +++++++++++---------
 drivers/rtc/rtc-opal.c                      |  33 ++--
 9 files changed, 203 insertions(+), 131 deletions(-)

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #1 lore raw

PATCH v2 1/9 powerpc/powernv: define a standard delay for OPAL_BUSY type retry loops

This is the start of an effort to tidy up and standardise all the delays. Existing loops have a range of delay/sleep periods from 1ms to 20ms, and some have no delay. They all loop forever except rtc, which times out after 10 retries, and that uses 10ms delays. So use 10ms as our standard delay. The OPAL maintainer agrees 10ms is a reasonable starting point.

The idea is to use the same recipe everywhere, once this is proven to work then it will be documented as an OPAL API standard. Then both firmware and OS can agree, and if a particular call needs something else, then that can be documented with reasoning.

This is not the end-all of this effort, it's just a relatively easy change that fixes some existing high latency delays. There should be provision for standardising timeouts and/or interruptible loops where possible, so non-fatal firmware errors don't cause hangs.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/opal.h | 3 +++
 1 file changed, 3 insertions(+)

arch/powerpc/include/asm/opal.h +3 -0

index 7159e1a6a61a..03e1a920491e 100644
@@ -21,6 +21,9 @@
 /* We calculate number of sg entries based on PAGE_SIZE */
 #define SG_ENTRIES_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct opal_sg_entry))
 
+/* Default time to sleep or delay between OPAL_BUSY/OPAL_BUSY_EVENT loops */
+#define OPAL_BUSY_DELAY_MS	10
+
 /* /sys/firmware/opal */
 extern struct kobject *opal_kobj;

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #2 lore raw

PATCH v2 2/9 powerpc/powernv: OPAL RTC driver standardise OPAL_BUSY loops

In reply to Nicholas Piggin

Convert to using the standard delay poll/delay form.

The OPAL RTC driver:

- Did not previously delay or sleep in the OPAL_BUSY_EVENT case. There have been scheduling delays of up to 50 seconds observed here (BMC reboot can do it), which this should fix.

Cc [email protected]

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal-rtc.c |  6 +++--
 drivers/rtc/rtc-opal.c                    | 33 ++++++++++++++---------
 2 files changed, 25 insertions(+), 14 deletions(-)

arch/powerpc/platforms/powernv/opal-rtc.c +4 -2

index f8868864f373..f530cf62594d 100644
@@ -48,10 +48,12 @@ unsigned long __init opal_get_boot_time(void)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			mdelay(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (rc == OPAL_BUSY)
+		} else if (rc == OPAL_BUSY) {
 			mdelay(10);
+		}
 	}
 	if (rc != OPAL_SUCCESS)
 		return 0;

drivers/rtc/rtc-opal.c +21 ~~-12~~

index 304e891e35fc..cddcc4749d39 100644
@@ -57,7 +57,7 @@ static void tm_to_opal(struct rtc_time *tm, u32 *y_m_d, u64 *h_m_s_ms)
 
 static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 {
-	long rc = OPAL_BUSY;
+	s64 rc = OPAL_BUSY;
 	int retries = 10;
 	u32 y_m_d;
 	u64 h_m_s_ms;

@@ -66,13 +66,17 @@ static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (retries-- && (rc == OPAL_HARDWARE
-				       || rc == OPAL_INTERNAL_ERROR))
+		} else if (rc == OPAL_BUSY) {
 			msleep(10);
-		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
-			break;
+		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
+			if (retries--) {
+				msleep(10); /* Wait 10ms before retry */
+				rc = OPAL_BUSY; /* go around again */
+			}
+		}
 	}
 
 	if (rc != OPAL_SUCCESS)

@@ -87,21 +91,26 @@ static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 
 static int opal_set_rtc_time(struct device *dev, struct rtc_time *tm)
 {
-	long rc = OPAL_BUSY;
+	s64 rc = OPAL_BUSY;
 	int retries = 10;
 	u32 y_m_d = 0;
 	u64 h_m_s_ms = 0;
 
 	tm_to_opal(tm, &y_m_d, &h_m_s_ms);
+
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_write(y_m_d, h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (retries-- && (rc == OPAL_HARDWARE
-				       || rc == OPAL_INTERNAL_ERROR))
+		} else if (rc == OPAL_BUSY) {
 			msleep(10);
-		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
-			break;
+		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
+			if (retries--) {
+				msleep(10); /* Wait 10ms before retry */
+				rc = OPAL_BUSY; /* go around again */
+			}
+		}
 	}
 
 	return rc == OPAL_SUCCESS ? 0 : -EIO;

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #3 lore raw

PATCH v2 3/9 powerpc/powernv: opal_put_chars partial write fix

In reply to Nicholas Piggin

The intention here is to consume and discard the remaining buffer upon error. This works if there has not been a previous partial write. If there has been, then total_len is no longer total number of bytes to copy. total_len is always "bytes left to copy", so it should be added to written bytes.

This code may not be exercised any more if partial writes will not be hit, but this is a small bugfix before a larger change.

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/platforms/powernv/opal.c +1 -1

index 516e23de5a3d..87d4c0aa7f64 100644
@@ -388,7 +388,7 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 		/* Closed or other error drop */
 		if (rc != OPAL_SUCCESS && rc != OPAL_BUSY &&
 		    rc != OPAL_BUSY_EVENT) {
-			written = total_len;
+			written += total_len;
 			break;
 		}
 		if (rc == OPAL_SUCCESS) {

Signature · 1 line

2.17.0

Benjamin Herrenschmidt

Reviewed-by on v2 3/9

2018-04-09 05:50 UTC #4 lore raw

Re: [PATCH v2 3/9] powerpc/powernv: opal_put_chars partial write fix

Quoted 9 lines · Nicholas Piggin

The intention here is to consume and discard the remaining buffer upon error. This works if there has not been a previous partial write. If there has been, then total_len is no longer total number of bytes to copy. total_len is always "bytes left to copy", so it should be added to written bytes.
This code may not be exercised any more if partial writes will not be hit, but this is a small bugfix before a larger change.

Reviewed-by Benjamin Herrenschmidt <[email protected]>

Quoted 18 lines

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/platforms/powernv/opal.c +1 -1

index 516e23de5a3d..87d4c0aa7f64 100644
@@ -388,7 +388,7 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 		/* Closed or other error drop */
 		if (rc != OPAL_SUCCESS && rc != OPAL_BUSY &&
 		    rc != OPAL_BUSY_EVENT) {
-			written = total_len;
+			written += total_len;
 			break;
 		}
 		if (rc == OPAL_SUCCESS) {

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #5 lore raw

PATCH v2 4/9 powerpc/powernv: OPAL console standardise OPAL_BUSY loops

In reply to Nicholas Piggin

Convert to using the standard delay poll/delay form.

The console code:

- Did not previously delay or sleep in its busy loop.

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c | 38 ++++++++++++++++-----------
 1 file changed, 23 insertions(+), 15 deletions(-)

arch/powerpc/platforms/powernv/opal.c +23 ~~-15~~

index 87d4c0aa7f64..473c8ce14a34 100644
@@ -378,33 +378,41 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	/* We still try to handle partial completions, though they
 	 * should no longer happen.
 	 */
-	rc = OPAL_BUSY;
-	while(total_len > 0 && (rc == OPAL_BUSY ||
-				rc == OPAL_BUSY_EVENT || rc == OPAL_SUCCESS)) {
+
+	while (total_len > 0) {
 		olen = cpu_to_be64(total_len);
-		rc = opal_console_write(vtermno, &olen, data);
+
+		rc = OPAL_BUSY;
+		while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+			rc = opal_console_write(vtermno, &olen, data);
+			if (rc == OPAL_BUSY_EVENT) {
+				mdelay(OPAL_BUSY_DELAY_MS);
+				opal_poll_events(NULL);
+			} else if (rc == OPAL_BUSY) {
+				mdelay(OPAL_BUSY_DELAY_MS);
+			}
+		}
+
 		len = be64_to_cpu(olen);
 
 		/* Closed or other error drop */
-		if (rc != OPAL_SUCCESS && rc != OPAL_BUSY &&
-		    rc != OPAL_BUSY_EVENT) {
-			written += total_len;
+		if (rc != OPAL_SUCCESS) {
+			written += total_len; /* drop remaining chars */
 			break;
 		}
-		if (rc == OPAL_SUCCESS) {
-			total_len -= len;
-			data += len;
-			written += len;
-		}
+
+		total_len -= len;
+		data += len;
+		written += len;
+
 		/* This is a bit nasty but we need that for the console to
 		 * flush when there aren't any interrupts. We will clean
 		 * things a bit later to limit that to synchronous path
 		 * such as the kernel console and xmon/udbg
 		 */
-		do
+		do {
 			opal_poll_events(&evt);
-		while(rc == OPAL_SUCCESS &&
-			(be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT));
+		} while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT);
 	}
 	spin_unlock_irqrestore(&opal_write_lock, flags);
 	return written;

Signature · 1 line

2.17.0

Benjamin Herrenschmidt

2018-04-09 05:53 UTC #6 lore raw

Re: [PATCH v2 4/9] powerpc/powernv: OPAL console standardise OPAL_BUSY loops

Quoted 8 lines · Nicholas Piggin

Convert to using the standard delay poll/delay form.
The console code:
- Did not previously delay or sleep in its busy loop.
Cc Benjamin Herrenschmidt <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>

Does it help with anything ? We don't technically *have* to delay or wait, I thought it would be good to try to hit the console as fast as possible in that case...

Ben.

Quoted 65 lines

---
 arch/powerpc/platforms/powernv/opal.c | 38 ++++++++++++++++-----------
 1 file changed, 23 insertions(+), 15 deletions(-)

arch/powerpc/platforms/powernv/opal.c +23 ~~-15~~

index 87d4c0aa7f64..473c8ce14a34 100644
@@ -378,33 +378,41 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	/* We still try to handle partial completions, though they
 	 * should no longer happen.
 	 */
-	rc = OPAL_BUSY;
-	while(total_len > 0 && (rc == OPAL_BUSY ||
-				rc == OPAL_BUSY_EVENT || rc == OPAL_SUCCESS)) {
+
+	while (total_len > 0) {
 		olen = cpu_to_be64(total_len);
-		rc = opal_console_write(vtermno, &olen, data);
+
+		rc = OPAL_BUSY;
+		while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+			rc = opal_console_write(vtermno, &olen, data);
+			if (rc == OPAL_BUSY_EVENT) {
+				mdelay(OPAL_BUSY_DELAY_MS);
+				opal_poll_events(NULL);
+			} else if (rc == OPAL_BUSY) {
+				mdelay(OPAL_BUSY_DELAY_MS);
+			}
+		}
+
 		len = be64_to_cpu(olen);
 
 		/* Closed or other error drop */
-		if (rc != OPAL_SUCCESS && rc != OPAL_BUSY &&
-		    rc != OPAL_BUSY_EVENT) {
-			written += total_len;
+		if (rc != OPAL_SUCCESS) {
+			written += total_len; /* drop remaining chars */
 			break;
 		}
-		if (rc == OPAL_SUCCESS) {
-			total_len -= len;
-			data += len;
-			written += len;
-		}
+
+		total_len -= len;
+		data += len;
+		written += len;
+
 		/* This is a bit nasty but we need that for the console to
 		 * flush when there aren't any interrupts. We will clean
 		 * things a bit later to limit that to synchronous path
 		 * such as the kernel console and xmon/udbg
 		 */
-		do
+		do {
 			opal_poll_events(&evt);
-		while(rc == OPAL_SUCCESS &&
-			(be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT));
+		} while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT);
 	}
 	spin_unlock_irqrestore(&opal_write_lock, flags);
 	return written;

Nicholas Piggin

author reviewer

2018-04-09 06:13 UTC #7 lore raw

Re: [PATCH v2 4/9] powerpc/powernv: OPAL console standardise OPAL_BUSY loops

Quoted 5 lines · On Mon, 09 Apr 2018 15:53:33 +1000 Benjamin Herrenschmidt…

Quoted 4 lines · Nicholas Piggin
Convert to using the standard delay poll/delay form.
The console code:

Quoted 8 lines

Quoted 4 lines
- Did not previously delay or sleep in its busy loop.
Cc Benjamin Herrenschmidt <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>
Does it help with anything ? We don't technically *have* to delay or wait, I thought it would be good to try to hit the console as fast as possible in that case...

We can always make exceptions to the standard form, but in those cases I would like to document it in the OPAL API and comment for the Linux side.

My thinking in this case is that it reduces time in firmware and in particular holding console locks. Is it likely / possible that we don't have enough buffering or some other issue makes it worth retrying so quickly?

Thanks,
Nick

Benjamin Herrenschmidt

2018-04-09 08:22 UTC #8 lore raw

Re: [PATCH v2 4/9] powerpc/powernv: OPAL console standardise OPAL_BUSY loops

Quoted 8 lines · Nicholas Piggin

We can always make exceptions to the standard form, but in those cases I would like to document it in the OPAL API and comment for the Linux side.
My thinking in this case is that it reduces time in firmware and in particular holding console locks. Is it likely / possible that we don't have enough buffering or some other issue makes it worth retrying so quickly?

Not sure to be honest, but yeah limiting the lock contention inside OPAL is probably not a bad idea.

Cheers,
Ben.

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #9 lore raw

PATCH v2 5/9 powerpc/powernv: OPAL platform standardise OPAL_BUSY loops

In reply to Nicholas Piggin

Convert to using the standard delay poll/delay form.

The platform code:

- Used delay when called from a schedule()able context.
- Did not previously delay or sleep in the OPAL_BUSY_EVENT case.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c  |  8 +++++---
 arch/powerpc/platforms/powernv/setup.c | 16 ++++++++++------
 2 files changed, 15 insertions(+), 9 deletions(-)

arch/powerpc/platforms/powernv/opal.c +5 -3

index 473c8ce14a34..0f03199a8664 100644
@@ -907,10 +907,12 @@ void opal_shutdown(void)
 	 */
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_sync_host_reboot();
-		if (rc == OPAL_BUSY)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else
-			mdelay(10);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 
 	/* Unregister memory dump region */

arch/powerpc/platforms/powernv/setup.c +10 -6

index ef8c9ce53a61..ddaea5ba9008 100644
@@ -217,10 +217,12 @@ static void  __noreturn pnv_restart(char *cmd)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_cec_reboot();
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else
-			mdelay(10);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	for (;;)
 		opal_poll_events(NULL);

@@ -234,10 +236,12 @@ static void __noreturn pnv_power_off(void)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_cec_power_down(0);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else
-			mdelay(10);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	for (;;)
 		opal_poll_events(NULL);

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #10 lore raw

PATCH v2 6/9 powerpc/powernv: OPAL NVRAM driver standardise OPAL_BUSY delays

In reply to Nicholas Piggin

Convert to using the standard delay poll/delay form.

The NVRAM driver:

- Did not previously delay or sleep in its busy loop.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal-nvram.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

arch/powerpc/platforms/powernv/opal-nvram.c +6 -1

index ba2ff06a2c98..1bceb95f422d 100644
@@ -11,6 +11,7 @@
 
 #define DEBUG
 
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/of.h>

@@ -56,8 +57,12 @@ static ssize_t opal_nvram_write(char *buf, size_t count, loff_t *index)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_write_nvram(__pa(buf), count, off);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 
 	if (rc)

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #11 lore raw

PATCH v2 7/9 powerpc/powernv: OPAL dump support standardise OPAL_BUSY delays

In reply to Nicholas Piggin

Convert to using the standard delay poll/delay form.

The dump code:

- Did not previously delay or sleep in the OPAL_BUSY case.
- Used a 20ms sleep.

Cc Vasant Hegde <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal-dump.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

arch/powerpc/platforms/powernv/opal-dump.c +4 -2

index 0dc8fa4e0af2..4ab749ceafc2 100644
@@ -260,12 +260,14 @@ static int64_t dump_read_data(struct dump_obj *dump)
 	addr = __pa(list);
 
 	/* Fetch data */
-	rc = OPAL_BUSY_EVENT;
+	rc = OPAL_BUSY;
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_dump_read(dump->id, addr);
 		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-			msleep(20);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
 		}
 	}

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #12 lore raw

PATCH v2 8/9 powerpc/xive: standardise OPAL_BUSY delays

In reply to Nicholas Piggin

Convert to using the standard delay poll/delay form.

The XIVE driver:

- Did not previously loop on the OPAL_BUSY_EVENT case.
- Used a 1ms sleep.

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/sysdev/xive/native.c | 193 +++++++++++++++++-------------
 1 file changed, 111 insertions(+), 82 deletions(-)

arch/powerpc/sysdev/xive/native.c +111 ~~-82~~

index d22aeb0b69e1..682f79dabb4a 100644
@@ -103,14 +103,18 @@ EXPORT_SYMBOL_GPL(xive_native_populate_irq_data);
 
 int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_irq_config(hw_irq, target, prio, sw_irq);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	return rc == 0 ? 0 : -ENXIO;
 }
 EXPORT_SYMBOL_GPL(xive_native_configure_irq);

@@ -159,12 +163,17 @@ int xive_native_configure_queue(u32 vp_id, struct xive_q *q, u8 prio,
 	}
 
 	/* Configure and enable the queue in HW */
-	for (;;) {
+	rc = OPAL_BUSY;
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_queue_info(vp_id, prio, qpage_phys, order, flags);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	if (rc) {
 		pr_err("Error %lld setting queue for prio %d\n", rc, prio);
 		rc = -EIO;

@@ -183,14 +192,17 @@ EXPORT_SYMBOL_GPL(xive_native_configure_queue);
 
 static void __xive_native_disable_queue(u32 vp_id, struct xive_q *q, u8 prio)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
 	/* Disable the queue in HW */
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_queue_info(vp_id, prio, 0, 0, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	if (rc)
 		pr_err("Error %lld disabling queue for prio %d\n", rc, prio);

@@ -240,7 +252,7 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
 {
 	struct device_node *np;
 	unsigned int chip_id;
-	s64 irq;
+	s64 rc = OPAL_BUSY;
 
 	/* Find the chip ID */
 	np = of_get_cpu_node(cpu, NULL);

@@ -250,33 +262,39 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
 	}
 
 	/* Allocate an IPI and populate info about it */
-	for (;;) {
-		irq = opal_xive_allocate_irq(chip_id);
-		if (irq == OPAL_BUSY) {
-			msleep(1);
-			continue;
-		}
-		if (irq < 0) {
-			pr_err("Failed to allocate IPI on CPU %d\n", cpu);
-			return -ENXIO;
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+		rc = opal_xive_allocate_irq(chip_id);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
 		}
-		xc->hw_ipi = irq;
-		break;
 	}
+	if (rc < 0) {
+		pr_err("Failed to allocate IPI on CPU %d\n", cpu);
+		return -ENXIO;
+	}
+	xc->hw_ipi = rc;
+
 	return 0;
 }
 #endif /* CONFIG_SMP */
 
 u32 xive_native_alloc_irq(void)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_allocate_irq(OPAL_XIVE_ANY_CHIP);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	if (rc < 0)
 		return 0;
 	return rc;

@@ -285,11 +303,16 @@ EXPORT_SYMBOL_GPL(xive_native_alloc_irq);
 
 void xive_native_free_irq(u32 irq)
 {
-	for (;;) {
-		s64 rc = opal_xive_free_irq(irq);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+	s64 rc = OPAL_BUSY;
+
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+		rc = opal_xive_free_irq(irq);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 }
 EXPORT_SYMBOL_GPL(xive_native_free_irq);

@@ -297,20 +320,11 @@ EXPORT_SYMBOL_GPL(xive_native_free_irq);
 #ifdef CONFIG_SMP
 static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
 {
-	s64 rc;
-
 	/* Free the IPI */
 	if (!xc->hw_ipi)
 		return;
-	for (;;) {
-		rc = opal_xive_free_irq(xc->hw_ipi);
-		if (rc == OPAL_BUSY) {
-			msleep(1);
-			continue;
-		}
-		xc->hw_ipi = 0;
-		break;
-	}
+	xive_native_free_irq(xc->hw_ipi);
+	xc->hw_ipi = 0;
 }
 #endif /* CONFIG_SMP */

@@ -381,7 +395,7 @@ static void xive_native_eoi(u32 hw_irq)
 
 static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u32 vp;
 	__be64 vp_cam_be;
 	u64 vp_cam;

@@ -392,12 +406,16 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 	/* Enable the pool VP */
 	vp = xive_pool_vps + cpu;
 	pr_debug("CPU %d setting up pool VP 0x%x\n", cpu, vp);
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp, OPAL_XIVE_VP_ENABLED, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	if (rc) {
 		pr_err("Failed to enable pool VP on CPU %d\n", cpu);
 		return;

@@ -425,7 +443,7 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 
 static void xive_native_teardown_cpu(unsigned int cpu, struct xive_cpu *xc)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u32 vp;
 
 	if (xive_pool_vps == XIVE_INVALID_VP)

@@ -436,11 +454,14 @@ static void xive_native_teardown_cpu(unsigned int cpu, struct xive_cpu *xc)
 
 	/* Disable it */
 	vp = xive_pool_vps + cpu;
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp, 0, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 }

@@ -627,7 +648,7 @@ static bool xive_native_provision_pages(void)
 
 u32 xive_native_alloc_vp_block(u32 max_vcpus)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u32 order;
 
 	order = fls(max_vcpus) - 1;

@@ -637,25 +658,25 @@ u32 xive_native_alloc_vp_block(u32 max_vcpus)
 	pr_debug("VP block alloc, for max VCPUs %d use order %d\n",
 		 max_vcpus, order);
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_alloc_vp_block(order);
-		switch (rc) {
-		case OPAL_BUSY:
-			msleep(1);
-			break;
-		case OPAL_XIVE_PROVISIONING:
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		} else if (rc == OPAL_XIVE_PROVISIONING) {
 			if (!xive_native_provision_pages())
 				return XIVE_INVALID_VP;
-			break;
-		default:
-			if (rc < 0) {
-				pr_err("OPAL failed to allocate VCPUs order %d, err %lld\n",
-				       order, rc);
-				return XIVE_INVALID_VP;
-			}
-			return rc;
+			rc = OPAL_BUSY; /* go around again */
 		}
 	}
+	if (rc < 0) {
+		pr_err("OPAL failed to allocate VCPUs order %d, err %lld\n",
+		       order, rc);
+		return XIVE_INVALID_VP;
+	}
+	return rc;
 }
 EXPORT_SYMBOL_GPL(xive_native_alloc_vp_block);

@@ -674,30 +695,38 @@ EXPORT_SYMBOL_GPL(xive_native_free_vp_block);
 
 int xive_native_enable_vp(u32 vp_id, bool single_escalation)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 	u64 flags = OPAL_XIVE_VP_ENABLED;
 
 	if (single_escalation)
 		flags |= OPAL_XIVE_VP_SINGLE_ESCALATION;
-	for (;;) {
+
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp_id, flags, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
+
 	return rc ? -EIO : 0;
 }
 EXPORT_SYMBOL_GPL(xive_native_enable_vp);
 
 int xive_native_disable_vp(u32 vp_id)
 {
-	s64 rc;
+	s64 rc = OPAL_BUSY;
 
-	for (;;) {
+	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_xive_set_vp_info(vp_id, 0, 0);
-		if (rc != OPAL_BUSY)
-			break;
-		msleep(1);
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	return rc ? -EIO : 0;
 }

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-09 05:25 UTC #13 lore raw

PATCH v2 9/9 powerpc/powernv: opal-kmsg standardise OPAL_BUSY handling

In reply to Nicholas Piggin

OPAL_CONSOLE_FLUSH is documented as being able to return OPAL_BUSY, so implement the standard OPAL_BUSY handling for it.

Cc Russell Currey <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal-kmsg.c | 24 ++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

arch/powerpc/platforms/powernv/opal-kmsg.c +16 -8

index 6f1214d4de92..f8f41ccce75f 100644
@@ -12,6 +12,7 @@
  */
 
 #include <linux/kmsg_dump.h>
+#include <linux/delay.h>
 
 #include <asm/opal.h>
 #include <asm/opal-api.h>

@@ -26,8 +27,7 @@
 static void force_opal_console_flush(struct kmsg_dumper *dumper,
 				     enum kmsg_dump_reason reason)
 {
-	int i;
-	int64_t ret;
+	s64 rc;
 
 	/*
 	 * Outside of a panic context the pollers will continue to run,

@@ -37,14 +37,22 @@ static void force_opal_console_flush(struct kmsg_dumper *dumper,
 		return;
 
 	if (opal_check_token(OPAL_CONSOLE_FLUSH)) {
-		ret = opal_console_flush(0);
+		do  {
+			rc = OPAL_BUSY;
+			while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+				rc = opal_console_flush(0);
+				if (rc == OPAL_BUSY_EVENT) {
+					mdelay(OPAL_BUSY_DELAY_MS);
+					opal_poll_events(NULL);
+				} else if (rc == OPAL_BUSY) {
+					mdelay(OPAL_BUSY_DELAY_MS);
+				}
+			}
+		} while (rc == OPAL_PARTIAL); /* More to flush */
 
-		if (ret == OPAL_UNSUPPORTED || ret == OPAL_PARAMETER)
-			return;
-
-		/* Incrementally flush until there's nothing left */
-		while (opal_console_flush(0) != OPAL_SUCCESS);
 	} else {
+		int i;
+
 		/*
 		 * If OPAL_CONSOLE_FLUSH is not implemented in the firmware,
 		 * the console can still be flushed by calling the polling

Signature · 1 line

2.17.0

Russell Currey

Reviewed-by on v2 9/9

2018-04-10 05:01 UTC #14 lore raw

Re: [PATCH v2 9/9] powerpc/powernv: opal-kmsg standardise OPAL_BUSY handling

Quoted 5 lines · Nicholas Piggin

OPAL_CONSOLE_FLUSH is documented as being able to return OPAL_BUSY, so implement the standard OPAL_BUSY handling for it.
Cc Russell Currey <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>

Reviewed-by Russell Currey <[email protected]>