[PATCH v2 2/2] powerpc/perf: Power11 Performance Monitoring support

2024-02-04 Thread Madhavan Srinivasan
Base enablement patch to register performance monitoring
hardware support for Power11. Most of fields are copied
from power10_pmu struct for power11_pmu struct.

Signed-off-by: Madhavan Srinivasan 
---
Changelog v1:
- Copied power10 struct for power11 with name change
  
 arch/powerpc/perf/core-book3s.c |  2 ++
 arch/powerpc/perf/internal.h|  1 +
 arch/powerpc/perf/power10-pmu.c | 27 +++
 3 files changed, 30 insertions(+)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index b7ff680cde96..6f0d46c53027 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2593,6 +2593,8 @@ static int __init init_ppc64_pmu(void)
return 0;
else if (!init_power10_pmu())
return 0;
+   else if (!init_power11_pmu())
+   return 0;
else if (!init_ppc970_pmu())
return 0;
else
diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
index 4c18b5504326..a70ac471a5a5 100644
--- a/arch/powerpc/perf/internal.h
+++ b/arch/powerpc/perf/internal.h
@@ -10,4 +10,5 @@ int __init init_power7_pmu(void);
 int __init init_power8_pmu(void);
 int __init init_power9_pmu(void);
 int __init init_power10_pmu(void);
+int __init init_power11_pmu(void);
 int __init init_generic_compat_pmu(void);
diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
index 9b5133e361a7..62a68b6b2d4b 100644
--- a/arch/powerpc/perf/power10-pmu.c
+++ b/arch/powerpc/perf/power10-pmu.c
@@ -634,3 +634,30 @@ int __init init_power10_pmu(void)
 
return 0;
 }
+
+static struct power_pmu power11_pmu;
+
+int __init init_power11_pmu(void)
+{
+   unsigned int pvr;
+   int rc;
+
+   pvr = mfspr(SPRN_PVR);
+   if (PVR_VER(pvr) != PVR_POWER11)
+   return -ENODEV;
+
+   /* Set the PERF_REG_EXTENDED_MASK here */
+   PERF_REG_EXTENDED_MASK = PERF_REG_PMU_MASK_31;
+
+   power11_pmu = power10_pmu;
+   power11_pmu.name = "Power11";
+
+   rc = register_power_pmu(_pmu);
+   if (rc)
+   return rc;
+
+   /* Tell userspace that EBB is supported */
+   cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_EBB;
+
+   return 0;
+}
-- 
2.43.0



[PATCH v2 1/2] powerpc: Add Power11 architected and raw mode

2024-02-04 Thread Madhavan Srinivasan
reg.h is updated with Power11 pvr. pvr_mask value of 0x0F07
means we are arch v3.1 compliant. This is used by phyp and
kvm when booting as a pseries guest to detect and enable
the appropriate hwcap, facility bits and PMU related fields.
Copied most of fields from Power10 table entry and added relevant
Power11 setup/restore and device tree routines.

Signed-off-by: Madhavan Srinivasan 
---
Changelog v1:
- no change in this patch.

 arch/powerpc/include/asm/cpu_setup.h  |  2 ++
 arch/powerpc/include/asm/cputable.h   |  3 ++
 arch/powerpc/include/asm/mce.h|  1 +
 arch/powerpc/include/asm/mmu.h|  1 +
 arch/powerpc/include/asm/reg.h|  1 +
 arch/powerpc/kernel/cpu_setup_power.c | 10 +++
 arch/powerpc/kernel/cpu_specs_book3s_64.h | 34 +++
 arch/powerpc/kernel/dt_cpu_ftrs.c | 15 ++
 arch/powerpc/kernel/mce_power.c   |  5 
 arch/powerpc/kernel/prom_init.c   | 10 ++-
 10 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cpu_setup.h 
b/arch/powerpc/include/asm/cpu_setup.h
index 30e2fe389502..ce800650bb8b 100644
--- a/arch/powerpc/include/asm/cpu_setup.h
+++ b/arch/powerpc/include/asm/cpu_setup.h
@@ -9,10 +9,12 @@ void __setup_cpu_power7(unsigned long offset, struct cpu_spec 
*spec);
 void __setup_cpu_power8(unsigned long offset, struct cpu_spec *spec);
 void __setup_cpu_power9(unsigned long offset, struct cpu_spec *spec);
 void __setup_cpu_power10(unsigned long offset, struct cpu_spec *spec);
+void __setup_cpu_power11(unsigned long offset, struct cpu_spec *spec);
 void __restore_cpu_power7(void);
 void __restore_cpu_power8(void);
 void __restore_cpu_power9(void);
 void __restore_cpu_power10(void);
+void __restore_cpu_power11(void);
 
 void __setup_cpu_e500v1(unsigned long offset, struct cpu_spec *spec);
 void __setup_cpu_e500v2(unsigned long offset, struct cpu_spec *spec);
diff --git a/arch/powerpc/include/asm/cputable.h 
b/arch/powerpc/include/asm/cputable.h
index 8765d5158324..3bd6e6e0224c 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -454,6 +454,9 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
CPU_FTR_DAWR | CPU_FTR_DAWR1 | \
CPU_FTR_DEXCR_NPHIE)
+
+#define CPU_FTRS_POWER11   CPU_FTRS_POWER10
+
 #define CPU_FTRS_CELL  (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index c9f0936bd3c9..241eee743fc5 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -257,6 +257,7 @@ long __machine_check_early_realmode_p7(struct pt_regs 
*regs);
 long __machine_check_early_realmode_p8(struct pt_regs *regs);
 long __machine_check_early_realmode_p9(struct pt_regs *regs);
 long __machine_check_early_realmode_p10(struct pt_regs *regs);
+long __machine_check_early_realmode_p11(struct pt_regs *regs);
 #endif /* CONFIG_PPC_BOOK3S_64 */
 
 #ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index d8b7e246a32f..61ebe5eff2c9 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -133,6 +133,7 @@
 #define MMU_FTRS_POWER8MMU_FTRS_POWER6
 #define MMU_FTRS_POWER9MMU_FTRS_POWER6
 #define MMU_FTRS_POWER10   MMU_FTRS_POWER6
+#define MMU_FTRS_POWER11   MMU_FTRS_POWER6
 #define MMU_FTRS_CELL  MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | \
MMU_FTR_CI_LARGE_PAGE
 #define MMU_FTRS_PA6T  MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | \
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 7fd09f25452d..7a7aa24bf57a 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1364,6 +1364,7 @@
 #define PVR_HX_C2000   0x0066
 #define PVR_POWER9 0x004E
 #define PVR_POWER100x0080
+#define PVR_POWER110x0082
 #define PVR_BE 0x0070
 #define PVR_PA6T   0x0090
 
diff --git a/arch/powerpc/kernel/cpu_setup_power.c 
b/arch/powerpc/kernel/cpu_setup_power.c
index 98bd4e6c1770..8c24fc67d90f 100644
--- a/arch/powerpc/kernel/cpu_setup_power.c
+++ b/arch/powerpc/kernel/cpu_setup_power.c
@@ -286,3 +286,13 @@ void __restore_cpu_power10(void)
init_HFSCR();
init_PMU_HV();
 }
+
+void __setup_cpu_power11(unsigned long offset, struct cpu_spec *t)
+{
+   return __setup_cpu_power10(offset, t);
+}
+
+void __restore_cpu_power11(void)
+{
+   return __restore_cpu_power10();
+}
diff --git a/arch/powerpc/kernel/cpu_specs_book3s_64.h 
b/arch/powerpc/kernel/cpu_specs_book3s_64.h
index 3ff9757df4c0..886fdfc7d05f 100644
--- a/arch/powerpc/kernel/cpu_specs_book3s_64.h
+++ b/arch/powerpc/kernel/cpu_specs_book3s_64.h
@@ -60,6 +60,9 @@
 

[PATCH v2] soc: fsl: fix kernel-doc warnings and typos

2024-02-04 Thread Randy Dunlap
Correct spelling of "list".

Fix a kernel-doc warning by describing the nested structure completely:

include/soc/fsl/dpaa2-fd.h:52: warning: Function parameter or member 'simple' 
not described in 'dpaa2_fd'

Signed-off-by: Randy Dunlap 
Cc: Li Yang 
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: Frank Li 
Cc: Guanhua Gao 
Cc: Roy Pledge 
---
v2:  update Cc: list, rebase

 include/soc/fsl/dpaa2-fd.h |   19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff -- a/include/soc/fsl/dpaa2-fd.h b/include/soc/fsl/dpaa2-fd.h
--- a/include/soc/fsl/dpaa2-fd.h
+++ b/include/soc/fsl/dpaa2-fd.h
@@ -25,14 +25,15 @@
 
 /**
  * struct dpaa2_fd - Struct describing FDs
- * @words: for easier/faster copying the whole FD structure
- * @addr:  address in the FD
- * @len:   length in the FD
- * @bpid:  buffer pool ID
- * @format_offset: format, offset, and short-length fields
- * @frc:   frame context
- * @ctrl:  control bits...including dd, sc, va, err, etc
- * @flc:   flow context address
+ * @words:for easier/faster copying the whole FD structure
+ * @simple:   struct for the FD fields
+ * @simple.addr:  address in the FD
+ * @simple.len:   length in the FD
+ * @simple.bpid:  buffer pool ID
+ * @simple.format_offset: format, offset, and short-length fields
+ * @simple.frc:   frame context
+ * @simple.ctrl:  control bits...including dd, sc, va, err, etc
+ * @simple.flc:   flow context address
  *
  * This structure represents the basic Frame Descriptor used in the system.
  */
