* [RFC 02/10] powerpc/rtas: do not schedule in rtas_os_term()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
rtas_os_term() is called in the panic path and should immediately
re-call the RTAS ibm,os-term function as long as it returns a busy
status. It's not safe to use rtas_busy_delay() in this context, which
potentially can schedule away. Use rtas_spin_if_busy().
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/kernel/rtas.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 4a1dfbfa51ba..4177f7385ea2 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -960,7 +960,7 @@ void rtas_os_term(char *str)
do {
status = rtas_call(rtas_token("ibm,os-term"), 1, 1, NULL,
__pa(rtas_os_term_buf));
- } while (rtas_busy_delay(status));
+ } while (rtas_spin_if_busy(status));
if (status != 0)
printk(KERN_EMERG "ibm,os-term call failed %d\n", status);
--
2.30.2
^ permalink raw reply related
* [RFC 01/10] powerpc/rtas: new APIs for busy and extended delay statuses
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
Add new APIs for handling busy (-2) and extended delay
hint (9900...9905) statuses from RTAS. These are intended to be
drop-in replacements for existing uses of rtas_busy_delay().
A problem with rtas_busy_delay() and rtas_busy_delay_time() is that
they consider -2/busy to be equivalent to 9900 (wait 1ms). In fact,
the OS should call again as soon as it wants on -2, which at least on
PowerVM means RTAS is returning only to uphold the general requirement
that RTAS must return control to the OS in a "timely fashion" (250us).
Combine this with the fact that msleep(1) actually sleeps for more
like 20ms in practice: on busy VMs we schedule away for much longer
than necessary on -2 and 9900.
This is fixed in rtas_sched_if_busy(), which uses usleep_range() for
small delay hints, and only schedules away on -2 if there is other
work available. It also refuses to sleep longer than one second
regardless of the hinted value, on the assumption that even longer
running operations can tolerate polling at 1HZ.
rtas_spin_if_busy() and rtas_force_spin_if_busy() are provided for
atomic contexts which need to handle busy status and extended delay
hints.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/include/asm/rtas.h | 4 +
arch/powerpc/kernel/rtas.c | 168 ++++++++++++++++++++++++++++++++
2 files changed, 172 insertions(+)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 9dc97d2f9d27..555ff3290f92 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -266,6 +266,10 @@ extern int rtas_set_rtc_time(struct rtc_time *rtc_time);
extern unsigned int rtas_busy_delay_time(int status);
extern unsigned int rtas_busy_delay(int status);
+bool rtas_sched_if_busy(int status);
+bool rtas_spin_if_busy(int status);
+bool rtas_force_spin_if_busy(int status);
+
extern int early_init_dt_scan_rtas(unsigned long node,
const char *uname, int depth, void *data);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 6bada744402b..4a1dfbfa51ba 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -519,6 +519,174 @@ unsigned int rtas_busy_delay(int status)
}
EXPORT_SYMBOL(rtas_busy_delay);
+/**
+ * rtas_force_spin_if_busy() - Consume a busy or extended delay status
+ * in atomic context.
+ * @status: Return value from rtas_call() or similar function.
+ *
+ * Use this function when you cannot avoid using an RTAS function
+ * which may return an extended delay hint in atomic context. If
+ * possible, use rtas_spin_if_busy() or rtas_sched_if_busy() instead
+ * of this function.
+ *
+ * Return: True if @status is -2 or 990x, in which case
+ * rtas_spin_if_busy() will have delayed an appropriate amount
+ * of time, and the caller should call the RTAS function
+ * again. False otherwise.
+ */
+bool rtas_force_spin_if_busy(int status)
+{
+ bool was_busy = true;
+
+ switch (status) {
+ case RTAS_BUSY:
+ /* OK to call again immediately; do nothing. */
+ break;
+ case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
+ mdelay(1);
+ break;
+ default:
+ was_busy = false;
+ break;
+ }
+
+ return was_busy;
+}
+
+/**
+ * rtas_spin_if_busy() - Consume a busy status in atomic context.
+ * @status: Return value from rtas_call() or similar function.
+ *
+ * Prefer rtas_sched_if_busy() over this function. Prefer this
+ * function over rtas_force_spin_if_busy(). Use this function in
+ * atomic contexts with RTAS calls that are specified to return -2 but
+ * not 990x. This function will complain and execute a minimal delay
+ * if passed a 990x status.
+ *
+ * Return: True if @status is -2 or 990x, in which case
+ * rtas_spin_if_busy() will have delayed an appropriate amount
+ * of time, and the caller should call the RTAS function
+ * again. False otherwise.
+ */
+bool rtas_spin_if_busy(int status)
+{
+ bool was_busy = true;
+
+ switch (status) {
+ case RTAS_BUSY:
+ /* OK to call again immediately; do nothing. */
+ break;
+ case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
+ /*
+ * Generally, RTAS functions which can return this
+ * status should be considered too expensive to use in
+ * atomic context. Change the calling code to use
+ * rtas_sched_if_busy(), or if that's not possible,
+ * use rtas_force_spin_if_busy().
+ */
+ pr_warn_once("%pS may use RTAS call in atomic context which returns extended delay.\n",
+ __builtin_return_address(0));
+ mdelay(1);
+ break;
+ default:
+ was_busy = false;
+ break;
+ }
+
+ return was_busy;
+}
+
+static unsigned long extended_delay_ms(unsigned int status)
+{
+ unsigned int extdelay;
+ unsigned int order;
+ unsigned int ms;
+
+ extdelay = clamp((int)status, RTAS_EXTENDED_DELAY_MIN, RTAS_EXTENDED_DELAY_MAX);
+ WARN_ONCE(extdelay != status, "%s passed invalid status %u\n", __func__, status);
+
+ order = status - RTAS_EXTENDED_DELAY_MIN;
+ for (ms = 1; order > 0; order--)
+ ms *= 10;
+
+ return ms;
+}
+
+static void handle_extended_delay(unsigned int status)
+{
+ unsigned long usecs;
+
+ usecs = 1000 * extended_delay_ms(status);
+
+ /*
+ * If we have no other work pending, there's no reason to
+ * sleep.
+ */
+ if (!need_resched())
+ return;
+
+ /*
+ * The extended delay hint can be as high as 100
+ * seconds. Surely any function returning such a status is
+ * either buggy or isn't going to be significantly slowed by
+ * us polling at 1HZ. Clamp the sleep time to one second.
+ */
+ usecs = clamp(usecs, 1000UL, 1000000UL);
+
+ /*
+ * The delay hint is an order-of-magnitude suggestion, not a
+ * minimum. It is fine, possibly even advantageous, for us to
+ * pause for less time than suggested. For small values, use
+ * usleep_range() to ensure we don't sleep much longer than
+ * actually suggested.
+ *
+ * See Documentation/timers/timers-howto.rst for explanation
+ * of the threshold used here.
+ */
+ if (usecs <= 20000)
+ usleep_range(usecs / 2, 2 * usecs);
+ else
+ msleep(DIV_ROUND_UP(usecs, 1000));
+}
+
+/**
+ * rtas_sched_if_busy() - Consume a busy or extended delay status.
+ * @status: Return value from rtas_call() or similar function.
+ *
+ * Prefer this function over rtas_spin_if_busy().
+ *
+ * Context: This function may sleep.
+ *
+ * Return: True if @status is -2 or 990x, in which case
+ * rtas_sched_if_busy() will have slept an appropriate amount
+ * of time, and the caller should call the RTAS function
+ * again. False otherwise.
+ */
+bool rtas_sched_if_busy(int status)
+{
+ bool was_busy = true;
+
+ might_sleep();
+
+ switch (status) {
+ case RTAS_BUSY:
+ /*
+ * OK to call again immediately. Schedule if there's
+ * other work to do, but no sleep is necessary.
+ */
+ cond_resched();
+ break;
+ case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
+ handle_extended_delay(status);
+ break;
+ default:
+ was_busy = false;
+ break;
+ }
+
+ return was_busy;
+}
+
static int rtas_error_rc(int rtas_rc)
{
int rc;
--
2.30.2
^ permalink raw reply related
* [RFC 03/10] powerpc/rtas-rtc: convert get-time-of-day to rtas_force_spin_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
The functions in rtas-rtc which call get-time-of-day can be invoked in
boot, suspend, and resume paths with interrupts off. Unfortunately
get-time-of-day can return an extended delay status, so we use
rtas_force_spin_if_busy().
In the specific case of rtas_get_rtc_time(), it is not clear why
returning an incorrect result is better than calling again even if we
are in interrupt context. Remove this logic.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/kernel/rtas-rtc.c | 28 ++--------------------------
1 file changed, 2 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/rtas-rtc.c b/arch/powerpc/kernel/rtas-rtc.c
index a28239b8b0c0..82cb95f29a11 100644
--- a/arch/powerpc/kernel/rtas-rtc.c
+++ b/arch/powerpc/kernel/rtas-rtc.c
@@ -17,19 +17,12 @@ time64_t __init rtas_get_boot_time(void)
{
int ret[8];
int error;
- unsigned int wait_time;
u64 max_wait_tb;
max_wait_tb = get_tb() + tb_ticks_per_usec * 1000 * MAX_RTC_WAIT;
do {
error = rtas_call(rtas_token("get-time-of-day"), 0, 8, ret);
-
- wait_time = rtas_busy_delay_time(error);
- if (wait_time) {
- /* This is boot time so we spin. */
- udelay(wait_time*1000);
- }
- } while (wait_time && (get_tb() < max_wait_tb));
+ } while (rtas_force_spin_if_busy(error) && (get_tb() < max_wait_tb));
if (error != 0) {
printk_ratelimited(KERN_WARNING
@@ -41,33 +34,16 @@ time64_t __init rtas_get_boot_time(void)
return mktime64(ret[0], ret[1], ret[2], ret[3], ret[4], ret[5]);
}
-/* NOTE: get_rtc_time will get an error if executed in interrupt context
- * and if a delay is needed to read the clock. In this case we just
- * silently return without updating rtc_tm.
- */
void rtas_get_rtc_time(struct rtc_time *rtc_tm)
{
int ret[8];
int error;
- unsigned int wait_time;
u64 max_wait_tb;
max_wait_tb = get_tb() + tb_ticks_per_usec * 1000 * MAX_RTC_WAIT;
do {
error = rtas_call(rtas_token("get-time-of-day"), 0, 8, ret);
-
- wait_time = rtas_busy_delay_time(error);
- if (wait_time) {
- if (in_interrupt()) {
- memset(rtc_tm, 0, sizeof(struct rtc_time));
- printk_ratelimited(KERN_WARNING
- "error: reading clock "
- "would delay interrupt\n");
- return; /* delay not allowed */
- }
- msleep(wait_time);
- }
- } while (wait_time && (get_tb() < max_wait_tb));
+ } while (rtas_sched_if_busy(error) && (get_tb() < max_wait_tb));
if (error != 0) {
printk_ratelimited(KERN_WARNING
--
2.30.2
^ permalink raw reply related
* [RFC 04/10] powerpc/rtas-rtc: convert set-time-of-day to rtas_sched_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
rtas_set_rtc_time() is called only in process context; convert this to
rtas_sched_if_busy().
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/kernel/rtas-rtc.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/rtas-rtc.c b/arch/powerpc/kernel/rtas-rtc.c
index 82cb95f29a11..421b92f95669 100644
--- a/arch/powerpc/kernel/rtas-rtc.c
+++ b/arch/powerpc/kernel/rtas-rtc.c
@@ -62,7 +62,7 @@ void rtas_get_rtc_time(struct rtc_time *rtc_tm)
int rtas_set_rtc_time(struct rtc_time *tm)
{
- int error, wait_time;
+ int error;
u64 max_wait_tb;
max_wait_tb = get_tb() + tb_ticks_per_usec * 1000 * MAX_RTC_WAIT;
@@ -72,13 +72,7 @@ int rtas_set_rtc_time(struct rtc_time *tm)
tm->tm_mday, tm->tm_hour, tm->tm_min,
tm->tm_sec, 0);
- wait_time = rtas_busy_delay_time(error);
- if (wait_time) {
- if (in_interrupt())
- return 1; /* probably decrementer */
- msleep(wait_time);
- }
- } while (wait_time && (get_tb() < max_wait_tb));
+ } while (rtas_sched_if_busy(error) && (get_tb() < max_wait_tb));
if (error != 0)
printk_ratelimited(KERN_WARNING
--
2.30.2
^ permalink raw reply related
* [RFC 05/10] powerpc/pseries/fadump: convert to rtas_sched_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
None of these call sites need to use mdelay(); convert them to
rtas_sched_if_busy().
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/rtas-fadump.c | 22 +++-----------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.c b/arch/powerpc/platforms/pseries/rtas-fadump.c
index f8f73b47b107..9a200d3bf5e0 100644
--- a/arch/powerpc/platforms/pseries/rtas-fadump.c
+++ b/arch/powerpc/platforms/pseries/rtas-fadump.c
@@ -129,7 +129,6 @@ static u64 rtas_fadump_get_bootmem_min(void)
static int rtas_fadump_register(struct fw_dump *fadump_conf)
{
- unsigned int wait_time;
int rc, err = -EIO;
/* TODO: Add upper time limit for the delay */
@@ -137,12 +136,7 @@ static int rtas_fadump_register(struct fw_dump *fadump_conf)
rc = rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
NULL, FADUMP_REGISTER, &fdm,
sizeof(struct rtas_fadump_mem_struct));
-
- wait_time = rtas_busy_delay_time(rc);
- if (wait_time)
- mdelay(wait_time);
-
- } while (wait_time);
+ } while (rtas_sched_if_busy(rc));
switch (rc) {
case 0:
@@ -177,7 +171,6 @@ static int rtas_fadump_register(struct fw_dump *fadump_conf)
static int rtas_fadump_unregister(struct fw_dump *fadump_conf)
{
- unsigned int wait_time;
int rc;
/* TODO: Add upper time limit for the delay */
@@ -185,11 +178,7 @@ static int rtas_fadump_unregister(struct fw_dump *fadump_conf)
rc = rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
NULL, FADUMP_UNREGISTER, &fdm,
sizeof(struct rtas_fadump_mem_struct));
-
- wait_time = rtas_busy_delay_time(rc);
- if (wait_time)
- mdelay(wait_time);
- } while (wait_time);
+ } while (rtas_sched_if_busy(rc));
if (rc) {
pr_err("Failed to un-register - unexpected error(%d).\n", rc);
@@ -202,7 +191,6 @@ static int rtas_fadump_unregister(struct fw_dump *fadump_conf)
static int rtas_fadump_invalidate(struct fw_dump *fadump_conf)
{
- unsigned int wait_time;
int rc;
/* TODO: Add upper time limit for the delay */
@@ -210,11 +198,7 @@ static int rtas_fadump_invalidate(struct fw_dump *fadump_conf)
rc = rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
NULL, FADUMP_INVALIDATE, fdm_active,
sizeof(struct rtas_fadump_mem_struct));
-
- wait_time = rtas_busy_delay_time(rc);
- if (wait_time)
- mdelay(wait_time);
- } while (wait_time);
+ } while (rtas_sched_if_busy(rc));
if (rc) {
pr_err("Failed to invalidate - unexpected error (%d).\n", rc);
--
2.30.2
^ permalink raw reply related
* [RFC 06/10] powerpc/pseries/msi: convert to rtas_sched_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
rtas_sched_if_busy() has better behavior for RTAS_BUSY (-2) and small
extended delay values.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/msi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 637300330507..df434b8a3aa7 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -49,7 +49,7 @@ static int rtas_change_msi(struct pci_dn *pdn, u32 func, u32 num_irqs)
func, num_irqs, seq_num);
seq_num = rtas_ret[1];
- } while (rtas_busy_delay(rc));
+ } while (rtas_sched_if_busy(rc));
/*
* If the RTAS call succeeded, return the number of irqs allocated.
@@ -100,7 +100,7 @@ static int rtas_query_irq_number(struct pci_dn *pdn, int offset)
do {
rc = rtas_call(query_token, 4, 3, rtas_ret, addr,
BUID_HI(buid), BUID_LO(buid), offset);
- } while (rtas_busy_delay(rc));
+ } while (rtas_sched_if_busy(rc));
if (rc) {
pr_debug("rtas_msi: error (%d) querying source number\n", rc);
--
2.30.2
^ permalink raw reply related
* [RFC 07/10] powerpc/pseries/iommu: convert to rtas_sched_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
rtas_sched_if_busy() has better behavior for RTAS_BUSY (-2) and small
extended delay values.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 0c55b991f665..0f0e7a51b863 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1016,7 +1016,7 @@ static int create_ddw(struct pci_dev *dev, const u32 *ddw_avail,
ret = rtas_call(ddw_avail[DDW_CREATE_PE_DMA_WIN], 5, 4,
(u32 *)create, cfg_addr, BUID_HI(buid),
BUID_LO(buid), page_shift, window_shift);
- } while (rtas_busy_delay(ret));
+ } while (rtas_sched_if_busy(ret));
dev_info(&dev->dev,
"ibm,create-pe-dma-window(%x) %x %x %x %x %x returned %d "
"(liobn = 0x%x starting addr = %x %x)\n",
--
2.30.2
^ permalink raw reply related
* [RFC 08/10] powerpc/pseries/dlpar: convert to rtas_sched_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
rtas_sched_if_busy() has better behavior for RTAS_BUSY (-2) and small
extended delay values.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 3ac70790ec7a..3ba77bc09a6e 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -167,7 +167,7 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
spin_unlock(&rtas_data_buf_lock);
- if (rtas_busy_delay(rc))
+ if (rtas_sched_if_busy(rc))
continue;
switch (rc) {
--
2.30.2
^ permalink raw reply related
* [RFC 10/10] powerpc/rtas_flash: convert to rtas_sched_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
rtas_sched_if_busy() has better behavior for RTAS_BUSY (-2) and small
extended delay values.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/kernel/rtas_flash.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index a99179d83538..bedefb9178ec 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -378,7 +378,7 @@ static void manage_flash(struct rtas_manage_flash_t *args_buf, unsigned int op)
do {
rc = rtas_call(rtas_token("ibm,manage-flash-image"), 1, 1,
NULL, op);
- } while (rtas_busy_delay(rc));
+ } while (rtas_sched_if_busy(rc));
args_buf->status = rc;
}
@@ -456,7 +456,7 @@ static void validate_flash(struct rtas_validate_flash_t *args_buf)
(u32) __pa(rtas_data_buf), args_buf->buf_size);
memcpy(args_buf->buf, rtas_data_buf, VALIDATE_BUF_SIZE);
spin_unlock(&rtas_data_buf_lock);
- } while (rtas_busy_delay(rc));
+ } while (rtas_sched_if_busy(rc));
args_buf->status = rc;
args_buf->update_results = update_results;
--
2.30.2
^ permalink raw reply related
* [RFC 09/10] powerpc/rtas: convert to rtas_sched_if_busy()
From: Nathan Lynch @ 2021-05-04 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aik, ajd, tyreld
In-Reply-To: <20210504030358.1715034-1-nathanl@linux.ibm.com>
rtas_sched_if_busy() has better behavior for RTAS_BUSY (-2) and small
extended delay values.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/kernel/rtas.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 4177f7385ea2..c5cc4542856f 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -743,7 +743,7 @@ int rtas_set_power_level(int powerdomain, int level, int *setlevel)
do {
rc = rtas_call(token, 2, 2, setlevel, powerdomain, level);
- } while (rtas_busy_delay(rc));
+ } while (rtas_sched_if_busy(rc));
if (rc < 0)
return rtas_error_rc(rc);
@@ -761,7 +761,7 @@ int rtas_get_sensor(int sensor, int index, int *state)
do {
rc = rtas_call(token, 2, 2, state, sensor, index);
- } while (rtas_busy_delay(rc));
+ } while (rtas_sched_if_busy(rc));
if (rc < 0)
return rtas_error_rc(rc);
@@ -822,7 +822,7 @@ int rtas_set_indicator(int indicator, int index, int new_value)
do {
rc = rtas_call(token, 3, 1, NULL, indicator, index, new_value);
- } while (rtas_busy_delay(rc));
+ } while (rtas_sched_if_busy(rc));
if (rc < 0)
return rtas_error_rc(rc);
@@ -990,7 +990,7 @@ void rtas_activate_firmware(void)
do {
fwrc = rtas_call(token, 0, 1, NULL);
- } while (rtas_busy_delay(fwrc));
+ } while (rtas_sched_if_busy(fwrc));
if (fwrc)
pr_err("ibm,activate-firmware failed (%i)\n", fwrc);
--
2.30.2
^ permalink raw reply related
* Re: [PATCH v3 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Paul Mackerras @ 2021-05-04 4:28 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <1619833560.k4eybr40bg.astroid@bobo.none>
On Sat, May 01, 2021 at 11:58:36AM +1000, Nicholas Piggin wrote:
> Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
> > As one of the arguments of the H_ENTER_NESTED hypercall, the nested
> > hypervisor (L1) prepares a structure containing the values of various
> > hypervisor-privileged registers with which it wants the nested guest
> > (L2) to run. Since the nested HV runs in supervisor mode it needs the
> > host to write to these registers.
> >
> > To stop a nested HV manipulating this mechanism and using a nested
> > guest as a proxy to access a facility that has been made unavailable
> > to it, we have a routine that sanitises the values of the HV registers
> > before copying them into the nested guest's vcpu struct.
> >
> > However, when coming out of the guest the values are copied as they
> > were back into L1 memory, which means that any sanitisation we did
> > during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
> >
> > This patch alters this sanitisation to have effect on the vcpu->arch
> > registers directly before entering and after exiting the guest,
> > leaving the structure that is copied back into L1 unchanged (except
> > when we really want L1 to access the value, e.g the Cause bits of
> > HFSCR).
> >
> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> > ---
> > arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++-----------
> > 1 file changed, 34 insertions(+), 21 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> > index 0cd0e7aad588..270552dd42c5 100644
> > --- a/arch/powerpc/kvm/book3s_hv_nested.c
> > +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> > @@ -102,8 +102,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> > {
> > struct kvmppc_vcore *vc = vcpu->arch.vcore;
> >
> > + /*
> > + * When loading the hypervisor-privileged registers to run L2,
> > + * we might have used bits from L1 state to restrict what the
> > + * L2 state is allowed to be. Since L1 is not allowed to read
> > + * the HV registers, do not include these modifications in the
> > + * return state.
> > + */
> > + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
> > + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
> > +
> > hr->dpdes = vc->dpdes;
> > - hr->hfscr = vcpu->arch.hfscr;
> > hr->purr = vcpu->arch.purr;
> > hr->spurr = vcpu->arch.spurr;
> > hr->ic = vcpu->arch.ic;
>
> Do we still have the problem here that hfac interrupts due to bits cleared
> by the hfscr sanitisation would have the cause bits returned to the L1,
> so in theory it could probe hfscr directly that way? I don't see a good
> solution to this except either have the L0 intercept these faults and do
> "something" transparent, or return error from H_ENTER_NESTED (which would
> also allow trivial probing of the facilities).
It seems to me that there are various specific reasons why L0 would
clear HFSCR bits, and if we think about the specific reasons, what we
should do becomes clear. (I say "L0" but in fact the same reasoning
applies to any hypervisor that lets its guest do hypervisor-ish
things.)
1. Emulating a version of the architecture which doesn't have the
feature in question - in that case the bit should appear to L1 as a
reserved bit in HFSCR (i.e. always read 0), the associated facility
code should never appear in the top 8 bits of any HFSCR value that L1
sees, and any HFU interrupt received by L0 for the facility should be
changed into an illegal instruction interrupt (or HEAI) forwarded to
L1. In this case the real HFSCR should always have the enable bit for
the facility set to 0.
2. Lazy save/restore of the state associated with a facility - in this
case, while the system is in the "lazy" state (i.e. the state is not
that of the currently running guest), the real HFSCR bit for the
facility should be 0. On an HFU interrupt for the facility, L0 looks
at L1's HFSCR value: if it's 0, forward the HFU interrupt to L1; if
it's 1, load up the facility state, set the facility's bit in HFSCR,
and resume the guest.
3. Emulating a facility in software - in this case, the real HFSCR
bit for the facility would always be 0. On an HFU interrupt, L0 reads
the instruction and emulates it, then resumes the guest.
One thing this all makes clear is that the IC field of the "virtual"
HFSCR value seen by L1 should only ever be changed when L0 forwards a
HFU interrupt to L1.
In fact we currently never do (1) or (2), and we only do (3) for
msgsndp etc., so this discussion is mostly theoretical.
> Returning an hfac interrupt to a hypervisor that thought it enabled the
> bit would be strange. But so does appearing to modify the register
> underneath it and then returning a fault.
I don't think we should ever do either of those things. The closest
would be (1) above, but in that case the fault has to be either an
illegal instruction type program interrupt, or a HEAI.
> I think the sanest thing would actually be to return failure from the
> hcall.
I don't think we should do that either.
Paul.
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christophe Leroy @ 2021-05-04 4:56 UTC (permalink / raw)
To: Christian Zigotzky, Michael Ellerman
Cc: Darren Stevens, linuxppc-dev, mad skateman, R.T.Dickinson,
Christian Zigotzky
In-Reply-To: <c5b0ac7c-525f-0208-7587-c90427eae137@xenosoft.de>
Le 04/05/2021 à 00:25, Christian Zigotzky a écrit :
> Hello,
>
> Xorg always restarts again and again after the the PowerPC updates 5.13-1 [1] on my FSL P5040 Cyrus+
> board (A-EON AmigaOne X5000) [2]. Xorg doesn't start anymore in a virtual e5500 QEMU machine [3].
>
> I bisected today [4].
>
> Result: powerpc/signal32: Convert do_setcontext[_tm]() to user access block
> (887f3ceb51cd34109ac17bfc98695162e299e657) [5] is the first bad commit.
>
> Please find attached the kernel config.
>
> Please check the first bad commit.
I'm not sure you can conclude anything here. There is a problem in that commit, but it is fixed by
525642624783 ("powerpc/signal32: Fix erroneous SIGSEGV on RT signal return") which is the last
commit of powerpc-5.13-1.
So any bisect from there will for sure point to 887f3ceb51cd ("powerpc/signal32: Convert
do_setcontext[_tm]() to user access block") but that's unconclusive. If the problem is still there
at the HEAD of powerpc-5.13-1, the problem is likely somewhere else.
I think you need to do the bisect again with a cherry-pick of 525642624783 at each step.
Thanks
Christophe
>
> Thanks,
> Christian
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c70a4be130de333ea079c59da41cc959712bb01c
>
> [2] http://wiki.amiga.org/index.php?title=X5000
> [3] qemu-system-ppc64 -M ppce500 -cpu e5500 -m 1024 -kernel uImage -drive
> format=raw,file=fedora28-2.img,index=0,if=virtio -netdev user,id=mynet0 -device
> virtio-net-pci,netdev=mynet0 -append "rw root=/dev/vda" -device virtio-vga -usb -device
> usb-ehci,id=ehci -device usb-tablet -device virtio-keyboard-pci -smp 4 -vnc :1
> [4] https://forum.hyperion-entertainment.com/viewtopic.php?p=53101#p53101
> [5]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=887f3ceb51cd34109ac17bfc98695162e299e657
>
^ permalink raw reply
* Re: [PATCH v3 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Nicholas Piggin @ 2021-05-04 5:26 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <YJDNbFQlB9DHnI6Z@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of May 4, 2021 2:28 pm:
> On Sat, May 01, 2021 at 11:58:36AM +1000, Nicholas Piggin wrote:
>> Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
>> > As one of the arguments of the H_ENTER_NESTED hypercall, the nested
>> > hypervisor (L1) prepares a structure containing the values of various
>> > hypervisor-privileged registers with which it wants the nested guest
>> > (L2) to run. Since the nested HV runs in supervisor mode it needs the
>> > host to write to these registers.
>> >
>> > To stop a nested HV manipulating this mechanism and using a nested
>> > guest as a proxy to access a facility that has been made unavailable
>> > to it, we have a routine that sanitises the values of the HV registers
>> > before copying them into the nested guest's vcpu struct.
>> >
>> > However, when coming out of the guest the values are copied as they
>> > were back into L1 memory, which means that any sanitisation we did
>> > during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
>> >
>> > This patch alters this sanitisation to have effect on the vcpu->arch
>> > registers directly before entering and after exiting the guest,
>> > leaving the structure that is copied back into L1 unchanged (except
>> > when we really want L1 to access the value, e.g the Cause bits of
>> > HFSCR).
>> >
>> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> > ---
>> > arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++-----------
>> > 1 file changed, 34 insertions(+), 21 deletions(-)
>> >
>> > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
>> > index 0cd0e7aad588..270552dd42c5 100644
>> > --- a/arch/powerpc/kvm/book3s_hv_nested.c
>> > +++ b/arch/powerpc/kvm/book3s_hv_nested.c
>> > @@ -102,8 +102,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
>> > {
>> > struct kvmppc_vcore *vc = vcpu->arch.vcore;
>> >
>> > + /*
>> > + * When loading the hypervisor-privileged registers to run L2,
>> > + * we might have used bits from L1 state to restrict what the
>> > + * L2 state is allowed to be. Since L1 is not allowed to read
>> > + * the HV registers, do not include these modifications in the
>> > + * return state.
>> > + */
>> > + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
>> > + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
>> > +
>> > hr->dpdes = vc->dpdes;
>> > - hr->hfscr = vcpu->arch.hfscr;
>> > hr->purr = vcpu->arch.purr;
>> > hr->spurr = vcpu->arch.spurr;
>> > hr->ic = vcpu->arch.ic;
>>
>> Do we still have the problem here that hfac interrupts due to bits cleared
>> by the hfscr sanitisation would have the cause bits returned to the L1,
>> so in theory it could probe hfscr directly that way? I don't see a good
>> solution to this except either have the L0 intercept these faults and do
>> "something" transparent, or return error from H_ENTER_NESTED (which would
>> also allow trivial probing of the facilities).
>
> It seems to me that there are various specific reasons why L0 would
> clear HFSCR bits, and if we think about the specific reasons, what we
> should do becomes clear. (I say "L0" but in fact the same reasoning
> applies to any hypervisor that lets its guest do hypervisor-ish
> things.)
>
> 1. Emulating a version of the architecture which doesn't have the
> feature in question - in that case the bit should appear to L1 as a
> reserved bit in HFSCR (i.e. always read 0), the associated facility
> code should never appear in the top 8 bits of any HFSCR value that L1
> sees, and any HFU interrupt received by L0 for the facility should be
> changed into an illegal instruction interrupt (or HEAI) forwarded to
> L1. In this case the real HFSCR should always have the enable bit for
> the facility set to 0.
>
> 2. Lazy save/restore of the state associated with a facility - in this
> case, while the system is in the "lazy" state (i.e. the state is not
> that of the currently running guest), the real HFSCR bit for the
> facility should be 0. On an HFU interrupt for the facility, L0 looks
> at L1's HFSCR value: if it's 0, forward the HFU interrupt to L1; if
> it's 1, load up the facility state, set the facility's bit in HFSCR,
> and resume the guest.
>
> 3. Emulating a facility in software - in this case, the real HFSCR
> bit for the facility would always be 0. On an HFU interrupt, L0 reads
> the instruction and emulates it, then resumes the guest.
>
> One thing this all makes clear is that the IC field of the "virtual"
> HFSCR value seen by L1 should only ever be changed when L0 forwards a
> HFU interrupt to L1.
>
> In fact we currently never do (1) or (2), and we only do (3) for
> msgsndp etc., so this discussion is mostly theoretical.
Yeah it's somewhat theoretical, and I guess I mostly agree with you.
Missing is the case where the L0 does not implement a feature at all.
Let's say TM is broken so it disables it, or nobody uses TAR so it
doesn't bother to switch it.
In those cases what do you tell the L1 if it enables a bit that you
don't support at all, and it takes a fault?
I guess the right thing to do is advertise that to the guest by some
other means, and expect it does the right thing. And you could have
the proviso in the nested HV specification that the returned IC field
might trip for a feature you enabled in the L1 HFSCR.
>
>> Returning an hfac interrupt to a hypervisor that thought it enabled the
>> bit would be strange. But so does appearing to modify the register
>> underneath it and then returning a fault.
>
> I don't think we should ever do either of those things. The closest
> would be (1) above, but in that case the fault has to be either an
> illegal instruction type program interrupt, or a HEAI.
>
>> I think the sanest thing would actually be to return failure from the
>> hcall.
>
> I don't think we should do that either.
I still think it's preferable for case 4. No point waiting for the
guest to boot and some user program eventually hits a bad instruction,
even if it was due to some host vs guest configuration problem.
At any rate, this patch 1 not overwriting the L2 HV state with the
sanitization step is fine and clearly required for any kind of non
trivial handling of missing bits.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Alexander Dahl @ 2021-05-04 5:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Albert Ou, Catalin Marinas, Linux Kbuild mailing list,
Greg Kroah-Hartman, Masahiro Yamada, Jonathan Corbet,
Linux Doc Mailing List, linux-kernel, Matthew Wilcox,
Miguel Ojeda, Will Deacon, Palmer Dabbelt, Paul Walmsley,
Joe Perches, Paul Mackerras, linux-riscv, Miguel Ojeda,
Linus Torvalds, Linux ARM, linuxppc-dev
In-Reply-To: <CAK8P3a0kV4ZfMEFh0DcMDjXqxA0yhj8a8CL-YFGV6B4pszHeGg@mail.gmail.com>
Hello Arnd,
Am Mon, May 03, 2021 at 11:25:21AM +0200 schrieb Arnd Bergmann:
> On Mon, May 3, 2021 at 9:35 AM Alexander Dahl <ada@thorsis.com> wrote:
> >
> > Desktops and servers are all nice, however I just want to make you
> > aware, there are embedded users forced to stick to older cross
> > toolchains for different reasons as well, e.g. in industrial
> > environment. :-)
> >
> > This is no show stopper for us, I just wanted to let you be aware.
>
> Can you be more specific about what scenarios you are thinking of,
> what the motivations are for using an old compiler with a new kernel
> on embedded systems, and what you think a realistic maximum
> time would be between compiler updates?
One reason might be certification. For certain industrial applications
like support for complex field bus protocols, you need to get your
devices tested by an external partner running extensive test suites.
This is time consuming and expensive.
Changing the toolchain of your system then, would be a massive change
which would require recertification, while you could argue just
updating a single component like the kernel and building everything
again, does not require the whole testing process again.
Thin ice, I know.
> One scenario that I've seen previously is where user space and
> kernel are built together as a source based distribution (OE, buildroot,
> openwrt, ...), and the compiler is picked to match the original sources
> of the user space because that is best tested, but the same compiler
> then gets used to build the kernel as well because that is the default
> in the build environment.
One problem we actually ran into in BSPs like that (we build with
ptxdist, however build system doesn't matter here, it could as well
have been buildroot etc.) was things* failing to build with newer
compilers, things we could not or did not want to fix, so staying with
an older toolchain was the obvious choice.
*Things as in bootloaders for an armv5 platform.
> There are two problems I see with this logic:
>
> - Running the latest kernel to avoid security problems is of course
> a good idea, but if one runs that with ten year old user space that
> is never updated, the system is likely to end up just as insecure.
> Not all bugs are in the kernel.
Agreed.
> - The same logic that applies to ancient user space staying with
> an ancient compiler (it's better tested in this combination) also
> applies to the kernel: running the latest kernel on an old compiler
> is something that few people test, and tends to run into more bugs
> than using the compiler that other developers used to test that
> kernel.
What we actually did: building recent userspace and kernel with older
toolchains, because bootloader. I know, there are several
possibilities to solve this kind of lock:
- built bootloader with different compiler
- update bootloader
- …
As said before, this is no problem for me now, I can work around it,
but to give an idea what could keep people on older toolchains.
Greets
Alex
^ permalink raw reply
* [PATCH v2 net-next] ibmvnic: remove default label from to_string switch
From: Michal Suchanek @ 2021-05-04 5:40 UTC (permalink / raw)
To: netdev
Cc: Sukadev Bhattiprolu, Lijun Pan, linux-kernel, Thomas Falcon,
Paul Mackerras, Dany Madden, Jakub Kicinski, Michal Suchanek,
linuxppc-dev, David S. Miller
In-Reply-To: <20210503.134721.2149322673805635760.davem@davemloft.net>
This way the compiler warns when a new value is added to the enum but
not to the string translation like:
drivers/net/ethernet/ibm/ibmvnic.c: In function 'adapter_state_to_string':
drivers/net/ethernet/ibm/ibmvnic.c:832:2: warning: enumeration value 'VNIC_FOOBAR' not handled in switch [-Wswitch]
switch (state) {
^~~~~~
drivers/net/ethernet/ibm/ibmvnic.c: In function 'reset_reason_to_string':
drivers/net/ethernet/ibm/ibmvnic.c:1935:2: warning: enumeration value 'VNIC_RESET_FOOBAR' not handled in switch [-Wswitch]
switch (reason) {
^~~~~~
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v2: Fix typo in commit message
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5788bb956d73..4d439413f6d9 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -846,9 +846,8 @@ static const char *adapter_state_to_string(enum vnic_state state)
return "REMOVING";
case VNIC_REMOVED:
return "REMOVED";
- default:
- return "UNKNOWN";
}
+ return "UNKNOWN";
}
static int ibmvnic_login(struct net_device *netdev)
@@ -1946,9 +1945,8 @@ static const char *reset_reason_to_string(enum ibmvnic_reset_reason reason)
return "TIMEOUT";
case VNIC_RESET_CHANGE_PARAM:
return "CHANGE_PARAM";
- default:
- return "UNKNOWN";
}
+ return "UNKNOWN";
}
/*
--
2.26.2
^ permalink raw reply related
* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Christophe Leroy @ 2021-05-04 6:33 UTC (permalink / raw)
To: Arnd Bergmann, Matthew Wilcox, Linus Torvalds, Segher Boessenkool,
Joe Perches, Miguel Ojeda, Masahiro Yamada, Albert Ou,
Linux Kbuild mailing list, Greg Kroah-Hartman, Jonathan Corbet,
Linux Doc Mailing List, linux-kernel, Palmer Dabbelt,
Paul Walmsley, Catalin Marinas, Miguel Ojeda, Paul Mackerras,
linux-riscv, linuxppc-dev, Will Deacon, Linux ARM
In-Reply-To: <YJDb9uLQBgoy94Ub@ada-deb-carambola.ifak-system.com>
Le 04/05/2021 à 07:30, Alexander Dahl a écrit :
> Hello Arnd,
>
> Am Mon, May 03, 2021 at 11:25:21AM +0200 schrieb Arnd Bergmann:
>> On Mon, May 3, 2021 at 9:35 AM Alexander Dahl <ada@thorsis.com> wrote:
>>>
>>> Desktops and servers are all nice, however I just want to make you
>>> aware, there are embedded users forced to stick to older cross
>>> toolchains for different reasons as well, e.g. in industrial
>>> environment. :-)
>>>
>>> This is no show stopper for us, I just wanted to let you be aware.
>>
>> Can you be more specific about what scenarios you are thinking of,
>> what the motivations are for using an old compiler with a new kernel
>> on embedded systems, and what you think a realistic maximum
>> time would be between compiler updates?
>
> One reason might be certification. For certain industrial applications
> like support for complex field bus protocols, you need to get your
> devices tested by an external partner running extensive test suites.
> This is time consuming and expensive.
>
> Changing the toolchain of your system then, would be a massive change
> which would require recertification, while you could argue just
> updating a single component like the kernel and building everything
> again, does not require the whole testing process again.
Not sure to follow you.
Our company provides systems for Air Trafic Control, so we have the same kind of assurance quality
process, but then I can't understand why you would need to upgrade your kernel at all.
Today our system is based on GCC 5 and Kernel 4.14. At the time being we are using GCC 5.5 (Latest
GCC 5) and kernel 4.14.232 (Latest 4.14.y). Kernel 4.14 is maintained until 2024.
The day we do an upgrade, we upgrade everything including the tool chain then we go for another 6
years without major changes/re-qualification, because we can't afford a new qualitication every now
and then.
So really, I can't see your approach.
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: imx-akcodec: Add imx-akcodec machine driver
From: Marco Felsch @ 2021-05-04 6:57 UTC (permalink / raw)
To: Shengjiu Wang
Cc: linux-arm-kernel, devicetree, alsa-devel, timur, Xiubo.Lee,
shawnguo, s.hauer, linuxppc-dev, tiwai, robh+dt, lgirdwood,
nicoleotsuka, broonie, linux-imx, kernel, perex, festevam,
linux-kernel
In-Reply-To: <1619157107-3734-1-git-send-email-shengjiu.wang@nxp.com>
On 21-04-23 13:51, Shengjiu Wang wrote:
> Add machine driver for i.MX boards that have
> AK4458/AK5558/AK4497/AK5552 DAC/ADC attached to
> SAI interface.
Why? Does simple-audio-card don't fit?
Regards,
Marco
^ permalink raw reply
* Re: [PATCH v3] pseries/drmem: update LMBs after LPM
From: Laurent Dufour @ 2021-05-04 7:03 UTC (permalink / raw)
To: Tyrel Datwyler, Aneesh Kumar K.V, mpe, benh, paulus
Cc: nathanl, linuxppc-dev, linux-kernel
In-Reply-To: <bdc510ff-f9f6-b032-0f0d-52a274fb9dab@linux.ibm.com>
Le 03/05/2021 à 22:44, Tyrel Datwyler a écrit :
> On 5/3/21 10:28 AM, Laurent Dufour wrote:
>> Le 01/05/2021 à 01:58, Tyrel Datwyler a écrit :
>>> On 4/30/21 9:13 AM, Laurent Dufour wrote:
>>>> Le 29/04/2021 à 21:12, Tyrel Datwyler a écrit :
>>>>> On 4/29/21 3:27 AM, Aneesh Kumar K.V wrote:
>>>>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>>>>
>
> Snip
>
>>>
>>> As of today I don't have a problem with your patch. This was more of me pointing
>>> out things that I think are currently wrong with our memory hotplug
>>> implementation, and that we need to take a long hard look at it down the road.
>>
>> I do agree, there is a lot of odd things there to address in this area.
>> If you're ok with that patch, do you mind to add a reviewed-by?
>>
>
> Can you send a v4 with the fix for the duplicate update included?
Of course !
I wrote it last week, but let in the to-be-sent list, my mistake.
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-04 7:21 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman
Cc: Darren Stevens, linuxppc-dev, mad skateman, R.T.Dickinson,
Christian Zigotzky
In-Reply-To: <0886c1dc-e946-69cb-a0a9-57247acfd080@csgroup.eu>
Hi Christophe,
Thanks for your answer but I think I don't know how it works with the
cherry-pick.
$ git bisect start
$ git bisect good 68a32ba14177d4a21c4a9a941cf1d7aea86d436f
$ git bisect bad c70a4be130de333ea079c59da41cc959712bb01c
Bisecting: 2462 revisions left to test after this (roughly 11 steps)
[47a6959fa331fe892a4fc3b48ca08e92045c6bda] netfilter: allow to turn off
xtables compat layer
$ git cherry-pick 525642624783
error: could not apply 525642624783... powerpc/signal32: Fix erroneous
SIGSEGV on RT signal return
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
How can I fix this error?
Thanks,
Christian
On 04 May 2021 at 06:56 am, Christophe Leroy wrote:
>
>
> Le 04/05/2021 à 00:25, Christian Zigotzky a écrit :
>> Hello,
>>
>> Xorg always restarts again and again after the the PowerPC updates
>> 5.13-1 [1] on my FSL P5040 Cyrus+ board (A-EON AmigaOne X5000) [2].
>> Xorg doesn't start anymore in a virtual e5500 QEMU machine [3].
>>
>> I bisected today [4].
>>
>> Result: powerpc/signal32: Convert do_setcontext[_tm]() to user access
>> block (887f3ceb51cd34109ac17bfc98695162e299e657) [5] is the first bad
>> commit.
>>
>> Please find attached the kernel config.
>>
>> Please check the first bad commit.
>
> I'm not sure you can conclude anything here. There is a problem in
> that commit, but it is fixed by 525642624783 ("powerpc/signal32: Fix
> erroneous SIGSEGV on RT signal return") which is the last commit of
> powerpc-5.13-1.
>
> So any bisect from there will for sure point to 887f3ceb51cd
> ("powerpc/signal32: Convert do_setcontext[_tm]() to user access
> block") but that's unconclusive. If the problem is still there at the
> HEAD of powerpc-5.13-1, the problem is likely somewhere else.
>
> I think you need to do the bisect again with a cherry-pick of
> 525642624783 at each step.
>
> Thanks
> Christophe
>
>
>>
>> Thanks,
>> Christian
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c70a4be130de333ea079c59da41cc959712bb01c
>>
>> [2] http://wiki.amiga.org/index.php?title=X5000
>> [3] qemu-system-ppc64 -M ppce500 -cpu e5500 -m 1024 -kernel uImage
>> -drive format=raw,file=fedora28-2.img,index=0,if=virtio -netdev
>> user,id=mynet0 -device virtio-net-pci,netdev=mynet0 -append "rw
>> root=/dev/vda" -device virtio-vga -usb -device usb-ehci,id=ehci
>> -device usb-tablet -device virtio-keyboard-pci -smp 4 -vnc :1
>> [4]
>> https://forum.hyperion-entertainment.com/viewtopic.php?p=53101#p53101
>> [5]
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=887f3ceb51cd34109ac17bfc98695162e299e657
>>
^ permalink raw reply
* Re: [PATCH v3 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Paul Mackerras @ 2021-05-04 7:36 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <1620105163.ok9nw6k5yz.astroid@bobo.none>
On Tue, May 04, 2021 at 03:26:24PM +1000, Nicholas Piggin wrote:
> Excerpts from Paul Mackerras's message of May 4, 2021 2:28 pm:
> > On Sat, May 01, 2021 at 11:58:36AM +1000, Nicholas Piggin wrote:
> >> Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
> >> > As one of the arguments of the H_ENTER_NESTED hypercall, the nested
> >> > hypervisor (L1) prepares a structure containing the values of various
> >> > hypervisor-privileged registers with which it wants the nested guest
> >> > (L2) to run. Since the nested HV runs in supervisor mode it needs the
> >> > host to write to these registers.
> >> >
> >> > To stop a nested HV manipulating this mechanism and using a nested
> >> > guest as a proxy to access a facility that has been made unavailable
> >> > to it, we have a routine that sanitises the values of the HV registers
> >> > before copying them into the nested guest's vcpu struct.
> >> >
> >> > However, when coming out of the guest the values are copied as they
> >> > were back into L1 memory, which means that any sanitisation we did
> >> > during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
> >> >
> >> > This patch alters this sanitisation to have effect on the vcpu->arch
> >> > registers directly before entering and after exiting the guest,
> >> > leaving the structure that is copied back into L1 unchanged (except
> >> > when we really want L1 to access the value, e.g the Cause bits of
> >> > HFSCR).
> >> >
> >> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> >> > ---
> >> > arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++-----------
> >> > 1 file changed, 34 insertions(+), 21 deletions(-)
> >> >
> >> > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> >> > index 0cd0e7aad588..270552dd42c5 100644
> >> > --- a/arch/powerpc/kvm/book3s_hv_nested.c
> >> > +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> >> > @@ -102,8 +102,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> >> > {
> >> > struct kvmppc_vcore *vc = vcpu->arch.vcore;
> >> >
> >> > + /*
> >> > + * When loading the hypervisor-privileged registers to run L2,
> >> > + * we might have used bits from L1 state to restrict what the
> >> > + * L2 state is allowed to be. Since L1 is not allowed to read
> >> > + * the HV registers, do not include these modifications in the
> >> > + * return state.
> >> > + */
> >> > + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
> >> > + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
> >> > +
> >> > hr->dpdes = vc->dpdes;
> >> > - hr->hfscr = vcpu->arch.hfscr;
> >> > hr->purr = vcpu->arch.purr;
> >> > hr->spurr = vcpu->arch.spurr;
> >> > hr->ic = vcpu->arch.ic;
> >>
> >> Do we still have the problem here that hfac interrupts due to bits cleared
> >> by the hfscr sanitisation would have the cause bits returned to the L1,
> >> so in theory it could probe hfscr directly that way? I don't see a good
> >> solution to this except either have the L0 intercept these faults and do
> >> "something" transparent, or return error from H_ENTER_NESTED (which would
> >> also allow trivial probing of the facilities).
> >
> > It seems to me that there are various specific reasons why L0 would
> > clear HFSCR bits, and if we think about the specific reasons, what we
> > should do becomes clear. (I say "L0" but in fact the same reasoning
> > applies to any hypervisor that lets its guest do hypervisor-ish
> > things.)
> >
> > 1. Emulating a version of the architecture which doesn't have the
> > feature in question - in that case the bit should appear to L1 as a
> > reserved bit in HFSCR (i.e. always read 0), the associated facility
> > code should never appear in the top 8 bits of any HFSCR value that L1
> > sees, and any HFU interrupt received by L0 for the facility should be
> > changed into an illegal instruction interrupt (or HEAI) forwarded to
> > L1. In this case the real HFSCR should always have the enable bit for
> > the facility set to 0.
> >
> > 2. Lazy save/restore of the state associated with a facility - in this
> > case, while the system is in the "lazy" state (i.e. the state is not
> > that of the currently running guest), the real HFSCR bit for the
> > facility should be 0. On an HFU interrupt for the facility, L0 looks
> > at L1's HFSCR value: if it's 0, forward the HFU interrupt to L1; if
> > it's 1, load up the facility state, set the facility's bit in HFSCR,
> > and resume the guest.
> >
> > 3. Emulating a facility in software - in this case, the real HFSCR
> > bit for the facility would always be 0. On an HFU interrupt, L0 reads
> > the instruction and emulates it, then resumes the guest.
> >
> > One thing this all makes clear is that the IC field of the "virtual"
> > HFSCR value seen by L1 should only ever be changed when L0 forwards a
> > HFU interrupt to L1.
> >
> > In fact we currently never do (1) or (2), and we only do (3) for
> > msgsndp etc., so this discussion is mostly theoretical.
>
> Yeah it's somewhat theoretical, and I guess I mostly agree with you.
>
> Missing is the case where the L0 does not implement a feature at all.
> Let's say TM is broken so it disables it, or nobody uses TAR so it
> doesn't bother to switch it.
I think that's the same as my case (1), where L0 is presenting an
architecture to L1 that doesn't implement the feature. (Unless you
mean that L0 is running on a machine that has features that didn't
exist at the time that L0's code was written; to handle that, L0
should clear all HFSCR bits that it doesn't know about, and map any
unknown HFSCR[IC] code into illegal interrupt/HEAI.)
> In those cases what do you tell the L1 if it enables a bit that you
> don't support at all, and it takes a fault?
Illegal instruction interrupt, or HEAI.
> I guess the right thing to do is advertise that to the guest by some
> other means, and expect it does the right thing. And you could have
> the proviso in the nested HV specification that the returned IC field
> might trip for a feature you enabled in the L1 HFSCR.
I think that is confusing; better to return illegal instruction/HEAI.
> >
> >> Returning an hfac interrupt to a hypervisor that thought it enabled the
> >> bit would be strange. But so does appearing to modify the register
> >> underneath it and then returning a fault.
> >
> > I don't think we should ever do either of those things. The closest
> > would be (1) above, but in that case the fault has to be either an
> > illegal instruction type program interrupt, or a HEAI.
> >
> >> I think the sanest thing would actually be to return failure from the
> >> hcall.
> >
> > I don't think we should do that either.
>
> I still think it's preferable for case 4. No point waiting for the
> guest to boot and some user program eventually hits a bad instruction,
> even if it was due to some host vs guest configuration problem.
If you let it boot to userspace, the administrator has a better chance
of recovering the situation.
Paul.
^ permalink raw reply
* [PATCH] powerpc/pmu: Make the generic compat PMU use the architected events
From: Paul Mackerras @ 2021-05-04 7:43 UTC (permalink / raw)
To: Madhavan Srinivasan, linuxppc-dev; +Cc: Michael Ellerman
This changes generic-compat-pmu.c so that it only uses architected
events defined in Power ISA v3.0B, rather than event encodings which,
while common to all the IBM Power Systems implementations, are
nevertheless implementation-specific rather than architected. The
intention is that any CPU implementation designed to conform to Power
ISA v3.0B or later can use generic-compat-pmu.c.
In addition to the existing events for cycles and instructions, this
adds several other architected events, including alternative encodings
for some events. In order to make it possible to measure cycles and
instructions at the same time as each other, we set the CC5-6RUN bit
in MMCR0, which makes PMC5 and PMC6 count instructions and cycles
regardless of the run bit, so their events are now PM_CYC and
PM_INST_CMPL rather than PM_RUN_CYC and PM_RUN_INST_CMPL (the latter
are still available via other event codes).
Note that POWER9 has an erratum where one architected event
(PM_FLOP_CMPL, floating-point operations completed, code 0x100f4) does
not work correctly. Given that there is a specific PMU driver for P9
which will be used in preference to generic-compat-pmu.c, that is not
a real problem.
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
---
arch/powerpc/perf/generic-compat-pmu.c | 170 +++++++++++++++++++------
1 file changed, 134 insertions(+), 36 deletions(-)
diff --git a/arch/powerpc/perf/generic-compat-pmu.c b/arch/powerpc/perf/generic-compat-pmu.c
index eb8a6aaf4cc1..695975227e60 100644
--- a/arch/powerpc/perf/generic-compat-pmu.c
+++ b/arch/powerpc/perf/generic-compat-pmu.c
@@ -14,45 +14,119 @@
*
* 28 24 20 16 12 8 4 0
* | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
- * [ pmc ] [unit ] [ ] m [ pmcxsel ]
- * | |
- * | *- mark
- * |
- * |
- * *- combine
- *
- * Below uses IBM bit numbering.
- *
- * MMCR1[x:y] = unit (PMCxUNIT)
- * MMCR1[24] = pmc1combine[0]
- * MMCR1[25] = pmc1combine[1]
- * MMCR1[26] = pmc2combine[0]
- * MMCR1[27] = pmc2combine[1]
- * MMCR1[28] = pmc3combine[0]
- * MMCR1[29] = pmc3combine[1]
- * MMCR1[30] = pmc4combine[0]
- * MMCR1[31] = pmc4combine[1]
- *
+ * [ pmc ] [ pmcxsel ]
*/
/*
- * Some power9 event codes.
+ * Event codes defined in ISA v3.0B
*/
#define EVENT(_name, _code) _name = _code,
enum {
-EVENT(PM_CYC, 0x0001e)
-EVENT(PM_INST_CMPL, 0x00002)
+ /* Cycles, alternate code */
+ EVENT(PM_CYC_ALT, 0x100f0)
+ /* One or more instructions completed in a cycle */
+ EVENT(PM_CYC_INST_CMPL, 0x100f2)
+ /* Floating-point instruction completed */
+ EVENT(PM_FLOP_CMPL, 0x100f4)
+ /* Instruction ERAT/L1-TLB miss */
+ EVENT(PM_L1_ITLB_MISS, 0x100f6)
+ /* All instructions completed and none available */
+ EVENT(PM_NO_INST_AVAIL, 0x100f8)
+ /* A load-type instruction completed (ISA v3.0+) */
+ EVENT(PM_LD_CMPL, 0x100fc)
+ /* Instruction completed, alternate code (ISA v3.0+) */
+ EVENT(PM_INST_CMPL_ALT, 0x100fe)
+ /* A store-type instruction completed */
+ EVENT(PM_ST_CMPL, 0x200f0)
+ /* Instruction Dispatched */
+ EVENT(PM_INST_DISP, 0x200f2)
+ /* Run_cycles */
+ EVENT(PM_RUN_CYC, 0x200f4)
+ /* Data ERAT/L1-TLB miss/reload */
+ EVENT(PM_L1_DTLB_RELOAD, 0x200f6)
+ /* Taken branch completed */
+ EVENT(PM_BR_TAKEN_CMPL, 0x200fa)
+ /* Demand iCache Miss */
+ EVENT(PM_L1_ICACHE_MISS, 0x200fc)
+ /* L1 Dcache reload from memory */
+ EVENT(PM_L1_RELOAD_FROM_MEM, 0x200fe)
+ /* L1 Dcache store miss */
+ EVENT(PM_ST_MISS_L1, 0x300f0)
+ /* Alternate code for PM_INST_DISP */
+ EVENT(PM_INST_DISP_ALT, 0x300f2)
+ /* Branch direction or target mispredicted */
+ EVENT(PM_BR_MISPREDICT, 0x300f6)
+ /* Data TLB miss/reload */
+ EVENT(PM_DTLB_MISS, 0x300fc)
+ /* Demand LD - L3 Miss (not L2 hit and not L3 hit) */
+ EVENT(PM_DATA_FROM_L3MISS, 0x300fe)
+ /* L1 Dcache load miss */
+ EVENT(PM_LD_MISS_L1, 0x400f0)
+ /* Cycle when instruction(s) dispatched */
+ EVENT(PM_CYC_INST_DISP, 0x400f2)
+ /* Branch or branch target mispredicted */
+ EVENT(PM_BR_MPRED_CMPL, 0x400f6)
+ /* Instructions completed with run latch set */
+ EVENT(PM_RUN_INST_CMPL, 0x400fa)
+ /* Instruction TLB miss/reload */
+ EVENT(PM_ITLB_MISS, 0x400fc)
+ /* Load data not cached */
+ EVENT(PM_LD_NOT_CACHED, 0x400fe)
+ /* Instructions */
+ EVENT(PM_INST_CMPL, 0x500fa)
+ /* Cycles */
+ EVENT(PM_CYC, 0x600f4)
};
#undef EVENT
+/* Table of alternatives, sorted in increasing order of column 0 */
+/* Note that in each row, column 0 must be the smallest */
+static const unsigned int generic_event_alternatives[][MAX_ALT] = {
+ { PM_CYC_ALT, PM_CYC },
+ { PM_INST_CMPL_ALT, PM_INST_CMPL },
+ { PM_INST_DISP, PM_INST_DISP_ALT },
+};
+
+static int generic_get_alternatives(u64 event, unsigned int flags, u64 alt[])
+{
+ int num_alt = 0;
+
+ num_alt = isa207_get_alternatives(event, alt,
+ ARRAY_SIZE(generic_event_alternatives), flags,
+ generic_event_alternatives);
+
+ return num_alt;
+}
+
GENERIC_EVENT_ATTR(cpu-cycles, PM_CYC);
GENERIC_EVENT_ATTR(instructions, PM_INST_CMPL);
+GENERIC_EVENT_ATTR(stalled-cycles-frontend, PM_NO_INST_AVAIL);
+GENERIC_EVENT_ATTR(branch-misses, PM_BR_MPRED_CMPL);
+GENERIC_EVENT_ATTR(cache-misses, PM_LD_MISS_L1);
+
+CACHE_EVENT_ATTR(L1-dcache-load-misses, PM_LD_MISS_L1);
+CACHE_EVENT_ATTR(L1-dcache-store-misses, PM_ST_MISS_L1);
+CACHE_EVENT_ATTR(L1-icache-load-misses, PM_L1_ICACHE_MISS);
+CACHE_EVENT_ATTR(LLC-load-misses, PM_DATA_FROM_L3MISS);
+CACHE_EVENT_ATTR(branch-load-misses, PM_BR_MPRED_CMPL);
+CACHE_EVENT_ATTR(dTLB-load-misses, PM_DTLB_MISS);
+CACHE_EVENT_ATTR(iTLB-load-misses, PM_ITLB_MISS);
static struct attribute *generic_compat_events_attr[] = {
GENERIC_EVENT_PTR(PM_CYC),
GENERIC_EVENT_PTR(PM_INST_CMPL),
+ GENERIC_EVENT_PTR(PM_NO_INST_AVAIL),
+ GENERIC_EVENT_PTR(PM_BR_MPRED_CMPL),
+ GENERIC_EVENT_PTR(PM_LD_MISS_L1),
+ CACHE_EVENT_PTR(PM_LD_MISS_L1),
+ CACHE_EVENT_PTR(PM_ST_MISS_L1),
+ CACHE_EVENT_PTR(PM_L1_ICACHE_MISS),
+ CACHE_EVENT_PTR(PM_DATA_FROM_L3MISS),
+ CACHE_EVENT_PTR(PM_BR_MPRED_CMPL),
+ CACHE_EVENT_PTR(PM_DTLB_MISS),
+ CACHE_EVENT_PTR(PM_ITLB_MISS),
NULL
};
@@ -63,17 +137,11 @@ static struct attribute_group generic_compat_pmu_events_group = {
PMU_FORMAT_ATTR(event, "config:0-19");
PMU_FORMAT_ATTR(pmcxsel, "config:0-7");
-PMU_FORMAT_ATTR(mark, "config:8");
-PMU_FORMAT_ATTR(combine, "config:10-11");
-PMU_FORMAT_ATTR(unit, "config:12-15");
PMU_FORMAT_ATTR(pmc, "config:16-19");
static struct attribute *generic_compat_pmu_format_attr[] = {
&format_attr_event.attr,
&format_attr_pmcxsel.attr,
- &format_attr_mark.attr,
- &format_attr_combine.attr,
- &format_attr_unit.attr,
&format_attr_pmc.attr,
NULL,
};
@@ -92,6 +160,9 @@ static const struct attribute_group *generic_compat_pmu_attr_groups[] = {
static int compat_generic_events[] = {
[PERF_COUNT_HW_CPU_CYCLES] = PM_CYC,
[PERF_COUNT_HW_INSTRUCTIONS] = PM_INST_CMPL,
+ [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = PM_NO_INST_AVAIL,
+ [PERF_COUNT_HW_BRANCH_MISSES] = PM_BR_MPRED_CMPL,
+ [PERF_COUNT_HW_CACHE_MISSES] = PM_LD_MISS_L1,
};
#define C(x) PERF_COUNT_HW_CACHE_##x
@@ -105,11 +176,11 @@ static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[ C(L1D) ] = {
[ C(OP_READ) ] = {
[ C(RESULT_ACCESS) ] = 0,
- [ C(RESULT_MISS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_LD_MISS_L1,
},
[ C(OP_WRITE) ] = {
[ C(RESULT_ACCESS) ] = 0,
- [ C(RESULT_MISS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_ST_MISS_L1,
},
[ C(OP_PREFETCH) ] = {
[ C(RESULT_ACCESS) ] = 0,
@@ -119,7 +190,7 @@ static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[ C(L1I) ] = {
[ C(OP_READ) ] = {
[ C(RESULT_ACCESS) ] = 0,
- [ C(RESULT_MISS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_L1_ICACHE_MISS,
},
[ C(OP_WRITE) ] = {
[ C(RESULT_ACCESS) ] = 0,
@@ -133,7 +204,7 @@ static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[ C(LL) ] = {
[ C(OP_READ) ] = {
[ C(RESULT_ACCESS) ] = 0,
- [ C(RESULT_MISS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_DATA_FROM_L3MISS,
},
[ C(OP_WRITE) ] = {
[ C(RESULT_ACCESS) ] = 0,
@@ -147,7 +218,7 @@ static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[ C(DTLB) ] = {
[ C(OP_READ) ] = {
[ C(RESULT_ACCESS) ] = 0,
- [ C(RESULT_MISS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_DTLB_MISS,
},
[ C(OP_WRITE) ] = {
[ C(RESULT_ACCESS) ] = -1,
@@ -161,7 +232,7 @@ static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[ C(ITLB) ] = {
[ C(OP_READ) ] = {
[ C(RESULT_ACCESS) ] = 0,
- [ C(RESULT_MISS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_ITLB_MISS,
},
[ C(OP_WRITE) ] = {
[ C(RESULT_ACCESS) ] = -1,
@@ -175,7 +246,7 @@ static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
[ C(BPU) ] = {
[ C(OP_READ) ] = {
[ C(RESULT_ACCESS) ] = 0,
- [ C(RESULT_MISS) ] = 0,
+ [ C(RESULT_MISS) ] = PM_BR_MPRED_CMPL,
},
[ C(OP_WRITE) ] = {
[ C(RESULT_ACCESS) ] = -1,
@@ -204,13 +275,30 @@ static u64 generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
#undef C
+/*
+ * We set MMCR0[CC5-6RUN] so we can use counters 5 and 6 for
+ * PM_INST_CMPL and PM_CYC.
+ */
+static int generic_compute_mmcr(u64 event[], int n_ev,
+ unsigned int hwc[], struct mmcr_regs *mmcr,
+ struct perf_event *pevents[], u32 flags)
+{
+ int ret;
+
+ ret = isa207_compute_mmcr(event, n_ev, hwc, mmcr, pevents, flags);
+ if (!ret)
+ mmcr->mmcr0 |= MMCR0_C56RUN;
+ return ret;
+}
+
static struct power_pmu generic_compat_pmu = {
.name = "GENERIC_COMPAT",
.n_counter = MAX_PMU_COUNTERS,
.add_fields = ISA207_ADD_FIELDS,
.test_adder = ISA207_TEST_ADDER,
- .compute_mmcr = isa207_compute_mmcr,
+ .compute_mmcr = generic_compute_mmcr,
.get_constraint = isa207_get_constraint,
+ .get_alternatives = generic_get_alternatives,
.disable_pmc = isa207_disable_pmc,
.flags = PPMU_HAS_SIER | PPMU_ARCH_207S,
.n_generic = ARRAY_SIZE(compat_generic_events),
@@ -223,6 +311,16 @@ int init_generic_compat_pmu(void)
{
int rc = 0;
+ /*
+ * From ISA v2.07 on, PMU features are architected;
+ * we require >= v3.0 because (a) that has PM_LD_CMPL and
+ * PM_INST_CMPL_ALT, which v2.07 doesn't have, and
+ * (b) we don't expect any non-IBM Power ISA
+ * implementations that conform to v2.07 but not v3.0.
+ */
+ if (!cpu_has_feature(CPU_FTR_ARCH_300))
+ return -ENODEV;
+
rc = register_power_pmu(&generic_compat_pmu);
if (rc)
return rc;
--
2.26.2
^ permalink raw reply related
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christophe Leroy @ 2021-05-04 7:47 UTC (permalink / raw)
To: Christian Zigotzky, Michael Ellerman
Cc: Darren Stevens, linuxppc-dev, mad skateman, R.T.Dickinson,
Christian Zigotzky
In-Reply-To: <9864cd72-f1aa-4cf5-1cda-b3a10233b24d@xenosoft.de>
Hi
Le 04/05/2021 à 09:21, Christian Zigotzky a écrit :
> Hi Christophe,
>
> Thanks for your answer but I think I don't know how it works with the cherry-pick.
>
> $ git bisect start
As you suspect the problem to be specific to powerpc, I can do
git bisect start -- arch/powerpc
> $ git bisect good 68a32ba14177d4a21c4a9a941cf1d7aea86d436f
> $ git bisect bad c70a4be130de333ea079c59da41cc959712bb01c
You said that powerpc-5.13-1 is bad so you can narrow the search I think:
git bisect bad powerpc-5.13-1
git bisect good 887f3ceb51cd3~
>
> Bisecting: 2462 revisions left to test after this (roughly 11 steps)
> [47a6959fa331fe892a4fc3b48ca08e92045c6bda] netfilter: allow to turn off xtables compat layer
>
> $ git cherry-pick 525642624783
> error: could not apply 525642624783... powerpc/signal32: Fix erroneous SIGSEGV on RT signal return
> hint: after resolving the conflicts, mark the corrected paths
> hint: with 'git add <paths>' or 'git rm <paths>'
> hint: and commit the result with 'git commit'
>
> How can I fix this error?
This problably means that the step is at a commit which is prior to the first bad commit you
identified at previous step. If you narrow the bisect as explained above, it shouldn't happen unless
git decides it needs to descend a branch to a merge point. In that case just do 'git cherry-pick
--abort" and go without the fix.
Note that once you have cherry picked the fix and tested the result, you have to apply the result to
HEAD~ (the commit before the cherry-pick).
- git bisect good HEAD~
- git bisect bad HEAD~
Christophe
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-04 8:29 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman
Cc: Darren Stevens, linuxppc-dev, mad skateman, R.T.Dickinson,
Christian Zigotzky
In-Reply-To: <1b0307be-05cd-ab62-8b22-75ffb59ff76b@csgroup.eu>
On 04 May 2021 at 09:47am, Christophe Leroy wrote:
> Hi
>
> Le 04/05/2021 à 09:21, Christian Zigotzky a écrit :
>> Hi Christophe,
>>
>> Thanks for your answer but I think I don't know how it works with the
>> cherry-pick.
>>
>> $ git bisect start
>
> As you suspect the problem to be specific to powerpc, I can do
>
> git bisect start -- arch/powerpc
>
>
>> $ git bisect good 68a32ba14177d4a21c4a9a941cf1d7aea86d436f
>> $ git bisect bad c70a4be130de333ea079c59da41cc959712bb01c
>
> You said that powerpc-5.13-1 is bad so you can narrow the search I think:
>
> git bisect bad powerpc-5.13-1
> git bisect good 887f3ceb51cd3~
I tried it but without any success.
git bisect bad powerpc-5.13-1
Output:
fatal: Needed a single revision
Bad rev input: powerpc-5.13-1
Maybe we should look in the PowerPC updates directly. The CPUs of the
AmigaOne X5000 and virtual e5500 QEMU machine belong to BookE cpu
family. The AmigaOne X1000 isn't affected by this issue because the PA6T
belongs to the Book3S cpu family. [1]
I found this in the PowerPC updates 5.13-1: - Convert 64-bit BookE to
do interrupt entry/exit in C.
Maybe we should look more in the modified BookE files:
arch/powerpc/kernel/head_booke.h [2]
arch/powerpc/kernel/head_fsl_booke.S [3]
Please check the BookE commits in the PowerPC updates 5.13-1. You don't
need an AmigaOne X5000 for testing because the virtual e5500 QEMU
machine is also affected.
Thanks,
Christian
[1] https://www.kernel.org/doc/Documentation/powerpc/cpu_families.txt
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/arch/powerpc/kernel/head_booke.h?id=c70a4be130de333ea079c59da41cc959712bb01c
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/arch/powerpc/kernel/head_fsl_booke.S?id=c70a4be130de333ea079c59da41cc959712bb01c
>
>
>>
>> Bisecting: 2462 revisions left to test after this (roughly 11 steps)
>> [47a6959fa331fe892a4fc3b48ca08e92045c6bda] netfilter: allow to turn
>> off xtables compat layer
>>
>> $ git cherry-pick 525642624783
>> error: could not apply 525642624783... powerpc/signal32: Fix
>> erroneous SIGSEGV on RT signal return
>> hint: after resolving the conflicts, mark the corrected paths
>> hint: with 'git add <paths>' or 'git rm <paths>'
>> hint: and commit the result with 'git commit'
>>
>> How can I fix this error?
>
> This problably means that the step is at a commit which is prior to
> the first bad commit you identified at previous step. If you narrow
> the bisect as explained above, it shouldn't happen unless git decides
> it needs to descend a branch to a merge point. In that case just do
> 'git cherry-pick --abort" and go without the fix.
>
> Note that once you have cherry picked the fix and tested the result,
> you have to apply the result to HEAD~ (the commit before the
> cherry-pick).
> - git bisect good HEAD~
> - git bisect bad HEAD~
>
> Christophe
^ permalink raw reply
* Re: [PATCH v3 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Nicholas Piggin @ 2021-05-04 8:35 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, Fabiano Rosas
In-Reply-To: <YJD5lwY4JXyS1VgH@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of May 4, 2021 5:36 pm:
> On Tue, May 04, 2021 at 03:26:24PM +1000, Nicholas Piggin wrote:
>> Excerpts from Paul Mackerras's message of May 4, 2021 2:28 pm:
>> > On Sat, May 01, 2021 at 11:58:36AM +1000, Nicholas Piggin wrote:
>> >> Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
>> >> > As one of the arguments of the H_ENTER_NESTED hypercall, the nested
>> >> > hypervisor (L1) prepares a structure containing the values of various
>> >> > hypervisor-privileged registers with which it wants the nested guest
>> >> > (L2) to run. Since the nested HV runs in supervisor mode it needs the
>> >> > host to write to these registers.
>> >> >
>> >> > To stop a nested HV manipulating this mechanism and using a nested
>> >> > guest as a proxy to access a facility that has been made unavailable
>> >> > to it, we have a routine that sanitises the values of the HV registers
>> >> > before copying them into the nested guest's vcpu struct.
>> >> >
>> >> > However, when coming out of the guest the values are copied as they
>> >> > were back into L1 memory, which means that any sanitisation we did
>> >> > during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
>> >> >
>> >> > This patch alters this sanitisation to have effect on the vcpu->arch
>> >> > registers directly before entering and after exiting the guest,
>> >> > leaving the structure that is copied back into L1 unchanged (except
>> >> > when we really want L1 to access the value, e.g the Cause bits of
>> >> > HFSCR).
>> >> >
>> >> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> >> > ---
>> >> > arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++-----------
>> >> > 1 file changed, 34 insertions(+), 21 deletions(-)
>> >> >
>> >> > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
>> >> > index 0cd0e7aad588..270552dd42c5 100644
>> >> > --- a/arch/powerpc/kvm/book3s_hv_nested.c
>> >> > +++ b/arch/powerpc/kvm/book3s_hv_nested.c
>> >> > @@ -102,8 +102,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
>> >> > {
>> >> > struct kvmppc_vcore *vc = vcpu->arch.vcore;
>> >> >
>> >> > + /*
>> >> > + * When loading the hypervisor-privileged registers to run L2,
>> >> > + * we might have used bits from L1 state to restrict what the
>> >> > + * L2 state is allowed to be. Since L1 is not allowed to read
>> >> > + * the HV registers, do not include these modifications in the
>> >> > + * return state.
>> >> > + */
>> >> > + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
>> >> > + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
>> >> > +
>> >> > hr->dpdes = vc->dpdes;
>> >> > - hr->hfscr = vcpu->arch.hfscr;
>> >> > hr->purr = vcpu->arch.purr;
>> >> > hr->spurr = vcpu->arch.spurr;
>> >> > hr->ic = vcpu->arch.ic;
>> >>
>> >> Do we still have the problem here that hfac interrupts due to bits cleared
>> >> by the hfscr sanitisation would have the cause bits returned to the L1,
>> >> so in theory it could probe hfscr directly that way? I don't see a good
>> >> solution to this except either have the L0 intercept these faults and do
>> >> "something" transparent, or return error from H_ENTER_NESTED (which would
>> >> also allow trivial probing of the facilities).
>> >
>> > It seems to me that there are various specific reasons why L0 would
>> > clear HFSCR bits, and if we think about the specific reasons, what we
>> > should do becomes clear. (I say "L0" but in fact the same reasoning
>> > applies to any hypervisor that lets its guest do hypervisor-ish
>> > things.)
>> >
>> > 1. Emulating a version of the architecture which doesn't have the
>> > feature in question - in that case the bit should appear to L1 as a
>> > reserved bit in HFSCR (i.e. always read 0), the associated facility
>> > code should never appear in the top 8 bits of any HFSCR value that L1
>> > sees, and any HFU interrupt received by L0 for the facility should be
>> > changed into an illegal instruction interrupt (or HEAI) forwarded to
>> > L1. In this case the real HFSCR should always have the enable bit for
>> > the facility set to 0.
>> >
>> > 2. Lazy save/restore of the state associated with a facility - in this
>> > case, while the system is in the "lazy" state (i.e. the state is not
>> > that of the currently running guest), the real HFSCR bit for the
>> > facility should be 0. On an HFU interrupt for the facility, L0 looks
>> > at L1's HFSCR value: if it's 0, forward the HFU interrupt to L1; if
>> > it's 1, load up the facility state, set the facility's bit in HFSCR,
>> > and resume the guest.
>> >
>> > 3. Emulating a facility in software - in this case, the real HFSCR
>> > bit for the facility would always be 0. On an HFU interrupt, L0 reads
>> > the instruction and emulates it, then resumes the guest.
>> >
>> > One thing this all makes clear is that the IC field of the "virtual"
>> > HFSCR value seen by L1 should only ever be changed when L0 forwards a
>> > HFU interrupt to L1.
>> >
>> > In fact we currently never do (1) or (2), and we only do (3) for
>> > msgsndp etc., so this discussion is mostly theoretical.
>>
>> Yeah it's somewhat theoretical, and I guess I mostly agree with you.
>>
>> Missing is the case where the L0 does not implement a feature at all.
>> Let's say TM is broken so it disables it, or nobody uses TAR so it
>> doesn't bother to switch it.
>
> I think that's the same as my case (1), where L0 is presenting an
> architecture to L1 that doesn't implement the feature. (Unless you
> mean that L0 is running on a machine that has features that didn't
> exist at the time that L0's code was written; to handle that, L0
> should clear all HFSCR bits that it doesn't know about, and map any
> unknown HFSCR[IC] code into illegal interrupt/HEAI.)
I don't think it's quite either of those. There is no ISA v3.0 where the
target address register facility does not exist, and the L0 is not going
to emulate it, for argument's sake.
>> In those cases what do you tell the L1 if it enables a bit that you
>> don't support at all, and it takes a fault?
>
> Illegal instruction interrupt, or HEAI.
Okay, so that's just a roundabout way to tell the nested HV "this
feature doesn't exist", asynchronously at some point when its guest
tries to use the facility. It's contrary to the architecture it
expected (maybe wrongly because it didn't query the right capability)
but it set HFSCR for the facility so it thought it should work.
>> I guess the right thing to do is advertise that to the guest by some
>> other means, and expect it does the right thing. And you could have
>> the proviso in the nested HV specification that the returned IC field
>> might trip for a feature you enabled in the L1 HFSCR.
>
> I think that is confusing; better to return illegal instruction/HEAI.
I think both are more confusing than returning error from the hcall.
>
>> >
>> >> Returning an hfac interrupt to a hypervisor that thought it enabled the
>> >> bit would be strange. But so does appearing to modify the register
>> >> underneath it and then returning a fault.
>> >
>> > I don't think we should ever do either of those things. The closest
>> > would be (1) above, but in that case the fault has to be either an
>> > illegal instruction type program interrupt, or a HEAI.
>> >
>> >> I think the sanest thing would actually be to return failure from the
>> >> hcall.
>> >
>> > I don't think we should do that either.
>>
>> I still think it's preferable for case 4. No point waiting for the
>> guest to boot and some user program eventually hits a bad instruction,
>> even if it was due to some host vs guest configuration problem.
>
> If you let it boot to userspace, the administrator has a better chance
> of recovering the situation.
Maybe. What about if it runs userspace until something executes
pthread_mutex_lock
An error message when you try to start the nested guest telling you
pass -machine cap-htm=off would be better... I guess that should
really all work with caps etc today though so TM's a bad example.
But assume we don't have a cap for the bit we disable? Maybe we
should have caps for all HFSCR bits, or I'm just worried about
something not very important.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Miguel Ojeda @ 2021-05-04 8:38 UTC (permalink / raw)
To: Ben Dooks
Cc: Albert Ou, Arnd Bergmann, Linux Kbuild mailing list,
Greg Kroah-Hartman, Masahiro Yamada, Jonathan Corbet,
Linux Doc Mailing List, linux-kernel, Miguel Ojeda, Will Deacon,
Palmer Dabbelt, Paul Walmsley, Catalin Marinas, Joe Perches,
Paul Mackerras, linux-riscv, linuxppc-dev, Linus Torvalds,
Linux ARM
In-Reply-To: <65cda2bb-1b02-6ebc-0ea2-c48927524aa0@codethink.co.uk>
On Tue, May 4, 2021 at 9:57 AM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> Some of us are a bit stuck as either customer refuses to upgrade
> their build infrastructure or has paid for some old but safety
> blessed version of gcc. These often lag years behind the recent
> gcc releases :(
In those scenarios, why do you need to build mainline? Aren't your
customers using longterm or frozen kernels? If they are paying for
certified GCC images, aren't they already paying for supported kernel
images from some vendor too?
I understand where you are coming from -- I have also dealt with
projects/machines running ancient, unsupported software/toolchains for
various reasons; but nobody expected upstream (and in particular the
mainline kernel source) to support them. In the cases I experienced,
those use cases require not touching anything at all, and when the
time came of doing so, everything would be updated at once,
re-certified/validated as needed and frozen again.
Cheers,
Miguel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox