LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping()
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack
In-Reply-To: <20210406093557.1073423-1-maz@kernel.org>

Instead of playing games with using irq_create_identity_mapping()
and irq_domain_associate(), drop the use of the former and only
use the latter, together with the allocation of the irq_desc
as needed.

It doesn't make the code less awful, but at least the intent
is clearer.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/sh/intc/core.c | 50 ++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
index a14684ffe4c1..6c57ee1ce6c4 100644
--- a/drivers/sh/intc/core.c
+++ b/drivers/sh/intc/core.c
@@ -179,6 +179,23 @@ static unsigned int __init save_reg(struct intc_desc_int *d,
 	return 0;
 }
 
+static bool __init intc_map(struct irq_domain *domain, int irq)
+{
+	int res;
+
+	if (!irq_to_desc(irq) && irq_alloc_desc_at(irq, NUMA_NO_NODE) != irq) {
+		pr_err("uname to allocate IRQ %d\n", irq);
+		return false;
+	}
+
+	if (irq_domain_associate(domain, irq, irq)) {
+		pr_err("domain association failure\n");
+		return false;
+	}
+
+	return true;
+}
+
 int __init register_intc_controller(struct intc_desc *desc)
 {
 	unsigned int i, k, smp;
@@ -316,19 +333,8 @@ int __init register_intc_controller(struct intc_desc *desc)
 		if (!vect->enum_id)
 			continue;
 
-		res = irq_create_identity_mapping(d->domain, irq);
-		if (unlikely(res)) {
-			if (res == -EEXIST) {
-				res = irq_domain_associate(d->domain, irq, irq);
-				if (unlikely(res)) {
-					pr_err("domain association failure\n");
-					continue;
-				}
-			} else {
-				pr_err("can't identity map IRQ %d\n", irq);
-				continue;
-			}
-		}
+		if (!intc_map(d->domain, irq))
+			continue;
 
 		intc_irq_xlate_set(irq, vect->enum_id, d);
 		intc_register_irq(desc, d, vect->enum_id, irq);
@@ -345,22 +351,8 @@ int __init register_intc_controller(struct intc_desc *desc)
 			 * IRQ support, each vector still needs to have
 			 * its own backing irq_desc.
 			 */
-			res = irq_create_identity_mapping(d->domain, irq2);
-			if (unlikely(res)) {
-				if (res == -EEXIST) {
-					res = irq_domain_associate(d->domain,
-								   irq2, irq2);
-					if (unlikely(res)) {
-						pr_err("domain association "
-						       "failure\n");
-						continue;
-					}
-				} else {
-					pr_err("can't identity map IRQ %d\n",
-					       irq);
-					continue;
-				}
-			}
+			if (!intc_map(d->domain, irq2))
+				continue;
 
 			vect2->enum_id = 0;
 
-- 
2.29.2


^ permalink raw reply related

* [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack
In-Reply-To: <20210406093557.1073423-1-maz@kernel.org>

Use the generic irq_domain_simple_ops structure instead of
a home-grown one.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/mips/netlogic/common/irq.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/mips/netlogic/common/irq.c b/arch/mips/netlogic/common/irq.c
index cf33dd8a487e..c25a2ce5e29f 100644
--- a/arch/mips/netlogic/common/irq.c
+++ b/arch/mips/netlogic/common/irq.c
@@ -276,10 +276,6 @@ asmlinkage void plat_irq_dispatch(void)
 }
 
 #ifdef CONFIG_CPU_XLP
-static const struct irq_domain_ops xlp_pic_irq_domain_ops = {
-	.xlate = irq_domain_xlate_onetwocell,
-};
-
 static int __init xlp_of_pic_init(struct device_node *node,
 					struct device_node *parent)
 {
@@ -324,7 +320,7 @@ static int __init xlp_of_pic_init(struct device_node *node,
 
 	xlp_pic_domain = irq_domain_add_legacy(node, n_picirqs,
 		nlm_irq_to_xirq(socid, PIC_IRQ_BASE), PIC_IRQ_BASE,
-		&xlp_pic_irq_domain_ops, NULL);
+		&irq_domain_simple_ops, NULL);
 	if (xlp_pic_domain == NULL) {
 		pr_err("PIC %pOFn: Creating legacy domain failed!\n", node);
 		return -EINVAL;
-- 
2.29.2


^ permalink raw reply related

* [PATCH 7/9] irqdomain: Drop references to recusive irqdomain setup
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack
In-Reply-To: <20210406093557.1073423-1-maz@kernel.org>

It was never completely implemented, and was removed a long time
ago. Adjust the documentation to reflect this.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 kernel/irq/irqdomain.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 0ba761e6b0a8..89474aa88220 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1659,12 +1659,10 @@ void irq_domain_free_irqs(unsigned int virq, unsigned int nr_irqs)
 
 /**
  * irq_domain_alloc_irqs_parent - Allocate interrupts from parent domain
+ * @domain:	Domain below which interrupts must be allocated
  * @irq_base:	Base IRQ number
  * @nr_irqs:	Number of IRQs to allocate
  * @arg:	Allocation data (arch/domain specific)
- *
- * Check whether the domain has been setup recursive. If not allocate
- * through the parent domain.
  */
 int irq_domain_alloc_irqs_parent(struct irq_domain *domain,
 				 unsigned int irq_base, unsigned int nr_irqs,
@@ -1680,11 +1678,9 @@ EXPORT_SYMBOL_GPL(irq_domain_alloc_irqs_parent);
 
 /**
  * irq_domain_free_irqs_parent - Free interrupts from parent domain
+ * @domain:	Domain below which interrupts must be freed
  * @irq_base:	Base IRQ number
  * @nr_irqs:	Number of IRQs to free
- *
- * Check whether the domain has been setup recursive. If not free
- * through the parent domain.
  */
 void irq_domain_free_irqs_parent(struct irq_domain *domain,
 				 unsigned int irq_base, unsigned int nr_irqs)
-- 
2.29.2


^ permalink raw reply related

* [PATCH 5/9] irqdomain: Kill irq_create_strict_mappings()/irq_create_identity_mapping()
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack
In-Reply-To: <20210406093557.1073423-1-maz@kernel.org>

No user of these APIs are left, remove them.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/irqdomain.h |  9 ---------
 kernel/irq/irqdomain.c    | 35 -----------------------------------
 2 files changed, 44 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index b9600f24878a..3997ed9e4d7d 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -411,15 +411,6 @@ static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
 }
 
 extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
-extern int irq_create_strict_mappings(struct irq_domain *domain,
-				      unsigned int irq_base,
-				      irq_hw_number_t hwirq_base, int count);
-
-static inline int irq_create_identity_mapping(struct irq_domain *host,
-					      irq_hw_number_t hwirq)
-{
-	return irq_create_strict_mappings(host, hwirq, hwirq, 1);
-}
 
 extern const struct irq_domain_ops irq_domain_simple_ops;
 
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index dfa716305ea9..0ba761e6b0a8 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -703,41 +703,6 @@ unsigned int irq_create_mapping_affinity(struct irq_domain *domain,
 }
 EXPORT_SYMBOL_GPL(irq_create_mapping_affinity);
 
-/**
- * irq_create_strict_mappings() - Map a range of hw irqs to fixed linux irqs
- * @domain: domain owning the interrupt range
- * @irq_base: beginning of linux IRQ range
- * @hwirq_base: beginning of hardware IRQ range
- * @count: Number of interrupts to map
- *
- * This routine is used for allocating and mapping a range of hardware
- * irqs to linux irqs where the linux irq numbers are at pre-defined
- * locations. For use by controllers that already have static mappings
- * to insert in to the domain.
- *
- * Non-linear users can use irq_create_identity_mapping() for IRQ-at-a-time
- * domain insertion.
- *
- * 0 is returned upon success, while any failure to establish a static
- * mapping is treated as an error.
- */
-int irq_create_strict_mappings(struct irq_domain *domain, unsigned int irq_base,
-			       irq_hw_number_t hwirq_base, int count)
-{
-	struct device_node *of_node;
-	int ret;
-
-	of_node = irq_domain_get_of_node(domain);
-	ret = irq_alloc_descs(irq_base, irq_base, count,
-			      of_node_to_nid(of_node));
-	if (unlikely(ret < 0))
-		return ret;
-
-	irq_domain_associate_many(domain, irq_base, hwirq_base, count);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(irq_create_strict_mappings);
-
 static int irq_domain_translate(struct irq_domain *d,
 				struct irq_fwspec *fwspec,
 				irq_hw_number_t *hwirq, unsigned int *type)
-- 
2.29.2


^ permalink raw reply related

* [PATCH 8/9] powerpc: Convert irq_domain_add_legacy_isa use to irq_domain_add_legacy
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack
In-Reply-To: <20210406093557.1073423-1-maz@kernel.org>

irq_domain_add_legacy_isa is a pain. It only exists for the benefit of
two PPC-specific drivers, and creates an ugly dependency between asm/irq.h
and linux/irqdomain.h

Instead, let's convert these two drivers to irq_domain_add_legacy(),
stop using NUM_ISA_INTERRUPTS by directly setting NR_IRQS_LEGACY.

The dependency cannot be broken yet as there is a lot of PPC-related
code that depends on it, but that's the first step towards it.

A followup patch will remove irq_domain_add_legacy_isa.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/powerpc/include/asm/irq.h         | 4 ++--
 arch/powerpc/platforms/ps3/interrupt.c | 4 ++--
 arch/powerpc/sysdev/i8259.c            | 3 ++-
 arch/powerpc/sysdev/mpic.c             | 2 +-
 arch/powerpc/sysdev/tsi108_pci.c       | 3 ++-
 arch/powerpc/sysdev/xics/xics-common.c | 2 +-
 6 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index f3f264e441a7..aeb209144c68 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -23,8 +23,8 @@ extern atomic_t ppc_n_lost_interrupts;
 /* Total number of virq in the platform */
 #define NR_IRQS		CONFIG_NR_IRQS
 
-/* Same thing, used by the generic IRQ code */
-#define NR_IRQS_LEGACY		NUM_ISA_INTERRUPTS
+/* Number of irqs reserved for a legacy isa controller */
+#define NR_IRQS_LEGACY		16
 
 extern irq_hw_number_t virq_to_hw(unsigned int virq);
 
diff --git a/arch/powerpc/platforms/ps3/interrupt.c b/arch/powerpc/platforms/ps3/interrupt.c
index 78f2339ed5cb..93e367a00452 100644
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -45,7 +45,7 @@
  * implementation equates HV plug value to Linux virq value, constrains each
  * interrupt to have a system wide unique plug number, and limits the range
  * of the plug values to map into the first dword of the bitmaps.  This
- * gives a usable range of plug values of  {NUM_ISA_INTERRUPTS..63}.  Note
+ * gives a usable range of plug values of  {NR_IRQS_LEGACY..63}.  Note
  * that there is no constraint on how many in this set an individual thread
  * can acquire.
  *
@@ -721,7 +721,7 @@ static unsigned int ps3_get_irq(void)
 	}
 
 #if defined(DEBUG)
-	if (unlikely(plug < NUM_ISA_INTERRUPTS || plug > PS3_PLUG_MAX)) {
+	if (unlikely(plug < NR_IRQS_LEGACY || plug > PS3_PLUG_MAX)) {
 		dump_bmp(&per_cpu(ps3_private, 0));
 		dump_bmp(&per_cpu(ps3_private, 1));
 		BUG();
diff --git a/arch/powerpc/sysdev/i8259.c b/arch/powerpc/sysdev/i8259.c
index c1d76c344351..dc1a151c63d7 100644
--- a/arch/powerpc/sysdev/i8259.c
+++ b/arch/powerpc/sysdev/i8259.c
@@ -260,7 +260,8 @@ void i8259_init(struct device_node *node, unsigned long intack_addr)
 	raw_spin_unlock_irqrestore(&i8259_lock, flags);
 
 	/* create a legacy host */
-	i8259_host = irq_domain_add_legacy_isa(node, &i8259_host_ops, NULL);
+	i8259_host = irq_domain_add_legacy(node, NR_IRQS_LEGACY, 0, 0,
+					   &i8259_host_ops, NULL);
 	if (i8259_host == NULL) {
 		printk(KERN_ERR "i8259: failed to allocate irq host !\n");
 		return;
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index b0426f28946a..995fb2ada507 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -602,7 +602,7 @@ static void __init mpic_scan_ht_pics(struct mpic *mpic)
 /* Find an mpic associated with a given linux interrupt */
 static struct mpic *mpic_find(unsigned int irq)
 {
-	if (irq < NUM_ISA_INTERRUPTS)
+	if (irq < NR_IRQS_LEGACY)
 		return NULL;
 
 	return irq_get_chip_data(irq);
diff --git a/arch/powerpc/sysdev/tsi108_pci.c b/arch/powerpc/sysdev/tsi108_pci.c
index 49f9541954f8..042bb38fa5c2 100644
--- a/arch/powerpc/sysdev/tsi108_pci.c
+++ b/arch/powerpc/sysdev/tsi108_pci.c
@@ -404,7 +404,8 @@ void __init tsi108_pci_int_init(struct device_node *node)
 {
 	DBG("Tsi108_pci_int_init: initializing PCI interrupts\n");
 
-	pci_irq_host = irq_domain_add_legacy_isa(node, &pci_irq_domain_ops, NULL);
+	pci_irq_host = irq_domain_add_legacy(node, NR_IRQS_LEGACY, 0, 0,
+					     &pci_irq_domain_ops, NULL);
 	if (pci_irq_host == NULL) {
 		printk(KERN_ERR "pci_irq_host: failed to allocate irq domain!\n");
 		return;
diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
index 7e4305c01bac..fdf8db4444b6 100644
--- a/arch/powerpc/sysdev/xics/xics-common.c
+++ b/arch/powerpc/sysdev/xics/xics-common.c
@@ -201,7 +201,7 @@ void xics_migrate_irqs_away(void)
 		struct ics *ics;
 
 		/* We can't set affinity on ISA interrupts */
-		if (virq < NUM_ISA_INTERRUPTS)
+		if (virq < NR_IRQS_LEGACY)
 			continue;
 		/* We only need to migrate enabled IRQS */
 		if (!desc->action)
-- 
2.29.2


^ permalink raw reply related

* [PATCH 9/9] irqdomain: Kill irq_domain_add_legacy_isa
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack
In-Reply-To: <20210406093557.1073423-1-maz@kernel.org>

This helper doesn't have a user anymore, let's remove it.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 Documentation/core-api/irq/irq-domain.rst |  1 -
 include/linux/irqdomain.h                 | 11 -----------
 2 files changed, 12 deletions(-)

diff --git a/Documentation/core-api/irq/irq-domain.rst b/Documentation/core-api/irq/irq-domain.rst
index a77c24c27f7b..84e561db468f 100644
--- a/Documentation/core-api/irq/irq-domain.rst
+++ b/Documentation/core-api/irq/irq-domain.rst
@@ -146,7 +146,6 @@ Legacy
 
 	irq_domain_add_simple()
 	irq_domain_add_legacy()
-	irq_domain_add_legacy_isa()
 	irq_domain_create_legacy()
 
 The Legacy mapping is a special case for drivers that already have a
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 3997ed9e4d7d..2a7ecf08d56e 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -45,9 +45,6 @@ struct cpumask;
 struct seq_file;
 struct irq_affinity_desc;
 
-/* Number of irqs reserved for a legacy isa controller */
-#define NUM_ISA_INTERRUPTS	16
-
 #define IRQ_DOMAIN_IRQ_SPEC_PARAMS 16
 
 /**
@@ -346,14 +343,6 @@ static inline struct irq_domain *irq_domain_add_nomap(struct device_node *of_nod
 {
 	return __irq_domain_add(of_node_to_fwnode(of_node), 0, max_irq, max_irq, ops, host_data);
 }
-static inline struct irq_domain *irq_domain_add_legacy_isa(
-				struct device_node *of_node,
-				const struct irq_domain_ops *ops,
-				void *host_data)
-{
-	return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS, 0, 0, ops,
-				     host_data);
-}
 static inline struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
 					 const struct irq_domain_ops *ops,
 					 void *host_data)
-- 
2.29.2


^ permalink raw reply related

* [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32
From: Christophe Leroy @ 2021-04-06  9:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e1723a06c5a26398ff03cb669ec99de2516045a9.1617701875.git.christophe.leroy@csgroup.eu>

For that, create a 32 bits version of patch_imm64_load_insns()
and create a patch_imm_load_insns() which calls
patch_imm32_load_insns() on PPC32 and patch_imm64_load_insns()
on PPC64.

Adapt optprobes_head.S for PPC32. Use PPC_LL/PPC_STL macros instead
of raw ld/std, opt out things linked to paca and use stmw/lmw to
save/restore registers.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/Kconfig                 |  2 +-
 arch/powerpc/kernel/optprobes.c      | 24 +++++++++++++--
 arch/powerpc/kernel/optprobes_head.S | 46 +++++++++++++++++++---------
 3 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c1344c05226c..49b538e54efb 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -227,7 +227,7 @@ config PPC
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI				if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
 	select HAVE_HARDLOCKUP_DETECTOR_ARCH	if PPC64 && PPC_BOOK3S && SMP
-	select HAVE_OPTPROBES			if PPC64
+	select HAVE_OPTPROBES
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_EVENTS_NMI		if PPC64
 	select HAVE_HARDLOCKUP_DETECTOR_PERF	if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCH
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 58fdb9f66e0f..cdf87086fa33 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -141,11 +141,21 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
 	}
 }
 
+static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
+{
+	patch_instruction((struct ppc_inst *)addr,
+			  ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
+	addr++;
+
+	patch_instruction((struct ppc_inst *)addr,
+			  ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
+}
+
 /*
  * Generate instructions to load provided immediate 64-bit value
  * to register 'reg' and patch these instructions at 'addr'.
  */
-static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
+static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)
 {
 	/* lis reg,(op)@highest */
 	patch_instruction((struct ppc_inst *)addr,
@@ -177,6 +187,14 @@ static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *
 				   ___PPC_RS(reg) | (val & 0xffff)));
 }
 
+static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
+{
+	if (IS_ENABLED(CONFIG_PPC64))
+		patch_imm64_load_insns(val, reg, addr);
+	else
+		patch_imm32_load_insns(val, reg, addr);
+}
+
 int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 {
 	struct ppc_inst branch_op_callback, branch_emulate_step, temp;
@@ -230,7 +248,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	 * Fixup the template with instructions to:
 	 * 1. load the address of the actual probepoint
 	 */
-	patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
+	patch_imm_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
 
 	/*
 	 * 2. branch to optimized_callback() and emulate_step()
@@ -264,7 +282,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	 * 3. load instruction to be emulated into relevant register, and
 	 */
 	temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
-	patch_imm64_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
+	patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
 
 	/*
 	 * 4. branch back from trampoline
diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
index ff8ba4d3824d..49f31e554573 100644
--- a/arch/powerpc/kernel/optprobes_head.S
+++ b/arch/powerpc/kernel/optprobes_head.S
@@ -30,39 +30,47 @@ optinsn_slot:
 	.global optprobe_template_entry
 optprobe_template_entry:
 	/* Create an in-memory pt_regs */
-	stdu	r1,-INT_FRAME_SIZE(r1)
+	PPC_STLU	r1,-INT_FRAME_SIZE(r1)
 	SAVE_GPR(0,r1)
 	/* Save the previous SP into stack */
 	addi	r0,r1,INT_FRAME_SIZE
-	std	r0,GPR1(r1)
+	PPC_STL	r0,GPR1(r1)
+#ifdef CONFIG_PPC64
 	SAVE_10GPRS(2,r1)
 	SAVE_10GPRS(12,r1)
 	SAVE_10GPRS(22,r1)
+#else
+	stmw	r2, GPR2(r1)
+#endif
 	/* Save SPRS */
 	mfmsr	r5
-	std	r5,_MSR(r1)
+	PPC_STL	r5,_MSR(r1)
 	li	r5,0x700
-	std	r5,_TRAP(r1)
+	PPC_STL	r5,_TRAP(r1)
 	li	r5,0
-	std	r5,ORIG_GPR3(r1)
-	std	r5,RESULT(r1)
+	PPC_STL	r5,ORIG_GPR3(r1)
+	PPC_STL	r5,RESULT(r1)
 	mfctr	r5
-	std	r5,_CTR(r1)
+	PPC_STL	r5,_CTR(r1)
 	mflr	r5
-	std	r5,_LINK(r1)
+	PPC_STL	r5,_LINK(r1)
 	mfspr	r5,SPRN_XER
-	std	r5,_XER(r1)
+	PPC_STL	r5,_XER(r1)
 	mfcr	r5
-	std	r5,_CCR(r1)
+	PPC_STL	r5,_CCR(r1)
+#ifdef CONFIG_PPC64
 	lbz     r5,PACAIRQSOFTMASK(r13)
 	std     r5,SOFTE(r1)
+#endif
 
 	/*
 	 * We may get here from a module, so load the kernel TOC in r2.
 	 * The original TOC gets restored when pt_regs is restored
 	 * further below.
 	 */
+#ifdef CONFIG_PPC64
 	ld	r2,PACATOC(r13)
+#endif
 
 	.global optprobe_template_op_address
 optprobe_template_op_address:
@@ -72,9 +80,11 @@ optprobe_template_op_address:
 	 */
 	nop
 	nop
+#ifdef CONFIG_PPC64
 	nop
 	nop
 	nop
+#endif
 	/* 2. pt_regs pointer in r4 */
 	addi	r4,r1,STACK_FRAME_OVERHEAD
 
@@ -94,9 +104,11 @@ optprobe_template_insn:
 	/* 2, Pass instruction to be emulated in r4 */
 	nop
 	nop
+#ifdef CONFIG_PPC64
 	nop
 	nop
 	nop
+#endif
 
 	.global optprobe_template_call_emulate
 optprobe_template_call_emulate:
@@ -107,20 +119,24 @@ optprobe_template_call_emulate:
 	 * All done.
 	 * Now, restore the registers...
 	 */
-	ld	r5,_MSR(r1)
+	PPC_LL	r5,_MSR(r1)
 	mtmsr	r5
-	ld	r5,_CTR(r1)
+	PPC_LL	r5,_CTR(r1)
 	mtctr	r5
-	ld	r5,_LINK(r1)
+	PPC_LL	r5,_LINK(r1)
 	mtlr	r5
-	ld	r5,_XER(r1)
+	PPC_LL	r5,_XER(r1)
 	mtxer	r5
-	ld	r5,_CCR(r1)
+	PPC_LL	r5,_CCR(r1)
 	mtcr	r5
 	REST_GPR(0,r1)
+#ifdef CONFIG_PPC64
 	REST_10GPRS(2,r1)
 	REST_10GPRS(12,r1)
 	REST_10GPRS(22,r1)
+#else
+	lmw	r2, GPR2(r1)
+#endif
 	/* Restore the previous SP */
 	addi	r1,r1,INT_FRAME_SIZE
 
-- 
2.25.0


^ permalink raw reply related

* [PATCH v1 1/2] powerpc/inst: ppc_inst_as_u64() becomes ppc_inst_as_ulong()
From: Christophe Leroy @ 2021-04-06  9:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao
  Cc: linuxppc-dev, linux-kernel

In order to simplify use on PPC32, change ppc_inst_as_u64()
into ppc_inst_as_ulong() that returns the 32 bits instruction
on PPC32.

Will be used when porting OPTPROBES to PPC32.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h  | 13 +++++++------
 arch/powerpc/kernel/optprobes.c  |  2 +-
 arch/powerpc/lib/code-patching.c |  2 +-
 arch/powerpc/xmon/xmon.c         |  2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index cc73c1267572..8ea0b503f32f 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -113,13 +113,14 @@ static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst *va
 	return location + ppc_inst_len(tmp);
 }
 
-static inline u64 ppc_inst_as_u64(struct ppc_inst x)
+static inline unsigned long ppc_inst_as_ulong(struct ppc_inst x)
 {
-#ifdef CONFIG_CPU_LITTLE_ENDIAN
-	return (u64)ppc_inst_suffix(x) << 32 | ppc_inst_val(x);
-#else
-	return (u64)ppc_inst_val(x) << 32 | ppc_inst_suffix(x);
-#endif
+	if (IS_ENABLED(CONFIG_PPC32))
+		return ppc_inst_val(x);
+	else if (IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN))
+		return (u64)ppc_inst_suffix(x) << 32 | ppc_inst_val(x);
+	else
+		return (u64)ppc_inst_val(x) << 32 | ppc_inst_suffix(x);
 }
 
 #define PPC_INST_STR_LEN sizeof("00000000 00000000")
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 7f7cdbeacd1a..58fdb9f66e0f 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -264,7 +264,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	 * 3. load instruction to be emulated into relevant register, and
 	 */
 	temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
-	patch_imm64_load_insns(ppc_inst_as_u64(temp), 4, buff + TMPL_INSN_IDX);
+	patch_imm64_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
 
 	/*
 	 * 4. branch back from trampoline
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 65aec4d6d9ba..870b30d9be2f 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -26,7 +26,7 @@ static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr
 
 		__put_kernel_nofault(patch_addr, &val, u32, failed);
 	} else {
-		u64 val = ppc_inst_as_u64(instr);
+		u64 val = ppc_inst_as_ulong(instr);
 
 		__put_kernel_nofault(patch_addr, &val, u64, failed);
 	}
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3fe37495f63d..ff10a357d41d 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2980,7 +2980,7 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
 		if (!ppc_inst_prefixed(inst))
 			dump_func(ppc_inst_val(inst), adr);
 		else
-			dump_func(ppc_inst_as_u64(inst), adr);
+			dump_func(ppc_inst_as_ulong(inst), adr);
 		printf("\n");
 	}
 	return adr - first_adr;
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping()
From: Geert Uytterhoeven @ 2021-04-06 10:32 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Rich Felker, Yoshinori Sato, Linux-sh list, Robert Jarzmik,
	Linux Kernel Mailing List, Haojian Zhuang,
	open list:BROADCOM NVRAM DRIVER, Thomas Bogendoerfer, Linux ARM,
	Thomas Gleixner, linuxppc-dev, Daniel Mack
In-Reply-To: <20210406093557.1073423-5-maz@kernel.org>

Hi Marc,

On Tue, Apr 6, 2021 at 11:44 AM Marc Zyngier <maz@kernel.org> wrote:
> Instead of playing games with using irq_create_identity_mapping()
> and irq_domain_associate(), drop the use of the former and only
> use the latter, together with the allocation of the irq_desc
> as needed.
>
> It doesn't make the code less awful, but at least the intent
> is clearer.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Thanks for your patch!

> --- a/drivers/sh/intc/core.c
> +++ b/drivers/sh/intc/core.c
> @@ -179,6 +179,23 @@ static unsigned int __init save_reg(struct intc_desc_int *d,
>         return 0;
>  }
>
> +static bool __init intc_map(struct irq_domain *domain, int irq)
> +{
> +       int res;

warning: unused variable ‘res’ [-Wunused-variable]

> +
> +       if (!irq_to_desc(irq) && irq_alloc_desc_at(irq, NUMA_NO_NODE) != irq) {
> +               pr_err("uname to allocate IRQ %d\n", irq);
> +               return false;
> +       }
> +
> +       if (irq_domain_associate(domain, irq, irq)) {
> +               pr_err("domain association failure\n");
> +               return false;
> +       }
> +
> +       return true;
> +}
> +
>  int __init register_intc_controller(struct intc_desc *desc)
>  {
>         unsigned int i, k, smp;
> @@ -316,19 +333,8 @@ int __init register_intc_controller(struct intc_desc *desc)

warning: unused variable ‘res’ [-Wunused-variable]

>                 if (!vect->enum_id)
>                         continue;
>
> -               res = irq_create_identity_mapping(d->domain, irq);


> -               if (unlikely(res)) {
> -                       if (res == -EEXIST) {
> -                               res = irq_domain_associate(d->domain, irq, irq);
> -                               if (unlikely(res)) {
> -                                       pr_err("domain association failure\n");
> -                                       continue;
> -                               }
> -                       } else {
> -                               pr_err("can't identity map IRQ %d\n", irq);
> -                               continue;
> -                       }
> -               }
> +               if (!intc_map(d->domain, irq))
> +                       continue;
>
>                 intc_irq_xlate_set(irq, vect->enum_id, d);
>                 intc_register_irq(desc, d, vect->enum_id, irq);

Otherwise this seems to work fine on real hardware (landisk) and qemu
(rts7751r2d).  I did verify that the new function intc_map() is called.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH printk v2 2/5] printk: remove safe buffers
From: John Ogness @ 2021-04-06 11:01 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Alexey Kardashevskiy, Paul Mackerras,
	Tiezhu Yang, Rafael Aquini, Aneesh Kumar K.V, Peter Zijlstra,
	Yue Hu, Jordan Niethe, Kees Cook, Paul E. McKenney,
	Alistair Popple, Guilherme G. Piccoli, Nicholas Piggin,
	Steven Rostedt, Thomas Gleixner, kexec, linux-kernel,
	Sergey Senozhatsky, Eric Biederman, Andrew Morton, linuxppc-dev
In-Reply-To: <YGXV8LJarjUJDhvy@alley>

On 2021-04-01, Petr Mladek <pmladek@suse.com> wrote:
>> Caller-id solves this problem and is easy to sort for anyone with
>> `grep'. Yes, it is a shame that `dmesg' does not show it, but
>> directly using any of the printk interfaces does show it (kmsg_dump,
>> /dev/kmsg, syslog, console).
>
> True but frankly, the current situation is _far_ from convenient:
>
>    + consoles do not show it by default
>    + none userspace tool (dmesg, journalctl, crash) is able to show it
>    + grep is a nightmare, especially if you have more than handful of CPUs
>
> Yes, everything is solvable but not easily.
>
>> >     I get this with "echo l >/proc/sysrq-trigger" and this patchset:
>> 
>> Of course. Without caller-id, it is a mess. But this has nothing to do
>> with NMI. The same problem exists for WARN_ON() on multiple CPUs
>> simultaneously. If the user is not using caller-id, they are
>> lost. Caller-id is the current solution to the interlaced logs.
>
> Sure. But in reality, the risk of mixed WARN_ONs is small. While
> this patch makes backtraces from all CPUs always unusable without
> caller_id and non-trivial effort.

I would prefer we solve the situation for non-NMI as well, not just for
the sysrq "l" case.

>> For the long term, we should introduce a printk-context API that allows
>> callers to perfectly pack their multi-line output into a single
>> entry. We discussed [0][1] this back in August 2020.
>
> We need a "short" term solution. There are currently 3 solutions:
>
> 1. Keep nmi_safe() and all the hacks around.
>
> 2. Serialize nmi_cpu_backtrace() by a spin lock and later by
>    the special lock used also by atomic consoles.
>
> 3. Tell complaining people how to sort the messed logs.

Or we look into the long term solution now. If caller-id's cannot not be
used as the solution (because nobody turns it on, nobody knows about it,
and/or distros do not enable it), then we should look at how to make at
least the backtraces contiguous. I have a few ideas here.

John Ogness

^ permalink raw reply

* Re: [PATCH printk v2 2/5] printk: remove safe buffers
From: Petr Mladek @ 2021-04-06 11:17 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Sergey Senozhatsky, Peter Zijlstra, Paul Mackerras, Tiezhu Yang,
	Rafael Aquini, Paul E. McKenney, Aneesh Kumar K.V,
	Alexey Kardashevskiy, Yue Hu, Jordan Niethe, Kees Cook,
	John Ogness, Alistair Popple, Guilherme G. Piccoli,
	Nicholas Piggin, Steven Rostedt, Thomas Gleixner, kexec,
	linux-kernel, Sergey Senozhatsky, Eric Biederman, Andrew Morton,
	linuxppc-dev
In-Reply-To: <YGZ9+kfQKxASmVDR@google.com>

On Fri 2021-04-02 11:14:18, Sergey Senozhatsky wrote:
> On (21/04/01 16:17), Petr Mladek wrote:
> > > For the long term, we should introduce a printk-context API that allows
> > > callers to perfectly pack their multi-line output into a single
> > > entry. We discussed [0][1] this back in August 2020.
> > 
> > We need a "short" term solution. There are currently 3 solutions:
> > 
> > 1. Keep nmi_safe() and all the hacks around.
> > 
> > 2. Serialize nmi_cpu_backtrace() by a spin lock and later by
> >    the special lock used also by atomic consoles.
> > 
> > 3. Tell complaining people how to sort the messed logs.
> 
> Are we talking about nmi_cpu_backtrace()->dump_stack() or some
> other path?

It is about serializing

			if (regs)
				show_regs(regs);
			else
				dump_stack();

in nmi_cpu_backtrace() when it is triggered on many(all) CPUs
at the same time.


> dump_stack() seems to be already serialized by `dump_lock`. Hmm,
> show_regs() is not serialized, seems like it should be under the
> same `dump_lock` as dump_stack().

Ah, I think that you already mentioned it in the past and I forget it.

Yes, we would need to synchronize all these dump/show functions using
the same lock. It is already the lock that might be taken recursively
on the same CPU.

In each case, we must not introduce another lock in
nmi_cpu_backtrace() because it might cause deadlock with
@dump_lock.

Anyway, I would really like to keep the dumps serialized. So, we
either need to use the same lock everywhere or we need to keep
nmi_safe buffers for now.

I would like to remove the nmi_safe buffers in the long term
but I am fine with doing it later after the consoles rework.
I'll leave the prioritization for John who is doing the work
and might have some preferences.

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping()
From: Christophe Leroy @ 2021-04-06 11:21 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, linux-kernel, linux-mips,
	linuxppc-dev, linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack
In-Reply-To: <20210406093557.1073423-2-maz@kernel.org>



Le 06/04/2021 à 11:35, Marc Zyngier a écrit :
> irq_linear_revmap() is supposed to be a fast path for domain
> lookups, but it only exposes low-level details of the irqdomain
> implementation, details which are better kept private.

Can you elaborate with more details ?

> 
> The *overhead* between the two is only a function call and
> a couple of tests, so it is likely that noone can show any
> meaningful difference compared to the cost of taking an
> interrupt.

Do you have any measurement ?

Can you make the "likely" a certitude ?

> 
> Reimplement irq_linear_revmap() with irq_find_mapping()
> in order to preserve source code compatibility, and
> rename the internal field for a measure.

This is in complete contradiction with commit https://github.com/torvalds/linux/commit/d3dcb436

At that time, irq_linear_revmap() was less complex than what irq_find_mapping() is today, and 
nevertheless it was considered worth restoring in as a fast path. What has changed since then ?

Can you also explain the reason for the renaming of "linear_revmap" into "revmap" ? What is that 
"measure" ?

> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>   include/linux/irqdomain.h | 22 +++++++++-------------
>   kernel/irq/irqdomain.c    |  6 +++---
>   2 files changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 33cacc8af26d..b9600f24878a 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -154,9 +154,9 @@ struct irq_domain_chip_generic;
>    * Revmap data, used internally by irq_domain
>    * @revmap_direct_max_irq: The largest hwirq that can be set for controllers that
>    *                         support direct mapping
> - * @revmap_size: Size of the linear map table @linear_revmap[]
> + * @revmap_size: Size of the linear map table @revmap[]
>    * @revmap_tree: Radix map tree for hwirqs that don't fit in the linear map
> - * @linear_revmap: Linear table of hwirq->virq reverse mappings
> + * @revmap: Linear table of hwirq->virq reverse mappings
>    */
>   struct irq_domain {
>   	struct list_head link;
> @@ -180,7 +180,7 @@ struct irq_domain {
>   	unsigned int revmap_size;
>   	struct radix_tree_root revmap_tree;
>   	struct mutex revmap_tree_mutex;
> -	unsigned int linear_revmap[];
> +	unsigned int revmap[];
>   };
>   
>   /* Irq domain flags */
> @@ -396,24 +396,20 @@ static inline unsigned int irq_create_mapping(struct irq_domain *host,
>   	return irq_create_mapping_affinity(host, hwirq, NULL);
>   }
>   
> -
>   /**
> - * irq_linear_revmap() - Find a linux irq from a hw irq number.
> + * irq_find_mapping() - Find a linux irq from a hw irq number.
>    * @domain: domain owning this hardware interrupt
>    * @hwirq: hardware irq number in that domain space
> - *
> - * This is a fast path alternative to irq_find_mapping() that can be
> - * called directly by irq controller code to save a handful of
> - * instructions. It is always safe to call, but won't find irqs mapped
> - * using the radix tree.
>    */
> +extern unsigned int irq_find_mapping(struct irq_domain *host,
> +				     irq_hw_number_t hwirq);
> +
>   static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
>   					     irq_hw_number_t hwirq)
>   {
> -	return hwirq < domain->revmap_size ? domain->linear_revmap[hwirq] : 0;
> +	return irq_find_mapping(domain, hwirq);
>   }
> -extern unsigned int irq_find_mapping(struct irq_domain *host,
> -				     irq_hw_number_t hwirq);
> +
>   extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
>   extern int irq_create_strict_mappings(struct irq_domain *domain,
>   				      unsigned int irq_base,
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index d10ab1d689d5..dfa716305ea9 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -486,7 +486,7 @@ static void irq_domain_clear_mapping(struct irq_domain *domain,
>   				     irq_hw_number_t hwirq)
>   {
>   	if (hwirq < domain->revmap_size) {
> -		domain->linear_revmap[hwirq] = 0;
> +		domain->revmap[hwirq] = 0;
>   	} else {
>   		mutex_lock(&domain->revmap_tree_mutex);
>   		radix_tree_delete(&domain->revmap_tree, hwirq);
> @@ -499,7 +499,7 @@ static void irq_domain_set_mapping(struct irq_domain *domain,
>   				   struct irq_data *irq_data)
>   {
>   	if (hwirq < domain->revmap_size) {
> -		domain->linear_revmap[hwirq] = irq_data->irq;
> +		domain->revmap[hwirq] = irq_data->irq;
>   	} else {
>   		mutex_lock(&domain->revmap_tree_mutex);
>   		radix_tree_insert(&domain->revmap_tree, hwirq, irq_data);
> @@ -920,7 +920,7 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>   
>   	/* Check if the hwirq is in the linear revmap. */
>   	if (hwirq < domain->revmap_size)
> -		return domain->linear_revmap[hwirq];
> +		return domain->revmap[hwirq];
>   
>   	rcu_read_lock();
>   	data = radix_tree_lookup(&domain->revmap_tree, hwirq);
> 

^ permalink raw reply

* Re: [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC
From: Thomas Bogendoerfer @ 2021-04-06 11:36 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Rich Felker, Yoshinori Sato, linux-sh, Robert Jarzmik,
	linux-kernel, Haojian Zhuang, linux-mips, linux-arm-kernel,
	Thomas Gleixner, linuxppc-dev, Daniel Mack
In-Reply-To: <20210406093557.1073423-7-maz@kernel.org>

On Tue, Apr 06, 2021 at 10:35:54AM +0100, Marc Zyngier wrote:
> Use the generic irq_domain_simple_ops structure instead of
> a home-grown one.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/mips/netlogic/common/irq.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping()
From: Marc Zyngier @ 2021-04-06 12:12 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Thomas Bogendoerfer, Yoshinori Sato, linux-sh, Robert Jarzmik,
	linux-kernel, Haojian Zhuang, linux-mips, Rich Felker,
	Daniel Mack, Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <15be426f-4429-ebeb-1b4a-8342bce391e5@csgroup.eu>

Christophe,

On Tue, 06 Apr 2021 12:21:33 +0100,
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 06/04/2021 à 11:35, Marc Zyngier a écrit :
> > irq_linear_revmap() is supposed to be a fast path for domain
> > lookups, but it only exposes low-level details of the irqdomain
> > implementation, details which are better kept private.
> 
> Can you elaborate with more details ?

Things like directly picking into the revmap are positively awful, and
doesn't work if the domain has been constructed using the radix
tree. Which on its own is totally broken, because things like
irq_domain_create_hierarchy() will pick an implementation or the
other.

> 
> > 
> > The *overhead* between the two is only a function call and
> > a couple of tests, so it is likely that noone can show any
> > meaningful difference compared to the cost of taking an
> > interrupt.
> 
> Do you have any measurement ?

I did measure things on arm64, and couldn't come up with any
difference other than noise.

> Can you make the "likely" a certitude ?

Of course not. You can always come up with an artificial CPU
implementation that has a very small exception entry overhead, and a
ridiculously slow memory subsystem. Do I care about these? No.

If you can come up with realistic platforms that show a regression
with this patch, I'm all ears.

> 
> > 
> > Reimplement irq_linear_revmap() with irq_find_mapping()
> > in order to preserve source code compatibility, and
> > rename the internal field for a measure.
> 
> This is in complete contradiction with commit https://github.com/torvalds/linux/commit/d3dcb436
> 
> At that time, irq_linear_revmap() was less complex than what
> irq_find_mapping() is today, and nevertheless it was considered worth
> restoring in as a fast path. What has changed since then ?

Over 8 years? Plenty. The use of irqdomains has been generalised, we
have domain hierarchies, and if anything, this commit introduces the
buggy behaviour I was mentioning above. I also don't see any mention
of actual performance in that commit.

And if we're worried about a fast path, being able to directly cache
the irq_data in the revmap, hence skipping the irq_desc lookup that
inevitable follows, is a much more interesting prospect than the "get
useless data quick" that irq_linear_revmap() implements.

This latter optimisation is what I am after.

> Can you also explain the reason for the renaming of "linear_revmap"
> into "revmap" ? What is that "measure" ?

To catch the potential direct use of the reverse map field.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply

* Re: [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping()
From: Marc Zyngier @ 2021-04-06 13:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Rich Felker, Yoshinori Sato, Linux-sh list, Robert Jarzmik,
	Linux Kernel Mailing List, Haojian Zhuang,
	open list:BROADCOM NVRAM DRIVER, Thomas Bogendoerfer, Linux ARM,
	Thomas Gleixner, linuxppc-dev, Daniel Mack
In-Reply-To: <CAMuHMdU3vo1AWuDr69WK8-1w38+Q7KturAoL=cFpspZiYr2ssA@mail.gmail.com>

On Tue, 06 Apr 2021 11:32:13 +0100,
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Marc,
> 
> On Tue, Apr 6, 2021 at 11:44 AM Marc Zyngier <maz@kernel.org> wrote:
> > Instead of playing games with using irq_create_identity_mapping()
> > and irq_domain_associate(), drop the use of the former and only
> > use the latter, together with the allocation of the irq_desc
> > as needed.
> >
> > It doesn't make the code less awful, but at least the intent
> > is clearer.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> 
> Thanks for your patch!
> 
> > --- a/drivers/sh/intc/core.c
> > +++ b/drivers/sh/intc/core.c
> > @@ -179,6 +179,23 @@ static unsigned int __init save_reg(struct intc_desc_int *d,
> >         return 0;
> >  }
> >
> > +static bool __init intc_map(struct irq_domain *domain, int irq)
> > +{
> > +       int res;
> 
> warning: unused variable ‘res’ [-Wunused-variable]
> 
> > +
> > +       if (!irq_to_desc(irq) && irq_alloc_desc_at(irq, NUMA_NO_NODE) != irq) {
> > +               pr_err("uname to allocate IRQ %d\n", irq);
> > +               return false;
> > +       }
> > +
> > +       if (irq_domain_associate(domain, irq, irq)) {
> > +               pr_err("domain association failure\n");
> > +               return false;
> > +       }
> > +
> > +       return true;
> > +}
> > +
> >  int __init register_intc_controller(struct intc_desc *desc)
> >  {
> >         unsigned int i, k, smp;
> > @@ -316,19 +333,8 @@ int __init register_intc_controller(struct intc_desc *desc)
> 
> warning: unused variable ‘res’ [-Wunused-variable]

Ah, thanks for spotting these.

> 
> >                 if (!vect->enum_id)
> >                         continue;
> >
> > -               res = irq_create_identity_mapping(d->domain, irq);
> 
> 
> > -               if (unlikely(res)) {
> > -                       if (res == -EEXIST) {
> > -                               res = irq_domain_associate(d->domain, irq, irq);
> > -                               if (unlikely(res)) {
> > -                                       pr_err("domain association failure\n");
> > -                                       continue;
> > -                               }
> > -                       } else {
> > -                               pr_err("can't identity map IRQ %d\n", irq);
> > -                               continue;
> > -                       }
> > -               }
> > +               if (!intc_map(d->domain, irq))
> > +                       continue;
> >
> >                 intc_irq_xlate_set(irq, vect->enum_id, d);
> >                 intc_register_irq(desc, d, vect->enum_id, irq);
> 
> Otherwise this seems to work fine on real hardware (landisk) and qemu
> (rts7751r2d).  I did verify that the new function intc_map() is called.
> 
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Awesome, thanks Geert.

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply

* Re: [PATCH v6 30/48] KVM: PPC: Book3S HV P9: Implement the rest of the P9 path in C
From: Nicholas Piggin @ 2021-04-06 13:02 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <1617699912.sfw989xp02.astroid@bobo.none>

Excerpts from Nicholas Piggin's message of April 6, 2021 7:12 pm:
> Excerpts from Paul Mackerras's message of April 6, 2021 5:27 pm:
>> On Mon, Apr 05, 2021 at 11:19:30AM +1000, Nicholas Piggin wrote:
>>> Almost all logic is moved to C, by introducing a new in_guest mode for
>>> the P9 path that branches very early in the KVM interrupt handler to
>>> P9 exit code.
>>> 
>>> The main P9 entry and exit assembly is now only about 160 lines of low
>>> level stack setup and register save/restore, plus a bad-interrupt
>>> handler.
>>> 
>>> There are two motivations for this, the first is just make the code more
>>> maintainable being in C. The second is to reduce the amount of code
>>> running in a special KVM mode, "realmode". In quotes because with radix
>>> it is no longer necessarily real-mode in the MMU, but it still has to be
>>> treated specially because it may be in real-mode, and has various
>>> important registers like PID, DEC, TB, etc set to guest. This is hostile
>>> to the rest of Linux and can't use arbitrary kernel functionality or be
>>> instrumented well.
>>> 
>>> This initial patch is a reasonably faithful conversion of the asm code,
>>> but it does lack any loop to return quickly back into the guest without
>>> switching out of realmode in the case of unimportant or easily handled
>>> interrupts. As explained in previous changes, handling HV interrupts
>>> in real mode is not so important for P9.
>>> 
>>> Use of Linux 64s interrupt entry code register conventions including
>>> paca EX_ save areas are brought into the KVM code. There is no point
>>> shuffling things into different paca save areas and making up a
>>> different calling convention for KVM.
>>> 
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> 
>> [snip]
>> 
>>> +/*
>>> + * Took an interrupt somewhere right before HRFID to guest, so registers are
>>> + * in a bad way. Return things hopefully enough to run host virtual code and
>>> + * run the Linux interrupt handler (SRESET or MCE) to print something useful.
>>> + *
>>> + * We could be really clever and save all host registers in known locations
>>> + * before setting HSTATE_IN_GUEST, then restoring them all here, and setting
>>> + * return address to a fixup that sets them up again. But that's a lot of
>>> + * effort for a small bit of code. Lots of other things to do first.
>>> + */
>>> +kvmppc_p9_bad_interrupt:
>>> +	/*
>>> +	 * Set GUEST_MODE_NONE so the handler won't branch to KVM, and clear
>>> +	 * MSR_RI in r12 ([H]SRR1) so the handler won't try to return.
>>> +	 */
>>> +	li	r10,KVM_GUEST_MODE_NONE
>>> +	stb	r10,HSTATE_IN_GUEST(r13)
>>> +	li	r10,MSR_RI
>>> +	andc	r12,r12,r10
>>> +
>>> +	/*
>>> +	 * Clean up guest registers to give host a chance to run.
>>> +	 */
>>> +	li	r10,0
>>> +	mtspr	SPRN_AMR,r10
>>> +	mtspr	SPRN_IAMR,r10
>>> +	mtspr	SPRN_CIABR,r10
>>> +	mtspr	SPRN_DAWRX0,r10
>>> +BEGIN_FTR_SECTION
>>> +	mtspr	SPRN_DAWRX1,r10
>>> +END_FTR_SECTION_IFSET(CPU_FTR_DAWR1)
>>> +	mtspr	SPRN_PID,r10
>>> +
>>> +	/*
>>> +	 * Switch to host MMU mode
>>> +	 */
>>> +	ld	r10, HSTATE_KVM_VCPU(r13)
>>> +	ld	r10, VCPU_KVM(r10)
>>> +	lwz	r10, KVM_HOST_LPID(r10)
>>> +	mtspr	SPRN_LPID,r10
>>> +
>>> +	ld	r10, HSTATE_KVM_VCPU(r13)
>>> +	ld	r10, VCPU_KVM(r10)
>>> +	ld	r10, KVM_HOST_LPCR(r10)
>>> +	mtspr	SPRN_LPCR,r10
>>> +
>>> +	/*
>>> +	 * Go back to interrupt handler
>>> +	 */
>>> +	ld	r10,HSTATE_SCRATCH0(r13)
>>> +	cmpwi	r10,BOOK3S_INTERRUPT_MACHINE_CHECK
>>> +	beq	machine_check_common
>>> +
>>> +	ld	r10,HSTATE_SCRATCH0(r13)
>>> +	cmpwi	r10,BOOK3S_INTERRUPT_SYSTEM_RESET
>>> +	beq	system_reset_common
>>> +
>>> +	b	.
>> 
>> So you only handle machine check and system reset here?  I would think
>> that program check would also be useful, for the cases where people
>> put BUG_ON in sensitive places (see below).  DSI and ISI could also be
>> useful for the null pointer dereference cases, I would think.
> 
> Those ones have their own stack, so a bit simpler to run them (and
> they obviously have to be handled as they are NMIs). I'll see if we
> can do something to improve the others a bit. Maybe just call program
> check for any other exception might work, making sure that it'll use
> the emergency stack rather than something that looks like a kernel
> stack but is a guest value, I'll see what we can get to work.

So program check isn't tested in virt mode, and neither are ISI or
DSI unless PR is possible, so the bad host interrupt check in
rmhandlers doesn't really catch those cases either.

We may want to improve on that but I might wait until after this
series.

I did get rid of the BUG_ON though.

Thanks,
Nick

>>> +static inline void mtslb(unsigned int idx, u64 slbee, u64 slbev)
>>> +{
>>> +	BUG_ON((slbee & 0xfff) != idx);
>>> +
>>> +	asm volatile("slbmte %0,%1" :: "r" (slbev), "r" (slbee));
>>> +}
>> 
>> Using BUG_ON here feels dangerous, and the condition it is testing is
>> certainly not one where the host kernel is in such trouble that it
>> can't continue to run.  If the index was wrong then at worst the guest
>> kernel would be in trouble.  So I don't believe BUG_ON is appropriate.
> 
> Yeah good point, some of it was a bit of development paranoia but I 
> do have to go through and tighten these up.
> 
>>> +
>>> +/*
>>> + * Malicious or buggy radix guests may have inserted SLB entries
>>> + * (only 0..3 because radix always runs with UPRT=1), so these must
>>> + * be cleared here to avoid side-channels. slbmte is used rather
>>> + * than slbia, as it won't clear cached translations.
>>> + */
>>> +static void radix_clear_slb(void)
>>> +{
>>> +	u64 slbee, slbev;
>>> +	int i;
>>> +
>>> +	for (i = 0; i < 4; i++) {
>>> +		mfslb(i, &slbee, &slbev);
>>> +		if (unlikely(slbee || slbev)) {
>>> +			slbee = i;
>>> +			slbev = 0;
>>> +			mtslb(i, slbee, slbev);
>>> +		}
>>> +	}
>> 
>> Are four slbmfee + slbmfev really faster than four slbmte?
> 
> I'd thought yes if they behaved similarly to mfspr, but from the look of 
> some workbooks it doesn't look like it's quite that simple.
> 
> I'll have to measure it.
> 
> Thanks,
> Nick
> 

^ permalink raw reply

* [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Andy Shevchenko @ 2021-04-06 13:31 UTC (permalink / raw)
  To: Michael Ellerman, Joerg Roedel, Wei Liu, Michael Kelley,
	Mike Rapoport, Corey Minyard, Bjorn Andersson, Mathieu Poirier,
	Andy Shevchenko, Andrew Morton, Tetsuo Handa, Vlastimil Babka,
	Paul E. McKenney, Steven Rostedt (VMware), linuxppc-dev,
	linux-kernel, linux-hyperv, openipmi-developer, linux-remoteproc,
	linux-arch, kexec, rcu, linux-fsdevel
  Cc: Ohad Ben-Cohen, Iurii Zaikin, Stephen Hemminger, Arnd Bergmann,
	Corey Minyard, Haiyang Zhang, x86, Lai Jiangshan, Josh Triplett,
	Joel Fernandes, Luis Chamberlain, Ingo Molnar, Borislav Petkov,
	Eric Biederman, H. Peter Anvin, Mathieu Desnoyers, Paul Mackerras,
	Thomas Gleixner, K. Y. Srinivasan, Kees Cook

kernel.h is being used as a dump for all kinds of stuff for a long time.
Here is the attempt to start cleaning it up by splitting out panic and
oops helpers.

At the same time convert users in header and lib folder to use new header.
Though for time being include new header back to kernel.h to avoid twisted
indirected includes for existing users.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/powerpc/kernel/setup-common.c   |  1 +
 arch/x86/include/asm/desc.h          |  1 +
 arch/x86/kernel/cpu/mshyperv.c       |  1 +
 arch/x86/kernel/setup.c              |  1 +
 drivers/char/ipmi/ipmi_msghandler.c  |  1 +
 drivers/remoteproc/remoteproc_core.c |  1 +
 include/asm-generic/bug.h            |  3 +-
 include/linux/kernel.h               | 84 +-----------------------
 include/linux/panic.h                | 98 ++++++++++++++++++++++++++++
 include/linux/panic_notifier.h       | 12 ++++
 kernel/hung_task.c                   |  1 +
 kernel/kexec_core.c                  |  1 +
 kernel/panic.c                       |  1 +
 kernel/rcu/tree.c                    |  2 +
 kernel/sysctl.c                      |  1 +
 kernel/trace/trace.c                 |  1 +
 16 files changed, 126 insertions(+), 84 deletions(-)
 create mode 100644 include/linux/panic.h
 create mode 100644 include/linux/panic_notifier.h

diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 74a98fff2c2f..046fe21b5c3b 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -9,6 +9,7 @@
 #undef DEBUG
 
 #include <linux/export.h>
+#include <linux/panic_notifier.h>
 #include <linux/string.h>
 #include <linux/sched.h>
 #include <linux/init.h>
diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index 476082a83d1c..ceb12683b6d1 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -9,6 +9,7 @@
 #include <asm/irq_vectors.h>
 #include <asm/cpu_entry_area.h>
 
+#include <linux/debug_locks.h>
 #include <linux/smp.h>
 #include <linux/percpu.h>
 
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 22f13343b5da..9e5c6f2b044d 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -17,6 +17,7 @@
 #include <linux/irq.h>
 #include <linux/kexec.h>
 #include <linux/i8253.h>
+#include <linux/panic_notifier.h>
 #include <linux/random.h>
 #include <asm/processor.h>
 #include <asm/hypervisor.h>
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 59e5e0903b0c..570699eecf90 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -14,6 +14,7 @@
 #include <linux/initrd.h>
 #include <linux/iscsi_ibft.h>
 #include <linux/memblock.h>
+#include <linux/panic_notifier.h>
 #include <linux/pci.h>
 #include <linux/root_dev.h>
 #include <linux/hugetlb.h>
diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 8a0e97b33cae..e96cb5c4f97a 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -16,6 +16,7 @@
 
 #include <linux/module.h>
 #include <linux/errno.h>
+#include <linux/panic_notifier.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
 #include <linux/seq_file.h>
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 626a6b90fba2..76dd8e2b1e7e 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/device.h>
+#include <linux/panic_notifier.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
 #include <linux/dma-map-ops.h>
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 76a10e0dca9f..719410b93f99 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -17,7 +17,8 @@
 #endif
 
 #ifndef __ASSEMBLY__
-#include <linux/kernel.h>
+#include <linux/panic.h>
+#include <linux/printk.h>
 
 #ifdef CONFIG_BUG
 
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 09035ac67d4b..6c5a05ac1ecb 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -14,6 +14,7 @@
 #include <linux/math.h>
 #include <linux/minmax.h>
 #include <linux/typecheck.h>
+#include <linux/panic.h>
 #include <linux/printk.h>
 #include <linux/build_bug.h>
 #include <linux/static_call_types.h>
@@ -70,7 +71,6 @@
 #define lower_32_bits(n) ((u32)((n) & 0xffffffff))
 
 struct completion;
-struct pt_regs;
 struct user;
 
 #ifdef CONFIG_PREEMPT_VOLUNTARY
@@ -175,14 +175,6 @@ void __might_fault(const char *file, int line);
 static inline void might_fault(void) { }
 #endif
 
-extern struct atomic_notifier_head panic_notifier_list;
-extern long (*panic_blink)(int state);
-__printf(1, 2)
-void panic(const char *fmt, ...) __noreturn __cold;
-void nmi_panic(struct pt_regs *regs, const char *msg);
-extern void oops_enter(void);
-extern void oops_exit(void);
-extern bool oops_may_print(void);
 void do_exit(long error_code) __noreturn;
 void complete_and_exit(struct completion *, long) __noreturn;
 
@@ -368,52 +360,8 @@ extern int __kernel_text_address(unsigned long addr);
 extern int kernel_text_address(unsigned long addr);
 extern int func_ptr_is_kernel_text(void *ptr);
 
-#ifdef CONFIG_SMP
-extern unsigned int sysctl_oops_all_cpu_backtrace;
-#else
-#define sysctl_oops_all_cpu_backtrace 0
-#endif /* CONFIG_SMP */
-
 extern void bust_spinlocks(int yes);
-extern int panic_timeout;
-extern unsigned long panic_print;
-extern int panic_on_oops;
-extern int panic_on_unrecovered_nmi;
-extern int panic_on_io_nmi;
-extern int panic_on_warn;
-extern unsigned long panic_on_taint;
-extern bool panic_on_taint_nousertaint;
-extern int sysctl_panic_on_rcu_stall;
-extern int sysctl_max_rcu_stall_to_panic;
-extern int sysctl_panic_on_stackoverflow;
-
-extern bool crash_kexec_post_notifiers;
 
-/*
- * panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
- * holds a CPU number which is executing panic() currently. A value of
- * PANIC_CPU_INVALID means no CPU has entered panic() or crash_kexec().
- */
-extern atomic_t panic_cpu;
-#define PANIC_CPU_INVALID	-1
-
-/*
- * Only to be used by arch init code. If the user over-wrote the default
- * CONFIG_PANIC_TIMEOUT, honor it.
- */
-static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
-{
-	if (panic_timeout == arch_default_timeout)
-		panic_timeout = timeout;
-}
-extern const char *print_tainted(void);
-enum lockdep_ok {
-	LOCKDEP_STILL_OK,
-	LOCKDEP_NOW_UNRELIABLE
-};
-extern void add_taint(unsigned flag, enum lockdep_ok);
-extern int test_taint(unsigned flag);
-extern unsigned long get_taint(void);
 extern int root_mountflags;
 
 extern bool early_boot_irqs_disabled;
@@ -432,36 +380,6 @@ extern enum system_states {
 	SYSTEM_SUSPEND,
 } system_state;
 
-/* This cannot be an enum because some may be used in assembly source. */
-#define TAINT_PROPRIETARY_MODULE	0
-#define TAINT_FORCED_MODULE		1
-#define TAINT_CPU_OUT_OF_SPEC		2
-#define TAINT_FORCED_RMMOD		3
-#define TAINT_MACHINE_CHECK		4
-#define TAINT_BAD_PAGE			5
-#define TAINT_USER			6
-#define TAINT_DIE			7
-#define TAINT_OVERRIDDEN_ACPI_TABLE	8
-#define TAINT_WARN			9
-#define TAINT_CRAP			10
-#define TAINT_FIRMWARE_WORKAROUND	11
-#define TAINT_OOT_MODULE		12
-#define TAINT_UNSIGNED_MODULE		13
-#define TAINT_SOFTLOCKUP		14
-#define TAINT_LIVEPATCH			15
-#define TAINT_AUX			16
-#define TAINT_RANDSTRUCT		17
-#define TAINT_FLAGS_COUNT		18
-#define TAINT_FLAGS_MAX			((1UL << TAINT_FLAGS_COUNT) - 1)
-
-struct taint_flag {
-	char c_true;	/* character printed when tainted */
-	char c_false;	/* character printed when not tainted */
-	bool module;	/* also show as a per-module taint flag */
-};
-
-extern const struct taint_flag taint_flags[TAINT_FLAGS_COUNT];
-
 extern const char hex_asc[];
 #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
 #define hex_asc_hi(x)	hex_asc[((x) & 0xf0) >> 4]
diff --git a/include/linux/panic.h b/include/linux/panic.h
new file mode 100644
index 000000000000..f5844908a089
--- /dev/null
+++ b/include/linux/panic.h
@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_PANIC_H
+#define _LINUX_PANIC_H
+
+#include <linux/compiler_attributes.h>
+#include <linux/types.h>
+
+struct pt_regs;
+
+extern long (*panic_blink)(int state);
+__printf(1, 2)
+void panic(const char *fmt, ...) __noreturn __cold;
+void nmi_panic(struct pt_regs *regs, const char *msg);
+extern void oops_enter(void);
+extern void oops_exit(void);
+extern bool oops_may_print(void);
+
+#ifdef CONFIG_SMP
+extern unsigned int sysctl_oops_all_cpu_backtrace;
+#else
+#define sysctl_oops_all_cpu_backtrace 0
+#endif /* CONFIG_SMP */
+
+extern int panic_timeout;
+extern unsigned long panic_print;
+extern int panic_on_oops;
+extern int panic_on_unrecovered_nmi;
+extern int panic_on_io_nmi;
+extern int panic_on_warn;
+
+extern unsigned long panic_on_taint;
+extern bool panic_on_taint_nousertaint;
+
+extern int sysctl_panic_on_rcu_stall;
+extern int sysctl_max_rcu_stall_to_panic;
+extern int sysctl_panic_on_stackoverflow;
+
+extern bool crash_kexec_post_notifiers;
+
+/*
+ * panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
+ * holds a CPU number which is executing panic() currently. A value of
+ * PANIC_CPU_INVALID means no CPU has entered panic() or crash_kexec().
+ */
+extern atomic_t panic_cpu;
+#define PANIC_CPU_INVALID	-1
+
+/*
+ * Only to be used by arch init code. If the user over-wrote the default
+ * CONFIG_PANIC_TIMEOUT, honor it.
+ */
+static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
+{
+	if (panic_timeout == arch_default_timeout)
+		panic_timeout = timeout;
+}
+
+/* This cannot be an enum because some may be used in assembly source. */
+#define TAINT_PROPRIETARY_MODULE	0
+#define TAINT_FORCED_MODULE		1
+#define TAINT_CPU_OUT_OF_SPEC		2
+#define TAINT_FORCED_RMMOD		3
+#define TAINT_MACHINE_CHECK		4
+#define TAINT_BAD_PAGE			5
+#define TAINT_USER			6
+#define TAINT_DIE			7
+#define TAINT_OVERRIDDEN_ACPI_TABLE	8
+#define TAINT_WARN			9
+#define TAINT_CRAP			10
+#define TAINT_FIRMWARE_WORKAROUND	11
+#define TAINT_OOT_MODULE		12
+#define TAINT_UNSIGNED_MODULE		13
+#define TAINT_SOFTLOCKUP		14
+#define TAINT_LIVEPATCH			15
+#define TAINT_AUX			16
+#define TAINT_RANDSTRUCT		17
+#define TAINT_FLAGS_COUNT		18
+#define TAINT_FLAGS_MAX			((1UL << TAINT_FLAGS_COUNT) - 1)
+
+struct taint_flag {
+	char c_true;	/* character printed when tainted */
+	char c_false;	/* character printed when not tainted */
+	bool module;	/* also show as a per-module taint flag */
+};
+
+extern const struct taint_flag taint_flags[TAINT_FLAGS_COUNT];
+
+enum lockdep_ok {
+	LOCKDEP_STILL_OK,
+	LOCKDEP_NOW_UNRELIABLE,
+};
+
+extern const char *print_tainted(void);
+extern void add_taint(unsigned flag, enum lockdep_ok);
+extern int test_taint(unsigned flag);
+extern unsigned long get_taint(void);
+
+#endif	/* _LINUX_PANIC_H */
diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
new file mode 100644
index 000000000000..41e32483d7a7
--- /dev/null
+++ b/include/linux/panic_notifier.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_PANIC_NOTIFIERS_H
+#define _LINUX_PANIC_NOTIFIERS_H
+
+#include <linux/notifier.h>
+#include <linux/types.h>
+
+extern struct atomic_notifier_head panic_notifier_list;
+
+extern bool crash_kexec_post_notifiers;
+
+#endif	/* _LINUX_PANIC_NOTIFIERS_H */
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index bb2e3e15c84c..2871076e4d29 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -15,6 +15,7 @@
 #include <linux/kthread.h>
 #include <linux/lockdep.h>
 #include <linux/export.h>
+#include <linux/panic_notifier.h>
 #include <linux/sysctl.h>
 #include <linux/suspend.h>
 #include <linux/utsname.h>
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index f099baee3578..4b34a9aa32bc 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -26,6 +26,7 @@
 #include <linux/suspend.h>
 #include <linux/device.h>
 #include <linux/freezer.h>
+#include <linux/panic_notifier.h>
 #include <linux/pm.h>
 #include <linux/cpu.h>
 #include <linux/uaccess.h>
diff --git a/kernel/panic.c b/kernel/panic.c
index 332736a72a58..edad89660a2b 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -23,6 +23,7 @@
 #include <linux/reboot.h>
 #include <linux/delay.h>
 #include <linux/kexec.h>
+#include <linux/panic_notifier.h>
 #include <linux/sched.h>
 #include <linux/sysrq.h>
 #include <linux/init.h>
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index ce5b4cd6bd18..a58c9c86fa13 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -32,6 +32,8 @@
 #include <linux/export.h>
 #include <linux/completion.h>
 #include <linux/moduleparam.h>
+#include <linux/panic.h>
+#include <linux/panic_notifier.h>
 #include <linux/percpu.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 3601786ddaeb..e5cf9c4ef5e1 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -27,6 +27,7 @@
 #include <linux/sysctl.h>
 #include <linux/bitmap.h>
 #include <linux/signal.h>
+#include <linux/panic.h>
 #include <linux/printk.h>
 #include <linux/proc_fs.h>
 #include <linux/security.h>
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 507a30bf26e4..9612a1d8fa13 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -39,6 +39,7 @@
 #include <linux/slab.h>
 #include <linux/ctype.h>
 #include <linux/init.h>
+#include <linux/panic_notifier.h>
 #include <linux/poll.h>
 #include <linux/nmi.h>
 #include <linux/fs.h>
-- 
2.30.2


^ permalink raw reply related

* [PATCH] powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an overflown PMC
From: Athira Rajeev @ 2021-04-06 14:14 UTC (permalink / raw)
  To: mpe; +Cc: nasastry, maddy, linuxppc-dev, npiggin

Running perf fuzzer showed below in dmesg logs:
"Can't find PMC that caused IRQ"

This means a PMU exception happened, but none of the PMC's (Performance
Monitor Counter) were found to be overflown. There are some corner cases
that clears the PMCs after PMI gets masked. In such cases, the perf
interrupt handler will not find the active PMC values that had caused
the overflow and thus leads to this message while replaying.

Case 1: PMU Interrupt happens during replay of other interrupts and
counter values gets cleared by PMU callbacks before replay:

During replay of interrupts like timer, __do_irq and doorbell exception, we
conditionally enable interrupts via may_hard_irq_enable(). This could
potentially create a window to generate a PMI. Since irq soft mask is set
to ALL_DISABLED, the PMI will get masked here. We could get IPIs run before
perf interrupt is replayed and the PMU events could deleted or stopped.
This will change the PMU SPR values and resets the counters. Snippet of
ftrace log showing PMU callbacks invoked in "__do_irq":

<idle>-0 [051] dns. 132025441306354: __do_irq <-call_do_irq
<idle>-0 [051] dns. 132025441306430: irq_enter <-__do_irq
<idle>-0 [051] dns. 132025441306503: irq_enter_rcu <-__do_irq
<idle>-0 [051] dnH. 132025441306599: xive_get_irq <-__do_irq
<<>>
<idle>-0 [051] dnH. 132025441307770: generic_smp_call_function_single_interrupt <-smp_ipi_demux_relaxed
<idle>-0 [051] dnH. 132025441307839: flush_smp_call_function_queue <-smp_ipi_demux_relaxed
<idle>-0 [051] dnH. 132025441308057: _raw_spin_lock <-event_function
<idle>-0 [051] dnH. 132025441308206: power_pmu_disable <-perf_pmu_disable
<idle>-0 [051] dnH. 132025441308337: power_pmu_del <-event_sched_out
<idle>-0 [051] dnH. 132025441308407: power_pmu_read <-power_pmu_del
<idle>-0 [051] dnH. 132025441308477: read_pmc <-power_pmu_read
<idle>-0 [051] dnH. 132025441308590: isa207_disable_pmc <-power_pmu_del
<idle>-0 [051] dnH. 132025441308663: write_pmc <-power_pmu_del
<idle>-0 [051] dnH. 132025441308787: power_pmu_event_idx <-perf_event_update_userpage
<idle>-0 [051] dnH. 132025441308859: rcu_read_unlock_strict <-perf_event_update_userpage
<idle>-0 [051] dnH. 132025441308975: power_pmu_enable <-perf_pmu_enable
<<>>
<idle>-0 [051] dnH. 132025441311108: irq_exit <-__do_irq
<idle>-0 [051] dns. 132025441311319: performance_monitor_exception <-replay_soft_interrupts

Case 2: PMI's masked during local_* operations, example local_add.
If the local_add operation happens within a local_irq_save, replay of
PMI will be during local_irq_restore. Similar to case 1, this could
also create a window before replay where PMU events gets deleted or
stopped.

Patch adds a fix to update the PMU callback functions (del,stop,enable) to
check for pending perf interrupt. If there is an overflown PMC and pending
perf interrupt indicated in Paca, clear the PMI bit in paca to drop that
sample. In case of power_pmu_del, also clear the MMCR0 PMAO bit which
otherwise could lead to spurious interrupts in some corner cases. Example,
a timer after power_pmu_del which will re-enable interrupts since PMI is
cleared and triggers a PMI again since PMAO bit is still set.

We can't just replay PMI any time. Hence this approach is preferred rather
than replaying PMI before resetting overflown PMC. Patch also documents
core-book3s on a race condition which can trigger these PMC messages during
idle path in PowerNV.

Fixes: f442d004806e ("powerpc/64s: Add support to mask perf interrupts and replay them")
Reported-by: Nageswara R Sastry <nasastry@in.ibm.com>
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Suggested-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/pmc.h  | 11 +++++++++
 arch/powerpc/perf/core-book3s.c | 55 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/arch/powerpc/include/asm/pmc.h b/arch/powerpc/include/asm/pmc.h
index c6bbe9778d3c..97b4bd8de25b 100644
--- a/arch/powerpc/include/asm/pmc.h
+++ b/arch/powerpc/include/asm/pmc.h
@@ -34,11 +34,22 @@ static inline void ppc_set_pmu_inuse(int inuse)
 #endif
 }
 
+static inline int clear_paca_irq_pmi(void)
+{
+	if (get_paca()->irq_happened & PACA_IRQ_PMI) {
+		WARN_ON_ONCE(mfmsr() & MSR_EE);
+		get_paca()->irq_happened &= ~PACA_IRQ_PMI;
+		return 1;
+	}
+	return 0;
+}
+
 extern void power4_enable_pmcs(void);
 
 #else /* CONFIG_PPC64 */
 
 static inline void ppc_set_pmu_inuse(int inuse) { }
+static inline int clear_paca_irq_pmi(void) { return 0; }
 
 #endif
 
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 766f064f00fb..18ca3c90f866 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -847,6 +847,20 @@ static void write_pmc(int idx, unsigned long val)
 	}
 }
 
+static int pmc_overflown(int idx)
+{
+	unsigned long val[8];
+	int i;
+
+	for (i = 0; i < ppmu->n_counter; i++)
+		val[i] = read_pmc(i + 1);
+
+	if ((int)val[idx-1] < 0)
+		return 1;
+
+	return 0;
+}
+
 /* Called from sysrq_handle_showregs() */
 void perf_event_print_debug(void)
 {
@@ -1438,6 +1452,15 @@ static void power_pmu_enable(struct pmu *pmu)
 		event = cpuhw->event[i];
 		if (event->hw.idx && event->hw.idx != hwc_index[i] + 1) {
 			power_pmu_read(event);
+			/*
+			 * if the PMC corresponding to event->hw.idx is
+			 * overflown, check if there is any pending perf
+			 * interrupt set in paca. If so, disable the interrupt
+			 * by clearing the paca bit for PMI since we are going
+			 * to reset the PMC.
+			 */
+			if (pmc_overflown(event->hw.idx))
+				clear_paca_irq_pmi();
 			write_pmc(event->hw.idx, 0);
 			event->hw.idx = 0;
 		}
@@ -1474,6 +1497,10 @@ static void power_pmu_enable(struct pmu *pmu)
 		event->hw.idx = idx;
 		if (event->hw.state & PERF_HES_STOPPED)
 			val = 0;
+
+		/* See above for clear_paca_irq_pmi */
+		if (pmc_overflown(event->hw.idx))
+			clear_paca_irq_pmi();
 		write_pmc(idx, val);
 
 		perf_event_update_userpage(event);
@@ -1619,6 +1646,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
 	struct cpu_hw_events *cpuhw;
 	long i;
 	unsigned long flags;
+	unsigned long val_mmcr0;
 
 	local_irq_save(flags);
 	perf_pmu_disable(event->pmu);
@@ -1636,6 +1664,22 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
 			--cpuhw->n_events;
 			ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);
 			if (event->hw.idx) {
+				/*
+				 * if the PMC corresponding to event->hw.idx is
+				 * overflown, check if there is any pending perf
+				 * interrupt set in paca. If so, disable the interrupt
+				 * and clear the MMCR0 PMAO bit since we are going
+				 * to reset the PMC and delete the event.
+				 */
+				if (pmc_overflown(event->hw.idx)) {
+					if (clear_paca_irq_pmi()) {
+						val_mmcr0 = mfspr(SPRN_MMCR0);
+						val_mmcr0 &= ~MMCR0_PMAO;
+						write_mmcr0(cpuhw, val_mmcr0);
+						mb();
+						isync();
+					}
+				}
 				write_pmc(event->hw.idx, 0);
 				event->hw.idx = 0;
 			}
@@ -1714,6 +1758,8 @@ static void power_pmu_stop(struct perf_event *event, int ef_flags)
 
 	local_irq_save(flags);
 	perf_pmu_disable(event->pmu);
+	if (pmc_overflown(event->hw.idx))
+		clear_paca_irq_pmi();
 
 	power_pmu_read(event);
 	event->hw.state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
@@ -2343,6 +2389,15 @@ static void __perf_event_interrupt(struct pt_regs *regs)
 			}
 		}
 	}
+
+	/*
+	 * During system wide profling or while specific CPU
+	 * is monitored for an event, some corner cases could
+	 * cause PMC to overflow in idle path. This will trigger
+	 * a PMI after waking up from idle. Since counter values
+	 * are _not_ saved/restored in idle path, can lead to
+	 * below "Can't find PMC" message.
+	 */
 	if (unlikely(!found) && !arch_irq_disabled_regs(regs))
 		printk_ratelimited(KERN_WARNING "Can't find PMC that caused IRQ\n");
 
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Bjorn Andersson @ 2021-04-06 14:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Corey Minyard, linux-hyperv, Tetsuo Handa, linux-remoteproc,
	Michael Kelley, Paul Mackerras, H. Peter Anvin, Joel Fernandes,
	K. Y. Srinivasan, Thomas Gleixner, linux-arch, Wei Liu,
	Stephen Hemminger, Corey Minyard, x86, Ingo Molnar, Iurii Zaikin,
	Ohad Ben-Cohen, Joerg Roedel, Kees Cook, Paul E. McKenney,
	Lai Jiangshan, Haiyang Zhang, Josh Triplett,
	Steven Rostedt (VMware), rcu, Borislav Petkov, openipmi-developer,
	Vlastimil Babka, Mathieu Poirier, kexec, linux-kernel,
	Luis Chamberlain, Arnd Bergmann, Eric Biederman, linux-fsdevel,
	Mathieu Desnoyers, Andrew Morton, linuxppc-dev, Mike Rapoport
In-Reply-To: <20210406133158.73700-1-andriy.shevchenko@linux.intel.com>

On Tue 06 Apr 08:31 CDT 2021, Andy Shevchenko wrote:

> kernel.h is being used as a dump for all kinds of stuff for a long time.
> Here is the attempt to start cleaning it up by splitting out panic and
> oops helpers.
> 
> At the same time convert users in header and lib folder to use new header.
> Though for time being include new header back to kernel.h to avoid twisted
> indirected includes for existing users.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

^ permalink raw reply

* Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Mike Rapoport @ 2021-04-06 14:39 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Corey Minyard, linux-hyperv, Tetsuo Handa, linux-remoteproc,
	Bjorn Andersson, Paul Mackerras, H. Peter Anvin, Joel Fernandes,
	K. Y. Srinivasan, Thomas Gleixner, linux-arch, Wei Liu,
	Stephen Hemminger, Corey Minyard, x86, Ingo Molnar, Iurii Zaikin,
	Ohad Ben-Cohen, Joerg Roedel, Kees Cook, Paul E. McKenney,
	Lai Jiangshan, Haiyang Zhang, Josh Triplett,
	Steven Rostedt (VMware), rcu, Borislav Petkov, openipmi-developer,
	Michael Kelley, Vlastimil Babka, Mathieu Poirier, kexec,
	linux-kernel, Luis Chamberlain, Arnd Bergmann, Eric Biederman,
	linux-fsdevel, Mathieu Desnoyers, Andrew Morton, linuxppc-dev
In-Reply-To: <20210406133158.73700-1-andriy.shevchenko@linux.intel.com>

On Tue, Apr 06, 2021 at 04:31:58PM +0300, Andy Shevchenko wrote:
> kernel.h is being used as a dump for all kinds of stuff for a long time.
> Here is the attempt to start cleaning it up by splitting out panic and
> oops helpers.
> 
> At the same time convert users in header and lib folder to use new header.
> Though for time being include new header back to kernel.h to avoid twisted
> indirected includes for existing users.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
>  arch/powerpc/kernel/setup-common.c   |  1 +
>  arch/x86/include/asm/desc.h          |  1 +
>  arch/x86/kernel/cpu/mshyperv.c       |  1 +
>  arch/x86/kernel/setup.c              |  1 +
>  drivers/char/ipmi/ipmi_msghandler.c  |  1 +
>  drivers/remoteproc/remoteproc_core.c |  1 +
>  include/asm-generic/bug.h            |  3 +-
>  include/linux/kernel.h               | 84 +-----------------------
>  include/linux/panic.h                | 98 ++++++++++++++++++++++++++++
>  include/linux/panic_notifier.h       | 12 ++++
>  kernel/hung_task.c                   |  1 +
>  kernel/kexec_core.c                  |  1 +
>  kernel/panic.c                       |  1 +
>  kernel/rcu/tree.c                    |  2 +
>  kernel/sysctl.c                      |  1 +
>  kernel/trace/trace.c                 |  1 +
>  16 files changed, 126 insertions(+), 84 deletions(-)
>  create mode 100644 include/linux/panic.h
>  create mode 100644 include/linux/panic_notifier.h
> 
> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> index 476082a83d1c..ceb12683b6d1 100644
> --- a/arch/x86/include/asm/desc.h
> +++ b/arch/x86/include/asm/desc.h
> @@ -9,6 +9,7 @@
>  #include <asm/irq_vectors.h>
>  #include <asm/cpu_entry_area.h>
>  
> +#include <linux/debug_locks.h>

This seems unrelated, but I might be missing something.

>  #include <linux/smp.h>
>  #include <linux/percpu.h>
>  

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH] powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an overflown PMC
From: Athira Rajeev @ 2021-04-06 14:40 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linuxppc-dev, Madhavan Srinivasan, nasastry@in.ibm.com, npiggin
In-Reply-To: <1617718443-1422-1-git-send-email-atrajeev@linux.vnet.ibm.com>

Hi,

Cover letter is missing in this patch. I will resent the patch along with cover letter. 
Sorry for the noise.

Thanks,
Athira
> On 06-Apr-2021, at 7:44 PM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
> 
> Running perf fuzzer showed below in dmesg logs:
> "Can't find PMC that caused IRQ"
> 
> This means a PMU exception happened, but none of the PMC's (Performance
> Monitor Counter) were found to be overflown. There are some corner cases
> that clears the PMCs after PMI gets masked. In such cases, the perf
> interrupt handler will not find the active PMC values that had caused
> the overflow and thus leads to this message while replaying.
> 
> Case 1: PMU Interrupt happens during replay of other interrupts and
> counter values gets cleared by PMU callbacks before replay:
> 
> During replay of interrupts like timer, __do_irq and doorbell exception, we
> conditionally enable interrupts via may_hard_irq_enable(). This could
> potentially create a window to generate a PMI. Since irq soft mask is set
> to ALL_DISABLED, the PMI will get masked here. We could get IPIs run before
> perf interrupt is replayed and the PMU events could deleted or stopped.
> This will change the PMU SPR values and resets the counters. Snippet of
> ftrace log showing PMU callbacks invoked in "__do_irq":
> 
> <idle>-0 [051] dns. 132025441306354: __do_irq <-call_do_irq
> <idle>-0 [051] dns. 132025441306430: irq_enter <-__do_irq
> <idle>-0 [051] dns. 132025441306503: irq_enter_rcu <-__do_irq
> <idle>-0 [051] dnH. 132025441306599: xive_get_irq <-__do_irq
> <<>>
> <idle>-0 [051] dnH. 132025441307770: generic_smp_call_function_single_interrupt <-smp_ipi_demux_relaxed
> <idle>-0 [051] dnH. 132025441307839: flush_smp_call_function_queue <-smp_ipi_demux_relaxed
> <idle>-0 [051] dnH. 132025441308057: _raw_spin_lock <-event_function
> <idle>-0 [051] dnH. 132025441308206: power_pmu_disable <-perf_pmu_disable
> <idle>-0 [051] dnH. 132025441308337: power_pmu_del <-event_sched_out
> <idle>-0 [051] dnH. 132025441308407: power_pmu_read <-power_pmu_del
> <idle>-0 [051] dnH. 132025441308477: read_pmc <-power_pmu_read
> <idle>-0 [051] dnH. 132025441308590: isa207_disable_pmc <-power_pmu_del
> <idle>-0 [051] dnH. 132025441308663: write_pmc <-power_pmu_del
> <idle>-0 [051] dnH. 132025441308787: power_pmu_event_idx <-perf_event_update_userpage
> <idle>-0 [051] dnH. 132025441308859: rcu_read_unlock_strict <-perf_event_update_userpage
> <idle>-0 [051] dnH. 132025441308975: power_pmu_enable <-perf_pmu_enable
> <<>>
> <idle>-0 [051] dnH. 132025441311108: irq_exit <-__do_irq
> <idle>-0 [051] dns. 132025441311319: performance_monitor_exception <-replay_soft_interrupts
> 
> Case 2: PMI's masked during local_* operations, example local_add.
> If the local_add operation happens within a local_irq_save, replay of
> PMI will be during local_irq_restore. Similar to case 1, this could
> also create a window before replay where PMU events gets deleted or
> stopped.
> 
> Patch adds a fix to update the PMU callback functions (del,stop,enable) to
> check for pending perf interrupt. If there is an overflown PMC and pending
> perf interrupt indicated in Paca, clear the PMI bit in paca to drop that
> sample. In case of power_pmu_del, also clear the MMCR0 PMAO bit which
> otherwise could lead to spurious interrupts in some corner cases. Example,
> a timer after power_pmu_del which will re-enable interrupts since PMI is
> cleared and triggers a PMI again since PMAO bit is still set.
> 
> We can't just replay PMI any time. Hence this approach is preferred rather
> than replaying PMI before resetting overflown PMC. Patch also documents
> core-book3s on a race condition which can trigger these PMC messages during
> idle path in PowerNV.
> 
> Fixes: f442d004806e ("powerpc/64s: Add support to mask perf interrupts and replay them")
> Reported-by: Nageswara R Sastry <nasastry@in.ibm.com>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Suggested-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/pmc.h  | 11 +++++++++
> arch/powerpc/perf/core-book3s.c | 55 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/pmc.h b/arch/powerpc/include/asm/pmc.h
> index c6bbe9778d3c..97b4bd8de25b 100644
> --- a/arch/powerpc/include/asm/pmc.h
> +++ b/arch/powerpc/include/asm/pmc.h
> @@ -34,11 +34,22 @@ static inline void ppc_set_pmu_inuse(int inuse)
> #endif
> }
> 
> +static inline int clear_paca_irq_pmi(void)
> +{
> +	if (get_paca()->irq_happened & PACA_IRQ_PMI) {
> +		WARN_ON_ONCE(mfmsr() & MSR_EE);
> +		get_paca()->irq_happened &= ~PACA_IRQ_PMI;
> +		return 1;
> +	}
> +	return 0;
> +}
> +
> extern void power4_enable_pmcs(void);
> 
> #else /* CONFIG_PPC64 */
> 
> static inline void ppc_set_pmu_inuse(int inuse) { }
> +static inline int clear_paca_irq_pmi(void) { return 0; }
> 
> #endif
> 
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 766f064f00fb..18ca3c90f866 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -847,6 +847,20 @@ static void write_pmc(int idx, unsigned long val)
> 	}
> }
> 
> +static int pmc_overflown(int idx)
> +{
> +	unsigned long val[8];
> +	int i;
> +
> +	for (i = 0; i < ppmu->n_counter; i++)
> +		val[i] = read_pmc(i + 1);
> +
> +	if ((int)val[idx-1] < 0)
> +		return 1;
> +
> +	return 0;
> +}
> +
> /* Called from sysrq_handle_showregs() */
> void perf_event_print_debug(void)
> {
> @@ -1438,6 +1452,15 @@ static void power_pmu_enable(struct pmu *pmu)
> 		event = cpuhw->event[i];
> 		if (event->hw.idx && event->hw.idx != hwc_index[i] + 1) {
> 			power_pmu_read(event);
> +			/*
> +			 * if the PMC corresponding to event->hw.idx is
> +			 * overflown, check if there is any pending perf
> +			 * interrupt set in paca. If so, disable the interrupt
> +			 * by clearing the paca bit for PMI since we are going
> +			 * to reset the PMC.
> +			 */
> +			if (pmc_overflown(event->hw.idx))
> +				clear_paca_irq_pmi();
> 			write_pmc(event->hw.idx, 0);
> 			event->hw.idx = 0;
> 		}
> @@ -1474,6 +1497,10 @@ static void power_pmu_enable(struct pmu *pmu)
> 		event->hw.idx = idx;
> 		if (event->hw.state & PERF_HES_STOPPED)
> 			val = 0;
> +
> +		/* See above for clear_paca_irq_pmi */
> +		if (pmc_overflown(event->hw.idx))
> +			clear_paca_irq_pmi();
> 		write_pmc(idx, val);
> 
> 		perf_event_update_userpage(event);
> @@ -1619,6 +1646,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
> 	struct cpu_hw_events *cpuhw;
> 	long i;
> 	unsigned long flags;
> +	unsigned long val_mmcr0;
> 
> 	local_irq_save(flags);
> 	perf_pmu_disable(event->pmu);
> @@ -1636,6 +1664,22 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
> 			--cpuhw->n_events;
> 			ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);
> 			if (event->hw.idx) {
> +				/*
> +				 * if the PMC corresponding to event->hw.idx is
> +				 * overflown, check if there is any pending perf
> +				 * interrupt set in paca. If so, disable the interrupt
> +				 * and clear the MMCR0 PMAO bit since we are going
> +				 * to reset the PMC and delete the event.
> +				 */
> +				if (pmc_overflown(event->hw.idx)) {
> +					if (clear_paca_irq_pmi()) {
> +						val_mmcr0 = mfspr(SPRN_MMCR0);
> +						val_mmcr0 &= ~MMCR0_PMAO;
> +						write_mmcr0(cpuhw, val_mmcr0);
> +						mb();
> +						isync();
> +					}
> +				}
> 				write_pmc(event->hw.idx, 0);
> 				event->hw.idx = 0;
> 			}
> @@ -1714,6 +1758,8 @@ static void power_pmu_stop(struct perf_event *event, int ef_flags)
> 
> 	local_irq_save(flags);
> 	perf_pmu_disable(event->pmu);
> +	if (pmc_overflown(event->hw.idx))
> +		clear_paca_irq_pmi();
> 
> 	power_pmu_read(event);
> 	event->hw.state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
> @@ -2343,6 +2389,15 @@ static void __perf_event_interrupt(struct pt_regs *regs)
> 			}
> 		}
> 	}
> +
> +	/*
> +	 * During system wide profling or while specific CPU
> +	 * is monitored for an event, some corner cases could
> +	 * cause PMC to overflow in idle path. This will trigger
> +	 * a PMI after waking up from idle. Since counter values
> +	 * are _not_ saved/restored in idle path, can lead to
> +	 * below "Can't find PMC" message.
> +	 */
> 	if (unlikely(!found) && !arch_irq_disabled_regs(regs))
> 		printk_ratelimited(KERN_WARNING "Can't find PMC that caused IRQ\n");
> 
> -- 
> 1.8.3.1
> 


^ permalink raw reply

* Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Christian Brauner @ 2021-04-06 14:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Corey Minyard, linux-hyperv, Tetsuo Handa, linux-remoteproc,
	Michael Kelley, Paul Mackerras, H. Peter Anvin, Joel Fernandes,
	K. Y. Srinivasan, Thomas Gleixner, linux-arch, Wei Liu,
	Stephen Hemminger, Corey Minyard, x86, Ingo Molnar, Iurii Zaikin,
	Ohad Ben-Cohen, Joerg Roedel, Kees Cook, Paul E. McKenney,
	Lai Jiangshan, Haiyang Zhang, Josh Triplett,
	Steven Rostedt (VMware), rcu, Borislav Petkov, openipmi-developer,
	Bjorn Andersson, Vlastimil Babka, Mathieu Poirier, kexec,
	linux-kernel, Luis Chamberlain, Arnd Bergmann, Eric Biederman,
	linux-fsdevel, Mathieu Desnoyers, Andrew Morton, linuxppc-dev,
	Mike Rapoport
In-Reply-To: <20210406133158.73700-1-andriy.shevchenko@linux.intel.com>

On Tue, Apr 06, 2021 at 04:31:58PM +0300, Andy Shevchenko wrote:
> kernel.h is being used as a dump for all kinds of stuff for a long time.
> Here is the attempt to start cleaning it up by splitting out panic and
> oops helpers.
> 
> At the same time convert users in header and lib folder to use new header.
> Though for time being include new header back to kernel.h to avoid twisted
> indirected includes for existing users.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

(I think David has tried something like this a few years ago too?)
Good idea in any case. (Be good to see kbuild do an allmodconfig build
of this though.)
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

>  arch/powerpc/kernel/setup-common.c   |  1 +
>  arch/x86/include/asm/desc.h          |  1 +
>  arch/x86/kernel/cpu/mshyperv.c       |  1 +
>  arch/x86/kernel/setup.c              |  1 +
>  drivers/char/ipmi/ipmi_msghandler.c  |  1 +
>  drivers/remoteproc/remoteproc_core.c |  1 +
>  include/asm-generic/bug.h            |  3 +-
>  include/linux/kernel.h               | 84 +-----------------------
>  include/linux/panic.h                | 98 ++++++++++++++++++++++++++++
>  include/linux/panic_notifier.h       | 12 ++++
>  kernel/hung_task.c                   |  1 +
>  kernel/kexec_core.c                  |  1 +
>  kernel/panic.c                       |  1 +
>  kernel/rcu/tree.c                    |  2 +
>  kernel/sysctl.c                      |  1 +
>  kernel/trace/trace.c                 |  1 +
>  16 files changed, 126 insertions(+), 84 deletions(-)
>  create mode 100644 include/linux/panic.h
>  create mode 100644 include/linux/panic_notifier.h
> 
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 74a98fff2c2f..046fe21b5c3b 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -9,6 +9,7 @@
>  #undef DEBUG
>  
>  #include <linux/export.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/string.h>
>  #include <linux/sched.h>
>  #include <linux/init.h>
> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> index 476082a83d1c..ceb12683b6d1 100644
> --- a/arch/x86/include/asm/desc.h
> +++ b/arch/x86/include/asm/desc.h
> @@ -9,6 +9,7 @@
>  #include <asm/irq_vectors.h>
>  #include <asm/cpu_entry_area.h>
>  
> +#include <linux/debug_locks.h>
>  #include <linux/smp.h>
>  #include <linux/percpu.h>
>  
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 22f13343b5da..9e5c6f2b044d 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -17,6 +17,7 @@
>  #include <linux/irq.h>
>  #include <linux/kexec.h>
>  #include <linux/i8253.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/random.h>
>  #include <asm/processor.h>
>  #include <asm/hypervisor.h>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 59e5e0903b0c..570699eecf90 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -14,6 +14,7 @@
>  #include <linux/initrd.h>
>  #include <linux/iscsi_ibft.h>
>  #include <linux/memblock.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/pci.h>
>  #include <linux/root_dev.h>
>  #include <linux/hugetlb.h>
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index 8a0e97b33cae..e96cb5c4f97a 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -16,6 +16,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/errno.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/poll.h>
>  #include <linux/sched.h>
>  #include <linux/seq_file.h>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 626a6b90fba2..76dd8e2b1e7e 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -20,6 +20,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/device.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
>  #include <linux/dma-map-ops.h>
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 76a10e0dca9f..719410b93f99 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -17,7 +17,8 @@
>  #endif
>  
>  #ifndef __ASSEMBLY__
> -#include <linux/kernel.h>
> +#include <linux/panic.h>
> +#include <linux/printk.h>
>  
>  #ifdef CONFIG_BUG
>  
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 09035ac67d4b..6c5a05ac1ecb 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -14,6 +14,7 @@
>  #include <linux/math.h>
>  #include <linux/minmax.h>
>  #include <linux/typecheck.h>
> +#include <linux/panic.h>
>  #include <linux/printk.h>
>  #include <linux/build_bug.h>
>  #include <linux/static_call_types.h>
> @@ -70,7 +71,6 @@
>  #define lower_32_bits(n) ((u32)((n) & 0xffffffff))
>  
>  struct completion;
> -struct pt_regs;
>  struct user;
>  
>  #ifdef CONFIG_PREEMPT_VOLUNTARY
> @@ -175,14 +175,6 @@ void __might_fault(const char *file, int line);
>  static inline void might_fault(void) { }
>  #endif
>  
> -extern struct atomic_notifier_head panic_notifier_list;
> -extern long (*panic_blink)(int state);
> -__printf(1, 2)
> -void panic(const char *fmt, ...) __noreturn __cold;
> -void nmi_panic(struct pt_regs *regs, const char *msg);
> -extern void oops_enter(void);
> -extern void oops_exit(void);
> -extern bool oops_may_print(void);
>  void do_exit(long error_code) __noreturn;
>  void complete_and_exit(struct completion *, long) __noreturn;
>  
> @@ -368,52 +360,8 @@ extern int __kernel_text_address(unsigned long addr);
>  extern int kernel_text_address(unsigned long addr);
>  extern int func_ptr_is_kernel_text(void *ptr);
>  
> -#ifdef CONFIG_SMP
> -extern unsigned int sysctl_oops_all_cpu_backtrace;
> -#else
> -#define sysctl_oops_all_cpu_backtrace 0
> -#endif /* CONFIG_SMP */
> -
>  extern void bust_spinlocks(int yes);
> -extern int panic_timeout;
> -extern unsigned long panic_print;
> -extern int panic_on_oops;
> -extern int panic_on_unrecovered_nmi;
> -extern int panic_on_io_nmi;
> -extern int panic_on_warn;
> -extern unsigned long panic_on_taint;
> -extern bool panic_on_taint_nousertaint;
> -extern int sysctl_panic_on_rcu_stall;
> -extern int sysctl_max_rcu_stall_to_panic;
> -extern int sysctl_panic_on_stackoverflow;
> -
> -extern bool crash_kexec_post_notifiers;
>  
> -/*
> - * panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
> - * holds a CPU number which is executing panic() currently. A value of
> - * PANIC_CPU_INVALID means no CPU has entered panic() or crash_kexec().
> - */
> -extern atomic_t panic_cpu;
> -#define PANIC_CPU_INVALID	-1
> -
> -/*
> - * Only to be used by arch init code. If the user over-wrote the default
> - * CONFIG_PANIC_TIMEOUT, honor it.
> - */
> -static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
> -{
> -	if (panic_timeout == arch_default_timeout)
> -		panic_timeout = timeout;
> -}
> -extern const char *print_tainted(void);
> -enum lockdep_ok {
> -	LOCKDEP_STILL_OK,
> -	LOCKDEP_NOW_UNRELIABLE
> -};
> -extern void add_taint(unsigned flag, enum lockdep_ok);
> -extern int test_taint(unsigned flag);
> -extern unsigned long get_taint(void);
>  extern int root_mountflags;
>  
>  extern bool early_boot_irqs_disabled;
> @@ -432,36 +380,6 @@ extern enum system_states {
>  	SYSTEM_SUSPEND,
>  } system_state;
>  
> -/* This cannot be an enum because some may be used in assembly source. */
> -#define TAINT_PROPRIETARY_MODULE	0
> -#define TAINT_FORCED_MODULE		1
> -#define TAINT_CPU_OUT_OF_SPEC		2
> -#define TAINT_FORCED_RMMOD		3
> -#define TAINT_MACHINE_CHECK		4
> -#define TAINT_BAD_PAGE			5
> -#define TAINT_USER			6
> -#define TAINT_DIE			7
> -#define TAINT_OVERRIDDEN_ACPI_TABLE	8
> -#define TAINT_WARN			9
> -#define TAINT_CRAP			10
> -#define TAINT_FIRMWARE_WORKAROUND	11
> -#define TAINT_OOT_MODULE		12
> -#define TAINT_UNSIGNED_MODULE		13
> -#define TAINT_SOFTLOCKUP		14
> -#define TAINT_LIVEPATCH			15
> -#define TAINT_AUX			16
> -#define TAINT_RANDSTRUCT		17
> -#define TAINT_FLAGS_COUNT		18
> -#define TAINT_FLAGS_MAX			((1UL << TAINT_FLAGS_COUNT) - 1)
> -
> -struct taint_flag {
> -	char c_true;	/* character printed when tainted */
> -	char c_false;	/* character printed when not tainted */
> -	bool module;	/* also show as a per-module taint flag */
> -};
> -
> -extern const struct taint_flag taint_flags[TAINT_FLAGS_COUNT];
> -
>  extern const char hex_asc[];
>  #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
>  #define hex_asc_hi(x)	hex_asc[((x) & 0xf0) >> 4]
> diff --git a/include/linux/panic.h b/include/linux/panic.h
> new file mode 100644
> index 000000000000..f5844908a089
> --- /dev/null
> +++ b/include/linux/panic.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_PANIC_H
> +#define _LINUX_PANIC_H
> +
> +#include <linux/compiler_attributes.h>
> +#include <linux/types.h>
> +
> +struct pt_regs;
> +
> +extern long (*panic_blink)(int state);
> +__printf(1, 2)
> +void panic(const char *fmt, ...) __noreturn __cold;
> +void nmi_panic(struct pt_regs *regs, const char *msg);
> +extern void oops_enter(void);
> +extern void oops_exit(void);
> +extern bool oops_may_print(void);
> +
> +#ifdef CONFIG_SMP
> +extern unsigned int sysctl_oops_all_cpu_backtrace;
> +#else
> +#define sysctl_oops_all_cpu_backtrace 0
> +#endif /* CONFIG_SMP */
> +
> +extern int panic_timeout;
> +extern unsigned long panic_print;
> +extern int panic_on_oops;
> +extern int panic_on_unrecovered_nmi;
> +extern int panic_on_io_nmi;
> +extern int panic_on_warn;
> +
> +extern unsigned long panic_on_taint;
> +extern bool panic_on_taint_nousertaint;
> +
> +extern int sysctl_panic_on_rcu_stall;
> +extern int sysctl_max_rcu_stall_to_panic;
> +extern int sysctl_panic_on_stackoverflow;
> +
> +extern bool crash_kexec_post_notifiers;
> +
> +/*
> + * panic_cpu is used for synchronizing panic() and crash_kexec() execution. It
> + * holds a CPU number which is executing panic() currently. A value of
> + * PANIC_CPU_INVALID means no CPU has entered panic() or crash_kexec().
> + */
> +extern atomic_t panic_cpu;
> +#define PANIC_CPU_INVALID	-1
> +
> +/*
> + * Only to be used by arch init code. If the user over-wrote the default
> + * CONFIG_PANIC_TIMEOUT, honor it.
> + */
> +static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
> +{
> +	if (panic_timeout == arch_default_timeout)
> +		panic_timeout = timeout;
> +}
> +
> +/* This cannot be an enum because some may be used in assembly source. */
> +#define TAINT_PROPRIETARY_MODULE	0
> +#define TAINT_FORCED_MODULE		1
> +#define TAINT_CPU_OUT_OF_SPEC		2
> +#define TAINT_FORCED_RMMOD		3
> +#define TAINT_MACHINE_CHECK		4
> +#define TAINT_BAD_PAGE			5
> +#define TAINT_USER			6
> +#define TAINT_DIE			7
> +#define TAINT_OVERRIDDEN_ACPI_TABLE	8
> +#define TAINT_WARN			9
> +#define TAINT_CRAP			10
> +#define TAINT_FIRMWARE_WORKAROUND	11
> +#define TAINT_OOT_MODULE		12
> +#define TAINT_UNSIGNED_MODULE		13
> +#define TAINT_SOFTLOCKUP		14
> +#define TAINT_LIVEPATCH			15
> +#define TAINT_AUX			16
> +#define TAINT_RANDSTRUCT		17
> +#define TAINT_FLAGS_COUNT		18
> +#define TAINT_FLAGS_MAX			((1UL << TAINT_FLAGS_COUNT) - 1)
> +
> +struct taint_flag {
> +	char c_true;	/* character printed when tainted */
> +	char c_false;	/* character printed when not tainted */
> +	bool module;	/* also show as a per-module taint flag */
> +};
> +
> +extern const struct taint_flag taint_flags[TAINT_FLAGS_COUNT];
> +
> +enum lockdep_ok {
> +	LOCKDEP_STILL_OK,
> +	LOCKDEP_NOW_UNRELIABLE,
> +};
> +
> +extern const char *print_tainted(void);
> +extern void add_taint(unsigned flag, enum lockdep_ok);
> +extern int test_taint(unsigned flag);
> +extern unsigned long get_taint(void);
> +
> +#endif	/* _LINUX_PANIC_H */
> diff --git a/include/linux/panic_notifier.h b/include/linux/panic_notifier.h
> new file mode 100644
> index 000000000000..41e32483d7a7
> --- /dev/null
> +++ b/include/linux/panic_notifier.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_PANIC_NOTIFIERS_H
> +#define _LINUX_PANIC_NOTIFIERS_H
> +
> +#include <linux/notifier.h>
> +#include <linux/types.h>
> +
> +extern struct atomic_notifier_head panic_notifier_list;
> +
> +extern bool crash_kexec_post_notifiers;
> +
> +#endif	/* _LINUX_PANIC_NOTIFIERS_H */
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index bb2e3e15c84c..2871076e4d29 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -15,6 +15,7 @@
>  #include <linux/kthread.h>
>  #include <linux/lockdep.h>
>  #include <linux/export.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/sysctl.h>
>  #include <linux/suspend.h>
>  #include <linux/utsname.h>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index f099baee3578..4b34a9aa32bc 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -26,6 +26,7 @@
>  #include <linux/suspend.h>
>  #include <linux/device.h>
>  #include <linux/freezer.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/pm.h>
>  #include <linux/cpu.h>
>  #include <linux/uaccess.h>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 332736a72a58..edad89660a2b 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -23,6 +23,7 @@
>  #include <linux/reboot.h>
>  #include <linux/delay.h>
>  #include <linux/kexec.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/sched.h>
>  #include <linux/sysrq.h>
>  #include <linux/init.h>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index ce5b4cd6bd18..a58c9c86fa13 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -32,6 +32,8 @@
>  #include <linux/export.h>
>  #include <linux/completion.h>
>  #include <linux/moduleparam.h>
> +#include <linux/panic.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/percpu.h>
>  #include <linux/notifier.h>
>  #include <linux/cpu.h>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 3601786ddaeb..e5cf9c4ef5e1 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -27,6 +27,7 @@
>  #include <linux/sysctl.h>
>  #include <linux/bitmap.h>
>  #include <linux/signal.h>
> +#include <linux/panic.h>
>  #include <linux/printk.h>
>  #include <linux/proc_fs.h>
>  #include <linux/security.h>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 507a30bf26e4..9612a1d8fa13 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -39,6 +39,7 @@
>  #include <linux/slab.h>
>  #include <linux/ctype.h>
>  #include <linux/init.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/poll.h>
>  #include <linux/nmi.h>
>  #include <linux/fs.h>
> -- 
> 2.30.2
> 

^ permalink raw reply

* [PATCH] powerpc/perf: Clear pending PMI in ppmu callbacks
From: Athira Rajeev @ 2021-04-06 14:47 UTC (permalink / raw)
  To: mpe; +Cc: nasastry, maddy, linuxppc-dev, npiggin

Running perf fuzzer testsuite popped up below messages
in the dmesg logs:

"Can't find PMC that caused IRQ"

This means a PMU exception happened, but none of the PMC's (Performance
Monitor Counter) were found to be overflown. Perf interrupt handler checks
the PMC's to see which PMC has overflown and if none of the PMCs are
overflown ( counter value not >= 0x80000000 ), it throws warning:
"Can't find PMC that caused IRQ".

Powerpc has capability to mask and replay a performance monitoring
interrupt (PMI). In case of replayed PMI, there are some corner cases
that clears the PMCs after masking. In such cases, the perf interrupt
handler will not find the active PMC values that had caused the overflow
and thus leading to this message. This patchset attempts to fix those
corner cases.

However there is one more case in PowerNV where these messages are
emitted during system wide profiling or when a specific CPU is monitored
for an event. That is, when a counter overflow just before entering idle
and a PMI gets triggered after wakeup from idle. Since PMCs
are not saved in the idle path, perf interrupt handler will not
find overflown counter value and emits the "Can't find PMC" messages.
This patch documents this race condition in powerpc core-book3s.

Patch fixes the ppmu callbacks to disable pending interrupt before clearing
the overflown PMC and documents the race condition in idle path.

Athira Rajeev (1):
  powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting
    an overflown PMC

 arch/powerpc/include/asm/pmc.h  | 11 +++++++++
 arch/powerpc/perf/core-book3s.c | 55 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

-- 
1.8.3.1


^ permalink raw reply

* [PATCH] powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an overflown PMC
From: Athira Rajeev @ 2021-04-06 14:47 UTC (permalink / raw)
  To: mpe; +Cc: nasastry, maddy, linuxppc-dev, npiggin
In-Reply-To: <1617720464-1651-1-git-send-email-atrajeev@linux.vnet.ibm.com>

Running perf fuzzer showed below in dmesg logs:
"Can't find PMC that caused IRQ"

This means a PMU exception happened, but none of the PMC's (Performance
Monitor Counter) were found to be overflown. There are some corner cases
that clears the PMCs after PMI gets masked. In such cases, the perf
interrupt handler will not find the active PMC values that had caused
the overflow and thus leads to this message while replaying.

Case 1: PMU Interrupt happens during replay of other interrupts and
counter values gets cleared by PMU callbacks before replay:

During replay of interrupts like timer, __do_irq and doorbell exception, we
conditionally enable interrupts via may_hard_irq_enable(). This could
potentially create a window to generate a PMI. Since irq soft mask is set
to ALL_DISABLED, the PMI will get masked here. We could get IPIs run before
perf interrupt is replayed and the PMU events could deleted or stopped.
This will change the PMU SPR values and resets the counters. Snippet of
ftrace log showing PMU callbacks invoked in "__do_irq":

<idle>-0 [051] dns. 132025441306354: __do_irq <-call_do_irq
<idle>-0 [051] dns. 132025441306430: irq_enter <-__do_irq
<idle>-0 [051] dns. 132025441306503: irq_enter_rcu <-__do_irq
<idle>-0 [051] dnH. 132025441306599: xive_get_irq <-__do_irq
<<>>
<idle>-0 [051] dnH. 132025441307770: generic_smp_call_function_single_interrupt <-smp_ipi_demux_relaxed
<idle>-0 [051] dnH. 132025441307839: flush_smp_call_function_queue <-smp_ipi_demux_relaxed
<idle>-0 [051] dnH. 132025441308057: _raw_spin_lock <-event_function
<idle>-0 [051] dnH. 132025441308206: power_pmu_disable <-perf_pmu_disable
<idle>-0 [051] dnH. 132025441308337: power_pmu_del <-event_sched_out
<idle>-0 [051] dnH. 132025441308407: power_pmu_read <-power_pmu_del
<idle>-0 [051] dnH. 132025441308477: read_pmc <-power_pmu_read
<idle>-0 [051] dnH. 132025441308590: isa207_disable_pmc <-power_pmu_del
<idle>-0 [051] dnH. 132025441308663: write_pmc <-power_pmu_del
<idle>-0 [051] dnH. 132025441308787: power_pmu_event_idx <-perf_event_update_userpage
<idle>-0 [051] dnH. 132025441308859: rcu_read_unlock_strict <-perf_event_update_userpage
<idle>-0 [051] dnH. 132025441308975: power_pmu_enable <-perf_pmu_enable
<<>>
<idle>-0 [051] dnH. 132025441311108: irq_exit <-__do_irq
<idle>-0 [051] dns. 132025441311319: performance_monitor_exception <-replay_soft_interrupts

Case 2: PMI's masked during local_* operations, example local_add.
If the local_add operation happens within a local_irq_save, replay of
PMI will be during local_irq_restore. Similar to case 1, this could
also create a window before replay where PMU events gets deleted or
stopped.

Patch adds a fix to update the PMU callback functions (del,stop,enable) to
check for pending perf interrupt. If there is an overflown PMC and pending
perf interrupt indicated in Paca, clear the PMI bit in paca to drop that
sample. In case of power_pmu_del, also clear the MMCR0 PMAO bit which
otherwise could lead to spurious interrupts in some corner cases. Example,
a timer after power_pmu_del which will re-enable interrupts since PMI is
cleared and triggers a PMI again since PMAO bit is still set.

We can't just replay PMI any time. Hence this approach is preferred rather
than replaying PMI before resetting overflown PMC. Patch also documents
core-book3s on a race condition which can trigger these PMC messages during
idle path in PowerNV.

Fixes: f442d004806e ("powerpc/64s: Add support to mask perf interrupts and replay them")
Reported-by: Nageswara R Sastry <nasastry@in.ibm.com>
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Suggested-by: Madhavan Srinivasan <maddy@linux.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/pmc.h  | 11 +++++++++
 arch/powerpc/perf/core-book3s.c | 55 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/arch/powerpc/include/asm/pmc.h b/arch/powerpc/include/asm/pmc.h