@@ -497,7 +498,7 @@ static inline void dpaa2_fl_set_addr(str
  * dpaa2_fl_get_frc() - Get the frame context in the FLE
  * @fle: the given frame list entry
  *
- * Return the frame context field in the frame lsit entry.
+ * Return the frame context field in the frame list entry.
  */
 static inline u32 dpaa2_fl_get_frc(const struct dpaa2_fl_entry *fle)
 {


[PATCH v2] soc: fsl: dpio: fix kernel-doc typos

2024-02-04 Thread Randy Dunlap
Correct spelling of 2 words.

Signed-off-by: Randy Dunlap 
Cc: Li Yang 
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: Frank Li 
Cc: Guanhua Gao 
Cc: Roy Pledge 
---
v2:  update Cc: list, rebase

 include/soc/fsl/dpaa2-io.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -- a/include/soc/fsl/dpaa2-io.h b/include/soc/fsl/dpaa2-io.h
--- a/include/soc/fsl/dpaa2-io.h
+++ b/include/soc/fsl/dpaa2-io.h
@@ -22,7 +22,7 @@ struct device;
  * DOC: DPIO Service
  *
  * The DPIO service provides APIs for users to interact with the datapath
- * by enqueueing and dequeing frame descriptors.
+ * by enqueueing and dequeueing frame descriptors.
  *
  * The following set of APIs can be used to enqueue and dequeue frames
  * as well as producing notification callbacks when data is available
@@ -33,7 +33,7 @@ struct device;
 
 /**
  * struct dpaa2_io_desc - The DPIO descriptor
- * @receives_notifications: Use notificaton mode. Non-zero if the DPIO
+ * @receives_notifications: Use notification mode. Non-zero if the DPIO
  *  has a channel.
  * @has_8prio:  Set to non-zero for channel with 8 priority WQs.  Ignored
  *  unless receives_notification is TRUE.


[PATCH v2] powerpc: Avoid nmi_enter/nmi_exit in real mode interrupt.

2024-02-04 Thread Mahesh Salgaonkar
nmi_enter()/nmi_exit() touches per cpu variables which can lead to kernel
crash when invoked during real mode interrupt handling (e.g. early HMI/MCE
interrupt handler) if percpu allocation comes from vmalloc area.

Early HMI/MCE handlers are called through DEFINE_INTERRUPT_HANDLER_NMI()
wrapper which invokes nmi_enter/nmi_exit calls. We don't see any issue when
percpu allocation is from the embedded first chunk. However with
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK enabled there are chances where percpu
allocation can come from the vmalloc area.

With kernel command line "percpu_alloc=page" we can force percpu allocation
to come from vmalloc area and can see kernel crash in machine_check_early:

[1.215714] NIP [c0e49eb4] rcu_nmi_enter+0x24/0x110
[1.215717] LR [c00461a0] machine_check_early+0xf0/0x2c0
[1.215719] --- interrupt: 200
[1.215720] [c00fffd73180] [] 0x0 (unreliable)
[1.215722] [c00fffd731b0] [] 0x0
[1.215724] [c00fffd73210] [c0008364] 
machine_check_early_common+0x134/0x1f8

Fix this by avoiding use of nmi_enter()/nmi_exit() in real mode if percpu
first chunk is not embedded.

Signed-off-by: Mahesh Salgaonkar 
---
Changes in v2:
- Rebase to upstream master
- Use jump_labels, if CONFIG_JUMP_LABEL is enabled, to avoid redoing the
  test at each interrupt entry.
- v1 is at 
https://lore.kernel.org/linuxppc-dev/164578465828.74956.6065296024817333750.stgit@jupiter/
---
 arch/powerpc/include/asm/interrupt.h | 14 ++
 arch/powerpc/include/asm/percpu.h| 11 +++
 arch/powerpc/kernel/setup_64.c   | 12 
 3 files changed, 37 insertions(+)

diff --git a/arch/powerpc/include/asm/interrupt.h 
b/arch/powerpc/include/asm/interrupt.h
index a4196ab1d0167..3b4e17c23d9a9 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -336,6 +336,16 @@ static inline void interrupt_nmi_enter_prepare(struct 
pt_regs *regs, struct inte
if (IS_ENABLED(CONFIG_KASAN))
return;
 
+   /*
+* Likewise, do not use it in real mode if percpu first chunk is not
+* embedded. With CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK enabled there
+* are chances where percpu allocation can come from vmalloc area.
+*/
+#ifdef CONFIG_PPC64
+   if (IS_ENABLED(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) && 
!is_embed_first_chunk)
+   return;
+#endif
+
/* Otherwise, it should be safe to call it */
nmi_enter();
 }
@@ -351,6 +361,10 @@ static inline void interrupt_nmi_exit_prepare(struct 
pt_regs *regs, struct inter
// no nmi_exit for a pseries hash guest taking a real mode 
exception
} else if (IS_ENABLED(CONFIG_KASAN)) {
// no nmi_exit for KASAN in real mode
+#ifdef CONFIG_PPC64
+   } else if (IS_ENABLED(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) && 
!is_embed_first_chunk) {
+   // no nmi_exit if percpu first chunk is not embedded
+#endif
} else {
nmi_exit();
}
diff --git a/arch/powerpc/include/asm/percpu.h 
b/arch/powerpc/include/asm/percpu.h
index 8e5b7d0b851c6..6b4dce4e78d5f 100644
--- a/arch/powerpc/include/asm/percpu.h
+++ b/arch/powerpc/include/asm/percpu.h
@@ -12,6 +12,17 @@
 
 #define __my_cpu_offset local_paca->data_offset
 
+#ifdef CONFIG_JUMP_LABEL
+DECLARE_STATIC_KEY_FALSE(__percpu_embed_first_chunk);
+
+#define is_embed_first_chunk   \
+   (static_key_enabled(&__percpu_embed_first_chunk.key))
+
+#else /* !CONFIG_JUMP_LABEL */
+extern bool __percpu_embed_first_chunk;
+#define is_embed_first_chunk   __percpu_embed_first_chunk
+
+#endif /* CONFIG_JUMP_LABEL */
 #endif /* CONFIG_SMP */
 #endif /* __powerpc64__ */
 
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 2f19d5e944852..674b6e1bebe9a 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -834,6 +834,11 @@ static __init int pcpu_cpu_to_node(int cpu)
 
 unsigned long __per_cpu_offset[NR_CPUS] __read_mostly;
 EXPORT_SYMBOL(__per_cpu_offset);
+#ifdef CONFIG_JUMP_LABEL
+DEFINE_STATIC_KEY_FALSE(__percpu_embed_first_chunk);
+#else
+bool __percpu_embed_first_chunk;
+#endif
 
 void __init setup_per_cpu_areas(void)
 {
@@ -869,6 +874,13 @@ void __init setup_per_cpu_areas(void)
pr_warn("PERCPU: %s allocator failed (%d), "
"falling back to page size\n",
pcpu_fc_names[pcpu_chosen_fc], rc);
+   else {
+#ifdef CONFIG_JUMP_LABEL
+   static_key_enable(&__percpu_embed_first_chunk.key);
+#else
+   __percpu_embed_first_chunk = true;
+#endif
+   }
}
 
if (rc < 0)
-- 
2.43.0



[PATCH 4/4] powerpc: pseries: make suspend_subsys const

2024-02-04 Thread Ricardo B. Marliere
Now that the driver core can properly handle constant struct bus_type,
move the suspend_subsys variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.

Cc: Greg Kroah-Hartman 
Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Ricardo B. Marliere 
---
 arch/powerpc/platforms/pseries/suspend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/suspend.c 
b/arch/powerpc/platforms/pseries/suspend.c
index 382003dfdb9a..c51db63d3e88 100644
--- a/arch/powerpc/platforms/pseries/suspend.c
+++ b/arch/powerpc/platforms/pseries/suspend.c
@@ -126,7 +126,7 @@ static ssize_t show_hibernate(struct device *dev,
 
 static DEVICE_ATTR(hibernate, 0644, show_hibernate, store_hibernate);
 
-static struct bus_type suspend_subsys = {
+static const struct bus_type suspend_subsys = {
.name = "power",
.dev_name = "power",
 };

-- 
2.43.0



[PATCH 3/4] powerpc: pseries: make cmm_subsys const

2024-02-04 Thread Ricardo B. Marliere
Now that the driver core can properly handle constant struct bus_type,
move the cmm_subsys variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.

Cc: Greg Kroah-Hartman 
Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Ricardo B. Marliere 
---
 arch/powerpc/platforms/pseries/cmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/cmm.c 
b/arch/powerpc/platforms/pseries/cmm.c
index 5f4037c1d7fe..6307dacc3862 100644
--- a/arch/powerpc/platforms/pseries/cmm.c
+++ b/arch/powerpc/platforms/pseries/cmm.c
@@ -375,7 +375,7 @@ static struct device_attribute *cmm_attrs[] = {
 static DEVICE_ULONG_ATTR(simulate_loan_target_kb, 0644,
 simulate_loan_target_kb);
 
-static struct bus_type cmm_subsys = {
+static const struct bus_type cmm_subsys = {
.name = "cmm",
.dev_name = "cmm",
 };

-- 
2.43.0



[PATCH 2/4] powerpc: ps3: make ps3_system_bus_type const

2024-02-04 Thread Ricardo B. Marliere
Now that the driver core can properly handle constant struct bus_type,
move the ps3_system_bus_type variable to be a constant structure as
well, placing it into read-only memory which can not be modified at
runtime.

Cc: Greg Kroah-Hartman 
Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Ricardo B. Marliere 
---
 arch/powerpc/platforms/ps3/system-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/ps3/system-bus.c 
b/arch/powerpc/platforms/ps3/system-bus.c
index d6b5f5ecd515..b4298e98ffe8 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -466,7 +466,7 @@ static struct attribute *ps3_system_bus_dev_attrs[] = {
 };
 ATTRIBUTE_GROUPS(ps3_system_bus_dev);
 
-static struct bus_type ps3_system_bus_type = {
+static const struct bus_type ps3_system_bus_type = {
.name = "ps3_system_bus",
.match = ps3_system_bus_match,
.uevent = ps3_system_bus_uevent,

-- 
2.43.0



[PATCH 0/4] powerpc: struct bus_type cleanup

2024-02-04 Thread Ricardo B. Marliere
This series is part of an effort to cleanup the users of the driver
core, as can be seen in many recent patches authored by Greg across the
tree (e.g. [1]). Specifically, this series is part of the task of
splitting one of his TODOs [2].

---
[1]: 
https://lore.kernel.org/lkml/?q=f%3Agregkh%40linuxfoundation.org+s%3A%22make%22+and+s%3A%22const%22
[2]: 
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=bus_cleanup=26105f537f0c60eacfeb430abd2e05d7ddcdd8aa

Cc: Greg Kroah-Hartman 
Signed-off-by: Ricardo B. Marliere 

---
Ricardo B. Marliere (4):
  powerpc: cell: make spu_subsys const
  powerpc: ps3: make ps3_system_bus_type const
  powerpc: pseries: make cmm_subsys const
  powerpc: pseries: make suspend_subsys const

 arch/powerpc/platforms/cell/spu_base.c   | 2 +-
 arch/powerpc/platforms/ps3/system-bus.c  | 2 +-
 arch/powerpc/platforms/pseries/cmm.c | 2 +-
 arch/powerpc/platforms/pseries/suspend.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
---
base-commit: 44a1aad2fe6c10bfe0589d8047057b10a4c18a19
change-id: 20240204-bus_cleanup-powerpc-720c27d8f15c

Best regards,
-- 
Ricardo B. Marliere 



[PATCH 1/4] powerpc: cell: make spu_subsys const

2024-02-04 Thread Ricardo B. Marliere
Now that the driver core can properly handle constant struct bus_type,
move the spu_subsys variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.

Cc: Greg Kroah-Hartman 
Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Ricardo B. Marliere 
---
 arch/powerpc/platforms/cell/spu_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/cell/spu_base.c 
b/arch/powerpc/platforms/cell/spu_base.c
index dea6f0f25897..346e433d2706 100644
--- a/arch/powerpc/platforms/cell/spu_base.c
+++ b/arch/powerpc/platforms/cell/spu_base.c
@@ -465,7 +465,7 @@ void spu_init_channels(struct spu *spu)
 }
 EXPORT_SYMBOL_GPL(spu_init_channels);
 
-static struct bus_type spu_subsys = {
+static const struct bus_type spu_subsys = {
.name = "spu",
.dev_name = "spu",
 };

-- 
2.43.0



Re: [PATCH v15 1/5] crash: forward memory_notify arg to arch crash hotplug handler

2024-02-04 Thread Baoquan He
On 01/11/24 at 04:21pm, Sourabh Jain wrote:
> In the event of memory hotplug or online/offline events, the crash
> memory hotplug notifier `crash_memhp_notifier()` receives a
> `memory_notify` object but doesn't forward that object to the
> generic and architecture-specific crash hotplug handler.
> 
> The `memory_notify` object contains the starting PFN (Page Frame Number)
> and the number of pages in the hot-removed memory. This information is
> necessary for architectures like PowerPC to update/recreate the kdump
> image, specifically `elfcorehdr`.
> 
> So update the function signature of `crash_handle_hotplug_event()` and
> `arch_crash_handle_hotplug_event()` to accept the `memory_notify` object
> as an argument from crash memory hotplug notifier.
> 
> Since no such object is available in the case of CPU hotplug event, the
> crash CPU hotplug notifier `crash_cpuhp_online()` passes NULL to the
> crash hotplug handler.
> 
..
> ---
>  arch/x86/include/asm/kexec.h |  2 +-
>  arch/x86/kernel/crash.c  |  3 ++-
>  include/linux/kexec.h|  2 +-
>  kernel/crash_core.c  | 14 +++---
>  4 files changed, 11 insertions(+), 10 deletions(-)

LGTM,

Acked-by: Baoquan He 

> 
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index c9f6a6c5de3c..9bb6607e864e 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -208,7 +208,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage 
> *image);
>  extern void kdump_nmi_shootdown_cpus(void);
>  
>  #ifdef CONFIG_CRASH_HOTPLUG
> -void arch_crash_handle_hotplug_event(struct kimage *image);
> +void arch_crash_handle_hotplug_event(struct kimage *image, void *arg);
>  #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event
>  
>  #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index b6b044356f1b..44744e9c68ec 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -428,10 +428,11 @@ unsigned int arch_crash_get_elfcorehdr_size(void)
>  /**
>   * arch_crash_handle_hotplug_event() - Handle hotplug elfcorehdr changes
>   * @image: a pointer to kexec_crash_image
> + * @arg: struct memory_notify handler for memory hotplug case and NULL for 
> CPU hotplug case.
>   *
>   * Prepare the new elfcorehdr and replace the existing elfcorehdr.
>   */
> -void arch_crash_handle_hotplug_event(struct kimage *image)
> +void arch_crash_handle_hotplug_event(struct kimage *image, void *arg)
>  {
>   void *elfbuf = NULL, *old_elfcorehdr;
>   unsigned long nr_mem_ranges;
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 400cb6c02176..802052d9c64b 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -483,7 +483,7 @@ static inline void arch_kexec_pre_free_pages(void *vaddr, 
> unsigned int pages) {
>  #endif
>  
>  #ifndef arch_crash_handle_hotplug_event
> -static inline void arch_crash_handle_hotplug_event(struct kimage *image) { }
> +static inline void arch_crash_handle_hotplug_event(struct kimage *image, 
> void *arg) { }
>  #endif
>  
>  int crash_check_update_elfcorehdr(void);
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index d48315667752..ab1c8e79759d 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -914,7 +914,7 @@ int crash_check_update_elfcorehdr(void)
>   * list of segments it checks (since the elfcorehdr changes and thus
>   * would require an update to purgatory itself to update the digest).
>   */
> -static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int 
> cpu)
> +static void crash_handle_hotplug_event(unsigned int hp_action, unsigned int 
> cpu, void *arg)
>  {
>   struct kimage *image;
>  
> @@ -976,7 +976,7 @@ static void crash_handle_hotplug_event(unsigned int 
> hp_action, unsigned int cpu)
>   image->hp_action = hp_action;
>  
>   /* Now invoke arch-specific update handler */
> - arch_crash_handle_hotplug_event(image);
> + arch_crash_handle_hotplug_event(image, arg);
>  
>   /* No longer handling a hotplug event */
>   image->hp_action = KEXEC_CRASH_HP_NONE;
> @@ -992,17 +992,17 @@ static void crash_handle_hotplug_event(unsigned int 
> hp_action, unsigned int cpu)
>   crash_hotplug_unlock();
>  }
>  
> -static int crash_memhp_notifier(struct notifier_block *nb, unsigned long 
> val, void *v)
> +static int crash_memhp_notifier(struct notifier_block *nb, unsigned long 
> val, void *arg)
>  {
>   switch (val) {
>   case MEM_ONLINE:
>   crash_handle_hotplug_event(KEXEC_CRASH_HP_ADD_MEMORY,
> - KEXEC_CRASH_HP_INVALID_CPU);
> + KEXEC_CRASH_HP_INVALID_CPU, arg);
>   break;
>  
>   case MEM_OFFLINE:
>   crash_handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_MEMORY,
> - KEXEC_CRASH_HP_INVALID_CPU);
> + KEXEC_CRASH_HP_INVALID_CPU, arg);
>   break;
>   }
>   

Re: [PATCH v15 2/5] crash: add a new kexec flag for hotplug support

2024-02-04 Thread Baoquan He
Hi Sourabh,

Thanks for the great work. There are some concerns, please see inline
comments.

On 01/11/24 at 04:21pm, Sourabh Jain wrote:
..
> Now, if the kexec tool sends KEXEC_CRASH_HOTPLUG_SUPPORT kexec flag to
> the kernel, it indicates to the kernel that all the required kexec
> segment is skipped from SHA calculation and it is safe to update kdump
> image loaded using the kexec_load syscall.

So finally you add a new KEXEC_CRASH_HOTPLUG_SUPPORT flag, that's fine.

> 
.. 
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index 9bb6607e864e..e791129fdf6c 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -211,6 +211,9 @@ extern void kdump_nmi_shootdown_cpus(void);
>  void arch_crash_handle_hotplug_event(struct kimage *image, void *arg);
>  #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event
>  
> +int arch_crash_hotplug_support(struct kimage *image, unsigned long 
> kexec_flags);
> +#define arch_crash_hotplug_support arch_crash_hotplug_support
> +
>  #ifdef CONFIG_HOTPLUG_CPU
>  int arch_crash_hotplug_cpu_support(void);
>  #define crash_hotplug_cpu_support arch_crash_hotplug_cpu_support

Then crash_hotplug_cpu_support is not needed any more on x86_64, and
crash_hotplug_memory_support(), if you remove their implementation in
arch/x86/kernel/crash.c, won't it cause building warning or error on x86?

> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 44744e9c68ec..293b54bff706 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -398,20 +398,16 @@ int crash_load_segments(struct kimage *image)
>  #undef pr_fmt
>  #define pr_fmt(fmt) "crash hp: " fmt
>  
> -/* These functions provide the value for the sysfs crash_hotplug nodes */
> -#ifdef CONFIG_HOTPLUG_CPU
> -int arch_crash_hotplug_cpu_support(void)
> +int arch_crash_hotplug_support(struct kimage *image, unsigned long 
> kexec_flags)
>  {
> - return crash_check_update_elfcorehdr();
> -}
> -#endif
>  
> -#ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_crash_hotplug_memory_support(void)
> -{
> - return crash_check_update_elfcorehdr();
> -}
> +#ifdef CONFIG_KEXEC_FILE
> + if (image->file_mode)
> + return 1;
>  #endif
> + return (kexec_flags & KEXEC_UPDATE_ELFCOREHDR ||
> + kexec_flags & KEXEC_CRASH_HOTPLUG_SUPPORT);

Do we need add some document to tell why there are two kexec flags on
x86_64, except of checking this patch log?

> +}
>  
>  unsigned int arch_crash_get_elfcorehdr_size(void)
>  {
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 548491de818e..2f411ddfbd8b 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -306,7 +306,7 @@ static ssize_t crash_hotplug_show(struct device *dev,
>struct device_attribute *attr,
>char *buf)
>  {
> - return sysfs_emit(buf, "%d\n", crash_hotplug_cpu_support());
> + return sysfs_emit(buf, "%d\n", crash_check_hotplug_support());
>  }
>  static DEVICE_ATTR_ADMIN_RO(crash_hotplug);
>  #endif
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 8a13babd826c..e70ab1d3428e 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -514,7 +514,7 @@ static DEVICE_ATTR_RW(auto_online_blocks);
>  static ssize_t crash_hotplug_show(struct device *dev,
>  struct device_attribute *attr, char *buf)
>  {
> - return sysfs_emit(buf, "%d\n", crash_hotplug_memory_support());
> + return sysfs_emit(buf, "%d\n", crash_check_hotplug_support());
>  }
>  static DEVICE_ATTR_RO(crash_hotplug);
>  #endif
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 802052d9c64b..7880d74dc5c4 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -317,8 +317,8 @@ struct kimage {
>   /* If set, we are using file mode kexec syscall */
>   unsigned int file_mode:1;
>  #ifdef CONFIG_CRASH_HOTPLUG
> - /* If set, allow changes to elfcorehdr of kexec_load'd image */
> - unsigned int update_elfcorehdr:1;
> + /* If set, allow changes to kexec segments of kexec_load'd image */

The code comment doesn't reflect the usage of the flag. You set it too
when it's kexec_file_load. Speaking of this, I do wonder why you need
set it too for kexec_file_load, and why we have
arch_crash_hotplug_support(), then crash_check_hotplug_support() both of
which have the same effect.

> + unsigned int hotplug_support:1;
>  #endif
>  
>  #ifdef ARCH_HAS_KIMAGE_ARCH
> @@ -396,9 +396,10 @@ bool kexec_load_permitted(int kexec_image_type);
>  
>  /* List of defined/legal kexec flags */
>  #ifndef CONFIG_KEXEC_JUMP
> -#define KEXEC_FLAGS(KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR)
> +#define KEXEC_FLAGS(KEXEC_ON_CRASH | KEXEC_UPDATE_ELFCOREHDR | 
> KEXEC_CRASH_HOTPLUG_SUPPORT)
>  #else
> -#define KEXEC_FLAGS(KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT | 
> KEXEC_UPDATE_ELFCOREHDR)
> 

Re: [PATCH v2 1/3] powerpc/code-patching: Add generic memory patching

2024-02-04 Thread Benjamin Gray
On Thu, 2023-11-30 at 15:55 +0530, Naveen N Rao wrote:
> On Mon, Oct 16, 2023 at 04:01:45PM +1100, Benjamin Gray wrote:
> > patch_instruction() is designed for patching instructions in
> > otherwise
> > readonly memory. Other consumers also sometimes need to patch
> > readonly
> > memory, so have abused patch_instruction() for arbitrary data
> > patches.
> > 
> > This is a problem on ppc64 as patch_instruction() decides on the
> > patch
> > width using the 'instruction' opcode to see if it's a prefixed
> > instruction. Data that triggers this can lead to larger writes,
> > possibly
> > crossing a page boundary and failing the write altogether.
> > 
> > Introduce patch_uint(), and patch_ulong(), with aliases
> > patch_u32(), and
> > patch_u64() (on ppc64) designed for aligned data patches. The patch
> > size is now determined by the called function, and is passed as an
> > additional parameter to generic internals.
> > 
> > While the instruction flushing is not required for data patches,
> > the
> > use cases for data patching (mainly module loading and static
> > calls)
> > are less performance sensitive than for instruction patching
> > (ftrace activation).
> 
> That's debatable. While it is nice to be able to activate function 
> tracing quickly, it is not necessarily a hot path. On the flip side,
> I 
> do have a use case for data patching for ftrace activation :)
> 

True, but it's still correct to do so at least. Having a different path
for data writes is definitely a possibility, but would be added for
performance. This series is motivated by fixing a correctness issue
with data write sizes.

> > So the instruction flushing remains unconditional
> > in this patch.
> > 
> > ppc32 does not support prefixed instructions, so is unaffected by
> > the
> > original issue. Care is taken in not exposing the size parameter in
> > the
> > public (non-static) interface, so the compiler can const-propagate
> > it
> > away.
> > 
> > Signed-off-by: Benjamin Gray 
> > 
> > ---
> > 
> > v2: * Deduplicate patch_32() definition
> >     * Use u32 for val32
> >     * Remove noinline
> > ---
> >  arch/powerpc/include/asm/code-patching.h | 33 
> >  arch/powerpc/lib/code-patching.c | 66 ++--
> > 
> >  2 files changed, 83 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/code-patching.h
> > b/arch/powerpc/include/asm/code-patching.h
> > index 3f881548fb61..7c6056bb1706 100644
> > --- a/arch/powerpc/include/asm/code-patching.h
> > +++ b/arch/powerpc/include/asm/code-patching.h
> > @@ -75,6 +75,39 @@ int patch_branch(u32 *addr, unsigned long
> > target, int flags);
> >  int patch_instruction(u32 *addr, ppc_inst_t instr);
> >  int raw_patch_instruction(u32 *addr, ppc_inst_t instr);
> >  
> > +/*
> > + * patch_uint() and patch_ulong() should only be called on
> > addresses where the
> > + * patch does not cross a cacheline, otherwise it may not be
> > flushed properly
> > + * and mixes of new and stale data may be observed. It cannot
> > cross a page
> > + * boundary, as only the target page is mapped as writable.
> 
> Should we enforce alignment requirements, especially for
> patch_ulong() 
> on 64-bit powerpc? I am not sure if there are use cases for unaligned
> 64-bit writes. That should also ensure that the write doesn't cross a
> cacheline.

Yeah, the current description is more just the technical restrictions,
not an endorsement of usage. If the caller isn't working with aligned
data, it seems unlikely it would still be cacheline aligned. The caller
should break it into 32bit patches if this is the case.

By enforce, are you suggesting a WARN_ON in the code too?

> > + *
> > + * patch_instruction() and other instruction patchers
> > automatically satisfy this
> > + * requirement due to instruction alignment requirements.
> > + */
> > +
> > +#ifdef CONFIG_PPC64
> > +
> > +int patch_uint(void *addr, unsigned int val);
> > +int patch_ulong(void *addr, unsigned long val);
> > +
> > +#define patch_u64 patch_ulong
> > +
> > +#else
> > +
> > +static inline int patch_uint(u32 *addr, unsigned int val)
> 
> Is there a reason to use u32 * here?
> 

No, fixed to void* for next series

> > +{
> > +   return patch_instruction(addr, ppc_inst(val));
> > +}
> > +
> > +static inline int patch_ulong(void *addr, unsigned long val)
> > +{
> > +   return patch_instruction(addr, ppc_inst(val));
> > +}
> > +
> > +#endif
> > +
> > +#define patch_u32 patch_uint
> > +
> >  static inline unsigned long patch_site_addr(s32 *site)
> >  {
> >     return (unsigned long)site + *site;
> > diff --git a/arch/powerpc/lib/code-patching.c
> > b/arch/powerpc/lib/code-patching.c
> > index b00112d7ad46..60289332412f 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -20,15 +20,14 @@
> >  #include 
> >  #include 
> >  
> > -static int __patch_instruction(u32 *exec_addr, ppc_inst_t instr,
> > u32 *patch_addr)
> > +static int __patch_memory(void 

Re: [PATCH 2/4] powerpc: ps3: make ps3_system_bus_type const

2024-02-04 Thread Geoff Levand
On 2/4/24 23:21, Ricardo B. Marliere wrote:
> Now that the driver core can properly handle constant struct bus_type,
> move the ps3_system_bus_type variable to be a constant structure as
> well, placing it into read-only memory which can not be modified at
> runtime.

> -static struct bus_type ps3_system_bus_type = {
> +static const struct bus_type ps3_system_bus_type = {
>   .name = "ps3_system_bus",
>   .match = ps3_system_bus_match,
>   .uevent = ps3_system_bus_uevent,

Seems fine.

Acked-by: Geoff Levand 

 



Re: [PATCH 2/4] powerpc: ps3: make ps3_system_bus_type const

2024-02-04 Thread Geert Uytterhoeven
On Sun, Feb 4, 2024 at 3:23 PM Ricardo B. Marliere  wrote:
> Now that the driver core can properly handle constant struct bus_type,
> move the ps3_system_bus_type variable to be a constant structure as
> well, placing it into read-only memory which can not be modified at
> runtime.
>
> Cc: Greg Kroah-Hartman 
> Suggested-by: Greg Kroah-Hartman 
> Signed-off-by: Ricardo B. Marliere 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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


Re: [PATCH 0/4] powerpc: struct bus_type cleanup

2024-02-04 Thread Greg Kroah-Hartman
On Sun, Feb 04, 2024 at 11:21:54AM -0300, Ricardo B. Marliere wrote:
> This series is part of an effort to cleanup the users of the driver
> core, as can be seen in many recent patches authored by Greg across the
> tree (e.g. [1]). Specifically, this series is part of the task of
> splitting one of his TODOs [2].
> 
> ---
> [1]: 
> https://lore.kernel.org/lkml/?q=f%3Agregkh%40linuxfoundation.org+s%3A%22make%22+and+s%3A%22const%22
> [2]: 
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=bus_cleanup=26105f537f0c60eacfeb430abd2e05d7ddcdd8aa
> 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Ricardo B. Marliere 

Reviewed-by: Greg Kroah-Hartman