[PATCH] selftest/powerpc/pmu/ebb: remove fixed_instruction.S
Commit 3752e453f6ba ("selftests/powerpc: Add tests of PMU EBBs") added
selftest testcases to verify EBB interface. instruction_count_test.c
testcase needs a fixed loop function to count overhead. Instead of
using the thirty_two_instruction_loop() in fixed_instruction_loop.S
in ebb folder, file is linked with thirty_two_instruction_loop() in
loop.S from top folder. Since fixed_instruction_loop.S not used, patch
removes the file.
Signed-off-by: Madhavan Srinivasan
---
.../powerpc/pmu/ebb/fixed_instruction_loop.S | 43 ---
1 file changed, 43 deletions(-)
delete mode 100644
tools/testing/selftests/powerpc/pmu/ebb/fixed_instruction_loop.S
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/fixed_instruction_loop.S
b/tools/testing/selftests/powerpc/pmu/ebb/fixed_instruction_loop.S
deleted file mode 100644
index 08a7b5f133b9..
--- a/tools/testing/selftests/powerpc/pmu/ebb/fixed_instruction_loop.S
+++ /dev/null
@@ -1,43 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright 2014, Michael Ellerman, IBM Corp.
- */
-
-#include
-
- .text
-
-FUNC_START(thirty_two_instruction_loop)
- cmpwi r3,0
- beqlr
- addir4,r3,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1
- addir4,r4,1 # 28 addi's
- subir3,r3,1
- b FUNC_NAME(thirty_two_instruction_loop)
-FUNC_END(thirty_two_instruction_loop)
--
2.34.1
[PATCH 6/6] powerpc/fsl: update ifc node name to be memory-controller
Update the node name to be align with latest binding.
Signed-off-by: Li Yang
---
arch/powerpc/boot/dts/fsl/bsc9131rdb.dts| 2 +-
arch/powerpc/boot/dts/fsl/bsc9132qds.dts| 2 +-
arch/powerpc/boot/dts/fsl/c293pcie.dts | 2 +-
arch/powerpc/boot/dts/fsl/p1010rdb_32b.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/p1010rdb_36b.dtsi | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/bsc9131rdb.dts
b/arch/powerpc/boot/dts/fsl/bsc9131rdb.dts
index 8da984251abc..0ba86a6dce1b 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9131rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/bsc9131rdb.dts
@@ -15,7 +15,7 @@ memory {
device_type = "memory";
};
- board_ifc: ifc: ifc@ff71e000 {
+ board_ifc: ifc: memory-controller@ff71e000 {
/* NAND Flash on board */
ranges = <0x0 0x0 0x0 0xff80 0x4000>;
reg = <0x0 0xff71e000 0x0 0x2000>;
diff --git a/arch/powerpc/boot/dts/fsl/bsc9132qds.dts
b/arch/powerpc/boot/dts/fsl/bsc9132qds.dts
index 7cb2158dfe58..ce642e879a1b 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9132qds.dts
+++ b/arch/powerpc/boot/dts/fsl/bsc9132qds.dts
@@ -15,7 +15,7 @@ memory {
device_type = "memory";
};
- ifc: ifc@ff71e000 {
+ ifc: memory-controller@ff71e000 {
/* NOR, NAND Flash on board */
ranges = <0x0 0x0 0x0 0x8800 0x0800
0x1 0x0 0x0 0xff80 0x0001>;
diff --git a/arch/powerpc/boot/dts/fsl/c293pcie.dts
b/arch/powerpc/boot/dts/fsl/c293pcie.dts
index 5e905e0857cf..e2fdac2ed420 100644
--- a/arch/powerpc/boot/dts/fsl/c293pcie.dts
+++ b/arch/powerpc/boot/dts/fsl/c293pcie.dts
@@ -42,7 +42,7 @@ memory {
device_type = "memory";
};
- ifc: ifc@fffe1e000 {
+ ifc: memory-controller@fffe1e000 {
reg = <0xf 0xffe1e000 0 0x2000>;
ranges = <0x0 0x0 0xf 0xec00 0x0400
0x1 0x0 0xf 0xff80 0x0001
diff --git a/arch/powerpc/boot/dts/fsl/p1010rdb_32b.dtsi
b/arch/powerpc/boot/dts/fsl/p1010rdb_32b.dtsi
index fdc19aab2f70..583a6cd05079 100644
--- a/arch/powerpc/boot/dts/fsl/p1010rdb_32b.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010rdb_32b.dtsi
@@ -36,7 +36,7 @@ memory {
device_type = "memory";
};
-board_ifc: ifc: ifc@ffe1e000 {
+board_ifc: ifc: memory-controller@ffe1e000 {
/* NOR, NAND Flashes and CPLD on board */
ranges = <0x0 0x0 0x0 0xee00 0x0200
0x1 0x0 0x0 0xff80 0x0001
diff --git a/arch/powerpc/boot/dts/fsl/p1010rdb_36b.dtsi
b/arch/powerpc/boot/dts/fsl/p1010rdb_36b.dtsi
index de2fceed4f79..4d41efe0038f 100644
--- a/arch/powerpc/boot/dts/fsl/p1010rdb_36b.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010rdb_36b.dtsi
@@ -36,7 +36,7 @@ memory {
device_type = "memory";
};
-board_ifc: ifc: ifc@fffe1e000 {
+board_ifc: ifc: memory-controller@fffe1e000 {
/* NOR, NAND Flashes and CPLD on board */
ranges = <0x0 0x0 0xf 0xee00 0x0200
0x1 0x0 0xf 0xff80 0x0001
--
2.25.1
[PATCH 3/6] powerpc/mpc85xx: remove "simple-bus" compatible from ifc node
The binding of ifc device has been updated. Update dts to match
accordingly.
Signed-off-by: Li Yang
---
arch/powerpc/boot/dts/fsl/b4si-post.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/c293si-post.dtsi| 2 +-
arch/powerpc/boot/dts/fsl/p1010si-post.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/t1023si-post.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/t1040si-post.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/t2081si-post.dtsi | 2 +-
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
index 4f044b41a776..fb3200b006ad 100644
--- a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
@@ -50,7 +50,7 @@ _pfdr {
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
b/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
index 2a677fd323eb..5c53cee8755f 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
@@ -35,7 +35,7 @@
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <16 2 0 0 20 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
b/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
index b8e0edd1ac69..4da451e000d9 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
@@ -35,7 +35,7 @@
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
/* FIXME: Test whether interrupts are split */
interrupts = <16 2 0 0 20 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
b/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
index bec0fc36849d..ee3b45806ee3 100644
--- a/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
@@ -35,7 +35,7 @@
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <19 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
index ccda0a91abf0..4b5671462eb3 100644
--- a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
@@ -35,7 +35,7 @@
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <16 2 0 0 19 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
index d552044c5afc..c15a49df66e1 100644
--- a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
@@ -52,7 +52,7 @@ _pfdr {
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
index f58eb820eb5e..38703e58dd09 100644
--- a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
@@ -52,7 +52,7 @@ _pfdr {
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
index ecbb447920bc..58ef8bf6045c 100644
--- a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
@@ -50,7 +50,7 @@ _pfdr {
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
index fcac73486d48..65f3e17c0d41 100644
--- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
@@ -50,7 +50,7 @@ _pfdr {
{
#address-cells = <2>;
#size-cells = <1>;
- compatible = "fsl,ifc", "simple-bus";
+ compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
};
--
2.25.1
Re: [PATCH kernel] powerpc/boot: Stop using RELACOUNT
Alexey Kardashevskiy writes:
> So far the RELACOUNT tag from the ELF header was containing the exact
> number of R_PPC_RELATIVE/R_PPC64_RELATIVE relocations. However the LLVM's
> recent change [1] make it equal-or-less than the actual number which
> makes it useless.
>
> This replaces RELACOUNT in zImage loader with a pair of RELASZ and RELAENT.
> The vmlinux relocation code is fixed in [2].
That's committed so you can say:
in commit d79976918852 ("powerpc/64: Add UADDR64 relocation support")
> To make it more future proof, this walks through the entire .rela.dyn
> section instead of assuming that the section is sorter by a relocation
> type. Unlike [1], this does not add unaligned UADDR/UADDR64 relocations
^
that should be 2?
> as in hardly possible to see those in arch-specific zImage.
I don't quite parse that. Is it true we can never see them in zImage?
Maybe it's true that we don't see them in practice.
> [1] https://github.com/llvm/llvm-project/commit/da0e5b885b25cf4
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next=d799769188529a
> Signed-off-by: Alexey Kardashevskiy
> ---
> arch/powerpc/boot/crt0.S | 43 +---
> 1 file changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> index feadee18e271..6ea3417da3b7 100644
> --- a/arch/powerpc/boot/crt0.S
> +++ b/arch/powerpc/boot/crt0.S
> @@ -8,7 +8,8 @@
> #include "ppc_asm.h"
>
> RELA = 7
> -RELACOUNT = 0x6ff9
> +RELASZ = 8
> +RELAENT = 9
>
> .data
> /* A procedure descriptor used when booting this as a COFF file.
> @@ -75,34 +76,38 @@ p_base: mflrr10 /* r10 now points to
> runtime addr of p_base */
> bne 11f
> lwz r9,4(r12) /* get RELA pointer in r9 */
> b 12f
> -11: addis r8,r8,(-RELACOUNT)@ha
> - cmpwi r8,RELACOUNT@l
> +11: cmpwi r8,RELASZ
> + bne 111f
> + lwz r0,4(r12) /* get RELASZ value in r0 */
> + b 12f
> +111: cmpwi r8,RELAENT
Can you use named local labels for new labels you introduce?
This could be .Lcheck_for_relaent: perhaps.
> bne 12f
> - lwz r0,4(r12) /* get RELACOUNT value in r0 */
> + lwz r14,4(r12) /* get RELAENT value in r14 */
> 12: addir12,r12,8
> b 9b
>
> /* The relocation section contains a list of relocations.
>* We now do the R_PPC_RELATIVE ones, which point to words
> - * which need to be initialized with addend + offset.
> - * The R_PPC_RELATIVE ones come first and there are RELACOUNT
> - * of them. */
> + * which need to be initialized with addend + offset */
> 10: /* skip relocation if we don't have both */
> cmpwi r0,0
> beq 3f
> cmpwi r9,0
> beq 3f
> + cmpwi r14,0
> + beq 3f
>
> add r9,r9,r11 /* Relocate RELA pointer */
> + divdr0,r0,r14 /* RELASZ / RELAENT */
This is in the 32-bit portion isn't it. AFAIK 32-bit CPUs don't
implement divd. I'm not sure why the toolchain allowed it. I would
expect it to trap if run on real 32-bit hardware.
> mtctr r0
> 2: lbz r0,4+3(r9) /* ELF32_R_INFO(reloc->r_info) */
> cmpwi r0,22 /* R_PPC_RELATIVE */
> - bne 3f
> + bne 22f
> lwz r12,0(r9) /* reloc->r_offset */
> lwz r0,8(r9)/* reloc->r_addend */
> add r0,r0,r11
> stwxr0,r11,r12
> - addir9,r9,12
> +22: add r9,r9,r14
> bdnz2b
>
> /* Do a cache flush for our text, in case the loader didn't */
cheers
Re: [PATCH 1/2] drivers/nvdimm: Fix build failure when CONFIG_PERF_EVENTS is not set
On Fri, Mar 18, 2022 at 4:42 AM Kajol Jain wrote:
>
> The following build failure occures when CONFIG_PERF_EVENTS is not set
> as generic pmu functions are not visible in that scenario.
>
> |-- s390-randconfig-r044-20220313
> | |-- nd_perf.c:(.text):undefined-reference-to-perf_pmu_migrate_context
> | |-- nd_perf.c:(.text):undefined-reference-to-perf_pmu_register
> | `-- nd_perf.c:(.text):undefined-reference-to-perf_pmu_unregister
>
> Similar build failure in nds32 architecture:
> nd_perf.c:(.text+0x21e): undefined reference to `perf_pmu_migrate_context'
> nd_perf.c:(.text+0x434): undefined reference to `perf_pmu_register'
> nd_perf.c:(.text+0x57c): undefined reference to `perf_pmu_unregister'
>
> Fix this issue by adding check for CONFIG_PERF_EVENTS config option
> and disabling the nvdimm perf interface incase this config is not set.
>
> Also removed function declaration of perf_pmu_migrate_context,
> perf_pmu_register, perf_pmu_unregister functions from nd.h as these are
> common pmu functions which are part of perf_event.h and since we
> are disabling nvdimm perf interface incase CONFIG_PERF_EVENTS option
> is not set, we not need to declare them in nd.h
>
> Fixes: 0fab1ba6ad6b ("drivers/nvdimm: Add perf interface to expose
> nvdimm performance stats") (Commit id based on linux-next tree)
> Signed-off-by: Kajol Jain
> Link: https://lore.kernel.org/all/62317124.ybqfu33+s%2fwdvwgj%25...@intel.com/
> Reported-by: kernel test robot
> ---
> drivers/nvdimm/Makefile | 2 +-
> include/linux/nd.h | 7 ---
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> ---
> - This fix patch changes are added and tested on top of linux-next tree
> on the 'next-20220315' branch.
> ---
> diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> index 3fb806748716..ba0296dca9db 100644
> --- a/drivers/nvdimm/Makefile
> +++ b/drivers/nvdimm/Makefile
> @@ -15,7 +15,7 @@ nd_e820-y := e820.o
> libnvdimm-y := core.o
> libnvdimm-y += bus.o
> libnvdimm-y += dimm_devs.o
> -libnvdimm-y += nd_perf.o
> +libnvdimm-$(CONFIG_PERF_EVENTS) += nd_perf.o
> libnvdimm-y += dimm.o
> libnvdimm-y += region_devs.o
> libnvdimm-y += region.o
> diff --git a/include/linux/nd.h b/include/linux/nd.h
> index 7b2ccbdc1cbc..a4265eaf5ae8 100644
> --- a/include/linux/nd.h
> +++ b/include/linux/nd.h
> @@ -8,8 +8,10 @@
> #include
> #include
> #include
> +#ifdef CONFIG_PERF_EVENTS
> #include
Why must this not be included? Doesn't it already handle the
CONFIG_PERF_EVENTS=n case internally?
> #include
I notice now that this platform-device header should have never been
added in the first place, just forward declare:
struct platform_device;
> +#endif
>
> enum nvdimm_event {
> NVDIMM_REVALIDATE_POISON,
> @@ -25,6 +27,7 @@ enum nvdimm_claim_class {
> NVDIMM_CCLASS_UNKNOWN,
> };
>
> +#ifdef CONFIG_PERF_EVENTS
> #define NVDIMM_EVENT_VAR(_id) event_attr_##_id
> #define NVDIMM_EVENT_PTR(_id) (_attr_##_id.attr.attr)
Why must these be inside the ifdef guard?
>
> @@ -63,9 +66,7 @@ extern ssize_t nvdimm_events_sysfs_show(struct device *dev,
>
> int register_nvdimm_pmu(struct nvdimm_pmu *nvdimm, struct platform_device
> *pdev);
> void unregister_nvdimm_pmu(struct nvdimm_pmu *nd_pmu);
Shouldn't there also be stub functions in the CONFIG_PERF_EVENTS=n case?
static inline int register_nvdimm_pmu(struct nvdimm_pmu *nvdimm,
struct platform_device *pdev)
{
return -ENXIO;
}
static inline void unregister_nvdimm_pmu(struct nvdimm_pmu *nd_pmu)
{
}
> -void perf_pmu_migrate_context(struct pmu *pmu, int src_cpu, int dst_cpu);
> -int perf_pmu_register(struct pmu *pmu, const char *name, int type);
> -void perf_pmu_unregister(struct pmu *pmu);
Yeah, I should have caught these earlier.
> +#endif
>
> struct nd_device_driver {
> struct device_driver drv;
> --
> 2.31.1
>
Re: [PATCH 2/2] powerpc/papr_scm: Fix build failure when CONFIG_PERF_EVENTS is not set
On Mon, Mar 21, 2022 at 2:39 PM Dan Williams wrote:
>
> On Fri, Mar 18, 2022 at 4:42 AM Kajol Jain wrote:
> >
> > The following build failure occures when CONFIG_PERF_EVENTS is not set
> > as generic pmu functions are not visible in that scenario.
> >
> > arch/powerpc/platforms/pseries/papr_scm.c:372:35: error: ‘struct
> > perf_event’ has no member named ‘attr’
> > p->nvdimm_events_map[event->attr.config],
> >^~
> > In file included from ./include/linux/list.h:5,
> > from ./include/linux/kobject.h:19,
> > from ./include/linux/of.h:17,
> > from arch/powerpc/platforms/pseries/papr_scm.c:5:
> > arch/powerpc/platforms/pseries/papr_scm.c: In function
> > ‘papr_scm_pmu_event_init’:
> > arch/powerpc/platforms/pseries/papr_scm.c:389:49: error: ‘struct
> > perf_event’ has no member named ‘pmu’
> > struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
> > ^~
> > ./include/linux/container_of.h:18:26: note: in definition of macro
> > ‘container_of’
> > void *__mptr = (void *)(ptr); \
> > ^~~
> > arch/powerpc/platforms/pseries/papr_scm.c:389:30: note: in expansion of
> > macro ‘to_nvdimm_pmu’
> > struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
> > ^
> > In file included from ./include/linux/bits.h:22,
> > from ./include/linux/bitops.h:6,
> > from ./include/linux/of.h:15,
> > from arch/powerpc/platforms/pseries/papr_scm.c:5:
> >
> > Fix the build issue by adding check for CONFIG_PERF_EVENTS config option
> > and disabling the papr_scm perf interface support incase this config
> > is not set
> >
> > Fixes: 4c08d4bbc089 ("powerpc/papr_scm: Add perf interface support")
> > (Commit id
> > based on linux-next tree)
> > Signed-off-by: Kajol Jain
> > ---
> > arch/powerpc/platforms/pseries/papr_scm.c | 15 +++
>
> This is a bit messier than I would have liked mainly because it dumps
> a bunch of ifdefery into a C file contrary to coding style, "Wherever
> possible, don't use preprocessor conditionals (#if, #ifdef) in .c
> files". I would expect this all to move to an organization like:
>
> arch/powerpc/platforms/pseries/papr_scm/main.c
> arch/powerpc/platforms/pseries/papr_scm/perf.c
>
> ...and a new config symbol like:
>
> config PAPR_SCM_PERF
>depends on PAPR_SCM && PERF_EVENTS
>def_bool y
>
> ...with wrappers in header files to make everything compile away
> without any need for main.c to carry an ifdef.
>
> Can you turn a patch like that in the next couple days? Otherwise, I
> think if Linus saw me sending a late breaking compile fix that threw
> coding style out the window he'd have cause to just drop the pull
> request entirely.
Also, please base it on the current state of the libnvdimm-for-next
branch as -next includes some of the SMART health changes leading to
at least one conflict.
Re: [PATCH] Docs: admin/kernel-parameters: edit a few boot options
On 3/21/22 17:45, Michael Ellerman wrote: > Randy Dunlap writes: >> Clean up some of admin-guide/kernel-parameters.txt: >> >> a. "smt" should be "smt=" (S390) >> b. add "smt-enabled" for POWERPC > > I'd rather you didn't. It's not well tested and we ignore it entirely on > some platforms because it causes bugs. Eventually I'd like to remove it. > > If we ever get time we'd want to support the generic `nosmt` argument > instead. No problem. Thanks for replying. -- ~Randy
Re: [PATCH] Docs: admin/kernel-parameters: edit a few boot options
Randy Dunlap writes: > Clean up some of admin-guide/kernel-parameters.txt: > > a. "smt" should be "smt=" (S390) > b. add "smt-enabled" for POWERPC I'd rather you didn't. It's not well tested and we ignore it entirely on some platforms because it causes bugs. Eventually I'd like to remove it. If we ever get time we'd want to support the generic `nosmt` argument instead. cheers
Re: [PATCH v2] macintosh/via-pmu: Avoid compiler warnings when CONFIG_PROC_FS is disabled
On 3/21/22 02:28, Finn Thain wrote:
> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined
> but not used [-Wunused-function]
> static int pmu_battery_proc_show(struct seq_file *m, void *v)
> ^
> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined
> but not used [-Wunused-function]
> static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> ^~
> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but
> not used [-Wunused-function]
> static int pmu_info_proc_show(struct seq_file *m, void *v)
> ^~
>
> Add some #ifdefs to avoid unused code warnings when CONFIG_PROC_FS is
> disabled.
>
> Cc: Randy Dunlap
> Cc: Christophe Leroy
> Reported-by: Randy Dunlap
> Suggested-by: Christophe Leroy
> Signed-off-by: Finn Thain
Tested-by: Randy Dunlap
Acked-by: Randy Dunlap
Thanks.
> ---
> Changed since v1:
> - Simplified to take advantage of the fact that proc_mkdir() is stubbed
>out when CONFIG_PROC_FS=n. Hence that call doesn't need to move
>within the #ifdef.
> ---
> drivers/macintosh/via-pmu.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index 2109129ea1bb..495fd35b11de 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -204,9 +204,11 @@ static int init_pmu(void);
> static void pmu_start(void);
> static irqreturn_t via_pmu_interrupt(int irq, void *arg);
> static irqreturn_t gpio1_interrupt(int irq, void *arg);
> +#ifdef CONFIG_PROC_FS
> static int pmu_info_proc_show(struct seq_file *m, void *v);
> static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
> static int pmu_battery_proc_show(struct seq_file *m, void *v);
> +#endif
> static void pmu_pass_intr(unsigned char *data, int len);
> static const struct proc_ops pmu_options_proc_ops;
>
> @@ -857,6 +859,7 @@ query_battery_state(void)
> 2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
> }
>
> +#ifdef CONFIG_PROC_FS
> static int pmu_info_proc_show(struct seq_file *m, void *v)
> {
> seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
> @@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
> .proc_release = single_release,
> .proc_write = pmu_options_proc_write,
> };
> +#endif
>
> #ifdef CONFIG_ADB
> /* Send an ADB command */
--
~Randy
Re: [PATCH 2/2] powerpc/papr_scm: Fix build failure when CONFIG_PERF_EVENTS is not set
On Fri, Mar 18, 2022 at 4:42 AM Kajol Jain wrote:
>
> The following build failure occures when CONFIG_PERF_EVENTS is not set
> as generic pmu functions are not visible in that scenario.
>
> arch/powerpc/platforms/pseries/papr_scm.c:372:35: error: ‘struct perf_event’
> has no member named ‘attr’
> p->nvdimm_events_map[event->attr.config],
>^~
> In file included from ./include/linux/list.h:5,
> from ./include/linux/kobject.h:19,
> from ./include/linux/of.h:17,
> from arch/powerpc/platforms/pseries/papr_scm.c:5:
> arch/powerpc/platforms/pseries/papr_scm.c: In function
> ‘papr_scm_pmu_event_init’:
> arch/powerpc/platforms/pseries/papr_scm.c:389:49: error: ‘struct perf_event’
> has no member named ‘pmu’
> struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
> ^~
> ./include/linux/container_of.h:18:26: note: in definition of macro
> ‘container_of’
> void *__mptr = (void *)(ptr); \
> ^~~
> arch/powerpc/platforms/pseries/papr_scm.c:389:30: note: in expansion of macro
> ‘to_nvdimm_pmu’
> struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
> ^
> In file included from ./include/linux/bits.h:22,
> from ./include/linux/bitops.h:6,
> from ./include/linux/of.h:15,
> from arch/powerpc/platforms/pseries/papr_scm.c:5:
>
> Fix the build issue by adding check for CONFIG_PERF_EVENTS config option
> and disabling the papr_scm perf interface support incase this config
> is not set
>
> Fixes: 4c08d4bbc089 ("powerpc/papr_scm: Add perf interface support") (Commit
> id
> based on linux-next tree)
> Signed-off-by: Kajol Jain
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 15 +++
This is a bit messier than I would have liked mainly because it dumps
a bunch of ifdefery into a C file contrary to coding style, "Wherever
possible, don't use preprocessor conditionals (#if, #ifdef) in .c
files". I would expect this all to move to an organization like:
arch/powerpc/platforms/pseries/papr_scm/main.c
arch/powerpc/platforms/pseries/papr_scm/perf.c
...and a new config symbol like:
config PAPR_SCM_PERF
depends on PAPR_SCM && PERF_EVENTS
def_bool y
...with wrappers in header files to make everything compile away
without any need for main.c to carry an ifdef.
Can you turn a patch like that in the next couple days? Otherwise, I
think if Linus saw me sending a late breaking compile fix that threw
coding style out the window he'd have cause to just drop the pull
request entirely.
Re: [PATCH v1 4/7] arm64/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
On Mon, Mar 21, 2022 at 05:44:05PM +, Will Deacon wrote: > On Mon, Mar 21, 2022 at 04:07:48PM +0100, David Hildenbrand wrote: > > So the example you gave cannot possibly have that bit set. From what I > > understand, it should be fine. But I have no real preference: I can also > > just stick to the original patch, whatever you prefer. > > I think I'd prefer to stay on the safe side and stick with bit 2 as you > originally proposed. If we need to support crazy numbers of swapfiles > in future then we can revisit the idea of allocating bit 1. Sounds fine to me. David, feel free to keep my reviewed-by on the original patch. -- Catalin
[powerpc:next] BUILD SUCCESS fe2640bd7a62f1f7c3f55fbda31084085075bc30
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: fe2640bd7a62f1f7c3f55fbda31084085075bc30 powerpc/pseries: Fix use after free in remove_phb_dynamic() elapsed time: 755m configs tested: 167 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig i386 randconfig-c001-20220321 mips randconfig-c004-20220320 i386 randconfig-c001 arm corgi_defconfig s390 allmodconfig powerpccell_defconfig armoxnas_v6_defconfig h8300 edosk2674_defconfig mips xway_defconfig parisc64 alldefconfig powerpc mpc834x_mds_defconfig arm omap2plus_defconfig armshmobile_defconfig powerpcmpc7448_hpc2_defconfig microblaze mmu_defconfig m68kmac_defconfig armpleb_defconfig arm pxa3xx_defconfig sh defconfig powerpc ep8248e_defconfig mips bmips_be_defconfig sh se7712_defconfig shdreamcast_defconfig arm sunxi_defconfig armqcom_defconfig powerpcamigaone_defconfig powerpcwarp_defconfig arm exynos_defconfig m68kmvme16x_defconfig powerpc holly_defconfig powerpcsam440ep_defconfig arm pxa_defconfig powerpc pcm030_defconfig sh rsk7269_defconfig nios2 10m50_defconfig powerpc64 defconfig ia64defconfig shsh7757lcr_defconfig powerpc mpc837x_rdb_defconfig m68k allmodconfig arm vf610m4_defconfig openriscor1ksim_defconfig nds32 defconfig arc axs103_defconfig arc defconfig m68kq40_defconfig arc tb10x_defconfig sh urquell_defconfig csky alldefconfig arm64alldefconfig xtensa iss_defconfig armmps2_defconfig powerpc ppc64_defconfig archsdk_defconfig m68k amcore_defconfig powerpc tqm8548_defconfig sh sdk7780_defconfig powerpc mpc85xx_cds_defconfig arm randconfig-c002-20220321 arm randconfig-c002-20220320 ia64 allmodconfig ia64 allyesconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig cskydefconfig alpha defconfig nios2allyesconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig sh allmodconfig parisc defconfig parisc64defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig i386 debian-10.3-kselftests i386 debian-10.3 mips allyesconfig mips allmodconfig powerpc allnoconfig powerpc allmodconfig powerpc allyesconfig x86_64 randconfig-a016-20220321 x86_64 randconfig-a011-20220321 x86_64 randconfig-a012-20220321 x86_64 randconfig-a013-20220321 x86_64 randconfig-a014-20220321 x86_64
[powerpc:merge] BUILD SUCCESS e8833c5edc5903f8c8c4fa3dd4f34d6b813c87c8
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge branch HEAD: e8833c5edc5903f8c8c4fa3dd4f34d6b813c87c8 Automatic merge of 'next' into merge (2022-03-21 13:13) elapsed time: 756m configs tested: 129 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm allmodconfig arm allyesconfig arm64allyesconfig arm64 defconfig i386 randconfig-c001-20220321 powerpc mpc8540_ads_defconfig arc haps_hs_smp_defconfig mips db1xxx_defconfig sh ecovec24_defconfig arm rpc_defconfig powerpc maple_defconfig arm pxa_defconfig powerpc pcm030_defconfig powerpcsam440ep_defconfig arm pxa3xx_defconfig sh urquell_defconfig csky alldefconfig arm gemini_defconfig arm aspeed_g5_defconfig xtensa audio_kc705_defconfig arm randconfig-c002-20220320 arm randconfig-c002-20220321 ia64defconfig ia64 allmodconfig ia64 allyesconfig m68kdefconfig m68k allyesconfig m68k allmodconfig nds32 allnoconfig nios2 defconfig arc allyesconfig cskydefconfig alpha defconfig nds32 defconfig alphaallyesconfig nios2allyesconfig arc defconfig sh allmodconfig h8300allyesconfig xtensa allyesconfig parisc defconfig parisc64defconfig s390 allmodconfig parisc allyesconfig s390defconfig s390 allyesconfig i386 allyesconfig i386 debian-10.3 i386 debian-10.3-kselftests i386defconfig sparcallyesconfig sparc defconfig mips allyesconfig mips allmodconfig powerpc allnoconfig powerpc allmodconfig powerpc allyesconfig x86_64 randconfig-a013-20220321 x86_64 randconfig-a012-20220321 x86_64 randconfig-a014-20220321 x86_64 randconfig-a011-20220321 x86_64 randconfig-a016-20220321 x86_64 randconfig-a015-20220321 i386 randconfig-a013-20220321 i386 randconfig-a011-20220321 i386 randconfig-a012-20220321 i386 randconfig-a014-20220321 i386 randconfig-a016-20220321 i386 randconfig-a015-20220321 i386 randconfig-a014 i386 randconfig-a012 i386 randconfig-a016 arc randconfig-r043-20220320 riscvrandconfig-r042-20220321 s390 randconfig-r044-20220321 arc randconfig-r043-20220321 riscvnommu_k210_defconfig riscvnommu_virt_defconfig riscvallyesconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig um i386_defconfig um x86_64_defconfig x86_64 defconfig x86_64 allyesconfig x86_64 kexec x86_64 rhel-8.3 x86_64 rhel-8.3-func x86_64 rhel-8.3-kunit x86_64rhel-8.3-kselftests clang tested configs: x86_64randconfig-c007 mips randconfig-c004-20220320 powerpc randconfig-c003-20220320 i386 randconfig-c001 arm randconfig-c002-20220320 s390 randconfig-c005-20220320 riscvrandconfig-c006
Re: [PATCH v1 4/7] arm64/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
On Mon, Mar 21, 2022 at 04:07:48PM +0100, David Hildenbrand wrote:
> On 21.03.22 15:38, Will Deacon wrote:
> > On Wed, Mar 16, 2022 at 06:27:01PM +, Catalin Marinas wrote:
> >> On Tue, Mar 15, 2022 at 03:18:34PM +0100, David Hildenbrand wrote:
> >>> diff --git a/arch/arm64/include/asm/pgtable-prot.h
> >>> b/arch/arm64/include/asm/pgtable-prot.h
> >>> index b1e1b74d993c..62e0ebeed720 100644
> >>> --- a/arch/arm64/include/asm/pgtable-prot.h
> >>> +++ b/arch/arm64/include/asm/pgtable-prot.h
> >>> @@ -14,6 +14,7 @@
> >>> * Software defined PTE bits definition.
> >>> */
> >>> #define PTE_WRITE(PTE_DBM)/* same as DBM
> >>> (51) */
> >>> +#define PTE_SWP_EXCLUSIVE(_AT(pteval_t, 1) << 2) /* only for
> >>> swp ptes */
> >>
> >> I think we can use bit 1 here.
> >>
> >>> @@ -909,12 +925,13 @@ static inline pmd_t pmdp_establish(struct
> >>> vm_area_struct *vma,
> >>> /*
> >>> * Encode and decode a swap entry:
> >>> * bits 0-1: present (must be zero)
> >>> - * bits 2-7: swap type
> >>> + * bits 2: remember PG_anon_exclusive
> >>> + * bits 3-7: swap type
> >>> * bits 8-57: swap offset
> >>> * bit 58:PTE_PROT_NONE (must be zero)
> >>
> >> I don't remember exactly why we reserved bits 0 and 1 when, from the
> >> hardware perspective, it's sufficient for bit 0 to be 0 and the whole
> >> pte becomes invalid. We use bit 1 as the 'table' bit (when 0 at pmd
> >> level, it's a huge page) but we shouldn't check for this on a swap
> >> entry.
> >
> > I'm a little worried that when we're dealing with huge mappings at the
> > PMD level we might lose the ability to distinguish them from a pte-level
> > mapping with this new flag set if we use bit 1. A similar issue to this
> > was fixed a long time ago by 59911ca4325d ("ARM64: mm: Move PTE_PROT_NONE
> > bit") when we used to use bit 1 for PTE_PROT_NONE.
> >
> > Is something like:
> >
> > pmd_to_swp_entry(swp_entry_to_pmd(pmd));
>
> Note that __HAVE_ARCH_PTE_SWP_EXCLUSIVE currently only applies to actual
> swap entries, not non-swap entries (migration, hwpoison, ...). So it
> really only applies to PTEs -- PMDs are not applicable.
Right, thanks for the clarification.
> So the example you gave cannot possibly have that bit set. From what I
> understand, it should be fine. But I have no real preference: I can also
> just stick to the original patch, whatever you prefer.
I think I'd prefer to stay on the safe side and stick with bit 2 as you
originally proposed. If we need to support crazy numbers of swapfiles
in future then we can revisit the idea of allocating bit 1.
Thanks, and sorry for the trouble.
Will
[PATCH] powerpc: align address to page boundary in change_page_attr()
Aligning address to page boundary allows flush_tlb_kernel_range()
to know it's a single page flush and use tlbie instead of tlbia.
On 603 we now have the following code in first leg of
change_page_attr():
2c: 55 29 00 3c rlwinm r9,r9,0,0,30
30: 91 23 00 00 stw r9,0(r3)
34: 7c 00 22 64 tlbie r4,r0
38: 7c 00 04 ac hwsync
3c: 38 60 00 00 li r3,0
40: 4e 80 00 20 blr
Before we had:
28: 55 29 00 3c rlwinm r9,r9,0,0,30
2c: 91 23 00 00 stw r9,0(r3)
30: 54 89 00 26 rlwinm r9,r4,0,0,19
34: 38 84 10 00 addir4,r4,4096
38: 7c 89 20 50 subfr4,r9,r4
3c: 28 04 10 00 cmplwi r4,4096
40: 41 81 00 30 bgt 70
44: 7c 00 4a 64 tlbie r9,r0
48: 7c 00 04 ac hwsync
4c: 38 60 00 00 li r3,0
50: 4e 80 00 20 blr
...
70: 94 21 ff f0 stwur1,-16(r1)
74: 7c 08 02 a6 mflrr0
78: 90 01 00 14 stw r0,20(r1)
7c: 48 00 00 01 bl 7c
7c: R_PPC_REL24 _tlbia
80: 80 01 00 14 lwz r0,20(r1)
84: 38 60 00 00 li r3,0
88: 7c 08 03 a6 mtlrr0
8c: 38 21 00 10 addir1,r1,16
90: 4e 80 00 20 blr
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/pageattr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index 85753e32a4de..6163e484bc6d 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -31,6 +31,7 @@ static int change_page_attr(pte_t *ptep, unsigned long addr,
void *data)
{
long action = (long)data;
+ addr &= PAGE_MASK;
/* modify the PTE bits as desired */
switch (action) {
case SET_MEMORY_RO:
--
2.35.1
[PATCH] powerpc/8xx: Simplify flush_tlb_kernel_range()
In the same spirit as commit 63f501e07a85 ("powerpc/8xx: Simplify TLB
handling"), simplify flush_tlb_kernel_range() for 8xx.
8xx cannot be SMP, and has 'tlbie' and 'tlbia' instructions, so
an inline version of flush_tlb_kernel_range() for 8xx is worth it.
With this page, first leg of change_page_attr() is:
2c: 55 29 00 3c rlwinm r9,r9,0,0,30
30: 91 23 00 00 stw r9,0(r3)
34: 7c 00 22 64 tlbie r4,r0
38: 7c 00 04 ac hwsync
3c: 38 60 00 00 li r3,0
40: 4e 80 00 20 blr
Before the patch it was:
30: 55 29 00 3c rlwinm r9,r9,0,0,30
34: 91 2a 00 00 stw r9,0(r10)
38: 94 21 ff f0 stwur1,-16(r1)
3c: 7c 08 02 a6 mflrr0
40: 38 83 10 00 addir4,r3,4096
44: 90 01 00 14 stw r0,20(r1)
48: 48 00 00 01 bl 48
48: R_PPC_REL24 flush_tlb_kernel_range
4c: 80 01 00 14 lwz r0,20(r1)
50: 38 60 00 00 li r3,0
54: 7c 08 03 a6 mtlrr0
58: 38 21 00 10 addir1,r1,16
5c: 4e 80 00 20 blr
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/nohash/tlbflush.h | 12 +++-
arch/powerpc/mm/nohash/tlb.c | 2 ++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/nohash/tlbflush.h
b/arch/powerpc/include/asm/nohash/tlbflush.h
index c08d25e3e626..698935d4f72d 100644
--- a/arch/powerpc/include/asm/nohash/tlbflush.h
+++ b/arch/powerpc/include/asm/nohash/tlbflush.h
@@ -30,7 +30,6 @@ struct mm_struct;
extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
unsigned long end);
-extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
#ifdef CONFIG_PPC_8xx
static inline void local_flush_tlb_mm(struct mm_struct *mm)
@@ -45,7 +44,18 @@ static inline void local_flush_tlb_page(struct
vm_area_struct *vma, unsigned lon
{
asm volatile ("tlbie %0; sync" : : "r" (vmaddr) : "memory");
}
+
+static inline void flush_tlb_kernel_range(unsigned long start, unsigned long
end)
+{
+ start &= PAGE_MASK;
+
+ if (end - start <= PAGE_SIZE)
+ asm volatile ("tlbie %0; sync" : : "r" (start) : "memory");
+ else
+ asm volatile ("sync; tlbia; isync" : : : "memory");
+}
#else
+extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
extern void local_flush_tlb_mm(struct mm_struct *mm);
extern void local_flush_tlb_page(struct vm_area_struct *vma, unsigned long
vmaddr);
diff --git a/arch/powerpc/mm/nohash/tlb.c b/arch/powerpc/mm/nohash/tlb.c
index fd2c77af5c55..47f81d1c35dc 100644
--- a/arch/powerpc/mm/nohash/tlb.c
+++ b/arch/powerpc/mm/nohash/tlb.c
@@ -358,6 +358,7 @@ void __init early_init_mmu_47x(void)
/*
* Flush kernel TLB entries in the given range
*/
+#ifndef CONFIG_PPC_8xx
void flush_tlb_kernel_range(unsigned long start, unsigned long end)
{
#ifdef CONFIG_SMP
@@ -370,6 +371,7 @@ void flush_tlb_kernel_range(unsigned long start, unsigned
long end)
#endif
}
EXPORT_SYMBOL(flush_tlb_kernel_range);
+#endif
/*
* Currently, for range flushing, we just do a full mm flush. This should
--
2.35.1
Re: [PATCH v1 4/7] arm64/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
On 21.03.22 15:38, Will Deacon wrote:
> On Wed, Mar 16, 2022 at 06:27:01PM +, Catalin Marinas wrote:
>> On Tue, Mar 15, 2022 at 03:18:34PM +0100, David Hildenbrand wrote:
>>> diff --git a/arch/arm64/include/asm/pgtable-prot.h
>>> b/arch/arm64/include/asm/pgtable-prot.h
>>> index b1e1b74d993c..62e0ebeed720 100644
>>> --- a/arch/arm64/include/asm/pgtable-prot.h
>>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>>> @@ -14,6 +14,7 @@
>>> * Software defined PTE bits definition.
>>> */
>>> #define PTE_WRITE (PTE_DBM)/* same as DBM (51) */
>>> +#define PTE_SWP_EXCLUSIVE (_AT(pteval_t, 1) << 2) /* only for swp ptes */
>>
>> I think we can use bit 1 here.
>>
>>> @@ -909,12 +925,13 @@ static inline pmd_t pmdp_establish(struct
>>> vm_area_struct *vma,
>>> /*
>>> * Encode and decode a swap entry:
>>> * bits 0-1: present (must be zero)
>>> - * bits 2-7: swap type
>>> + * bits 2: remember PG_anon_exclusive
>>> + * bits 3-7: swap type
>>> * bits 8-57: swap offset
>>> * bit 58:PTE_PROT_NONE (must be zero)
>>
>> I don't remember exactly why we reserved bits 0 and 1 when, from the
>> hardware perspective, it's sufficient for bit 0 to be 0 and the whole
>> pte becomes invalid. We use bit 1 as the 'table' bit (when 0 at pmd
>> level, it's a huge page) but we shouldn't check for this on a swap
>> entry.
>
> I'm a little worried that when we're dealing with huge mappings at the
> PMD level we might lose the ability to distinguish them from a pte-level
> mapping with this new flag set if we use bit 1. A similar issue to this
> was fixed a long time ago by 59911ca4325d ("ARM64: mm: Move PTE_PROT_NONE
> bit") when we used to use bit 1 for PTE_PROT_NONE.
>
> Is something like:
>
> pmd_to_swp_entry(swp_entry_to_pmd(pmd));
Note that __HAVE_ARCH_PTE_SWP_EXCLUSIVE currently only applies to actual
swap entries, not non-swap entries (migration, hwpoison, ...). So it
really only applies to PTEs -- PMDs are not applicable.
So the example you gave cannot possibly have that bit set. From what I
understand, it should be fine. But I have no real preference: I can also
just stick to the original patch, whatever you prefer.
Thanks!
--
Thanks,
David / dhildenb
Re: Build regressions/improvements in v5.17
On Mon, Mar 21, 2022 at 3:00 PM Geert Uytterhoeven wrote:
> JFYI, when comparing v5.17[1] to v5.17-rc8[3], the summaries are:
> - build errors: +5/-0
+ /kisskb/src/crypto/blake2b_generic.c: error: the frame size of
2288 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]: =>
109:1
sparc64-gcc11/sparc-allmodconfig
+ /kisskb/src/drivers/vfio/pci/vfio_pci_rdwr.c: error: assignment
makes pointer from integer without a cast [-Werror=int-conversion]:
=> 324:9, 317:9
+ /kisskb/src/drivers/vfio/pci/vfio_pci_rdwr.c: error: implicit
declaration of function 'ioport_map'
[-Werror=implicit-function-declaration]: => 317:11
+ /kisskb/src/drivers/vfio/pci/vfio_pci_rdwr.c: error: implicit
declaration of function 'ioport_unmap'
[-Werror=implicit-function-declaration]: => 338:15
um-x86_64/um-all{mod,yes}config
+ error: arch/powerpc/kvm/book3s_64_entry.o: relocation truncated to
fit: R_PPC64_REL14 (stub) against symbol `machine_check_common'
defined in .text section in arch/powerpc/kernel/head_64.o: =>
(.text+0x3e4)
powerpc-gcc5/powerpc-allyesconfig
> [1]
> http://kisskb.ellerman.id.au/kisskb/branch/linus/head/f443e374ae131c168a065ea1748feac6b2e76613/
> (96 out of 98 configs)
> [3]
> http://kisskb.ellerman.id.au/kisskb/branch/linus/head/09688c0166e76ce2fb85e86b9d99be8b0084cdf9/
> (96 out of 98 configs)
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 v1 4/7] arm64/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
On Mon, Mar 21, 2022 at 02:38:02PM +, Will Deacon wrote:
> On Wed, Mar 16, 2022 at 06:27:01PM +, Catalin Marinas wrote:
> > On Tue, Mar 15, 2022 at 03:18:34PM +0100, David Hildenbrand wrote:
> > > diff --git a/arch/arm64/include/asm/pgtable-prot.h
> > > b/arch/arm64/include/asm/pgtable-prot.h
> > > index b1e1b74d993c..62e0ebeed720 100644
> > > --- a/arch/arm64/include/asm/pgtable-prot.h
> > > +++ b/arch/arm64/include/asm/pgtable-prot.h
> > > @@ -14,6 +14,7 @@
> > > * Software defined PTE bits definition.
> > > */
> > > #define PTE_WRITE(PTE_DBM)/* same as DBM
> > > (51) */
> > > +#define PTE_SWP_EXCLUSIVE(_AT(pteval_t, 1) << 2) /* only for
> > > swp ptes */
> >
> > I think we can use bit 1 here.
> >
> > > @@ -909,12 +925,13 @@ static inline pmd_t pmdp_establish(struct
> > > vm_area_struct *vma,
> > > /*
> > > * Encode and decode a swap entry:
> > > * bits 0-1: present (must be zero)
> > > - * bits 2-7: swap type
> > > + * bits 2: remember PG_anon_exclusive
> > > + * bits 3-7: swap type
> > > * bits 8-57: swap offset
> > > * bit 58:PTE_PROT_NONE (must be zero)
> >
> > I don't remember exactly why we reserved bits 0 and 1 when, from the
> > hardware perspective, it's sufficient for bit 0 to be 0 and the whole
> > pte becomes invalid. We use bit 1 as the 'table' bit (when 0 at pmd
> > level, it's a huge page) but we shouldn't check for this on a swap
> > entry.
>
> I'm a little worried that when we're dealing with huge mappings at the
> PMD level we might lose the ability to distinguish them from a pte-level
> mapping with this new flag set if we use bit 1. A similar issue to this
> was fixed a long time ago by 59911ca4325d ("ARM64: mm: Move PTE_PROT_NONE
> bit") when we used to use bit 1 for PTE_PROT_NONE.
>
> Is something like:
>
> pmd_to_swp_entry(swp_entry_to_pmd(pmd));
>
> supposed to preserve the original pmd? I'm not sure that's guaranteed
> after this change if bit 1 can be cleared in the process -- we could end
> up with a pte, which the hardware would interpret as a table entry and
> end up with really bad things happening.
(I got this back to front: having the bit set rather than cleared would
be an issue, but the overall point remains).
Will
Re: [PATCH v1 4/7] arm64/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
On Wed, Mar 16, 2022 at 06:27:01PM +, Catalin Marinas wrote:
> On Tue, Mar 15, 2022 at 03:18:34PM +0100, David Hildenbrand wrote:
> > diff --git a/arch/arm64/include/asm/pgtable-prot.h
> > b/arch/arm64/include/asm/pgtable-prot.h
> > index b1e1b74d993c..62e0ebeed720 100644
> > --- a/arch/arm64/include/asm/pgtable-prot.h
> > +++ b/arch/arm64/include/asm/pgtable-prot.h
> > @@ -14,6 +14,7 @@
> > * Software defined PTE bits definition.
> > */
> > #define PTE_WRITE (PTE_DBM)/* same as DBM (51) */
> > +#define PTE_SWP_EXCLUSIVE (_AT(pteval_t, 1) << 2) /* only for swp ptes */
>
> I think we can use bit 1 here.
>
> > @@ -909,12 +925,13 @@ static inline pmd_t pmdp_establish(struct
> > vm_area_struct *vma,
> > /*
> > * Encode and decode a swap entry:
> > * bits 0-1: present (must be zero)
> > - * bits 2-7: swap type
> > + * bits 2: remember PG_anon_exclusive
> > + * bits 3-7: swap type
> > * bits 8-57: swap offset
> > * bit 58:PTE_PROT_NONE (must be zero)
>
> I don't remember exactly why we reserved bits 0 and 1 when, from the
> hardware perspective, it's sufficient for bit 0 to be 0 and the whole
> pte becomes invalid. We use bit 1 as the 'table' bit (when 0 at pmd
> level, it's a huge page) but we shouldn't check for this on a swap
> entry.
I'm a little worried that when we're dealing with huge mappings at the
PMD level we might lose the ability to distinguish them from a pte-level
mapping with this new flag set if we use bit 1. A similar issue to this
was fixed a long time ago by 59911ca4325d ("ARM64: mm: Move PTE_PROT_NONE
bit") when we used to use bit 1 for PTE_PROT_NONE.
Is something like:
pmd_to_swp_entry(swp_entry_to_pmd(pmd));
supposed to preserve the original pmd? I'm not sure that's guaranteed
after this change if bit 1 can be cleared in the process -- we could end
up with a pte, which the hardware would interpret as a table entry and
end up with really bad things happening.
Will
Re: [PATCH v2] ASoC: fsl-asoc-card: Fix jack_event() always return 0
On Mon, 21 Mar 2022 14:57:54 +0800, Meng Tang wrote: > Today, hp_jack_event and mic_jack_event always return 0. However, > snd_soc_dapm_disable_pin and snd_soc_dapm_enable_pin may return a > non-zero value, this will cause the user who calling hp_jack_event > and mic_jack_event don't know whether the operation was really > successfully. > > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus Thanks! [1/1] ASoC: fsl-asoc-card: Fix jack_event() always return 0 commit: 5cb90dcb6ad569f4968da6dd841db10b91df5642 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[syzbot] WARNING in kvm_mmu_notifier_invalidate_range_start (2)
Hello,
syzbot found the following issue on:
HEAD commit:56e337f2cf13 Revert "gpio: Revert regression in sysfs-gpio..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13821b8d70
kernel config: https://syzkaller.appspot.com/x/.config?x=d35f9bc6884af6c9
dashboard link: https://syzkaller.appspot.com/bug?extid=6bde52d89cfdf9f61425
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for
Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12a2d0a970
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13d34fd970
The issue was bisected to:
commit ed922739c9199bf515a3e7fec3e319ce1edeef2a
Author: Maciej S. Szmigiero
Date: Mon Dec 6 19:54:28 2021 +
KVM: Use interval tree to do fast hva lookup in memslots
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=142aa59d70
final oops: https://syzkaller.appspot.com/x/report.txt?x=162aa59d70
console output: https://syzkaller.appspot.com/x/log.txt?x=122aa59d70
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+6bde52d89cfdf9f61...@syzkaller.appspotmail.com
Fixes: ed922739c919 ("KVM: Use interval tree to do fast hva lookup in memslots")
[ cut here ]
WARNING: CPU: 0 PID: 3599 at arch/x86/kvm/../../../virt/kvm/kvm_main.c:529
__kvm_handle_hva_range arch/x86/kvm/../../../virt/kvm/kvm_main.c:529 [inline]
WARNING: CPU: 0 PID: 3599 at arch/x86/kvm/../../../virt/kvm/kvm_main.c:529
kvm_mmu_notifier_invalidate_range_start+0x97a/0xb20
arch/x86/kvm/../../../virt/kvm/kvm_main.c:714
Modules linked in:
CPU: 0 PID: 3599 Comm: syz-executor221 Not tainted
5.17.0-rc8-syzkaller-3-g56e337f2cf13 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
01/01/2011
RIP: 0010:__kvm_handle_hva_range arch/x86/kvm/../../../virt/kvm/kvm_main.c:529
[inline]
RIP: 0010:kvm_mmu_notifier_invalidate_range_start+0x97a/0xb20
arch/x86/kvm/../../../virt/kvm/kvm_main.c:714
Code: 00 48 c7 c2 60 0c a2 89 be b9 01 00 00 48 c7 c7 c0 10 a2 89 c6 05 ed 71
76 0c 01 e8 79 84 ff 07 e9 73 ff ff ff e8 b6 cd 6f 00 <0f> 0b e9 88 fc ff ff e8
aa cd 6f 00 0f 0b e9 58 fc ff ff e8 9e cd
RSP: 0018:c90001caf948 EFLAGS: 00010293
RAX: RBX: 2000d000 RCX:
RDX: 888020d83a00 RSI: 8108f27a RDI: 0003
RBP: c90002b76290 R08: 2000d000 R09: c90002b762e3
R10: 8108eb1c R11: 0001 R12: c90002b7f240
R13: c90002b75000 R14: c90001cafc18 R15: 2000d000
FS: 55a55300() GS:8880b9c0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 2088 CR3: 74ce9000 CR4: 003526f0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
Call Trace:
mn_hlist_invalidate_range_start mm/mmu_notifier.c:493 [inline]
__mmu_notifier_invalidate_range_start+0x2ff/0x800 mm/mmu_notifier.c:548
mmu_notifier_invalidate_range_start include/linux/mmu_notifier.h:459 [inline]
move_page_tables+0x2642/0x2d20 mm/mremap.c:498
move_vma+0x48c/0xf40 mm/mremap.c:629
mremap_to mm/mremap.c:862 [inline]
__do_sys_mremap+0xf01/0x1560 mm/mremap.c:972
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f11faab5089
Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48
89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01
c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7ffc17608428 EFLAGS: 0246 ORIG_RAX: 0019
RAX: ffda RBX: RCX: 7f11faab5089
RDX: 1000 RSI: fe74 RDI: 2000d000
RBP: 7f11faa79070 R08: 20007000 R09:
R10: 0003 R11: 0246 R12: 7f11faa79100
R13: R14: R15:
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkal...@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
Re: [PATCH v2] ASoC: fsl-asoc-card: Fix jack_event() always return 0
On Mon, Mar 21, 2022 at 2:58 PM Meng Tang wrote:
> Today, hp_jack_event and mic_jack_event always return 0. However,
> snd_soc_dapm_disable_pin and snd_soc_dapm_enable_pin may return a
> non-zero value, this will cause the user who calling hp_jack_event
> and mic_jack_event don't know whether the operation was really
> successfully.
>
> Signed-off-by: Meng Tang
>
Acked-by: Shengjiu Wang
Best regards
Wang Shengjiu
> ---
> sound/soc/fsl/fsl-asoc-card.c | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index 370bc790c6ba..d9a0d4768c4d 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -462,11 +462,9 @@ static int hp_jack_event(struct notifier_block *nb,
> unsigned long event,
>
> if (event & SND_JACK_HEADPHONE)
> /* Disable speaker if headphone is plugged in */
> - snd_soc_dapm_disable_pin(dapm, "Ext Spk");
> + return snd_soc_dapm_disable_pin(dapm, "Ext Spk");
> else
> - snd_soc_dapm_enable_pin(dapm, "Ext Spk");
> -
> - return 0;
> + return snd_soc_dapm_enable_pin(dapm, "Ext Spk");
> }
>
> static struct notifier_block hp_jack_nb = {
> @@ -481,11 +479,9 @@ static int mic_jack_event(struct notifier_block *nb,
> unsigned long event,
>
> if (event & SND_JACK_MICROPHONE)
> /* Disable dmic if microphone is plugged in */
> - snd_soc_dapm_disable_pin(dapm, "DMIC");
> + return snd_soc_dapm_disable_pin(dapm, "DMIC");
> else
> - snd_soc_dapm_enable_pin(dapm, "DMIC");
> -
> - return 0;
> + return snd_soc_dapm_enable_pin(dapm, "DMIC");
> }
>
> static struct notifier_block mic_jack_nb = {
> --
> 2.20.1
>
>
>
>
[PATCH v2] macintosh/via-pmu: Avoid compiler warnings when CONFIG_PROC_FS is disabled
drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined
but not used [-Wunused-function]
static int pmu_battery_proc_show(struct seq_file *m, void *v)
^
drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined
but not used [-Wunused-function]
static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
^~
drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but
not used [-Wunused-function]
static int pmu_info_proc_show(struct seq_file *m, void *v)
^~
Add some #ifdefs to avoid unused code warnings when CONFIG_PROC_FS is
disabled.
Cc: Randy Dunlap
Cc: Christophe Leroy
Reported-by: Randy Dunlap
Suggested-by: Christophe Leroy
Signed-off-by: Finn Thain
---
Changed since v1:
- Simplified to take advantage of the fact that proc_mkdir() is stubbed
out when CONFIG_PROC_FS=n. Hence that call doesn't need to move
within the #ifdef.
---
drivers/macintosh/via-pmu.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 2109129ea1bb..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -204,9 +204,11 @@ static int init_pmu(void);
static void pmu_start(void);
static irqreturn_t via_pmu_interrupt(int irq, void *arg);
static irqreturn_t gpio1_interrupt(int irq, void *arg);
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v);
static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
static int pmu_battery_proc_show(struct seq_file *m, void *v);
+#endif
static void pmu_pass_intr(unsigned char *data, int len);
static const struct proc_ops pmu_options_proc_ops;
@@ -857,6 +859,7 @@ query_battery_state(void)
2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
}
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
@@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
.proc_release = single_release,
.proc_write = pmu_options_proc_write,
};
+#endif
#ifdef CONFIG_ADB
/* Send an ADB command */
--
2.32.0
Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand
Christophe Leroy wrote: Le 21/03/2022 à 09:19, Naveen N. Rao a écrit : Christophe Leroy wrote: We don't enable ftrace for vdso, so I suspect objtool run above will be a no-op. This needs to be confirmed, of course. I just checked without the series: recordmcount isn't run for VDSO, so objtool shouldn't be run either when the series is applied. Agree. I was only pointing out that it would be harmless, but it is good to ensure objtool is skipped for files/directories where ftrace isn't enabled. recordmcount keys off the presence of CC_FLAGS_FTRACE, and we should do the same for 'objtool --mcount'. - Naveen
Re: [PATCH] Subject: [PATCH] macintosh/via-pmu: Avoid compiler warnings when CONFIG_PROC_FS is disabled
Please disregard. I'll send v2 with corrected commit log entry.
[PATCH] Subject: [PATCH] macintosh/via-pmu: Avoid compiler warnings when CONFIG_PROC_FS is disabled
drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined
but not used [-Wunused-function]
static int pmu_battery_proc_show(struct seq_file *m, void *v)
^
drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined
but not used [-Wunused-function]
static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
^~
drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but
not used [-Wunused-function]
static int pmu_info_proc_show(struct seq_file *m, void *v)
^~
Rearrange some code and add some #ifdefs to avoid unused code warnings
when CONFIG_PROC_FS is disabled.
Reported-by: Randy Dunlap
Cc: Randy Dunlap
Cc: Christophe Leroy
Signed-off-by: Finn Thain
---
drivers/macintosh/via-pmu.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 2109129ea1bb..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -204,9 +204,11 @@ static int init_pmu(void);
static void pmu_start(void);
static irqreturn_t via_pmu_interrupt(int irq, void *arg);
static irqreturn_t gpio1_interrupt(int irq, void *arg);
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v);
static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
static int pmu_battery_proc_show(struct seq_file *m, void *v);
+#endif
static void pmu_pass_intr(unsigned char *data, int len);
static const struct proc_ops pmu_options_proc_ops;
@@ -857,6 +859,7 @@ query_battery_state(void)
2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
}
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
@@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
.proc_release = single_release,
.proc_write = pmu_options_proc_write,
};
+#endif
#ifdef CONFIG_ADB
/* Send an ADB command */
--
2.32.0
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
On Mon, 21 Mar 2022, Christophe Leroy wrote:
> Le 21/03/2022 à 09:50, Finn Thain a écrit :
> > Hi Christophe,
> >
> > On Mon, 21 Mar 2022, Finn Thain wrote:
> >
> >> On Mon, 21 Mar 2022, Christophe Leroy wrote:
> >>
> >>> Le 19/03/2022 à 08:20, Finn Thain a écrit :
> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
> defined but not used [-Wunused-function]
> static int pmu_battery_proc_show(struct seq_file *m, void *v)
> ^
> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
> defined but not used [-Wunused-function]
> static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> ^~
> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show'
> defined but not used [-Wunused-function]
> static int pmu_info_proc_show(struct seq_file *m, void *v)
> ^~
>
> Rearrange some code and add some #ifdefs to avoid unused code warnings
> when CONFIG_PROC_FS is disabled.
> >>>
> >>> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
> >>>
> >>
> >> You'd get a warning about the prototypes ("declared static but never
> >> defined"). Rather than add an ifdef around the prototypes as well, I
> >> just reordered things a little.
> >
> > Oops, I was forgetting that I also added an ifdef around the new
> > prototype.
> >
> > The simplest solution is probably the patch below, as it better exploits
> > the stubbed-out proc_* API in include/linux/proc_fs.h.
> >
> > Was this what you had in mind?
>
> Yes that's exactly what I had in mind.
>
OK, I'll submit it formally.
Thanks for your review.
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
On Mon, 21 Mar 2022, Geert Uytterhoeven wrote: > On Mon, Mar 21, 2022 at 9:29 AM Finn Thain wrote: > > On Mon, 21 Mar 2022, Christophe Leroy wrote: > > > Le 21/03/2022 à 05:30, Finn Thain a écrit : > > > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event': > > > > via-pmu-event.c:(.text+0x44): undefined reference to `input_event' > > > > via-pmu-event.c:(.text+0x68): undefined reference to `input_event' > > > > via-pmu-event.c:(.text+0x94): undefined reference to `input_event' > > > > via-pmu-event.c:(.text+0xb8): undefined reference to `input_event' > > > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init': > > > > via-pmu-event.c:(.init.text+0x20): undefined reference to > > > > `input_allocate_device' > > > > via-pmu-event.c:(.init.text+0xc4): undefined reference to > > > > `input_register_device' > > > > via-pmu-event.c:(.init.text+0xd4): undefined reference to > > > > `input_free_device' > > > > make[1]: *** [Makefile:1155: vmlinux] Error 1 > > > > make: *** [Makefile:350: __build_one_by_one] Error 2 > > > > > > > > Don't call into the input subsystem unless CONFIG_INPUT is built-in. > > > > > > > > Reported-by: kernel test robot > > > > Cc: Michael Ellerman > > > > Cc: Geert Uytterhoeven > > > > Cc: Randy Dunlap > > > > Signed-off-by: Finn Thain > > > > --- > > > > This is equivalent to the patch I sent a couple of days ago. This one > > > > is slightly longer and adds a new symbol so that Kconfig logic can been > > > > used instead of Makefile logic in case reviewers prefer that. > > > > --- > > > > drivers/macintosh/Kconfig | 5 + > > > > drivers/macintosh/Makefile | 3 ++- > > > > drivers/macintosh/via-pmu.c | 2 ++ > > > > 3 files changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig > > > > index 5cdc361da37c..b9102f051bbb 100644 > > > > --- a/drivers/macintosh/Kconfig > > > > +++ b/drivers/macintosh/Kconfig > > > > @@ -67,6 +67,11 @@ config ADB_PMU > > > > this device; you should do so if your machine is one of those > > > > mentioned above. > > > > > > > > +config ADB_PMU_EVENT > > > > + bool > > > > + depends on ADB_PMU && INPUT=y > > > > + default y > > > > > > Could be reduced to > > > > > > config ADB_PMU_EVENT > > > def_bool y if ADB_PMU && INPUT=y > > > > That's great but my question remains unanswered: why the aversion to > > conditionals in Makefiles, when that would be simpler (no new symbol)? > > While conditionals in Makefiles do exist, they are far less common, Perhaps the Kconfig solution is more common. I did look for it but didn't find it in recent commits. Maybe I didn't look hard enough. Mostly I see invisible Kconfig getting used for things that are hard to do in any simpler way. > and can be confusing. Kconfig can be confusing too. I don't think your approach reduces complexity, I think it increases it. > They're also harder to grep for. > E.g. "git grep via-pmu-event.o" after your alternative patch would > give: > > obj-$(CONFIG_ADB_PMU) += via-pmu-event.o > > but would miss the important surrounding part: > > ifeq ($(CONFIG_INPUT), y) > obj-$(CONFIG_ADB_PMU) += via-pmu-event.o > endif > > Keeping configuration logic in a single place (the Kconfig file) > avoids that. The extra symbol is invisible, so it doesn't hurt much. > The same argument applies to Kconfig. "git grep CONFIG_ADB_PMU_EVENT" doesn't even mention drivers/macintosh/Kconfig. If you do "git grep ADB_PMU_EVENT" you get: drivers/macintosh/Kconfig:config ADB_PMU_EVENT with no mention of the "important surrounding part" that you were concerned about: config ADB_PMU_EVENT def_bool y if ADB_PMU && INPUT=y Anyway, we don't have to debate this, as drivers/macintosh already has a maintainer who can decide the question. Both patches are available.
Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions
Le 21/03/2022 à 09:30, Christophe Leroy a écrit : Le 21/03/2022 à 08:56, Christophe Leroy a écrit : Le 21/03/2022 à 03:27, Michael Ellerman a écrit : Christophe Leroy writes: Le 18/03/2022 à 13:26, Peter Zijlstra a écrit : On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote: This patch adds powerpc specific functions required for 'objtool mcount' to work, and enables mcount for ppc. I would love to see more objtool enablement for Power :-) I have not received this series and I can't see it on powerpc patchwork either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/) Did you send it to linuxppc-dev list ? If not can you resend it there ? It is there, might have been delayed? http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129 There are some CI failures. On PPC32 I get : [ 0.00] ftrace: No functions to be traced? Without this series I get: [ 0.00] ftrace: allocating 22508 entries in 17 pages [ 0.00] ftrace: allocated 17 pages with 2 groups ppc64_defconfig on QEMU: [ 0.00][ T0] ftrace: No functions to be traced? Without your series: [ 0.00][ T0] ftrace: allocating 38750 entries in 15 pages [ 0.00][ T0] ftrace: allocated 15 pages with 4 groups [ 0.00][ T0] trace event string verifier disabled ppc64le_defconfig on QEMU: [0.00][T0] ftrace: allocating 37236 entries in 14 pages [0.00][T0] ftrace: allocated 14 pages with 3 groups Without your series: [0.00][T0] ftrace: allocating 37236 entries in 14 pages [0.00][T0] ftrace: allocated 14 pages with 3 groups So it seems it works only on Little Endian. Works neither on PPC32 nor on PPC64 Big Endian Christophe
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
Le 21/03/2022 à 09:50, Finn Thain a écrit :
> Hi Christophe,
>
> On Mon, 21 Mar 2022, Finn Thain wrote:
>
>> On Mon, 21 Mar 2022, Christophe Leroy wrote:
>>
>>> Le 19/03/2022 à 08:20, Finn Thain a écrit :
drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
defined but not used [-Wunused-function]
static int pmu_battery_proc_show(struct seq_file *m, void *v)
^
drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
defined but not used [-Wunused-function]
static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
^~
drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined
but not used [-Wunused-function]
static int pmu_info_proc_show(struct seq_file *m, void *v)
^~
Rearrange some code and add some #ifdefs to avoid unused code warnings
when CONFIG_PROC_FS is disabled.
>>>
>>> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
>>>
>>
>> You'd get a warning about the prototypes ("declared static but never
>> defined"). Rather than add an ifdef around the prototypes as well, I
>> just reordered things a little.
>
> Oops, I was forgetting that I also added an ifdef around the new
> prototype.
>
> The simplest solution is probably the patch below, as it better exploits
> the stubbed-out proc_* API in include/linux/proc_fs.h.
>
> Was this what you had in mind?
Yes that's exactly what I had in mind.
Thanks
Christophe
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
Le 21/03/2022 à 09:33, Finn Thain a écrit :
> On Mon, 21 Mar 2022, Christophe Leroy wrote:
>
>> Le 19/03/2022 à 08:20, Finn Thain a écrit :
>>> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
>>> defined but not used [-Wunused-function]
>>>static int pmu_battery_proc_show(struct seq_file *m, void *v)
>>> ^
>>> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
>>> defined but not used [-Wunused-function]
>>>static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
>>> ^~
>>> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined
>>> but not used [-Wunused-function]
>>>static int pmu_info_proc_show(struct seq_file *m, void *v)
>>> ^~
>>>
>>> Rearrange some code and add some #ifdefs to avoid unused code warnings
>>> when CONFIG_PROC_FS is disabled.
>>
>> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
>>
>
> You'd get a warning about the prototypes ("declared static but never
> defined"). Rather than add an ifdef around the prototypes as well, I just
> reordered things a little.
Then now you have callers of proc_create_single_data() inside the ifdefs
and then you have an additional #ifdef in the middle of
via_pmu_dev_init() and also have to ifdef out all proc_pmu_.
I thing ifdefing out the prototypes would be less churn.
Christophe
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
Hi Christophe,
On Mon, 21 Mar 2022, Finn Thain wrote:
> On Mon, 21 Mar 2022, Christophe Leroy wrote:
>
> > Le 19/03/2022 à 08:20, Finn Thain a écrit :
> > > drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
> > > defined but not used [-Wunused-function]
> > > static int pmu_battery_proc_show(struct seq_file *m, void *v)
> > > ^
> > > drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
> > > defined but not used [-Wunused-function]
> > > static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> > > ^~
> > > drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined
> > > but not used [-Wunused-function]
> > > static int pmu_info_proc_show(struct seq_file *m, void *v)
> > > ^~
> > >
> > > Rearrange some code and add some #ifdefs to avoid unused code warnings
> > > when CONFIG_PROC_FS is disabled.
> >
> > Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
> >
>
> You'd get a warning about the prototypes ("declared static but never
> defined"). Rather than add an ifdef around the prototypes as well, I
> just reordered things a little.
Oops, I was forgetting that I also added an ifdef around the new
prototype.
The simplest solution is probably the patch below, as it better exploits
the stubbed-out proc_* API in include/linux/proc_fs.h.
Was this what you had in mind?
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 2109129ea1bb..495fd35b11de 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -204,9 +204,11 @@ static int init_pmu(void);
static void pmu_start(void);
static irqreturn_t via_pmu_interrupt(int irq, void *arg);
static irqreturn_t gpio1_interrupt(int irq, void *arg);
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v);
static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
static int pmu_battery_proc_show(struct seq_file *m, void *v);
+#endif
static void pmu_pass_intr(unsigned char *data, int len);
static const struct proc_ops pmu_options_proc_ops;
@@ -857,6 +859,7 @@ query_battery_state(void)
2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
}
+#ifdef CONFIG_PROC_FS
static int pmu_info_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
@@ -977,6 +980,7 @@ static const struct proc_ops pmu_options_proc_ops = {
.proc_release = single_release,
.proc_write = pmu_options_proc_write,
};
+#endif
#ifdef CONFIG_ADB
/* Send an ADB command */
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
Hi Finn,
On Mon, Mar 21, 2022 at 9:29 AM Finn Thain wrote:
> On Mon, 21 Mar 2022, Christophe Leroy wrote:
> > Le 21/03/2022 à 05:30, Finn Thain a écrit :
> > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event':
> > > via-pmu-event.c:(.text+0x44): undefined reference to `input_event'
> > > via-pmu-event.c:(.text+0x68): undefined reference to `input_event'
> > > via-pmu-event.c:(.text+0x94): undefined reference to `input_event'
> > > via-pmu-event.c:(.text+0xb8): undefined reference to `input_event'
> > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init':
> > > via-pmu-event.c:(.init.text+0x20): undefined reference to
> > > `input_allocate_device'
> > > via-pmu-event.c:(.init.text+0xc4): undefined reference to
> > > `input_register_device'
> > > via-pmu-event.c:(.init.text+0xd4): undefined reference to
> > > `input_free_device'
> > > make[1]: *** [Makefile:1155: vmlinux] Error 1
> > > make: *** [Makefile:350: __build_one_by_one] Error 2
> > >
> > > Don't call into the input subsystem unless CONFIG_INPUT is built-in.
> > >
> > > Reported-by: kernel test robot
> > > Cc: Michael Ellerman
> > > Cc: Geert Uytterhoeven
> > > Cc: Randy Dunlap
> > > Signed-off-by: Finn Thain
> > > ---
> > > This is equivalent to the patch I sent a couple of days ago. This one
> > > is slightly longer and adds a new symbol so that Kconfig logic can been
> > > used instead of Makefile logic in case reviewers prefer that.
> > > ---
> > > drivers/macintosh/Kconfig | 5 +
> > > drivers/macintosh/Makefile | 3 ++-
> > > drivers/macintosh/via-pmu.c | 2 ++
> > > 3 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
> > > index 5cdc361da37c..b9102f051bbb 100644
> > > --- a/drivers/macintosh/Kconfig
> > > +++ b/drivers/macintosh/Kconfig
> > > @@ -67,6 +67,11 @@ config ADB_PMU
> > > this device; you should do so if your machine is one of those
> > > mentioned above.
> > >
> > > +config ADB_PMU_EVENT
> > > + bool
> > > + depends on ADB_PMU && INPUT=y
> > > + default y
> >
> > Could be reduced to
> >
> > config ADB_PMU_EVENT
> > def_bool y if ADB_PMU && INPUT=y
>
> That's great but my question remains unanswered: why the aversion to
> conditionals in Makefiles, when that would be simpler (no new symbol)?
While conditionals in Makefiles do exist, they are far less common, and
can be confusing. They're also harder to grep for.
E.g. "git grep via-pmu-event.o" after your alternative patch would
give:
obj-$(CONFIG_ADB_PMU) += via-pmu-event.o
but would miss the important surrounding part:
ifeq ($(CONFIG_INPUT), y)
obj-$(CONFIG_ADB_PMU) += via-pmu-event.o
endif
Keeping configuration logic in a single place (the Kconfig file)
avoids that. The extra symbol is invisible, so it doesn't hurt much.
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] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
On Mon, 21 Mar 2022, Christophe Leroy wrote:
> Le 19/03/2022 à 08:20, Finn Thain a écrit :
> > drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show'
> > defined but not used [-Wunused-function]
> > static int pmu_battery_proc_show(struct seq_file *m, void *v)
> > ^
> > drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show'
> > defined but not used [-Wunused-function]
> > static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> > ^~
> > drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined
> > but not used [-Wunused-function]
> > static int pmu_info_proc_show(struct seq_file *m, void *v)
> > ^~
> >
> > Rearrange some code and add some #ifdefs to avoid unused code warnings
> > when CONFIG_PROC_FS is disabled.
>
> Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
>
You'd get a warning about the prototypes ("declared static but never
defined"). Rather than add an ifdef around the prototypes as well, I just
reordered things a little.
Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions
Le 21/03/2022 à 08:56, Christophe Leroy a écrit : Le 21/03/2022 à 03:27, Michael Ellerman a écrit : Christophe Leroy writes: Le 18/03/2022 à 13:26, Peter Zijlstra a écrit : On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote: This patch adds powerpc specific functions required for 'objtool mcount' to work, and enables mcount for ppc. I would love to see more objtool enablement for Power :-) I have not received this series and I can't see it on powerpc patchwork either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/) Did you send it to linuxppc-dev list ? If not can you resend it there ? It is there, might have been delayed? http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129 There are some CI failures. On PPC32 I get : [ 0.00] ftrace: No functions to be traced? Without this series I get: [ 0.00] ftrace: allocating 22508 entries in 17 pages [ 0.00] ftrace: allocated 17 pages with 2 groups ppc64_defconfig on QEMU: [0.00][T0] ftrace: No functions to be traced? Without your series: [0.00][T0] ftrace: allocating 38750 entries in 15 pages [0.00][T0] ftrace: allocated 15 pages with 4 groups [0.00][T0] trace event string verifier disabled
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
On Mon, 21 Mar 2022, Christophe Leroy wrote: > > > Le 21/03/2022 à 05:30, Finn Thain a écrit : > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event': > > via-pmu-event.c:(.text+0x44): undefined reference to `input_event' > > via-pmu-event.c:(.text+0x68): undefined reference to `input_event' > > via-pmu-event.c:(.text+0x94): undefined reference to `input_event' > > via-pmu-event.c:(.text+0xb8): undefined reference to `input_event' > > drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init': > > via-pmu-event.c:(.init.text+0x20): undefined reference to > > `input_allocate_device' > > via-pmu-event.c:(.init.text+0xc4): undefined reference to > > `input_register_device' > > via-pmu-event.c:(.init.text+0xd4): undefined reference to > > `input_free_device' > > make[1]: *** [Makefile:1155: vmlinux] Error 1 > > make: *** [Makefile:350: __build_one_by_one] Error 2 > > > > Don't call into the input subsystem unless CONFIG_INPUT is built-in. > > > > Reported-by: kernel test robot > > Cc: Michael Ellerman > > Cc: Geert Uytterhoeven > > Cc: Randy Dunlap > > Signed-off-by: Finn Thain > > --- > > This is equivalent to the patch I sent a couple of days ago. This one > > is slightly longer and adds a new symbol so that Kconfig logic can been > > used instead of Makefile logic in case reviewers prefer that. > > --- > > drivers/macintosh/Kconfig | 5 + > > drivers/macintosh/Makefile | 3 ++- > > drivers/macintosh/via-pmu.c | 2 ++ > > 3 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig > > index 5cdc361da37c..b9102f051bbb 100644 > > --- a/drivers/macintosh/Kconfig > > +++ b/drivers/macintosh/Kconfig > > @@ -67,6 +67,11 @@ config ADB_PMU > > this device; you should do so if your machine is one of those > > mentioned above. > > > > +config ADB_PMU_EVENT > > + bool > > + depends on ADB_PMU && INPUT=y > > + default y > > Could be reduced to > > config ADB_PMU_EVENT > def_bool y if ADB_PMU && INPUT=y > That's great but my question remains unanswered: why the aversion to conditionals in Makefiles, when that would be simpler (no new symbol)?
Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand
Le 21/03/2022 à 09:19, Naveen N. Rao a écrit : > Christophe Leroy wrote: >> >> >> Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit : >>> This patch adds 'mcount' as a subcommand to objtool, and enables >>> the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL >>> is selected. Additionally, architectures can select HAVE_NOP_MCOUNT >>> if they choose to nop out mcount call sites. If that config option is >>> selected, then --mnop is passed as an option to 'objtool mcount' >>> >>> Signed-off-by: Sathvika Vasireddy >>> --- >>> Makefile | 6 ++ >>> arch/x86/Kconfig | 3 +- >>> scripts/Makefile.build | 12 +++ >>> tools/objtool/Build | 2 + >>> tools/objtool/Makefile | 4 +- >>> tools/objtool/builtin-mcount.c | 74 + >>> tools/objtool/include/objtool/builtin.h | 4 +- >>> tools/objtool/include/objtool/objtool.h | 1 + >>> tools/objtool/mcount.c | 138 >>> tools/objtool/objtool.c | 1 + >>> tools/objtool/weak.c | 5 + >>> 11 files changed, 247 insertions(+), 3 deletions(-) >>> create mode 100644 tools/objtool/builtin-mcount.c >>> create mode 100644 tools/objtool/mcount.c >>> >>> diff --git a/Makefile b/Makefile >>> index 55a30ca69350..316f7d08b30a 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC >>> endif >>> endif >>> ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL >>> + ifdef CONFIG_HAVE_NOP_MCOUNT >>> CC_FLAGS_USING += -DCC_USING_NOP_MCOUNT >>> + endif >>> endif >>> ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT >>> ifdef CONFIG_HAVE_C_RECORDMCOUNT >>> @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION >>> prepare: tools/objtool >>> endif >>> +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL >>> +prepare: tools/objtool >>> +endif >> >> I don't think that will work for powerpc. >> >> See arch/powerpc/Makefile >> >> powerpc builds the VDSO under prepare: , and powerpc has >> CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that >> seem to use objtool, allthough that looks odd to me. Must be something >> to fix somewhere. >> >> powerpc64-linux-gcc >> -Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc >> -I./arch/powerpc/include -I./arch/powerpc/include/generated >> -I./include -I./arch/powerpc/include/uapi >> -I./arch/powerpc/include/generated/uapi -I./include/uapi >> -I./include/generated/uapi -include ./include/linux/compiler-version.h >> -include ./include/linux/kconfig.h -include >> ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc >> -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef >> -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing >> -fno-common -fshort-wchar -fno-PIE >> -Werror=implicit-function-declaration -Werror=implicit-int >> -Werror=return-type -Wno-format-security -std=gnu89 -mbig-endian -m64 >> -msoft-float -pipe -mtraceback=no -mabi=elfv1 -mcall-aixdesc >> -mcmodel=medium -mno-pointers-to-nested-functions -mtune=power7 >> -mcpu=power5 -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables >> -mno-string -Wa,-maltivec -Wa,-mpower4 -Wa,-many -mabi=elfv1 >> -mcall-aixdesc -mbig-endian -mstack-protector-guard=tls >> -mstack-protector-guard-reg=r13 -fno-delete-null-pointer-checks >> -Wno-frame-address -Wno-format-truncation -Wno-format-overflow >> -Wno-address-of-packed-member -O2 -fno-allow-store-data-races >> -Wframe-larger-than=2048 -fstack-protector-strong >> -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable >> -Wno-unused-const-variable -fno-stack-clash-protection >> -Wdeclaration-after-statement -Wvla -Wno-pointer-sign >> -Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds >> -Wno-array-bounds -Wno-stringop-overflow -Wno-restrict >> -Wno-maybe-uninitialized -Wno-alloc-size-larger-than >> -fno-strict-overflow -fno-stack-check -fconserve-stack >> -Werror=date-time -Werror=incompatible-pointer-types >> -Werror=designated-init -Wno-packed-not-aligned >> -mstack-protector-guard-offset=3200 -shared -fno-common -fno-builtin >> -nostdlib -Wl,--hash-style=both -include >> /home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c >> -fno-stack-protector -DDISABLE_BRANCH_PROFILING -ffreestanding >> -fasynchronous-unwind-tables -ffixed-r30 >> -DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"' >> -DKBUILD_BASENAME='"vgettimeofday_64"' >> -DKBUILD_MODNAME='"vgettimeofday_64"' >> -D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o >> arch/powerpc/kernel/vdso/vgettimeofday-64.o >> arch/powerpc/kernel/vdso/vgettimeofday.c ; ./tools/objtool/objtool >> mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o > > We don't enable ftrace for vdso, so I suspect objtool run above will be > a no-op. This needs to be confirmed, of course. > I just checked
Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand
Christophe Leroy wrote: Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit : This patch adds 'mcount' as a subcommand to objtool, and enables the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL is selected. Additionally, architectures can select HAVE_NOP_MCOUNT if they choose to nop out mcount call sites. If that config option is selected, then --mnop is passed as an option to 'objtool mcount' Signed-off-by: Sathvika Vasireddy --- Makefile| 6 ++ arch/x86/Kconfig| 3 +- scripts/Makefile.build | 12 +++ tools/objtool/Build | 2 + tools/objtool/Makefile | 4 +- tools/objtool/builtin-mcount.c | 74 + tools/objtool/include/objtool/builtin.h | 4 +- tools/objtool/include/objtool/objtool.h | 1 + tools/objtool/mcount.c | 138 tools/objtool/objtool.c | 1 + tools/objtool/weak.c| 5 + 11 files changed, 247 insertions(+), 3 deletions(-) create mode 100644 tools/objtool/builtin-mcount.c create mode 100644 tools/objtool/mcount.c diff --git a/Makefile b/Makefile index 55a30ca69350..316f7d08b30a 100644 --- a/Makefile +++ b/Makefile @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC endif endif ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL + ifdef CONFIG_HAVE_NOP_MCOUNT CC_FLAGS_USING += -DCC_USING_NOP_MCOUNT + endif endif ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT ifdef CONFIG_HAVE_C_RECORDMCOUNT @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION prepare: tools/objtool endif +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL +prepare: tools/objtool +endif I don't think that will work for powerpc. See arch/powerpc/Makefile powerpc builds the VDSO under prepare: , and powerpc has CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that seem to use objtool, allthough that looks odd to me. Must be something to fix somewhere. powerpc64-linux-gcc -Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89 -mbig-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv1 -mcall-aixdesc -mcmodel=medium -mno-pointers-to-nested-functions -mtune=power7 -mcpu=power5 -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables -mno-string -Wa,-maltivec -Wa,-mpower4 -Wa,-many -mabi=elfv1 -mcall-aixdesc -mbig-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-address-of-packed-member -O2 -fno-allow-store-data-races -Wframe-larger-than=2048 -fstack-protector-strong -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -fno-stack-clash-protection -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds -Wno-array-bounds -Wno-stringop-overflow -Wno-restrict -Wno-maybe-uninitialized -Wno-alloc-size-larger-than -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wno-packed-not-aligned -mstack-protector-guard-offset=3200 -shared -fno-common -fno-builtin -nostdlib -Wl,--hash-style=both -include /home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c -fno-stack-protector -DDISABLE_BRANCH_PROFILING -ffreestanding -fasynchronous-unwind-tables -ffixed-r30 -DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"' -DKBUILD_BASENAME='"vgettimeofday_64"' -DKBUILD_MODNAME='"vgettimeofday_64"' -D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o arch/powerpc/kernel/vdso/vgettimeofday-64.o arch/powerpc/kernel/vdso/vgettimeofday.c ; ./tools/objtool/objtool mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o We don't enable ftrace for vdso, so I suspect objtool run above will be a no-op. This needs to be confirmed, of course. - Naveen
[RFC v3 PATCH 5/5] powerpc/crash hp: add crash hotplug support for kexec_load
The kernel changes needed to add support for crash hotplug support for
kexec_load system calls are similar to kexec_file_load (which has already
been implemented in earlier patches). Since kexec segment array is
prepared by kexec tool in the userspace, the kernel does aware of which
index FDT segment is present.
The only change was done to enabled crash hotplug support for kexec_load is
updated the crash hotplug handler to identify the FDT segment from kexec
segment array.
Signed-off-by: Sourabh Jain
---
arch/powerpc/kexec/core_64.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index a470fe6904e3..2c248dfb169b 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -480,7 +480,9 @@ int update_cpus_node(void *fdt)
void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
unsigned long a, unsigned long b)
{
- void *fdt;
+ void *fdt, *ptr;
+ unsigned int n;
+ unsigned long mem, memsz;
/* No action needed for CPU hot-unplug */
if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
@@ -492,6 +494,23 @@ void arch_crash_hotplug_handler(struct kimage *image,
unsigned int hp_action,
return;
}
+ /* Sine kexec segments for kexec_load system call is prepred by
+* kexec tool in userspace we need loop through all the segments
+* to find out segment index corresponds FDT segment. In case of
+* kexec_file_load it is discovered during the load itself.
+*/
+ if (!image->arch.fdt_index_valid) {
+ for (n = 0; n < image->nr_segments; n++) {
+ mem = image->segment[n].mem;
+ memsz = image->segment[n].memsz;
+ ptr = __va(mem);
+ if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
+ image->arch.fdt_index = n;
+ image->arch.fdt_index_valid = true;
+ }
+ }
+ }
+
/* Must have valid FDT index */
if (!image->arch.fdt_index_valid) {
pr_err("crash hp: unable to locate FDT segment");
--
2.35.1
[RFC v3 PATCH 3/5] powrepc/crash hp: update kimage struct
Two new members fdt_index and fdt_index_valid are added in kimage_arch
struct to track the FDT kexec segment. These new members of kimage_arch
struct will help the crash hotplug handler to easily access the FDT
segment from the kexec segment array. Otherwise, we have to loop through
all kexec segments to find the FDT segments.
Signed-off-by: Sourabh Jain
---
arch/powerpc/include/asm/kexec.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index e1288826e22e..19c2cab6a880 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -104,6 +104,8 @@ extern const struct kexec_file_ops kexec_elf64_ops;
struct kimage_arch {
struct crash_mem *exclude_ranges;
+ int fdt_index;
+ bool fdt_index_valid;
unsigned long backup_start;
void *backup_buf;
void *fdt;
--
2.35.1
[RFC v3 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load
Two major changes are done to enable the crash CPU hotplug handler.
Firstly, updated the kexec_load path to prepare kimage for hotplug
changes and secondly, implemented the crash hotplug handler itself.
On the kexec load path, memsz allocation of crash FDT segment is
updated to ensure that it has sufficient buffer space to accommodate
future hot add CPUs. Initialized the kimage members to track the FDT
segment.
The crash hotplug handler updates the cpus node of crash FDT. While
we update crash FDT the kexec_crash_image is marked invalid and restored
after FDT update to avoid race.
Since memory crash hotplug support is not there yet the crash hotplug
handler simply warn the user and return.
Signed-off-by: Sourabh Jain
---
arch/powerpc/kexec/core_64.c | 46
arch/powerpc/kexec/elf_64.c | 40 +++
2 files changed, 86 insertions(+)
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 249d2632526d..a470fe6904e3 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -466,6 +466,52 @@ int update_cpus_node(void *fdt)
return ret;
}
+#ifdef CONFIG_CRASH_HOTPLUG
+/**
+ * arch_crash_hotplug_handler() - Handle hotplug FDT changes
+ * @image: the active struct kimage
+ * @hp_action: the hot un/plug action being handled
+ * @a: first parameter dependent upon hp_action
+ * @b: first parameter dependent upon hp_action
+ *
+ * To accurately reflect CPU hot un/plug changes, the FDT
+ * must be updated with the new list of CPUs and memories.
+ */
+void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action,
+ unsigned long a, unsigned long b)
+{
+ void *fdt;
+
+ /* No action needed for CPU hot-unplug */
+ if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
+ return;
+
+ /* crash update on memory hotplug is not support yet */
+ if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action ==
KEXEC_CRASH_HP_ADD_MEMORY) {
+ pr_err("crash hp: crash update is not supported with memory
hotplug\n");
+ return;
+ }
+
+ /* Must have valid FDT index */
+ if (!image->arch.fdt_index_valid) {
+ pr_err("crash hp: unable to locate FDT segment");
+ return;
+ }
+
+ fdt = __va((void *)image->segment[image->arch.fdt_index].mem);
+
+ /* Temporarily invalidate the crash image while it is replaced */
+ xchg(_crash_image, NULL);
+
+ /* update FDT to refelect changes to CPU resrouces */
+ if (update_cpus_node(fdt))
+ pr_err("crash hp: failed to update crash FDT");
+
+ /* The crash image is now valid once again */
+ xchg(_crash_image, image);
+}
+#endif /* CONFIG_CRASH_HOTPLUG */
+
#ifdef CONFIG_PPC_64S_HASH_MMU
/* Values we need to export to the second kernel via the device tree. */
static unsigned long htab_base;
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..2ffe6d69e186 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,6 +24,33 @@
#include
#include
+
+#ifdef CONFIG_CRASH_HOTPLUG
+#define MAX_CORE 256
+#define PER_CORE_NODE_SIZE 1500
+
+/**
+ * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
+ * @fdt: pointer to crash kernel FDT
+ *
+ * Calculate the buffer space needed to add more CPU nodes in FDT after
+ * capture kenrel load due to hot add events.
+ *
+ * Some assumption are made to calculate the additional buffer size needed
+ * to accommodate future hot add CPUs to the crash FDT. The maximum core count
+ * in the system would not go beyond MAX_CORE and memory needed to store per
core
+ * date in FDT is PER_CORE_NODE_SIZE.
+ *
+ * Certainly MAX_CORE count can be replaced with possible core count and
+ * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
+ * core size value on Power9 LPAR.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt)
+{
+ return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
+}
+#endif
+
static void *elf64_load(struct kimage *image, char *kernel_buf,
unsigned long kernel_len, char *initrd,
unsigned long initrd_len, char *cmdline,
@@ -123,6 +150,19 @@ static void *elf64_load(struct kimage *image, char
*kernel_buf,
kbuf.buf_align = PAGE_SIZE;
kbuf.top_down = true;
kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+
+#ifdef CONFIG_CRASH_HOTPLUG
+ if (image->type == KEXEC_TYPE_CRASH) {
+ kbuf.memsz = get_crash_fdt_mem_sz(fdt);
+ fdt_set_totalsize(fdt, kbuf.memsz);
+ image->arch.fdt_index = image->nr_segments;
+ image->arch.fdt_index_valid = true;
+ } else
+#endif
+ {
+ kbuf.memsz = fdt_totalsize(fdt);
+ }
+
ret = kexec_add_buffer();
if (ret)
goto out_free_fdt;
[RFC v3 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
The option CRASH_HOTPLUG enables, in kernel update to kexec segments on hotplug events. All the updates needed on the capture kernel load path in the kernel for both kexec_load and kexec_file_load system will be kept under this config. Signed-off-by: Sourabh Jain --- arch/powerpc/Kconfig | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index b779603978e1..b816339ef8c7 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -623,6 +623,17 @@ config FA_DUMP If unsure, say "y". Only special kernels like petitboot may need to say "N" here. +config CRASH_HOTPLUG + bool "kernel updates of crash kexec segments" + depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE + help + An efficient way to keep the capture kernel up-to-date with CPU + hotplug events. On hotplug event (CPU/memory) the kexec segments + of capture kernel becomes stale and need to be updated with latest + CPU and memory regions. In this method the kernel performs minimal + update to only relevant kexec segments on CPU hotplug event, instead + of triggering full capture reload from userspace using udev rule. + config PRESERVE_FA_DUMP bool "Preserve Firmware-assisted dump" depends on PPC64 && PPC_POWERNV && !FA_DUMP -- 2.35.1
[RFC v3 PATCH 1/5] powerpc/kexec: make update_cpus_node non-static
Make the update_cpus_node function non-static and export it for
usage in other kexec components.
The update_cpus_node definition is moved to core_64.c so that it
can be used with both kexec_load and kexec_file_load system calls.
Signed-off-by: Sourabh Jain
---
arch/powerpc/include/asm/kexec.h | 1 +
arch/powerpc/kexec/core_64.c | 88 +++
arch/powerpc/kexec/file_load_64.c | 87 --
3 files changed, 89 insertions(+), 87 deletions(-)
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 8ebdd23d987c..e1288826e22e 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -127,6 +127,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage
*image);
int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
unsigned long initrd_load_addr,
unsigned long initrd_len, const char *cmdline);
+int update_cpus_node(void *fdt);
#endif /* CONFIG_PPC64 */
#endif /* CONFIG_KEXEC_FILE */
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 635b5fc30b53..249d2632526d 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -378,6 +379,93 @@ void default_machine_kexec(struct kimage *image)
/* NOTREACHED */
}
+/**
+ * add_node_props - Reads node properties from device node structure and add
+ * them to fdt.
+ * @fdt:Flattened device tree of the kernel
+ * @node_offset:offset of the node to add a property at
+ * @dn: device node pointer
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static int add_node_props(void *fdt, int node_offset, const struct device_node
*dn)
+{
+ int ret = 0;
+ struct property *pp;
+
+ if (!dn)
+ return -EINVAL;
+
+ for_each_property_of_node(dn, pp) {
+ ret = fdt_setprop(fdt, node_offset, pp->name, pp->value,
pp->length);
+ if (ret < 0) {
+ pr_err("Unable to add %s property: %s\n", pp->name,
fdt_strerror(ret));
+ return ret;
+ }
+ }
+ return ret;
+}
+
+/**
+ * update_cpus_node - Update cpus node of flattened device tree using of_root
+ *device node.
+ * @fdt: Flattened device tree of the kernel.
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+int update_cpus_node(void *fdt)
+{
+ struct device_node *cpus_node, *dn;
+ int cpus_offset, cpus_subnode_offset, ret = 0;
+
+ cpus_offset = fdt_path_offset(fdt, "/cpus");
+ if (cpus_offset < 0 && cpus_offset != -FDT_ERR_NOTFOUND) {
+ pr_err("Malformed device tree: error reading /cpus node: %s\n",
+ fdt_strerror(cpus_offset));
+ return cpus_offset;
+ }
+
+ if (cpus_offset > 0) {
+ ret = fdt_del_node(fdt, cpus_offset);
+ if (ret < 0) {
+ pr_err("Error deleting /cpus node: %s\n",
fdt_strerror(ret));
+ return -EINVAL;
+ }
+ }
+
+ /* Add cpus node to fdt */
+ cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"), "cpus");
+ if (cpus_offset < 0) {
+ pr_err("Error creating /cpus node: %s\n",
fdt_strerror(cpus_offset));
+ return -EINVAL;
+ }
+
+ /* Add cpus node properties */
+ cpus_node = of_find_node_by_path("/cpus");
+ ret = add_node_props(fdt, cpus_offset, cpus_node);
+ of_node_put(cpus_node);
+ if (ret < 0)
+ return ret;
+
+ /* Loop through all subnodes of cpus and add them to fdt */
+ for_each_node_by_type(dn, "cpu") {
+ cpus_subnode_offset = fdt_add_subnode(fdt, cpus_offset,
dn->full_name);
+ if (cpus_subnode_offset < 0) {
+ pr_err("Unable to add %s subnode: %s\n", dn->full_name,
+ fdt_strerror(cpus_subnode_offset));
+ ret = cpus_subnode_offset;
+ goto out;
+ }
+
+ ret = add_node_props(fdt, cpus_subnode_offset, dn);
+ if (ret < 0)
+ goto out;
+ }
+out:
+ of_node_put(dn);
+ return ret;
+}
+
#ifdef CONFIG_PPC_64S_HASH_MMU
/* Values we need to export to the second kernel via the device tree. */
static unsigned long htab_base;
diff --git a/arch/powerpc/kexec/file_load_64.c
b/arch/powerpc/kexec/file_load_64.c
index 07da6bf1cf24..57f991b0a9da 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -951,93 +951,6 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage
*image)
return (unsigned int)(usm_entries * sizeof(u64));
}
-/**
- * add_node_props - Reads node
[RFC v3 PATCH 0/5] In kernel handling of CPU hotplug events for crash kernel
This patch series implements the crash hotplug handler on PowerPC introduced
by https://lkml.org/lkml/2022/3/3/674 patch series.
The Problem:
Post hotplug/DLPAR events the capture kernel holds stale information about the
system. Dump collection with stale capture kernel might end up in dump capture
failure or an inaccurate dump collection.
Existing solution:
==
The existing solution to keep the capture kernel up-to-date is observe the
hotplug event via udev rule and trigger a full capture kernel reload post
hotplug event.
Shortcomings:
- Leaves a window where kernel crash might not lead to successful dump
collection.
- Reloading all kexec components for each hotplug is inefficient. Since only
one or two kexec components need to be updated due to hotplug event reloading
all kexec component is redundant.
- udev rules are prone to races if hotplug events are frequent.
More about issues with an existing solution is posted here:
- https://lkml.org/lkml/2020/12/14/532
- https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/240254.html
Proposed Solution:
==
Instead of reloading all kexec segments on hotplug event, this patch series
focuses on updating only the relevant kexec segment. Once the kexec
segments are loaded in the kernel reserved area then an arch-specific hotplug
handler
will update the relevant kexec segment based on hotplug event type.
As mentioned above this patch series implemented a PowerPC crash hotplug
handler for the CPU. The crash hotplug handler memory is in our TODO list.
A couple of minor changes are required to realize the benefit of the patch
series:
- disalble the udev rule:
comment out the below line in kdump udev rule file:
RHEL: /usr/lib/udev/rules.d/98-kexec.rules
# SUBSYSTEM=="cpu", ACTION=="online", GOTO="kdump_reload_cpu"
- kexec tool needs to be updated with patch for kexec_load system call
to work (not needed if -s option is used during kexec panic load):
---
diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c
b/kexec/arch/ppc64/kexec-elf-ppc64.c
index 695b8b0..1dc6490 100644
--- a/kexec/arch/ppc64/kexec-elf-ppc64.c
+++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
@@ -45,6 +45,29 @@ uint64_t initrd_base, initrd_size;
unsigned char reuse_initrd = 0;
const char *ramdisk;
+#define MAX_CORE 256
+#define PER_CORE_NODE_SIZE 1500
+
+/**
+ * get_crash_fdt_mem_sz() - calcuate mem size for crash kernel FDT
+ * @fdt: pointer to crash kernel FDT
+ *
+ * Calculate the buffer space needed to add more CPU nodes in FDT after
+ * capture kenrel load due to hot add events.
+ *
+ * Some assumption are made to calculate the additional buffer size needed
+ * to accommodate future hot add CPUs to the crash FDT. The maximum core count
+ * in the system would not go beyond MAX_CORE and memory needed to store per
core
+ * date in FDT is PER_CORE_NODE_SIZE.
+ *
+ * Certainly MAX_CORE count can be replaced with possible core count and
+ * PER_CORE_NODE_SIZE to some standard value instead of randomly observed
+ * core size value on Power9 LPAR.
+ */
+static unsigned int get_crash_fdt_mem_sz(void *fdt) {
+ return fdt_totalsize(fdt) + (PER_CORE_NODE_SIZE * MAX_CORE);
+}
+
int elf_ppc64_probe(const char *buf, off_t len)
{
struct mem_ehdr ehdr;
@@ -179,6 +202,7 @@ int elf_ppc64_load(int argc, char **argv, const char *buf,
off_t len,
uint64_t max_addr, hole_addr;
char *seg_buf = NULL;
off_t seg_size = 0;
+ unsigned int mem_sz = 0;
struct mem_phdr *phdr;
size_t size;
#ifdef NEED_RESERVE_DTB
@@ -329,7 +353,13 @@ int elf_ppc64_load(int argc, char **argv, const char *buf,
off_t len,
if (result < 0)
return result;
- my_dt_offset = add_buffer(info, seg_buf, seg_size, seg_size,
+ if (info->kexec_flags & KEXEC_ON_CRASH) {
+ mem_sz = get_crash_fdt_mem_sz((void *)seg_buf);
+ fdt_set_totalsize(seg_buf, mem_sz);
+ info->fdt_index = info->nr_segments;
+ }
+
+ my_dt_offset = add_buffer(info, seg_buf, seg_size, mem_sz,
0, 0, max_addr, -1);
#ifdef NEED_RESERVE_DTB
diff --git a/kexec/kexec.c b/kexec/kexec.c
index f63b36b..846b1a8 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -672,6 +672,9 @@ static void update_purgatory(struct kexec_info *info)
if (info->segment[i].mem == (void *)info->rhdr.rel_addr) {
continue;
}
+ if (info->fdt_index == i)
+ continue;
+
sha256_update(, info->segment[i].buf,
info->segment[i].bufsz);
nullsz = info->segment[i].memsz - info->segment[i].bufsz;
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 595dd68..0906a1b 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -169,6 +169,7 @@ struct kexec_info {
int command_line_len;
Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions
Le 21/03/2022 à 03:27, Michael Ellerman a écrit : > Christophe Leroy writes: >> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit : >>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote: This patch adds powerpc specific functions required for 'objtool mcount' to work, and enables mcount for ppc. >>> >>> I would love to see more objtool enablement for Power :-) >> >> I have not received this series and I can't see it on powerpc patchwork >> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/) >> >> Did you send it to linuxppc-dev list ? If not can you resend it there ? > > It is there, might have been delayed? > > http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129 > > There are some CI failures. > On PPC32 I get : [0.00] ftrace: No functions to be traced? Without this series I get: [0.00] ftrace: allocating 22508 entries in 17 pages [0.00] ftrace: allocated 17 pages with 2 groups Christophe
Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions
Le 21/03/2022 à 03:27, Michael Ellerman a écrit : > Christophe Leroy writes: >> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit : >>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote: This patch adds powerpc specific functions required for 'objtool mcount' to work, and enables mcount for ppc. >>> >>> I would love to see more objtool enablement for Power :-) >> >> I have not received this series and I can't see it on powerpc patchwork >> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/) >> >> Did you send it to linuxppc-dev list ? If not can you resend it there ? > > It is there, might have been delayed? > > http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129 > > There are some CI failures. > On PPC64 we randomly get: /bin/sh: 1: ./tools/objtool/objtool: not found make[2]: *** [arch/powerpc/kernel/vdso/vgettimeofday-64.o] Error 127 /linux/arch/powerpc/kernel/vdso/Makefile:77: recipe for target 'arch/powerpc/kernel/vdso/vgettimeofday-64.o' failed make[2]: *** Deleting file 'arch/powerpc/kernel/vdso/vgettimeofday-64.o' make[1]: *** [vdso_prepare] Error 2 make[1]: *** Waiting for unfinished jobs /linux/arch/powerpc/Makefile:423: recipe for target 'vdso_prepare' failed make: *** [__sub-make] Error 2 Makefile:219: recipe for target '__sub-make' failed This is linkely because prepare: target depends on vdso_prepare and prepare: target depends on objtool, but vdso_prepare: doesn't depend on objtool. I'm not sure it is correct to run objtool mcount on VDSO as it is kind of useless. Only VDSO64 seems to call objtool, not VDSO32. Christophe
Re: [PATCH v2] ASoC: fsl-asoc-card: Fix jack_event() always return 0
Le 21/03/2022 à 07:57, Meng Tang a écrit :
> Today, hp_jack_event and mic_jack_event always return 0. However,
> snd_soc_dapm_disable_pin and snd_soc_dapm_enable_pin may return a
> non-zero value, this will cause the user who calling hp_jack_event
> and mic_jack_event don't know whether the operation was really
> successfully.
>
> Signed-off-by: Meng Tang
Reviewed-by: Christophe Leroy
> ---
> sound/soc/fsl/fsl-asoc-card.c | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index 370bc790c6ba..d9a0d4768c4d 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -462,11 +462,9 @@ static int hp_jack_event(struct notifier_block *nb,
> unsigned long event,
>
> if (event & SND_JACK_HEADPHONE)
> /* Disable speaker if headphone is plugged in */
> - snd_soc_dapm_disable_pin(dapm, "Ext Spk");
> + return snd_soc_dapm_disable_pin(dapm, "Ext Spk");
> else
> - snd_soc_dapm_enable_pin(dapm, "Ext Spk");
> -
> - return 0;
> + return snd_soc_dapm_enable_pin(dapm, "Ext Spk");
> }
>
> static struct notifier_block hp_jack_nb = {
> @@ -481,11 +479,9 @@ static int mic_jack_event(struct notifier_block *nb,
> unsigned long event,
>
> if (event & SND_JACK_MICROPHONE)
> /* Disable dmic if microphone is plugged in */
> - snd_soc_dapm_disable_pin(dapm, "DMIC");
> + return snd_soc_dapm_disable_pin(dapm, "DMIC");
> else
> - snd_soc_dapm_enable_pin(dapm, "DMIC");
> -
> - return 0;
> + return snd_soc_dapm_enable_pin(dapm, "DMIC");
> }
>
> static struct notifier_block mic_jack_nb = {
[PATCH kernel] powerpc/boot: Stop using RELACOUNT
So far the RELACOUNT tag from the ELF header was containing the exact number of R_PPC_RELATIVE/R_PPC64_RELATIVE relocations. However the LLVM's recent change [1] make it equal-or-less than the actual number which makes it useless. This replaces RELACOUNT in zImage loader with a pair of RELASZ and RELAENT. The vmlinux relocation code is fixed in [2]. To make it more future proof, this walks through the entire .rela.dyn section instead of assuming that the section is sorter by a relocation type. Unlike [1], this does not add unaligned UADDR/UADDR64 relocations as in hardly possible to see those in arch-specific zImage. [1] https://github.com/llvm/llvm-project/commit/da0e5b885b25cf4 [2] https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next=d799769188529a Signed-off-by: Alexey Kardashevskiy --- arch/powerpc/boot/crt0.S | 43 +--- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S index feadee18e271..6ea3417da3b7 100644 --- a/arch/powerpc/boot/crt0.S +++ b/arch/powerpc/boot/crt0.S @@ -8,7 +8,8 @@ #include "ppc_asm.h" RELA = 7 -RELACOUNT = 0x6ff9 +RELASZ = 8 +RELAENT = 9 .data /* A procedure descriptor used when booting this as a COFF file. @@ -75,34 +76,38 @@ p_base: mflrr10 /* r10 now points to runtime addr of p_base */ bne 11f lwz r9,4(r12) /* get RELA pointer in r9 */ b 12f -11:addis r8,r8,(-RELACOUNT)@ha - cmpwi r8,RELACOUNT@l +11:cmpwi r8,RELASZ + bne 111f + lwz r0,4(r12) /* get RELASZ value in r0 */ + b 12f +111: cmpwi r8,RELAENT bne 12f - lwz r0,4(r12) /* get RELACOUNT value in r0 */ + lwz r14,4(r12) /* get RELAENT value in r14 */ 12:addir12,r12,8 b 9b /* The relocation section contains a list of relocations. * We now do the R_PPC_RELATIVE ones, which point to words -* which need to be initialized with addend + offset. -* The R_PPC_RELATIVE ones come first and there are RELACOUNT -* of them. */ +* which need to be initialized with addend + offset */ 10:/* skip relocation if we don't have both */ cmpwi r0,0 beq 3f cmpwi r9,0 beq 3f + cmpwi r14,0 + beq 3f add r9,r9,r11 /* Relocate RELA pointer */ + divdr0,r0,r14 /* RELASZ / RELAENT */ mtctr r0 2: lbz r0,4+3(r9) /* ELF32_R_INFO(reloc->r_info) */ cmpwi r0,22 /* R_PPC_RELATIVE */ - bne 3f + bne 22f lwz r12,0(r9) /* reloc->r_offset */ lwz r0,8(r9)/* reloc->r_addend */ add r0,r0,r11 stwxr0,r11,r12 - addir9,r9,12 +22:add r9,r9,r14 bdnz2b /* Do a cache flush for our text, in case the loader didn't */ @@ -160,32 +165,38 @@ p_base: mflrr10 /* r10 now points to runtime addr of p_base */ bne 10f ld r13,8(r11) /* get RELA pointer in r13 */ b 11f -10:addis r12,r12,(-RELACOUNT)@ha - cmpdi r12,RELACOUNT@l - bne 11f - ld r8,8(r11) /* get RELACOUNT value in r8 */ +10:cmpwi r12,RELASZ + bne 101f + lwz r8,8(r11) /* get RELASZ pointer in r8 */ + b 11f +101: cmpwi r12,RELAENT + bne 11f + lwz r14,8(r11) /* get RELAENT pointer in r14 */ 11:addir11,r11,16 b 9b 12: - cmpdi r13,0/* check we have both RELA and RELACOUNT */ + cmpdi r13,0/* check we have both RELA, RELASZ, RELAENT*/ cmpdi cr1,r8,0 beq 3f beq cr1,3f + cmpdi r14,0 + beq 3f /* Calcuate the runtime offset. */ subfr13,r13,r9 /* Run through the list of relocations and process the * R_PPC64_RELATIVE ones. */ + divdr8,r8,r14 /* RELASZ / RELAENT */ mtctr r8 13:ld r0,8(r9)/* ELF64_R_TYPE(reloc->r_info) */ cmpdi r0,22 /* R_PPC64_RELATIVE */ - bne 3f + bne 14f ld r12,0(r9)/* reloc->r_offset */ ld r0,16(r9) /* reloc->r_addend */ add r0,r0,r13 stdxr0,r13,r12 - addir9,r9,24 +14:add r9,r9,r14 bdnz13b /* Do a cache flush for our text, in case the loader didn't */ -- 2.30.2
Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand
Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit : > This patch adds 'mcount' as a subcommand to objtool, and enables > the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL > is selected. Additionally, architectures can select HAVE_NOP_MCOUNT > if they choose to nop out mcount call sites. If that config option is > selected, then --mnop is passed as an option to 'objtool mcount' > > Signed-off-by: Sathvika Vasireddy > --- > Makefile| 6 ++ > arch/x86/Kconfig| 3 +- > scripts/Makefile.build | 12 +++ > tools/objtool/Build | 2 + > tools/objtool/Makefile | 4 +- > tools/objtool/builtin-mcount.c | 74 + > tools/objtool/include/objtool/builtin.h | 4 +- > tools/objtool/include/objtool/objtool.h | 1 + > tools/objtool/mcount.c | 138 > tools/objtool/objtool.c | 1 + > tools/objtool/weak.c| 5 + > 11 files changed, 247 insertions(+), 3 deletions(-) > create mode 100644 tools/objtool/builtin-mcount.c > create mode 100644 tools/objtool/mcount.c > > diff --git a/Makefile b/Makefile > index 55a30ca69350..316f7d08b30a 100644 > --- a/Makefile > +++ b/Makefile > @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC > endif > endif > ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL > + ifdef CONFIG_HAVE_NOP_MCOUNT > CC_FLAGS_USING+= -DCC_USING_NOP_MCOUNT > + endif > endif > ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT > ifdef CONFIG_HAVE_C_RECORDMCOUNT > @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION > prepare: tools/objtool > endif > > +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL > +prepare: tools/objtool > +endif I don't think that will work for powerpc. See arch/powerpc/Makefile powerpc builds the VDSO under prepare: , and powerpc has CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that seem to use objtool, allthough that looks odd to me. Must be something to fix somewhere. powerpc64-linux-gcc -Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89 -mbig-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv1 -mcall-aixdesc -mcmodel=medium -mno-pointers-to-nested-functions -mtune=power7 -mcpu=power5 -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables -mno-string -Wa,-maltivec -Wa,-mpower4 -Wa,-many -mabi=elfv1 -mcall-aixdesc -mbig-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-address-of-packed-member -O2 -fno-allow-store-data-races -Wframe-larger-than=2048 -fstack-protector-strong -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -fno-stack-clash-protection -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds -Wno-array-bounds -Wno-stringop-overflow -Wno-restrict -Wno-maybe-uninitialized -Wno-alloc-size-larger-than -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wno-packed-not-aligned -mstack-protector-guard-offset=3200 -shared -fno-common -fno-builtin -nostdlib -Wl,--hash-style=both -include /home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c -fno-stack-protector -DDISABLE_BRANCH_PROFILING -ffreestanding -fasynchronous-unwind-tables -ffixed-r30 -DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"' -DKBUILD_BASENAME='"vgettimeofday_64"' -DKBUILD_MODNAME='"vgettimeofday_64"' -D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o arch/powerpc/kernel/vdso/vgettimeofday-64.o arch/powerpc/kernel/vdso/vgettimeofday.c ; ./tools/objtool/objtool mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o > + > ifdef CONFIG_BPF > ifdef CONFIG_DEBUG_INFO_BTF > prepare: tools/bpf/resolve_btfids
[PATCH v2] ASoC: fsl-asoc-card: Fix jack_event() always return 0
Today, hp_jack_event and mic_jack_event always return 0. However,
snd_soc_dapm_disable_pin and snd_soc_dapm_enable_pin may return a
non-zero value, this will cause the user who calling hp_jack_event
and mic_jack_event don't know whether the operation was really
successfully.
Signed-off-by: Meng Tang
---
sound/soc/fsl/fsl-asoc-card.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 370bc790c6ba..d9a0d4768c4d 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -462,11 +462,9 @@ static int hp_jack_event(struct notifier_block *nb,
unsigned long event,
if (event & SND_JACK_HEADPHONE)
/* Disable speaker if headphone is plugged in */
- snd_soc_dapm_disable_pin(dapm, "Ext Spk");
+ return snd_soc_dapm_disable_pin(dapm, "Ext Spk");
else
- snd_soc_dapm_enable_pin(dapm, "Ext Spk");
-
- return 0;
+ return snd_soc_dapm_enable_pin(dapm, "Ext Spk");
}
static struct notifier_block hp_jack_nb = {
@@ -481,11 +479,9 @@ static int mic_jack_event(struct notifier_block *nb,
unsigned long event,
if (event & SND_JACK_MICROPHONE)
/* Disable dmic if microphone is plugged in */
- snd_soc_dapm_disable_pin(dapm, "DMIC");
+ return snd_soc_dapm_disable_pin(dapm, "DMIC");
else
- snd_soc_dapm_enable_pin(dapm, "DMIC");
-
- return 0;
+ return snd_soc_dapm_enable_pin(dapm, "DMIC");
}
static struct notifier_block mic_jack_nb = {
--
2.20.1
Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions
Le 21/03/2022 à 03:27, Michael Ellerman a écrit : > Christophe Leroy writes: >> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit : >>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote: This patch adds powerpc specific functions required for 'objtool mcount' to work, and enables mcount for ppc. >>> >>> I would love to see more objtool enablement for Power :-) >> >> I have not received this series and I can't see it on powerpc patchwork >> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/) >> >> Did you send it to linuxppc-dev list ? If not can you resend it there ? > > It is there, might have been delayed? Yes something must have gone wrong. Peter's and myself's responses are not at http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220318105140.43914-4...@linux.ibm.com/ while yours and Naveen's ones are there. Christophe
Re: [PATCH] ASoC: fsl-asoc-card: Fix jack_event() always return 0
Le 18/03/2022 à 10:35, Meng Tang a écrit :
> Today, hp_jack_event and mic_jack_event always return 0. However,
> snd_soc_dapm_disable_pin and snd_soc_dapm_enable_pin may return a
> non-zero value, this will cause the user who calling hp_jack_event
> and mic_jack_event don't know whether the operation was really
> successfully.
>
> This patch corrects the behavior by properly returning 1 when the
> value gets updated.
>
> Signed-off-by: Meng Tang
> ---
> sound/soc/fsl/fsl-asoc-card.c | 16 ++--
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index 370bc790c6ba..f2641c2cb047 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -457,16 +457,18 @@ static int fsl_asoc_card_audmux_init(struct device_node
> *np,
> static int hp_jack_event(struct notifier_block *nb, unsigned long event,
>void *data)
> {
> + int ret;
> +
You don't need that new local var. There are coccinelle scripts that
track those unnecessary intermediates so don't add a new one.
> struct snd_soc_jack *jack = (struct snd_soc_jack *)data;
> struct snd_soc_dapm_context *dapm = >card->dapm;
>
> if (event & SND_JACK_HEADPHONE)
> /* Disable speaker if headphone is plugged in */
> - snd_soc_dapm_disable_pin(dapm, "Ext Spk");
> + ret = snd_soc_dapm_disable_pin(dapm, "Ext Spk");
Should be
return snd_soc_dapm_disable_pin(dapm, "Ext Spk");
> else
> - snd_soc_dapm_enable_pin(dapm, "Ext Spk");
> + ret = snd_soc_dapm_enable_pin(dapm, "Ext Spk");
and
return snd_soc_dapm_enable_pin(dapm, "Ext Spk");
>
> - return 0;
> + return ret;
And completely drop that.
> }
>
> static struct notifier_block hp_jack_nb = {
> @@ -476,16 +478,18 @@ static struct notifier_block hp_jack_nb = {
> static int mic_jack_event(struct notifier_block *nb, unsigned long event,
> void *data)
> {
> + int ret;
> +
And do exactly the same here.
> struct snd_soc_jack *jack = (struct snd_soc_jack *)data;
> struct snd_soc_dapm_context *dapm = >card->dapm;
>
> if (event & SND_JACK_MICROPHONE)
> /* Disable dmic if microphone is plugged in */
> - snd_soc_dapm_disable_pin(dapm, "DMIC");
> + ret = snd_soc_dapm_disable_pin(dapm, "DMIC");
> else
> - snd_soc_dapm_enable_pin(dapm, "DMIC");
> + ret = snd_soc_dapm_enable_pin(dapm, "DMIC");
>
> - return 0;
> + return ret;
> }
>
> static struct notifier_block mic_jack_nb = {
Re: [PATCH] macintosh/via-pmu: Fix compiler warnings when CONFIG_PROC_FS is disabled
Le 19/03/2022 à 08:20, Finn Thain a écrit :
> drivers/macintosh/via-pmu.c:897:12: warning: 'pmu_battery_proc_show' defined
> but not used [-Wunused-function]
> static int pmu_battery_proc_show(struct seq_file *m, void *v)
> ^
> drivers/macintosh/via-pmu.c:871:12: warning: 'pmu_irqstats_proc_show' defined
> but not used [-Wunused-function]
> static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
> ^~
> drivers/macintosh/via-pmu.c:860:12: warning: 'pmu_info_proc_show' defined but
> not used [-Wunused-function]
> static int pmu_info_proc_show(struct seq_file *m, void *v)
> ^~
>
> Rearrange some code and add some #ifdefs to avoid unused code warnings
> when CONFIG_PROC_FS is disabled.
Why not just put those three functions inside an #ifdef CONFIG_PROC_FS ?
Christophe
>
> Reported-by: Randy Dunlap
> Cc: Randy Dunlap
> Signed-off-by: Finn Thain
> ---
> drivers/macintosh/via-pmu.c | 61 ++---
> 1 file changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index 55afa6dfa263..5ffebf29b630 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -173,10 +173,15 @@ static unsigned long async_req_locks;
> #define NUM_IRQ_STATS 13
> static unsigned int pmu_irq_stats[NUM_IRQ_STATS];
>
> +#ifdef CONFIG_PROC_FS
> static struct proc_dir_entry *proc_pmu_root;
> static struct proc_dir_entry *proc_pmu_info;
> static struct proc_dir_entry *proc_pmu_irqstats;
> static struct proc_dir_entry *proc_pmu_options;
> +static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
> +static void pmu_proc_setup(void);
> +#endif
> +
> static int option_server_mode;
>
> int pmu_battery_count;
> @@ -185,7 +190,6 @@ unsigned int pmu_power_flags = PMU_PWR_AC_PRESENT;
> struct pmu_battery_info pmu_batteries[PMU_MAX_BATTERIES];
> static int query_batt_timer = BATTERY_POLLING_COUNT;
> static struct adb_request batt_req;
> -static struct proc_dir_entry *proc_pmu_batt[PMU_MAX_BATTERIES];
>
> int asleep;
>
> @@ -204,11 +208,7 @@ static int init_pmu(void);
> static void pmu_start(void);
> static irqreturn_t via_pmu_interrupt(int irq, void *arg);
> static irqreturn_t gpio1_interrupt(int irq, void *arg);
> -static int pmu_info_proc_show(struct seq_file *m, void *v);
> -static int pmu_irqstats_proc_show(struct seq_file *m, void *v);
> -static int pmu_battery_proc_show(struct seq_file *m, void *v);
> static void pmu_pass_intr(unsigned char *data, int len);
> -static const struct proc_ops pmu_options_proc_ops;
>
> #ifdef CONFIG_ADB
> const struct adb_driver via_pmu_driver = {
> @@ -551,26 +551,9 @@ static int __init via_pmu_dev_init(void)
> }
> #endif /* CONFIG_PPC32 */
>
> - /* Create /proc/pmu */
> - proc_pmu_root = proc_mkdir("pmu", NULL);
> - if (proc_pmu_root) {
> - long i;
> -
> - for (i=0; i - char title[16];
> - sprintf(title, "battery_%ld", i);
> - proc_pmu_batt[i] = proc_create_single_data(title, 0,
> - proc_pmu_root, pmu_battery_proc_show,
> - (void *)i);
> - }
> -
> - proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
> - pmu_info_proc_show);
> - proc_pmu_irqstats = proc_create_single("interrupts", 0,
> - proc_pmu_root, pmu_irqstats_proc_show);
> - proc_pmu_options = proc_create("options", 0600, proc_pmu_root,
> - _options_proc_ops);
> - }
> +#ifdef CONFIG_PROC_FS
> + pmu_proc_setup();
> +#endif
> return 0;
> }
>
> @@ -857,6 +840,7 @@ query_battery_state(void)
> 2, PMU_SMART_BATTERY_STATE, pmu_cur_battery+1);
> }
>
> +#ifdef CONFIG_PROC_FS
> static int pmu_info_proc_show(struct seq_file *m, void *v)
> {
> seq_printf(m, "PMU driver version : %d\n", PMU_DRIVER_VERSION);
> @@ -978,6 +962,33 @@ static const struct proc_ops pmu_options_proc_ops = {
> .proc_write = pmu_options_proc_write,
> };
>
> +static void pmu_proc_setup(void)
> +{
> + long i;
> +
> + /* Create /proc/pmu */
> + proc_pmu_root = proc_mkdir("pmu", NULL);
> + if (!proc_pmu_root)
> + return;
> +
> + for (i = 0; i < pmu_battery_count; i++) {
> + char title[16];
> +
> + sprintf(title, "battery_%ld", i);
> + proc_pmu_batt[i] =
> + proc_create_single_data(title, 0, proc_pmu_root,
> + pmu_battery_proc_show, (void
> *)i);
> + }
> +
> + proc_pmu_info = proc_create_single("info", 0, proc_pmu_root,
> +
Re: [PATCH] macintosh/via-pmu: Fix build failure when CONFIG_INPUT is disabled
Le 21/03/2022 à 05:30, Finn Thain a écrit :
> drivers/macintosh/via-pmu-event.o: In function `via_pmu_event':
> via-pmu-event.c:(.text+0x44): undefined reference to `input_event'
> via-pmu-event.c:(.text+0x68): undefined reference to `input_event'
> via-pmu-event.c:(.text+0x94): undefined reference to `input_event'
> via-pmu-event.c:(.text+0xb8): undefined reference to `input_event'
> drivers/macintosh/via-pmu-event.o: In function `via_pmu_event_init':
> via-pmu-event.c:(.init.text+0x20): undefined reference to
> `input_allocate_device'
> via-pmu-event.c:(.init.text+0xc4): undefined reference to
> `input_register_device'
> via-pmu-event.c:(.init.text+0xd4): undefined reference to `input_free_device'
> make[1]: *** [Makefile:1155: vmlinux] Error 1
> make: *** [Makefile:350: __build_one_by_one] Error 2
>
> Don't call into the input subsystem unless CONFIG_INPUT is built-in.
>
> Reported-by: kernel test robot
> Cc: Michael Ellerman
> Cc: Geert Uytterhoeven
> Cc: Randy Dunlap
> Signed-off-by: Finn Thain
> ---
> This is equivalent to the patch I sent a couple of days ago. This one
> is slightly longer and adds a new symbol so that Kconfig logic can been
> used instead of Makefile logic in case reviewers prefer that.
> ---
> drivers/macintosh/Kconfig | 5 +
> drivers/macintosh/Makefile | 3 ++-
> drivers/macintosh/via-pmu.c | 2 ++
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
> index 5cdc361da37c..b9102f051bbb 100644
> --- a/drivers/macintosh/Kconfig
> +++ b/drivers/macintosh/Kconfig
> @@ -67,6 +67,11 @@ config ADB_PMU
> this device; you should do so if your machine is one of those
> mentioned above.
>
> +config ADB_PMU_EVENT
> + bool
> + depends on ADB_PMU && INPUT=y
> + default y
Could be reduced to
config ADB_PMU_EVENT
def_bool y if ADB_PMU && INPUT=y
> +
> config ADB_PMU_LED
> bool "Support for the Power/iBook front LED"
> depends on PPC_PMAC && ADB_PMU
> diff --git a/drivers/macintosh/Makefile b/drivers/macintosh/Makefile
> index 49819b1b6f20..712edcb3e0b0 100644
> --- a/drivers/macintosh/Makefile
> +++ b/drivers/macintosh/Makefile
> @@ -12,7 +12,8 @@ obj-$(CONFIG_MAC_EMUMOUSEBTN) += mac_hid.o
> obj-$(CONFIG_INPUT_ADBHID) += adbhid.o
> obj-$(CONFIG_ANSLCD)+= ans-lcd.o
>
> -obj-$(CONFIG_ADB_PMU)+= via-pmu.o via-pmu-event.o
> +obj-$(CONFIG_ADB_PMU)+= via-pmu.o
> +obj-$(CONFIG_ADB_PMU_EVENT) += via-pmu-event.o
> obj-$(CONFIG_ADB_PMU_LED) += via-pmu-led.o
> obj-$(CONFIG_PMAC_BACKLIGHT)+= via-pmu-backlight.o
> obj-$(CONFIG_ADB_CUDA) += via-cuda.o
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index b1859e5340b3..022e2fd4397b 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -1468,12 +1468,14 @@ pmu_handle_data(unsigned char *data, int len)
> if (pmu_battery_count)
> query_battery_state();
> pmu_pass_intr(data, len);
> +#ifdef CONFIG_ADB_PMU_EVENT
> /* len == 6 is probably a bad check. But how do I
>* know what PMU versions send what events here? */
> if (len == 6) {
Can you replace that #ifdef stuff by
if (IS_ENABLED(CONFIG_ADB_PMU_EVENT) && len == 6) {
> via_pmu_event(PMU_EVT_POWER, !!(data[1]&8));
> via_pmu_event(PMU_EVT_LID, data[1]&1);
> }
> +#endif
> break;
>
> default:
Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions
Peter Zijlstra wrote:
On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
This patch adds powerpc specific functions required for
'objtool mcount' to work, and enables mcount for ppc.
I would love to see more objtool enablement for Power :-)
diff --git a/tools/objtool/arch/powerpc/include/arch/elf.h
b/tools/objtool/arch/powerpc/include/arch/elf.h
new file mode 100644
index ..3c8ebb7d2a6b
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/elf.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _OBJTOOL_ARCH_ELF
+#define _OBJTOOL_ARCH_ELF
+
+#define R_NONE R_PPC_NONE
+
+#endif /* _OBJTOOL_ARCH_ELF */
diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
index d1fc6a123a6e..c9c14fa0dfd7 100644
--- a/tools/objtool/utils.c
+++ b/tools/objtool/utils.c
@@ -179,11 +179,29 @@ int create_mcount_loc_sections(struct objtool_file *file)
loc = (unsigned long *)sec->data->d_buf + idx;
memset(loc, 0, sizeof(unsigned long));
- if (elf_add_reloc_to_insn(file->elf, sec,
- idx * sizeof(unsigned long),
- R_X86_64_64,
- insn->sec, insn->offset))
- return -1;
+ if (file->elf->ehdr.e_machine == EM_X86_64) {
+ if (elf_add_reloc_to_insn(file->elf, sec,
+ idx * sizeof(unsigned long),
+ R_X86_64_64,
+ insn->sec, insn->offset))
+ return -1;
+ }
+
+ if (file->elf->ehdr.e_machine == EM_PPC64) {
+ if (elf_add_reloc_to_insn(file->elf, sec,
+ idx * sizeof(unsigned long),
+ R_PPC64_ADDR64,
+ insn->sec, insn->offset))
+ return -1;
+ }
+
+ if (file->elf->ehdr.e_machine == EM_PPC) {
+ if (elf_add_reloc_to_insn(file->elf, sec,
+ idx * sizeof(unsigned long),
+ R_PPC_ADDR32,
+ insn->sec, insn->offset))
+ return -1;
+ }
It appears to me that repeating this code 3 times doesn't really scale
well, how about we introduce a helper like:
int elf_reloc_type_long(struct elf *elf)
{
switch (elf->ehdr.e_machine) {
case EM_X86_64:
return R_X86_64_64;
case EM_PPC64:
return R_PPC64_ADDR64;
case EM_PPC:
return R_PPC_ADDR32;
default:
WARN("unknown machine...")
exit(-1);
}
}
if (elf_add_reloc_to_insn(file->elf, sec,
idx * sizeof(unsigned long),
elf_reloc_type_long(file->elf),
insn->sec, insn->offset))
return -1;
Good point. I suppose we'll need a similar helper, say,
elf_reloc_type_none(), to replace R_NONE usage if we want to have
objtool work for cross-arch builds.
- Naveen