index c6bbe9778d3c..97b4bd8de25b 100644
--- a/arch/powerpc/include/asm/pmc.h
+++ b/arch/powerpc/include/asm/pmc.h
@@ -34,11 +34,22 @@ static inline void ppc_set_pmu_inuse(int inuse)
 #endif
 }
 
+static inline int clear_paca_irq_pmi(void)
+{
+	if (get_paca()->irq_happened & PACA_IRQ_PMI) {
+		WARN_ON_ONCE(mfmsr() & MSR_EE);
+		get_paca()->irq_happened &= ~PACA_IRQ_PMI;
+		return 1;
+	}
+	return 0;
+}
+
 extern void power4_enable_pmcs(void);
 
 #else /* CONFIG_PPC64 */
 
 static inline void ppc_set_pmu_inuse(int inuse) { }
+static inline int clear_paca_irq_pmi(void) { return 0; }
 
 #endif
 
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 766f064f00fb..18ca3c90f866 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -847,6 +847,20 @@ static void write_pmc(int idx, unsigned long val)
 	}
 }
 
+static int pmc_overflown(int idx)
+{
+	unsigned long val[8];
+	int i;
+
+	for (i = 0; i < ppmu->n_counter; i++)
+		val[i] = read_pmc(i + 1);
+
+	if ((int)val[idx-1] < 0)
+		return 1;
+
+	return 0;
+}
+
 /* Called from sysrq_handle_showregs() */
 void perf_event_print_debug(void)
 {
@@ -1438,6 +1452,15 @@ static void power_pmu_enable(struct pmu *pmu)
 		event = cpuhw->event[i];
 		if (event->hw.idx && event->hw.idx != hwc_index[i] + 1) {
 			power_pmu_read(event);
+			/*
+			 * if the PMC corresponding to event->hw.idx is
+			 * overflown, check if there is any pending perf
+			 * interrupt set in paca. If so, disable the interrupt
+			 * by clearing the paca bit for PMI since we are going
+			 * to reset the PMC.
+			 */
+			if (pmc_overflown(event->hw.idx))
+				clear_paca_irq_pmi();
 			write_pmc(event->hw.idx, 0);
 			event->hw.idx = 0;
 		}
@@ -1474,6 +1497,10 @@ static void power_pmu_enable(struct pmu *pmu)
 		event->hw.idx = idx;
 		if (event->hw.state & PERF_HES_STOPPED)
 			val = 0;
+
+		/* See above for clear_paca_irq_pmi */
+		if (pmc_overflown(event->hw.idx))
+			clear_paca_irq_pmi();
 		write_pmc(idx, val);
 
 		perf_event_update_userpage(event);
@@ -1619,6 +1646,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
 	struct cpu_hw_events *cpuhw;
 	long i;
 	unsigned long flags;
+	unsigned long val_mmcr0;
 
 	local_irq_save(flags);
 	perf_pmu_disable(event->pmu);
@@ -1636,6 +1664,22 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
 			--cpuhw->n_events;
 			ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);
 			if (event->hw.idx) {
+				/*
+				 * if the PMC corresponding to event->hw.idx is
+				 * overflown, check if there is any pending perf
+				 * interrupt set in paca. If so, disable the interrupt
+				 * and clear the MMCR0 PMAO bit since we are going
+				 * to reset the PMC and delete the event.
+				 */
+				if (pmc_overflown(event->hw.idx)) {
+					if (clear_paca_irq_pmi()) {
+						val_mmcr0 = mfspr(SPRN_MMCR0);
+						val_mmcr0 &= ~MMCR0_PMAO;
+						write_mmcr0(cpuhw, val_mmcr0);
+						mb();
+						isync();
+					}
+				}
 				write_pmc(event->hw.idx, 0);
 				event->hw.idx = 0;
 			}
