[PATCH] Fix the message in facility unavailable exception

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3565d

2 messages, 2 authors, 2016-11-07 · open the first message on its own page

[PATCH] Fix the message in facility unavailable exception

From: Balbir Singh <bsingharora@gmail.com>
Date: 2016-11-05 04:14:25

I ran into this during some testing on qemu. The current
facility_strings[] are correct when the trap address is
0xf80 (hypervisor facility unavailable). When the trap
address is 0xf60, IC (Interruption Cause) a.k.a status
in the code is undefined for values 0 and 1. This patch
adds a check to prevent printing the wrong information
and helps better direct debugging effort.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/kernel/traps.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d26605d..da0f634 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1520,8 +1520,14 @@ void facility_unavailable_exception(struct pt_regs *regs)
 	}
 
 	if ((status < ARRAY_SIZE(facility_strings)) &&
-	    facility_strings[status])
-		facility = facility_strings[status];
+	    facility_strings[status]) {
+		if (!hv && status < 2) {
+			pr_warn("Unexpected facility unavailable exception "
+				"interruption cause %d\n", status);
+			facility = "Unknown";
+		} else
+			facility = facility_strings[status];
+	}
 
 	/* We restore the interrupt state now */
 	if (!arch_irq_disabled_regs(regs))
-- 
2.5.5

Re: [PATCH] Fix the message in facility unavailable exception

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-11-07 07:53:44

Balbir Singh [off-list ref] writes:
I ran into this during some testing on qemu. The current
facility_strings[] are correct when the trap address is
0xf80 (hypervisor facility unavailable). When the trap
address is 0xf60, IC (Interruption Cause) a.k.a status
in the code is undefined for values 0 and 1.
OK. But how did you generate an exception with an undefined status code?
quoted hunk
This patch
adds a check to prevent printing the wrong information
and helps better direct debugging effort.
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d26605d..da0f634 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1520,8 +1520,14 @@ void facility_unavailable_exception(struct pt_regs *regs)
 	}
 
 	if ((status < ARRAY_SIZE(facility_strings)) &&
-	    facility_strings[status])
-		facility = facility_strings[status];
+	    facility_strings[status]) {
+		if (!hv && status < 2) {
+			pr_warn("Unexpected facility unavailable exception "
+				"interruption cause %d\n", status);
Please don't add un-ratelimited printks() in this function, otherwise if
they're user triggerable (which some are) it gives the user a way to
scrub the kernel log.
+			facility = "Unknown";
+		} else
+			facility = facility_strings[status];
+	}
I think we should instead tighten the condition on that top-level if, and
have an else clause for all cases that uses "Unknown". eg.

	if ((hv || status >= 2) &&
            (status < ARRAY_SIZE(facility_strings)) &&
	    facility_strings[status])
        {
		facility = facility_strings[status];
        } else {
		facility = "Unknown";
        }

And then if you want to we can also print the hex status value in the
existing printk().

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help