@@ -1714,6 +1758,8 @@ static void power_pmu_stop(struct perf_event *event, int ef_flags)
 
 	local_irq_save(flags);
 	perf_pmu_disable(event->pmu);
+	if (pmc_overflown(event->hw.idx))
+		clear_paca_irq_pmi();
 
 	power_pmu_read(event);
 	event->hw.state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
@@ -2343,6 +2389,15 @@ static void __perf_event_interrupt(struct pt_regs *regs)
 			}
 		}
 	}
+
+	/*
+	 * During system wide profling or while specific CPU
+	 * is monitored for an event, some corner cases could
+	 * cause PMC to overflow in idle path. This will trigger
+	 * a PMI after waking up from idle. Since counter values
+	 * are _not_ saved/restored in idle path, can lead to
+	 * below "Can't find PMC" message.
+	 */
 	if (unlikely(!found) && !arch_irq_disabled_regs(regs))
 		printk_ratelimited(KERN_WARNING "Can't find PMC that caused IRQ\n");
 
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v1 1/1] kernel.h: Split out panic and oops helpers
From: Arnd Bergmann @ 2021-04-06 15:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Corey Minyard, Linux on Hyper-V List, Tetsuo Handa,
	linux-remoteproc, Michael Kelley, Paul Mackerras, H. Peter Anvin,
	Joel Fernandes, K. Y. Srinivasan, Thomas Gleixner, linux-arch,
	Wei Liu, Stephen Hemminger, Corey Minyard,
	the arch/x86 maintainers, Ingo Molnar, Iurii Zaikin,
	Ohad Ben-Cohen, Joerg Roedel, Kees Cook, Paul E. McKenney,
	Lai Jiangshan, Haiyang Zhang, Josh Triplett,
	Steven Rostedt (VMware), rcu, Borislav Petkov, openipmi-developer,
	Bjorn Andersson, Vlastimil Babka, Mathieu Poirier, kexec,
	Linux Kernel Mailing List, Luis Chamberlain, Eric Biederman,
	Linux FS-devel Mailing List, Mathieu Desnoyers, Andrew Morton,
	linuxppc-dev, Mike Rapoport
In-Reply-To: <20210406133158.73700-1-andriy.shevchenko@linux.intel.com>

On Tue, Apr 6, 2021 at 3:31 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> kernel.h is being used as a dump for all kinds of stuff for a long time.
> Here is the attempt to start cleaning it up by splitting out panic and
> oops helpers.
>
> At the same time convert users in header and lib folder to use new header.
> Though for time being include new header back to kernel.h to avoid twisted
> indirected includes for existing users.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Nice!

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox