RE: [PATCH v2] vmscan: add trace events for lru_gen
>On Thu, 21 Sep 2023 09:12:30 -0700
>
>
>"T.J. Mercier" wrote:
>
>
>
>
>
>> > + TP_fast_assign(
>> >
>> >
>> > + __entry->nid = nid;
>> >
>> >
>> > + __entry->nr_reclaimed = nr_reclaimed;
>> >
>> >
>> > + __entry->nr_dirty = stat->nr_dirty;
>> >
>> >
>> > + __entry->nr_writeback = stat->nr_writeback;
>> >
>> >
>> > + __entry->nr_congested = stat->nr_congested;
>> >
>> >
>> > + __entry->nr_immediate = stat->nr_immediate;
>> >
>> >
>> > + __entry->nr_activate0 = stat->nr_activate[0];
>> >
>> >
>> > + __entry->nr_activate1 = stat->nr_activate[1];
>> >
>> >
>> > + __entry->nr_ref_keep = stat->nr_ref_keep;
>> >
>> >
>> > + __entry->nr_unmap_fail = stat->nr_unmap_fail;
>> >
>> >
>> > + __entry->priority = priority;
>> >
>> >
>> > + __entry->reclaim_flags = trace_reclaim_flags(file);
>> >
>> >
>> > + ),
>> >
>> >
>> > +
>> >
>> >
>> > + TP_printk("nid=%d nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld
>> > nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d
>> > nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
>>
>>
>>
>> Many of these values are unsigned so I think many of these format
>>
>>
>> specifiers should be %lu instead of %ld.
>>
>>
Hello T.J.
Thank you for your comment
As you expected I got this from the legacy lru trace.
I've changed as you recommended.
Actually I changed
[syzbot] [wpan?] [input?] [usb?] memory leak in hwsim_add_one (2)
Hello, syzbot found the following issue on: HEAD commit:e789286468a9 Merge tag 'x86-urgent-2023-09-17' of git://gi.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=16db487fa8 kernel config: https://syzkaller.appspot.com/x/.config?x=943a94479fa8e863 dashboard link: https://syzkaller.appspot.com/bug?extid=d2aa0f55c4ae66a9b75d compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15cc837268 Downloadable assets: disk image: https://storage.googleapis.com/syzbot-assets/60bec5b60566/disk-e7892864.raw.xz vmlinux: https://storage.googleapis.com/syzbot-assets/509a449f2ff0/vmlinux-e7892864.xz kernel image: https://storage.googleapis.com/syzbot-assets/36581da19789/bzImage-e7892864.xz IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+d2aa0f55c4ae66a9b...@syzkaller.appspotmail.com BUG: memory leak unreferenced object 0x8881042a8940 (size 64): comm "swapper/0", pid 1, jiffies 4294937901 (age 1085.750s) hex dump (first 32 bytes): 00 0d 00 00 00 00 00 00 ff ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [] kmalloc_trace+0x25/0x90 mm/slab_common.c:1114 [] kmalloc include/linux/slab.h:599 [inline] [] kzalloc include/linux/slab.h:720 [inline] [] hwsim_add_one+0x14a/0x650 drivers/net/ieee802154/mac802154_hwsim.c:949 [] hwsim_probe+0x23/0xe0 drivers/net/ieee802154/mac802154_hwsim.c:1022 [] platform_probe+0x83/0x110 drivers/base/platform.c:1404 [] call_driver_probe drivers/base/dd.c:579 [inline] [] really_probe+0x126/0x440 drivers/base/dd.c:658 [] __driver_probe_device+0xc3/0x190 drivers/base/dd.c:800 [] driver_probe_device+0x2a/0x120 drivers/base/dd.c:830 [] __driver_attach drivers/base/dd.c:1216 [inline] [] __driver_attach+0x107/0x1f0 drivers/base/dd.c:1156 [] bus_for_each_dev+0xb3/0x110 drivers/base/bus.c:368 [] bus_add_driver+0x126/0x2a0 drivers/base/bus.c:673 [] driver_register+0x85/0x180 drivers/base/driver.c:246 [] hwsim_init_module+0xc6/0x110 drivers/net/ieee802154/mac802154_hwsim.c:1073 [] do_one_initcall+0x76/0x430 init/main.c:1232 [] do_initcall_level init/main.c:1294 [inline] [] do_initcalls init/main.c:1310 [inline] [] do_basic_setup init/main.c:1329 [inline] [] kernel_init_freeable+0x25a/0x460 init/main.c:1547 [] kernel_init+0x1b/0x290 init/main.c:1437 [] ret_from_fork+0x45/0x50 arch/x86/kernel/process.c:147 BUG: memory leak unreferenced object 0x8881042a8780 (size 64): comm "swapper/0", pid 1, jiffies 4294937902 (age 1085.740s) hex dump (first 32 bytes): 00 0d 00 00 00 00 00 00 ff ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [] kmalloc_trace+0x25/0x90 mm/slab_common.c:1114 [] kmalloc include/linux/slab.h:599 [inline] [] kzalloc include/linux/slab.h:720 [inline] [] hwsim_add_one+0x14a/0x650 drivers/net/ieee802154/mac802154_hwsim.c:949 [] hwsim_probe+0x46/0xe0 drivers/net/ieee802154/mac802154_hwsim.c:1022 [] platform_probe+0x83/0x110 drivers/base/platform.c:1404 [] call_driver_probe drivers/base/dd.c:579 [inline] [] really_probe+0x126/0x440 drivers/base/dd.c:658 [] __driver_probe_device+0xc3/0x190 drivers/base/dd.c:800 [] driver_probe_device+0x2a/0x120 drivers/base/dd.c:830 [] __driver_attach drivers/base/dd.c:1216 [inline] [] __driver_attach+0x107/0x1f0 drivers/base/dd.c:1156 [] bus_for_each_dev+0xb3/0x110 drivers/base/bus.c:368 [] bus_add_driver+0x126/0x2a0 drivers/base/bus.c:673 [] driver_register+0x85/0x180 drivers/base/driver.c:246 [] hwsim_init_module+0xc6/0x110 drivers/net/ieee802154/mac802154_hwsim.c:1073 [] do_one_initcall+0x76/0x430 init/main.c:1232 [] do_initcall_level init/main.c:1294 [inline] [] do_initcalls init/main.c:1310 [inline] [] do_basic_setup init/main.c:1329 [inline] [] kernel_init_freeable+0x25a/0x460 init/main.c:1547 [] kernel_init+0x1b/0x290 init/main.c:1437 [] ret_from_fork+0x45/0x50 arch/x86/kernel/process.c:147 BUG: memory leak unreferenced object 0x8881007cc000 (size 768): comm "udevd", pid 4480, jiffies 4295045154 (age 13.270s) hex dump (first 32 bytes): 01 00 00 00 03 00 00 00 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [] alloc_inode_sb include/linux/fs.h:2909 [inline] [] sock_alloc_inode+0x25/0x90 net/socket.c:308 [] alloc_inode+0x23/0x100 fs/inode.c:259 [] new_inode_pseudo+0x16/0x50 fs/inode.c:1004 [] sock_alloc+0x1b/0x90 net/socket.c:634 [] __sock_create+0xbd/0x2e0 net/socket.c:1516 [] sock_create net/socket.c:1603 [inline] [] __sys_socket_create net/socket.c:1640 [inline] [] __sys_socket+0xb8/0x1a0
[PATCH] ARM: dts: qcom: apq8026-samsung-matisse-wifi: Fix inverted hall sensor
Fix hall sensor GPIO polarity and also allow disabling the sensor.
Remove unneeded interrupt.
Fixes: f15623bda1dc ("ARM: dts: qcom: Add support for Samsung Galaxy Tab 4 10.1
(SM-T530)")
Signed-off-by: Matti Lehtimäki
---
arch/arm/boot/dts/qcom/qcom-apq8026-samsung-matisse-wifi.dts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-matisse-wifi.dts
b/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-matisse-wifi.dts
index 884d99297d4c..f516e0426bb9 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-matisse-wifi.dts
+++ b/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-matisse-wifi.dts
@@ -45,11 +45,11 @@ gpio-hall-sensor {
event-hall-sensor {
label = "Hall Effect Sensor";
- gpios = < 110 GPIO_ACTIVE_HIGH>;
- interrupts = < 110 IRQ_TYPE_EDGE_FALLING>;
+ gpios = < 110 GPIO_ACTIVE_LOW>;
linux,input-type = ;
linux,code = ;
debounce-interval = <15>;
+ linux,can-disable;
wakeup-source;
};
};
--
2.39.2
[PATCH 2/2] ARM: qcom: msm8974: Add rpm-master-stats node
Add rpm-master-stats node for MSM8974 and the required RPM MSG RAM
slices for memory access.
Signed-off-by: Matti Lehtimäki
---
arch/arm/boot/dts/qcom/qcom-msm8974.dtsi | 32
1 file changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
index 706fef53767e..0bc2e66d15b1 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
@@ -116,6 +116,18 @@ pmu {
rpm: remoteproc {
compatible = "qcom,msm8974-rpm-proc", "qcom,rpm-proc";
+ master-stats {
+ compatible = "qcom,rpm-master-stats";
+ qcom,rpm-msg-ram = <_master_stats>,
+ <_master_stats>,
+ <_master_stats>,
+ <_master_stats>;
+ qcom,master-names = "APSS",
+ "MPSS",
+ "LPSS",
+ "PRONTO";
+ };
+
smd-edge {
interrupts = ;
qcom,ipc = < 8 0>;
@@ -1067,6 +1079,26 @@ gcc: clock-controller@fc40 {
rpm_msg_ram: sram@fc428000 {
compatible = "qcom,rpm-msg-ram";
reg = <0xfc428000 0x4000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0xfc428000 0x4000>;
+
+ apss_master_stats: sram@150 {
+ reg = <0x150 0x14>;
+ };
+
+ mpss_master_stats: sram@b50 {
+ reg = <0xb50 0x14>;
+ };
+
+ lpss_master_stats: sram@1550 {
+ reg = <0x1550 0x14>;
+ };
+
+ pronto_master_stats: sram@1f50 {
+ reg = <0x1f50 0x14>;
+ };
};
bimc: interconnect@fc38 {
--
2.39.2
[PATCH 1/2] ARM: qcom: msm8226: Add rpm-master-stats node
Add rpm-master-stats node for MSM8226 and the required RPM MSG RAM
slices for memory access.
Signed-off-by: Matti Lehtimäki
---
arch/arm/boot/dts/qcom/qcom-msm8226.dtsi | 32
1 file changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
index 44f3f0127fd7..98cc5ea637e1 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
@@ -56,6 +56,18 @@ pmu {
rpm: remoteproc {
compatible = "qcom,msm8226-rpm-proc", "qcom,rpm-proc";
+ master-stats {
+ compatible = "qcom,rpm-master-stats";
+ qcom,rpm-msg-ram = <_master_stats>,
+ <_master_stats>,
+ <_master_stats>,
+ <_master_stats>;
+ qcom,master-names = "APSS",
+ "MPSS",
+ "LPSS",
+ "PRONTO";
+ };
+
smd-edge {
interrupts = ;
qcom,ipc = < 8 0>;
@@ -742,6 +754,26 @@ sram@fc19 {
rpm_msg_ram: sram@fc428000 {
compatible = "qcom,rpm-msg-ram";
reg = <0xfc428000 0x4000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0xfc428000 0x4000>;
+
+ apss_master_stats: sram@150 {
+ reg = <0x150 0x14>;
+ };
+
+ mpss_master_stats: sram@b50 {
+ reg = <0xb50 0x14>;
+ };
+
+ lpss_master_stats: sram@1550 {
+ reg = <0x1550 0x14>;
+ };
+
+ pronto_master_stats: sram@1f50 {
+ reg = <0x1f50 0x14>;
+ };
};
tcsr_mutex: hwlock@fd484000 {
--
2.39.2
[PATCH 0/2] Add rpm-master-stats nodes for MSM8226 and MSM8974
Add rpm-master-stats nodes and the required RPM MSG RAM slices for MSM8226 and MSM8974. Matti Lehtimäki (2): ARM: qcom: msm8226: Add rpm-master-stats node ARM: qcom: msm8974: Add rpm-master-stats node arch/arm/boot/dts/qcom/qcom-msm8226.dtsi | 32 arch/arm/boot/dts/qcom/qcom-msm8974.dtsi | 32 2 files changed, 64 insertions(+) -- 2.39.2
Re: [PATCH v3 06/13] mm/execmem: introduce execmem_data_alloc()
On Mon, Sep 18, 2023 at 12:31 AM Mike Rapoport wrote:
>
[...]
> diff --git a/include/linux/execmem.h b/include/linux/execmem.h
> index 519bdfdca595..09d45ac786e9 100644
> --- a/include/linux/execmem.h
> +++ b/include/linux/execmem.h
> @@ -29,6 +29,7 @@
> * @EXECMEM_KPROBES: parameters for kprobes
> * @EXECMEM_FTRACE: parameters for ftrace
> * @EXECMEM_BPF: parameters for BPF
> + * @EXECMEM_MODULE_DATA: parameters for module data sections
> * @EXECMEM_TYPE_MAX:
> */
> enum execmem_type {
> @@ -37,6 +38,7 @@ enum execmem_type {
> EXECMEM_KPROBES,
> EXECMEM_FTRACE,
In longer term, I think we can improve the JITed code and merge
kprobe/ftrace/bpf. to use the same ranges. Also, do we need special
setting for FTRACE? If not, let's just remove it.
> EXECMEM_BPF,
> + EXECMEM_MODULE_DATA,
> EXECMEM_TYPE_MAX,
> };
Overall, it is great that kprobe/ftrace/bpf no longer depend on modules.
OTOH, I think we should merge execmem_type and existing mod_mem_type.
Otherwise, we still need to handle page permissions in multiple places.
What is our plan for that?
Thanks,
Song
>
> @@ -107,6 +109,23 @@ struct execmem_params *execmem_arch_params(void);
> */
> void *execmem_text_alloc(enum execmem_type type, size_t size);
>
> +/**
> + * execmem_data_alloc - allocate memory for data coupled to code
> + * @type: type of the allocation
> + * @size: how many bytes of memory are required
> + *
> + * Allocates memory that will contain data coupled with executable code,
> + * like data sections in kernel modules.
> + *
> + * The memory will have protections defined by architecture.
> + *
> + * The allocated memory will reside in an area that does not impose
> + * restrictions on the addressing modes.
> + *
> + * Return: a pointer to the allocated memory or %NULL
> + */
> +void *execmem_data_alloc(enum execmem_type type, size_t size);
> +
> /**
> * execmem_free - free executable memory
> * @ptr: pointer to the memory that should be freed
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c4146bfcd0a7..2ae83a6abf66 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1188,25 +1188,16 @@ void __weak module_arch_freeing_init(struct module
> *mod)
> {
> }
>
> -static bool mod_mem_use_vmalloc(enum mod_mem_type type)
> -{
> - return IS_ENABLED(CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC) &&
> - mod_mem_type_is_core_data(type);
> -}
> -
> static void *module_memory_alloc(unsigned int size, enum mod_mem_type type)
> {
> - if (mod_mem_use_vmalloc(type))
> - return vzalloc(size);
> + if (mod_mem_type_is_data(type))
> + return execmem_data_alloc(EXECMEM_MODULE_DATA, size);
> return execmem_text_alloc(EXECMEM_MODULE_TEXT, size);
> }
>
> static void module_memory_free(void *ptr, enum mod_mem_type type)
> {
> - if (mod_mem_use_vmalloc(type))
> - vfree(ptr);
> - else
> - execmem_free(ptr);
> + execmem_free(ptr);
> }
>
> static void free_mod_mem(struct module *mod)
> diff --git a/mm/execmem.c b/mm/execmem.c
> index abcbd07e05ac..aeff85261360 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -53,11 +53,23 @@ static void *execmem_alloc(size_t size, struct
> execmem_range *range)
> return kasan_reset_tag(p);
> }
>
> +static inline bool execmem_range_is_data(enum execmem_type type)
> +{
> + return type == EXECMEM_MODULE_DATA;
> +}
> +
> void *execmem_text_alloc(enum execmem_type type, size_t size)
> {
> return execmem_alloc(size, _params.ranges[type]);
> }
>
> +void *execmem_data_alloc(enum execmem_type type, size_t size)
> +{
> + WARN_ON_ONCE(!execmem_range_is_data(type));
> +
> + return execmem_alloc(size, _params.ranges[type]);
> +}
> +
> void execmem_free(void *ptr)
> {
> /*
> @@ -93,7 +105,10 @@ static void execmem_init_missing(struct execmem_params *p)
> struct execmem_range *r = >ranges[i];
>
> if (!r->start) {
> - r->pgprot = default_range->pgprot;
> + if (execmem_range_is_data(i))
> + r->pgprot = PAGE_KERNEL;
> + else
> + r->pgprot = default_range->pgprot;
> r->alignment = default_range->alignment;
> r->start = default_range->start;
> r->end = default_range->end;
> --
> 2.39.2
>
Re: [PATCH v3 02/13] mm: introduce execmem_text_alloc() and execmem_free()
On Mon, Sep 18, 2023 at 12:30 AM Mike Rapoport wrote:
>
[...]
> diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
> index 42215f9404af..db5561d0c233 100644
> --- a/arch/s390/kernel/module.c
> +++ b/arch/s390/kernel/module.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -76,7 +77,7 @@ void *module_alloc(unsigned long size)
> #ifdef CONFIG_FUNCTION_TRACER
> void module_arch_cleanup(struct module *mod)
> {
> - module_memfree(mod->arch.trampolines_start);
> + execmem_free(mod->arch.trampolines_start);
> }
> #endif
>
> @@ -510,7 +511,7 @@ static int
> module_alloc_ftrace_hotpatch_trampolines(struct module *me,
>
> size = FTRACE_HOTPATCH_TRAMPOLINES_SIZE(s->sh_size);
> numpages = DIV_ROUND_UP(size, PAGE_SIZE);
> - start = module_alloc(numpages * PAGE_SIZE);
> + start = execmem_text_alloc(EXECMEM_FTRACE, numpages * PAGE_SIZE);
This should be EXECMEM_MODULE_TEXT?
Thanks,
Song
> if (!start)
> return -ENOMEM;
> set_memory_rox((unsigned long)start, numpages);
[...]
Re: [PATCH v3 09/13] powerpc: extend execmem_params for kprobes allocations
On Mon, Sep 18, 2023 at 12:31 AM Mike Rapoport wrote: > [...] > @@ -135,5 +138,13 @@ struct execmem_params __init *execmem_arch_params(void) > > range->pgprot = prot; > > + execmem_params.ranges[EXECMEM_KPROBES].start = VMALLOC_START; > + execmem_params.ranges[EXECMEM_KPROBES].start = VMALLOC_END; .end = VMALLOC_END. Thanks, Song > + > + if (strict_module_rwx_enabled()) > + execmem_params.ranges[EXECMEM_KPROBES].pgprot = > PAGE_KERNEL_ROX; > + else > + execmem_params.ranges[EXECMEM_KPROBES].pgprot = > PAGE_KERNEL_EXEC; > + > return _params; > } > -- > 2.39.2 > >
Re: [PATCH v3 02/13] mm: introduce execmem_text_alloc() and execmem_free()
On Mon, Sep 18, 2023 at 12:30 AM Mike Rapoport wrote:
>
[...]
> +
> +/**
> + * enum execmem_type - types of executable memory ranges
> + *
> + * There are several subsystems that allocate executable memory.
> + * Architectures define different restrictions on placement,
> + * permissions, alignment and other parameters for memory that can be used
> + * by these subsystems.
> + * Types in this enum identify subsystems that allocate executable memory
> + * and let architectures define parameters for ranges suitable for
> + * allocations by each subsystem.
> + *
> + * @EXECMEM_DEFAULT: default parameters that would be used for types that
> + * are not explcitly defined.
> + * @EXECMEM_MODULE_TEXT: parameters for module text sections
> + * @EXECMEM_KPROBES: parameters for kprobes
> + * @EXECMEM_FTRACE: parameters for ftrace
> + * @EXECMEM_BPF: parameters for BPF
> + * @EXECMEM_TYPE_MAX:
> + */
> +enum execmem_type {
> + EXECMEM_DEFAULT,
I found EXECMEM_DEFAULT more confusing than helpful.
Song
> + EXECMEM_MODULE_TEXT = EXECMEM_DEFAULT,
> + EXECMEM_KPROBES,
> + EXECMEM_FTRACE,
> + EXECMEM_BPF,
> + EXECMEM_TYPE_MAX,
> +};
> +
[...]
Re: [PATCH v3 02/13] mm: introduce execmem_text_alloc() and execmem_free()
On Mon, Sep 18, 2023 at 12:30 AM Mike Rapoport wrote:
>
[...]
> +
> +#include
> +#include
> +#include
> +#include
> +
> +static void *execmem_alloc(size_t size)
> +{
> + return module_alloc(size);
> +}
> +
> +void *execmem_text_alloc(enum execmem_type type, size_t size)
> +{
> + return execmem_alloc(size);
> +}
execmem_text_alloc (and later execmem_data_alloc) both take "type" as
input. I guess we can just use execmem_alloc(type, size) for everything?
Thanks,
Song
> +
> +void execmem_free(void *ptr)
> +{
> + /*
> +* This memory may be RO, and freeing RO memory in an interrupt is not
> +* supported by vmalloc.
> +*/
> + WARN_ON(in_interrupt());
> + vfree(ptr);
> +}
> --
> 2.39.2
>
[PATCH v6 00/13] selftests/sgx: Fix compilation errors
Hi,
This patch series ensures that all SGX selftests succeed when compiling with
optimizations (as tested with -O{0,1,2,3,s} for both gcc 11.3.0 and clang
14.0.0). The aim of the patches is to avoid reliance on undefined,
compiler-specific behavior that can make the test results fragile.
As far as I see, all commits in this series have now been acked/reviewed.
Please let me know if any concerns remain and I'd happily address them.
Reference output below:
.. Testing gcc -O0[OK]
.. Testing gcc -O1[OK]
.. Testing gcc -O2[OK]
.. Testing gcc -O3[OK]
.. Testing gcc -Os[OK]
.. Testing gcc -Ofast [OK]
.. Testing gcc -Og[OK]
.. Testing clang -O0[OK]
.. Testing clang -O1[OK]
.. Testing clang -O2[OK]
.. Testing clang -O3[OK]
.. Testing clang -Os[OK]
.. Testing clang -Ofast [OK]
.. Testing clang -Og[OK]
Changelog
-
v6
- Collect final ack/reviewed-by tags (Jarkko, Kai)
v5
- Reorder patches (Jarkko, Kai)
- Include fixes tag for inline asm memory clobber patch (Kai)
- Include linker error in static-pie commit message (Kai)
- Include generated assembly in relocations commit (Kai)
v4
- Remove redundant -nostartfiles compiler flag (Jarkko)
- Split dynamic symbol table removal in separate commit (Kai)
- Split redundant push/pop elimination in separate commit (Kai)
- Remove (incomplete) register cleansing on enclave exit
- Fix possibly uninitialized pointer dereferences in load.c
v3
- Refactor encl_op_array declaration and indexing (Jarkko)
- Annotate encl_buffer with "used" attribute (Kai)
- Split encl_buffer size and placement commits (Kai)
v2
- Add additional check for NULL pointer (Kai)
- Refine to produce proper static-pie executable
- Fix linker script assertions
- Specify memory clobber for inline asm instead of volatile (Kai)
- Clarify why encl_buffer non-static (Jarkko, Kai)
- Clarify -ffreestanding (Jarkko)
Best,
Jo
Jo Van Bulck (13):
selftests/sgx: Fix uninitialized pointer dereference in error path
selftests/sgx: Fix uninitialized pointer dereferences in
encl_get_entry
selftests/sgx: Include memory clobber for inline asm in test enclave
selftests/sgx: Separate linker options
selftests/sgx: Specify freestanding environment for enclave
compilation
selftests/sgx: Remove redundant enclave base address save/restore
selftests/sgx: Produce static-pie executable for test enclave
selftests/sgx: Handle relocations in test enclave
selftests/sgx: Fix linker script asserts
selftests/sgx: Ensure test enclave buffer is entirely preserved
selftests/sgx: Ensure expected location of test enclave buffer
selftests/sgx: Discard unsupported ELF sections
selftests/sgx: Remove incomplete ABI sanitization code in test enclave
tools/testing/selftests/sgx/Makefile | 12 ++--
tools/testing/selftests/sgx/defines.h | 2 +
tools/testing/selftests/sgx/load.c| 9 ++-
tools/testing/selftests/sgx/sigstruct.c | 5 +-
tools/testing/selftests/sgx/test_encl.c | 67 +--
tools/testing/selftests/sgx/test_encl.lds | 10 +--
.../selftests/sgx/test_encl_bootstrap.S | 28 +++-
7 files changed, 77 insertions(+), 56 deletions(-)
--
2.25.1
Re: [PATCH] remoteproc: mediatek: Refactor single core check and fix retrocompatibility
On Tue, Sep 19, 2023 at 11:23:36AM +0200, AngeloGioacchino Del Regno wrote:
> In older devicetrees we had the ChromeOS EC in a node called "cros-ec"
> instead of the newer "cros-ec-rpmsg", but this driver is now checking
> only for the latter, breaking compatibility with those.
>
> Besides, we can check if the SCP is single or dual core by simply
> walking through the children of the main SCP node and checking if
> if there's more than one "mediatek,scp-core" compatible node.
>
> Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core
> SCP")
> Signed-off-by: AngeloGioacchino Del Regno
>
> ---
> drivers/remoteproc/mtk_scp.c | 18 +++---
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
Applied.
Thanks,
Mathieu
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index ea227b566c54..a35409eda0cf 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1144,29 +1144,25 @@ static int scp_add_multi_core(struct platform_device
> *pdev,
> return ret;
> }
>
> -static int scp_is_single_core(struct platform_device *pdev)
> +static bool scp_is_single_core(struct platform_device *pdev)
> {
> struct device *dev = >dev;
> struct device_node *np = dev_of_node(dev);
> struct device_node *child;
> + int num_cores = 0;
>
> - child = of_get_next_available_child(np, NULL);
> - if (!child)
> - return dev_err_probe(dev, -ENODEV, "No child node\n");
> + for_each_child_of_node(np, child)
> + if (of_device_is_compatible(child, "mediatek,scp-core"))
> + num_cores++;
>
> - of_node_put(child);
> - return of_node_name_eq(child, "cros-ec-rpmsg");
> + return num_cores < 2;
> }
>
> static int scp_cluster_init(struct platform_device *pdev, struct
> mtk_scp_of_cluster *scp_cluster)
> {
> int ret;
>
> - ret = scp_is_single_core(pdev);
> - if (ret < 0)
> - return ret;
> -
> - if (ret)
> + if (scp_is_single_core(pdev))
> ret = scp_add_single_core(pdev, scp_cluster);
> else
> ret = scp_add_multi_core(pdev, scp_cluster);
> --
> 2.42.0
>
[PATCH v6 12/13] selftests/sgx: Discard unsupported ELF sections
Building the test enclave with -static-pie may produce a dynamic symbol table, but this is not supported for enclaves and any relocations need to happen manually (e.g., as for "encl_op_array"). Thus, opportunistically discard ".dyn*" and ".gnu.hash" which the enclave loader cannot handle. Signed-off-by: Jo Van Bulck Reviewed-by: Jarkko Sakkinen --- tools/testing/selftests/sgx/test_encl.lds | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds index 333a3e78fdc9..ffe851a1cac4 100644 --- a/tools/testing/selftests/sgx/test_encl.lds +++ b/tools/testing/selftests/sgx/test_encl.lds @@ -33,6 +33,8 @@ SECTIONS *(.note*) *(.debug*) *(.eh_frame*) + *(.dyn*) + *(.gnu.hash) } } -- 2.25.1
[PATCH v6 10/13] selftests/sgx: Ensure test enclave buffer is entirely preserved
Attach the "used" attribute to instruct the compiler to preserve the static
encl_buffer, even if it appears it is not entirely referenced in the enclave
code, as expected by the external tests manipulating page permissions.
Link:
https://lore.kernel.org/all/a2732938-f3db-a0af-3d68-a18060f66...@cs.kuleuven.be/
Signed-off-by: Jo Van Bulck
Reviewed-by: Jarkko Sakkinen
Acked-by: Kai Huang
---
tools/testing/selftests/sgx/defines.h | 1 +
tools/testing/selftests/sgx/test_encl.c | 9 +
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/sgx/defines.h
b/tools/testing/selftests/sgx/defines.h
index d8587c971941..b8f482667ce1 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -13,6 +13,7 @@
#define __aligned(x) __attribute__((__aligned__(x)))
#define __packed __attribute__((packed))
+#define __used __attribute__((used))
#include "../../../../arch/x86/include/asm/sgx.h"
#include "../../../../arch/x86/include/asm/enclu.h"
diff --git a/tools/testing/selftests/sgx/test_encl.c
b/tools/testing/selftests/sgx/test_encl.c
index 649604c526e7..7465f121fb74 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -5,11 +5,12 @@
#include "defines.h"
/*
- * Data buffer spanning two pages that will be placed first in .data
- * segment. Even if not used internally the second page is needed by
- * external test manipulating page permissions.
+ * Data buffer spanning two pages that will be placed first in the .data
+ * segment. Even if not used internally the second page is needed by external
+ * test manipulating page permissions, so mark encl_buffer as "used" to make
+ * sure it is entirely preserved by the compiler.
*/
-static uint8_t encl_buffer[8192] = { 1 };
+static uint8_t __used encl_buffer[8192] = { 1 };
enum sgx_enclu_function {
EACCEPT = 0x5,
--
2.25.1
[PATCH v6 11/13] selftests/sgx: Ensure expected location of test enclave buffer
The external tests manipulating page permissions expect encl_buffer to be
placed at the start of the test enclave's .data section. As this is not
guaranteed per the C standard, explicitly place encl_buffer in a separate
section that is explicitly placed at the start of the .data segment in the
linker script to avoid the compiler placing it somewhere else in .data.
Signed-off-by: Jo Van Bulck
Reviewed-by: Jarkko Sakkinen
Acked-by: Kai Huang
---
tools/testing/selftests/sgx/defines.h | 1 +
tools/testing/selftests/sgx/test_encl.c | 8
tools/testing/selftests/sgx/test_encl.lds | 1 +
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/sgx/defines.h
b/tools/testing/selftests/sgx/defines.h
index b8f482667ce1..402f8787a71c 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -14,6 +14,7 @@
#define __aligned(x) __attribute__((__aligned__(x)))
#define __packed __attribute__((packed))
#define __used __attribute__((used))
+#define __section(x)__attribute__((__section__(x)))
#include "../../../../arch/x86/include/asm/sgx.h"
#include "../../../../arch/x86/include/asm/enclu.h"
diff --git a/tools/testing/selftests/sgx/test_encl.c
b/tools/testing/selftests/sgx/test_encl.c
index 7465f121fb74..2c4d709cce2d 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -6,11 +6,11 @@
/*
* Data buffer spanning two pages that will be placed first in the .data
- * segment. Even if not used internally the second page is needed by external
- * test manipulating page permissions, so mark encl_buffer as "used" to make
- * sure it is entirely preserved by the compiler.
+ * segment via the linker script. Even if not used internally the second page
+ * is needed by external test manipulating page permissions, so mark
+ * encl_buffer as "used" to make sure it is entirely preserved by the compiler.
*/
-static uint8_t __used encl_buffer[8192] = { 1 };
+static uint8_t __used __section(".data.encl_buffer") encl_buffer[8192] = { 1 };
enum sgx_enclu_function {
EACCEPT = 0x5,
diff --git a/tools/testing/selftests/sgx/test_encl.lds
b/tools/testing/selftests/sgx/test_encl.lds
index 6ffdfc9fb4cf..333a3e78fdc9 100644
--- a/tools/testing/selftests/sgx/test_encl.lds
+++ b/tools/testing/selftests/sgx/test_encl.lds
@@ -24,6 +24,7 @@ SECTIONS
} : text
.data : {
+ *(.data.encl_buffer)
*(.data*)
} : data
--
2.25.1
[PATCH v6 08/13] selftests/sgx: Handle relocations in test enclave
Static-pie binaries normally include a startup routine to perform any ELF
relocations from .rela.dyn. Since the enclave loading process is different
and glibc is not included, do the necessary relocation for encl_op_array
entries manually at runtime relative to the enclave base to ensure correct
function pointers.
When keeping encl_op_array as a local variable on the stack, gcc without
optimizations generates code that explicitly gets the right function
addresses and stores them to create the array on the stack:
encl_body:
/* snipped */
leado_encl_op_put_to_buf(%rip), %rax
mov%rax, -0x50(%rbp)
leado_encl_op_get_from_buf(%rip), %rax
mov%rax,-0x48(%rbp)
leado_encl_op_put_to_addr(%rip), %rax
/* snipped */
However, gcc -Os or clang generate more efficient code that initializes
encl_op_array by copying a "prepared copy" containing the absolute
addresses of the functions (i.e., relative to the image base starting from
0) generated by the compiler/linker:
encl_body:
/* snipped */
leaprepared_copy(%rip), %rsi
lea-0x48(%rsp), %rdi
mov$0x10,%ecx
rep movsl %ds:(%rsi),%es:(%rdi)
/* snipped */
When building the enclave with -static-pie, the compiler/linker includes
relocation entries for the function symbols in the "prepared copy":
Relocation section '.rela.dyn' at offset 0x4000 contains 12 entries:
Offset Info Type Symbol
/* snipped; "prepared_copy" starts at 0x6000 */
6000 0008 R_X86_64_RELATIVE
6008 0008 R_X86_64_RELATIVE
6010 0008 R_X86_64_RELATIVE
6018 0008 R_X86_64_RELATIVE
6020 0008 R_X86_64_RELATIVE
6028 0008 R_X86_64_RELATIVE
6030 0008 R_X86_64_RELATIVE
6038 0008 R_X86_64_RELATIVE
Static-pie binaries normally include a glibc "_dl_relocate_static_pie"
routine that will perform these relocations as part of the startup.
However, since the enclave loading process is different and glibc is not
included, we cannot rely on these relocations to be performed. Without
relocations, the code would erroneously jump to the _absolute_ function
address loaded from the local copy.
Thus, declare "encl_op_array" as global and manually relocate the loaded
function-pointer entries relative to the enclave base at runtime. This
generates the following code:
encl_body:
/* snipped */
leaencl_op_array(%rip), %rcx
lea__encl_base(%rip), %rax
add(%rcx,%rdx,8),%rax
jmp*%rax
Link:
https://lore.kernel.org/all/150d8ca8-2c66-60d1-f9fc-8e6279824...@cs.kuleuven.be/
Link:
https://lore.kernel.org/all/5c22de5a-4b3b-1f38-9771-409b4ec7f...@cs.kuleuven.be/#r
Signed-off-by: Jo Van Bulck
Reviewed-by: Jarkko Sakkinen
Acked-by: Kai Huang
---
tools/testing/selftests/sgx/test_encl.c | 50 +
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/sgx/test_encl.c
b/tools/testing/selftests/sgx/test_encl.c
index ae791df3e5a5..649604c526e7 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -121,21 +121,41 @@ static void do_encl_op_nop(void *_op)
}
+/*
+ * Symbol placed at the start of the enclave image by the linker script.
+ * Declare this extern symbol with visibility "hidden" to ensure the compiler
+ * does not access it through the GOT and generates position-independent
+ * addressing as __encl_base(%rip), so we can get the actual enclave base
+ * during runtime.
+ */
+extern const uint8_t __attribute__((visibility("hidden"))) __encl_base;
+
+typedef void (*encl_op_t)(void *);
+static const encl_op_t encl_op_array[ENCL_OP_MAX] = {
+ do_encl_op_put_to_buf,
+ do_encl_op_get_from_buf,
+ do_encl_op_put_to_addr,
+ do_encl_op_get_from_addr,
+ do_encl_op_nop,
+ do_encl_eaccept,
+ do_encl_emodpe,
+ do_encl_init_tcs_page,
+};
+
void encl_body(void *rdi, void *rsi)
{
- const void (*encl_op_array[ENCL_OP_MAX])(void *) = {
- do_encl_op_put_to_buf,
- do_encl_op_get_from_buf,
- do_encl_op_put_to_addr,
- do_encl_op_get_from_addr,
- do_encl_op_nop,
- do_encl_eaccept,
- do_encl_emodpe,
- do_encl_init_tcs_page,
- };
-
- struct encl_op_header *op = (struct encl_op_header *)rdi;
-
- if (op->type < ENCL_OP_MAX)
- (*encl_op_array[op->type])(op);
+ struct encl_op_header *header = (struct encl_op_header *)rdi;
+ encl_op_t op;
+
+ if (header->type >= ENCL_OP_MAX)
+ return;
+
+ /*
+* The enclave base address needs to be added, as this call site
+* *cannot be* made rip-relative by the compiler, or fixed up by
+* any other possible means.
+*/
+ op = ((uint64_t)&__encl_base) +
[PATCH v6 05/13] selftests/sgx: Specify freestanding environment for enclave compilation
Use -ffreestanding to assert the enclave compilation targets a freestanding environment (i.e., without "main" or standard libraries). This fixes clang reporting "undefined reference to `memset'" after erroneously optimizing away the provided memset/memcpy implementations. Still need to instruct the linker from using standard system startup functions, but drop -nostartfiles as it is implied by -nostdlib. Signed-off-by: Jo Van Bulck Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang --- tools/testing/selftests/sgx/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index dcdd04b322f8..7eb890bdd3f0 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -14,7 +14,7 @@ endif INCLUDES := -I$(top_srcdir)/tools/include HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC HOST_LDFLAGS := -z noexecstack -lcrypto -ENCL_CFLAGS += -Wall -Werror -static -nostdlib -nostartfiles -fPIC \ +ENCL_CFLAGS += -Wall -Werror -static -nostdlib -ffreestanding -fPIC \ -fno-stack-protector -mrdrnd $(INCLUDES) ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none -- 2.25.1
[PATCH v6 01/13] selftests/sgx: Fix uninitialized pointer dereference in error path
Ensure ctx is zero-initialized, such that the encl_measure function will
not call EVP_MD_CTX_destroy with an uninitialized ctx pointer in case of an
early error during key generation.
Fixes: 2adcba79e69d ("selftests/x86: Add a selftest for SGX")
Signed-off-by: Jo Van Bulck
Reviewed-by: Jarkko Sakkinen
Acked-by: Kai Huang
---
tools/testing/selftests/sgx/sigstruct.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/sgx/sigstruct.c
b/tools/testing/selftests/sgx/sigstruct.c
index a07896a46364..d73b29becf5b 100644
--- a/tools/testing/selftests/sgx/sigstruct.c
+++ b/tools/testing/selftests/sgx/sigstruct.c
@@ -318,9 +318,9 @@ bool encl_measure(struct encl *encl)
struct sgx_sigstruct *sigstruct = >sigstruct;
struct sgx_sigstruct_payload payload;
uint8_t digest[SHA256_DIGEST_LENGTH];
+ EVP_MD_CTX *ctx = NULL;
unsigned int siglen;
RSA *key = NULL;
- EVP_MD_CTX *ctx;
int i;
memset(sigstruct, 0, sizeof(*sigstruct));
@@ -384,7 +384,8 @@ bool encl_measure(struct encl *encl)
return true;
err:
- EVP_MD_CTX_destroy(ctx);
+ if (ctx)
+ EVP_MD_CTX_destroy(ctx);
RSA_free(key);
return false;
}
--
2.25.1
[PATCH v6 06/13] selftests/sgx: Remove redundant enclave base address save/restore
Remove redundant push/pop pair that stores and restores the enclave base address in the test enclave, as it is never used after the pop and can anyway be easily retrieved via the __encl_base symbol. Signed-off-by: Jo Van Bulck Acked-by: Kai Huang --- tools/testing/selftests/sgx/test_encl_bootstrap.S | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S index 03ae0f57e29d..e0ce993d3f2c 100644 --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S @@ -55,12 +55,9 @@ encl_entry_core: push%rax push%rcx # push the address after EENTER - push%rbx # push the enclave base address callencl_body - pop %rbx # pop the enclave base address - /* Clear volatile GPRs, except RAX (EEXIT function). */ xor %rcx, %rcx xor %rdx, %rdx -- 2.25.1
[PATCH v6 04/13] selftests/sgx: Separate linker options
Fixes "'linker' input unused [-Wunused-command-line-argument]" errors when compiling with clang. Signed-off-by: Jo Van Bulck Reviewed-by: Jarkko Sakkinen --- tools/testing/selftests/sgx/Makefile | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index 50aab6b57da3..dcdd04b322f8 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -12,9 +12,11 @@ OBJCOPY := $(CROSS_COMPILE)objcopy endif INCLUDES := -I$(top_srcdir)/tools/include -HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack -ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \ +HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC +HOST_LDFLAGS := -z noexecstack -lcrypto +ENCL_CFLAGS += -Wall -Werror -static -nostdlib -nostartfiles -fPIC \ -fno-stack-protector -mrdrnd $(INCLUDES) +ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx TEST_FILES := $(OUTPUT)/test_encl.elf @@ -28,7 +30,7 @@ $(OUTPUT)/test_sgx: $(OUTPUT)/main.o \ $(OUTPUT)/sigstruct.o \ $(OUTPUT)/call.o \ $(OUTPUT)/sign_key.o - $(CC) $(HOST_CFLAGS) -o $@ $^ -lcrypto + $(CC) $(HOST_CFLAGS) -o $@ $^ $(HOST_LDFLAGS) $(OUTPUT)/main.o: main.c $(CC) $(HOST_CFLAGS) -c $< -o $@ @@ -45,8 +47,8 @@ $(OUTPUT)/call.o: call.S $(OUTPUT)/sign_key.o: sign_key.S $(CC) $(HOST_CFLAGS) -c $< -o $@ -$(OUTPUT)/test_encl.elf: test_encl.lds test_encl.c test_encl_bootstrap.S - $(CC) $(ENCL_CFLAGS) -T $^ -o $@ -Wl,--build-id=none +$(OUTPUT)/test_encl.elf: test_encl.c test_encl_bootstrap.S + $(CC) $(ENCL_CFLAGS) $^ -o $@ $(ENCL_LDFLAGS) EXTRA_CLEAN := \ $(OUTPUT)/test_encl.elf \ -- 2.25.1
[PATCH v6 13/13] selftests/sgx: Remove incomplete ABI sanitization code in test enclave
As the selftest enclave is *not* intended for production, simplify the code by not initializing CPU configuration registers as expected by the ABI on enclave entry or cleansing caller-save registers on enclave exit. Link: https://lore.kernel.org/all/da0cfb1e-e347-f7f2-ac72-aec0ee0d8...@intel.com/ Signed-off-by: Jo Van Bulck Reviewed-by: Jarkko Sakkinen --- .../testing/selftests/sgx/test_encl_bootstrap.S | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S index 28fe5d2ac0af..d8c4ac94e032 100644 --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S @@ -59,21 +59,11 @@ encl_entry_core: push%rcx # push the address after EENTER + # NOTE: as the selftest enclave is *not* intended for production, + # simplify the code by not initializing ABI registers on entry or + # cleansing caller-save registers on exit. callencl_body - /* Clear volatile GPRs, except RAX (EEXIT function). */ - xor %rcx, %rcx - xor %rdx, %rdx - xor %rdi, %rdi - xor %rsi, %rsi - xor %r8, %r8 - xor %r9, %r9 - xor %r10, %r10 - xor %r11, %r11 - - # Reset status flags. - add %rdx, %rdx # OF = SF = AF = CF = 0; ZF = PF = 1 - # Prepare EEXIT target by popping the address of the instruction after # EENTER to RBX. pop %rbx -- 2.25.1
[PATCH v6 03/13] selftests/sgx: Include memory clobber for inline asm in test enclave
Add the "memory" clobber to the EMODPE and EACCEPT asm blocks to tell the
compiler the assembly code accesses to the secinfo struct. This ensures
the compiler treats the asm block as a memory barrier and the write to
secinfo will be visible to ENCLU.
Fixes: 20404a808593 ("selftests/sgx: Add test for EPCM permission changes")
Signed-off-by: Jo Van Bulck
Reviewed-by: Kai Huang
Reviewed-by: Jarkko Sakkinen
---
tools/testing/selftests/sgx/test_encl.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/sgx/test_encl.c
b/tools/testing/selftests/sgx/test_encl.c
index c0d6397295e3..ae791df3e5a5 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -24,10 +24,11 @@ static void do_encl_emodpe(void *_op)
secinfo.flags = op->flags;
asm volatile(".byte 0x0f, 0x01, 0xd7"
- :
+ : /* no outputs */
: "a" (EMODPE),
"b" (),
- "c" (op->epc_addr));
+ "c" (op->epc_addr)
+ : "memory" /* read from secinfo pointer */);
}
static void do_encl_eaccept(void *_op)
@@ -42,7 +43,8 @@ static void do_encl_eaccept(void *_op)
: "=a" (rax)
: "a" (EACCEPT),
"b" (),
- "c" (op->epc_addr));
+ "c" (op->epc_addr)
+ : "memory" /* read from secinfo pointer */);
op->ret = rax;
}
--
2.25.1
Re: [PATCH] remoteproc: mediatek: Refactor single core check and fix retrocompatibility
Il 20/09/23 17:03, Laura Nao ha scritto:
On 9/19/23 11:23, AngeloGioacchino Del Regno wrote:
In older devicetrees we had the ChromeOS EC in a node called "cros-ec"
instead of the newer "cros-ec-rpmsg", but this driver is now checking
only for the latter, breaking compatibility with those.
Besides, we can check if the SCP is single or dual core by simply
walking through the children of the main SCP node and checking if
if there's more than one "mediatek,scp-core" compatible node.
Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core
SCP")
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/remoteproc/mtk_scp.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
Tested on asurada (spherion) and jacuzzi (juniper). The issue was detected by
KernelCI, so:
Reported-by: "kernelci.org bot"
Tested-by: Laura Nao
Thanks for pointing out the correct Reported-by tag! :-)
Cheers,
Angelo
Re: [PATCH v2] vmscan: add trace events for lru_gen
On Wed, Sep 20, 2023 at 11:19 PM Jaewon Kim wrote:
>
> As the legacy lru provides, the lru_gen needs some trace events for
> debugging.
>
Hi Jaewon, thanks for adding this.
> This commit introduces 2 trace events.
> trace_mm_vmscan_lru_gen_scan
> trace_mm_vmscan_lru_gen_evict
>
> Each event is similar to the following legacy events.
> trace_mm_vmscan_lru_isolate,
> trace_mm_vmscan_lru_shrink_[in]active
>
> Here's an example
> mm_vmscan_lru_gen_scan: isolate_mode=0 classzone=1 order=9
> nr_requested=4096 nr_scanned=431 nr_skipped=0 nr_taken=55 lru=anon
> mm_vmscan_lru_gen_evict: nid=0 nr_reclaimed=42 nr_dirty=0 nr_writeback=0
> nr_congested=0 nr_immediate=0 nr_activate_anon=13 nr_activate_file=0
> nr_ref_keep=0 nr_unmap_fail=0 priority=2
> flags=RECLAIM_WB_ANON|RECLAIM_WB_ASYNC
> mm_vmscan_lru_gen_scan: isolate_mode=0 classzone=1 order=9
> nr_requested=4096 nr_scanned=66 nr_skipped=0 nr_taken=64 lru=file
> mm_vmscan_lru_gen_evict: nid=0 nr_reclaimed=62 nr_dirty=0 nr_writeback=0
> nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=2
> nr_ref_keep=0 nr_unmap_fail=0 priority=2
> flags=RECLAIM_WB_FILE|RECLAIM_WB_ASYNC
>
> Signed-off-by: Jaewon Kim
> ---
> v2: use condition and make it aligned
> v1: introduce trace events
> ---
> include/trace/events/mmflags.h | 5 ++
> include/trace/events/vmscan.h | 98 ++
> mm/vmscan.c| 17 --
> 3 files changed, 115 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> index 1478b9dd05fa..44e9b38f83e7 100644
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -274,6 +274,10 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY, "softdirty" )
> \
> EM (LRU_ACTIVE_FILE, "active_file") \
> EMe(LRU_UNEVICTABLE, "unevictable")
>
> +#define LRU_GEN_NAMES \
> + EM (LRU_GEN_ANON, "anon") \
> + EMe(LRU_GEN_FILE, "file")
> +
> /*
> * First define the enums in the above macros to be exported to userspace
> * via TRACE_DEFINE_ENUM().
> @@ -288,6 +292,7 @@ COMPACTION_PRIORITY
> /* COMPACTION_FEEDBACK are defines not enums. Not needed here. */
> ZONE_TYPE
> LRU_NAMES
> +LRU_GEN_NAMES
>
> /*
> * Now redefine the EM() and EMe() macros to map the enums to the strings
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index d2123dd960d5..f0c3a4bd72db 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -327,6 +327,57 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
> __print_symbolic(__entry->lru, LRU_NAMES))
> );
>
> +TRACE_EVENT_CONDITION(mm_vmscan_lru_gen_scan,
> + TP_PROTO(int highest_zoneidx,
> + int order,
> + unsigned long nr_requested,
> + unsigned long nr_scanned,
> + unsigned long nr_skipped,
> + unsigned long nr_taken,
> + isolate_mode_t isolate_mode,
> + int lru),
> +
> + TP_ARGS(highest_zoneidx, order, nr_requested, nr_scanned, nr_skipped,
> nr_taken, isolate_mode, lru),
> +
> + TP_CONDITION(nr_scanned),
> +
> + TP_STRUCT__entry(
> + __field(int, highest_zoneidx)
> + __field(int, order)
> + __field(unsigned long, nr_requested)
> + __field(unsigned long, nr_scanned)
> + __field(unsigned long, nr_skipped)
> + __field(unsigned long, nr_taken)
> + __field(unsigned int, isolate_mode)
> + __field(int, lru)
> + ),
> +
> + TP_fast_assign(
> + __entry->highest_zoneidx = highest_zoneidx;
> + __entry->order = order;
> + __entry->nr_requested = nr_requested;
> + __entry->nr_scanned = nr_scanned;
> + __entry->nr_skipped = nr_skipped;
> + __entry->nr_taken = nr_taken;
> + __entry->isolate_mode = (__force unsigned int)isolate_mode;
> + __entry->lru = lru;
> + ),
> +
> + /*
> +* classzone is previous name of the highest_zoneidx.
> +* Reason not to change it is the ABI requirement of the tracepoint.
> +*/
> + TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu
> nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s",
> + __entry->isolate_mode,
> + __entry->highest_zoneidx,
> + __entry->order,
> + __entry->nr_requested,
> + __entry->nr_scanned,
> + __entry->nr_skipped,
> + __entry->nr_taken,
> + __print_symbolic(__entry->lru, LRU_GEN_NAMES))
> +);
> +
> TRACE_EVENT(mm_vmscan_write_folio,
>
> TP_PROTO(struct folio *folio),
> @@ -437,6 +488,53 @@ TRACE_EVENT(mm_vmscan_lru_shrink_active,
>
Re: [RFC PATCH 3/4] net: drop_monitor: use drop_reason_lookup()
On Thu, 2023-09-21 at 10:51 +0200, Johannes Berg wrote:
>
> - if (!list ||
> - subsys_reason >= list->n_reasons ||
> - !list->reasons[subsys_reason] ||
> - strlen(list->reasons[subsys_reason]) > NET_DM_MAX_REASON_LEN) {
> - list =
> rcu_dereference(drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_CORE]);
> - subsys_reason = SKB_DROP_REASON_NOT_SPECIFIED;
> - }
Oops, I lost this translation of erroneous ones to the core
SKB_DROP_REASON_NOT_SPECIFIED.
Maybe we should just not have the attribute in that case? But that could
be a different change too.
> - if (nla_put_string(msg, NET_DM_ATTR_REASON,
> -list->reasons[subsys_reason])) {
> + reason_str = drop_reason_lookup(cb->reason);
> + if (nla_put_string(msg, NET_DM_ATTR_REASON, reason_str)) {
> rcu_read_unlock();
johannes
RE: (2) [PATCH] vmscan: add trace events for lru_gen
>> Great. Thank you for your comment. >> >> For the putting the struct scan_control *sc inside the trace, >> I couldn't do that because struct scan_control is defined in mm/vmscan.c. >> I think I should not move it to a seperate header file. > >Well if you ever decide to do so, one thing to do is to move the >trace/events/vmscan.h into mm/ as trace_vmscan.h so that it would have >access to local header files. Then all you need to do is to move the >struct scan_control into a local mm/X.h header file. > >> >> As you may expect, I just made this by copying the existing >> trace_mm_vmscan_lru_isolate and trace_mm_vmscan_lru_shrink_inactive >> >> I've tried to change like this. >> Would this be good for you? > >The below looks fine to me. Thanks. > >-- Steve Thank you for your comment. But I still don't know if I can move the strut scan_control into a new separate header file, mm/X.h. I guess it was meant to be located in vmscan.c only. Let me just send v2 patch with only this change, and wait for other mm guys comments regarding the moving the struct scan_control thing. Thank you very much. > >> >> >> --- a/include/trace/events/vmscan.h >> +++ b/include/trace/events/vmscan.h >> @@ -327,7 +327,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate, >> __print_symbolic(__entry->lru, LRU_NAMES)) >> ); >> >> -TRACE_EVENT(mm_vmscan_lru_gen_scan, >> +TRACE_EVENT_CONDITION(mm_vmscan_lru_gen_scan, >> TP_PROTO(int highest_zoneidx, >> int order, >> unsigned long nr_requested, >> @@ -339,6 +339,8 @@ TRACE_EVENT(mm_vmscan_lru_gen_scan, >> >> TP_ARGS(highest_zoneidx, order, nr_requested, nr_scanned, >> nr_skipped, nr_taken, isolate_mode, lru), >> >> + TP_CONDITION(nr_scanned), >> + >> TP_STRUCT__entry( >> __field(int, highest_zoneidx) >> __field(int, order) >> @@ -494,7 +496,6 @@ TRACE_EVENT(mm_vmscan_lru_gen_evict, >> TP_ARGS(nid, nr_reclaimed, stat, priority, file), >> >> TP_STRUCT__entry( >> - __field(int, nid) >> __field(unsigned long, nr_reclaimed) >> __field(unsigned long, nr_dirty) >> __field(unsigned long, nr_writeback) >> @@ -504,6 +505,7 @@ TRACE_EVENT(mm_vmscan_lru_gen_evict, >> __field(unsigned int, nr_activate1) >> __field(unsigned long, nr_ref_keep) >> __field(unsigned long, nr_unmap_fail) >> + __field(int, nid) >> __field(int, priority) >> __field(int, reclaim_flags) >> ), >> >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -5131,10 +5131,9 @@ static int scan_folios(struct lruvec *lruvec, struct >> scan_control *sc, >> __count_memcg_events(memcg, PGREFILL, sorted); >> __count_vm_events(PGSCAN_ANON + type, isolated); >> >> - if (scanned) >> - trace_mm_vmscan_lru_gen_scan(sc->reclaim_idx, sc->order, >> - MAX_LRU_BATCH, scanned, skipped, isolated, >> - sc->may_unmap ? 0 : ISOLATE_UNMAPPED, type); >> + trace_mm_vmscan_lru_gen_scan(sc->reclaim_idx, sc->order, >> MAX_LRU_BATCH, >> + scanned, skipped, isolated, >> + sc->may_unmap ? 0 : ISOLATE_UNMAPPED, type); >> >> >> >> >
[PATCH] ARM: dts: qcom: sdx65-mtp: Specify PM7250B SID to use
Now that the pm7250b.dtsi can be configured to be on a different SID, we
also need to specify it for this dts file. Set it to the SID 2/3 like it
was before commit 8e2d56f64572 ("arm64: dts: qcom: pm7250b: make SID
configurable").
Signed-off-by: Luca Weiss
---
arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts
b/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts
index fcf1c51c5e7a..9649c859a2c3 100644
--- a/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts
+++ b/arch/arm/boot/dts/qcom/qcom-sdx65-mtp.dts
@@ -4,6 +4,10 @@
*/
/dts-v1/;
+/* PM7250B is configured to use SID2/3 */
+#define PM7250B_SID 2
+#define PM7250B_SID1 3
+
#include "qcom-sdx65.dtsi"
#include
#include
---
base-commit: 7fc7222d9680366edeecc219c21ca96310bdbc10
change-id: 20230921-pm7250b-sid-fixup-0c35c3c59312
Best regards,
--
Luca Weiss
Re: [PATCH] remoteproc: mediatek: Refactor single core check and fix retrocompatibility
On Wed, Sep 20, 2023 at 11:03 PM Laura Nao wrote:
>
> On 9/19/23 11:23, AngeloGioacchino Del Regno wrote:
> > In older devicetrees we had the ChromeOS EC in a node called "cros-ec"
> > instead of the newer "cros-ec-rpmsg", but this driver is now checking
> > only for the latter, breaking compatibility with those.
> >
> > Besides, we can check if the SCP is single or dual core by simply
> > walking through the children of the main SCP node and checking if
> > if there's more than one "mediatek,scp-core" compatible node.
> >
> > Fixes: 1fdbf0cdde98 ("remoteproc: mediatek: Probe SCP cluster on multi-core
> > SCP")
> > Signed-off-by: AngeloGioacchino Del Regno
> >
> > ---
> > drivers/remoteproc/mtk_scp.c | 18 +++---
> > 1 file changed, 7 insertions(+), 11 deletions(-)
> >
>
> Tested on asurada (spherion) and jacuzzi (juniper). The issue was detected by
> KernelCI, so:
>
> Reported-by: "kernelci.org bot"
> Tested-by: Laura Nao
Reviewed-by: Chen-Yu Tsai
Tested-by: Chen-Yu Tsai
on Hayato (MT8192) and Juniper (MT8183).
[PATCH v2] vmscan: add trace events for lru_gen
As the legacy lru provides, the lru_gen needs some trace events for
debugging.
This commit introduces 2 trace events.
trace_mm_vmscan_lru_gen_scan
trace_mm_vmscan_lru_gen_evict
Each event is similar to the following legacy events.
trace_mm_vmscan_lru_isolate,
trace_mm_vmscan_lru_shrink_[in]active
Here's an example
mm_vmscan_lru_gen_scan: isolate_mode=0 classzone=1 order=9 nr_requested=4096
nr_scanned=431 nr_skipped=0 nr_taken=55 lru=anon
mm_vmscan_lru_gen_evict: nid=0 nr_reclaimed=42 nr_dirty=0 nr_writeback=0
nr_congested=0 nr_immediate=0 nr_activate_anon=13 nr_activate_file=0
nr_ref_keep=0 nr_unmap_fail=0 priority=2 flags=RECLAIM_WB_ANON|RECLAIM_WB_ASYNC
mm_vmscan_lru_gen_scan: isolate_mode=0 classzone=1 order=9 nr_requested=4096
nr_scanned=66 nr_skipped=0 nr_taken=64 lru=file
mm_vmscan_lru_gen_evict: nid=0 nr_reclaimed=62 nr_dirty=0 nr_writeback=0
nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=2
nr_ref_keep=0 nr_unmap_fail=0 priority=2 flags=RECLAIM_WB_FILE|RECLAIM_WB_ASYNC
Signed-off-by: Jaewon Kim
---
v2: use condition and make it aligned
v1: introduce trace events
---
include/trace/events/mmflags.h | 5 ++
include/trace/events/vmscan.h | 98 ++
mm/vmscan.c| 17 --
3 files changed, 115 insertions(+), 5 deletions(-)
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index 1478b9dd05fa..44e9b38f83e7 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -274,6 +274,10 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY, "softdirty" )
\
EM (LRU_ACTIVE_FILE, "active_file") \
EMe(LRU_UNEVICTABLE, "unevictable")
+#define LRU_GEN_NAMES \
+ EM (LRU_GEN_ANON, "anon") \
+ EMe(LRU_GEN_FILE, "file")
+
/*
* First define the enums in the above macros to be exported to userspace
* via TRACE_DEFINE_ENUM().
@@ -288,6 +292,7 @@ COMPACTION_PRIORITY
/* COMPACTION_FEEDBACK are defines not enums. Not needed here. */
ZONE_TYPE
LRU_NAMES
+LRU_GEN_NAMES
/*
* Now redefine the EM() and EMe() macros to map the enums to the strings
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index d2123dd960d5..f0c3a4bd72db 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -327,6 +327,57 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
__print_symbolic(__entry->lru, LRU_NAMES))
);
+TRACE_EVENT_CONDITION(mm_vmscan_lru_gen_scan,
+ TP_PROTO(int highest_zoneidx,
+ int order,
+ unsigned long nr_requested,
+ unsigned long nr_scanned,
+ unsigned long nr_skipped,
+ unsigned long nr_taken,
+ isolate_mode_t isolate_mode,
+ int lru),
+
+ TP_ARGS(highest_zoneidx, order, nr_requested, nr_scanned, nr_skipped,
nr_taken, isolate_mode, lru),
+
+ TP_CONDITION(nr_scanned),
+
+ TP_STRUCT__entry(
+ __field(int, highest_zoneidx)
+ __field(int, order)
+ __field(unsigned long, nr_requested)
+ __field(unsigned long, nr_scanned)
+ __field(unsigned long, nr_skipped)
+ __field(unsigned long, nr_taken)
+ __field(unsigned int, isolate_mode)
+ __field(int, lru)
+ ),
+
+ TP_fast_assign(
+ __entry->highest_zoneidx = highest_zoneidx;
+ __entry->order = order;
+ __entry->nr_requested = nr_requested;
+ __entry->nr_scanned = nr_scanned;
+ __entry->nr_skipped = nr_skipped;
+ __entry->nr_taken = nr_taken;
+ __entry->isolate_mode = (__force unsigned int)isolate_mode;
+ __entry->lru = lru;
+ ),
+
+ /*
+* classzone is previous name of the highest_zoneidx.
+* Reason not to change it is the ABI requirement of the tracepoint.
+*/
+ TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu
nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s",
+ __entry->isolate_mode,
+ __entry->highest_zoneidx,
+ __entry->order,
+ __entry->nr_requested,
+ __entry->nr_scanned,
+ __entry->nr_skipped,
+ __entry->nr_taken,
+ __print_symbolic(__entry->lru, LRU_GEN_NAMES))
+);
+
TRACE_EVENT(mm_vmscan_write_folio,
TP_PROTO(struct folio *folio),
@@ -437,6 +488,53 @@ TRACE_EVENT(mm_vmscan_lru_shrink_active,
show_reclaim_flags(__entry->reclaim_flags))
);
+TRACE_EVENT(mm_vmscan_lru_gen_evict,
+
+ TP_PROTO(int nid, unsigned long nr_reclaimed,
+ struct reclaim_stat *stat, int priority, int file),
+
+ TP_ARGS(nid, nr_reclaimed, stat, priority, file),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, nr_reclaimed)
+
[PATCH v6 09/13] selftests/sgx: Fix linker script asserts
DEFINED only considers symbols, not section names. Hence, replace the check for .got.plt with the _GLOBAL_OFFSET_TABLE_ symbol and remove other (non-essential) asserts. Signed-off-by: Jo Van Bulck Reviewed-by: Jarkko Sakkinen --- tools/testing/selftests/sgx/test_encl.lds | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds index 62d37160f59b..6ffdfc9fb4cf 100644 --- a/tools/testing/selftests/sgx/test_encl.lds +++ b/tools/testing/selftests/sgx/test_encl.lds @@ -35,8 +35,4 @@ SECTIONS } } -ASSERT(!DEFINED(.altinstructions), "ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.altinstr_replacement), "ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.discard.retpoline_safe), "RETPOLINE ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.discard.nospec), "RETPOLINE ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.got.plt), "Libcalls are not supported in enclaves") +ASSERT(!DEFINED(_GLOBAL_OFFSET_TABLE_), "Libcalls through GOT are not supported in enclaves") -- 2.25.1
[PATCH v6 02/13] selftests/sgx: Fix uninitialized pointer dereferences in encl_get_entry
Ensure sym_tab and sym_names are zero-initialized and add an early-out
condition in the unlikely (erroneous) case that the enclave ELF file would
not contain a symbol table.
This addresses -Werror=maybe-uninitialized compiler warnings for gcc -O2.
Fixes: 33c5aac3bf32 ("selftests/sgx: Test complete changing of page type flow")
Signed-off-by: Jo Van Bulck
Reviewed-by: Jarkko Sakkinen
---
tools/testing/selftests/sgx/load.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c
b/tools/testing/selftests/sgx/load.c
index 94bdeac1cf04..c9f658e44de6 100644
--- a/tools/testing/selftests/sgx/load.c
+++ b/tools/testing/selftests/sgx/load.c
@@ -136,11 +136,11 @@ static bool encl_ioc_add_pages(struct encl *encl, struct
encl_segment *seg)
*/
uint64_t encl_get_entry(struct encl *encl, const char *symbol)
{
+ Elf64_Sym *symtab = NULL;
+ char *sym_names = NULL;
Elf64_Shdr *sections;
- Elf64_Sym *symtab;
Elf64_Ehdr *ehdr;
- char *sym_names;
- int num_sym;
+ int num_sym = 0;
int i;
ehdr = encl->bin;
@@ -161,6 +161,9 @@ uint64_t encl_get_entry(struct encl *encl, const char
*symbol)
}
}
+ if (!symtab || !sym_names)
+ return 0;
+
for (i = 0; i < num_sym; i++) {
Elf64_Sym *sym = [i];
--
2.25.1
[PATCH v6 07/13] selftests/sgx: Produce static-pie executable for test enclave
The current combination of -static and -fPIC creates a static executable
with position-dependent addresses for global variables. Use -static-pie
and -fPIE to create a proper static position independent executable that
can be loaded at any address without a dynamic linker.
When building the original "lea (encl_stack)(%rbx), %rax" assembly code
with -static-pie -fPIE, the linker complains about a relocation it cannot
resolve:
/usr/local/bin/ld: /tmp/cchIWyfG.o: relocation R_X86_64_32S against
`.data' can not be used when making a PIE object; recompile with -fPIE
collect2: error: ld returned 1 exit status
Thus, since only RIP-relative addressing is legit for local symbols, use
"encl_stack(%rip)" and declare an explicit "__encl_base" symbol at the
start of the linker script to be able to calculate the stack address
relative to the current TCS in the enclave assembly entry code.
Link:
https://lore.kernel.org/all/f9c24d89-ed72-7d9e-c650-050d722c6...@cs.kuleuven.be/
Signed-off-by: Jo Van Bulck
Reviewed-by: Jarkko Sakkinen
Acked-by: Kai Huang
---
tools/testing/selftests/sgx/Makefile | 2 +-
tools/testing/selftests/sgx/test_encl.lds | 1 +
tools/testing/selftests/sgx/test_encl_bootstrap.S | 9 ++---
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/sgx/Makefile
b/tools/testing/selftests/sgx/Makefile
index 7eb890bdd3f0..8d2ba6adc92b 100644
--- a/tools/testing/selftests/sgx/Makefile
+++ b/tools/testing/selftests/sgx/Makefile
@@ -14,7 +14,7 @@ endif
INCLUDES := -I$(top_srcdir)/tools/include
HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC
HOST_LDFLAGS := -z noexecstack -lcrypto
-ENCL_CFLAGS += -Wall -Werror -static -nostdlib -ffreestanding -fPIC \
+ENCL_CFLAGS += -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE \
-fno-stack-protector -mrdrnd $(INCLUDES)
ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none
diff --git a/tools/testing/selftests/sgx/test_encl.lds
b/tools/testing/selftests/sgx/test_encl.lds
index a1ec64f7d91f..62d37160f59b 100644
--- a/tools/testing/selftests/sgx/test_encl.lds
+++ b/tools/testing/selftests/sgx/test_encl.lds
@@ -10,6 +10,7 @@ PHDRS
SECTIONS
{
. = 0;
+__encl_base = .;
.tcs : {
*(.tcs*)
} : tcs
diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S
b/tools/testing/selftests/sgx/test_encl_bootstrap.S
index e0ce993d3f2c..28fe5d2ac0af 100644
--- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
+++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
@@ -42,9 +42,12 @@
encl_entry:
# RBX contains the base address for TCS, which is the first address
# inside the enclave for TCS #1 and one page into the enclave for
- # TCS #2. By adding the value of encl_stack to it, we get
- # the absolute address for the stack.
- lea (encl_stack)(%rbx), %rax
+ # TCS #2. First make it relative by substracting __encl_base and
+ # then add the address of encl_stack to get the address for the stack.
+ lea __encl_base(%rip), %rax
+ sub %rax, %rbx
+ lea encl_stack(%rip), %rax
+ add %rbx, %rax
jmp encl_entry_core
encl_dyn_entry:
# Entry point for dynamically created TCS page expected to follow
--
2.25.1
[PATCH] ring-buffer: Fix bytes info in per_cpu buffer stats
The 'bytes' info in file 'per_cpu/cpu/stats' means the number of
bytes in cpu buffer that have not been consumed. However, currently
after consuming data by reading file 'trace_pipe', the 'bytes' info
was not changed as expected.
# cat per_cpu/cpu0/stats
entries: 0
overrun: 0
commit overrun: 0
bytes: 568 <--- 'bytes' is problematical !!!
oldest event ts: 8651.371479
now ts: 8653.912224
dropped events: 0
read events: 8
The root cause is incorrect stat on cpu_buffer->read_bytes. To fix it:
1. When stat 'read_bytes', account consumed event in rb_advance_reader();
2. When stat 'entries_bytes', exclude the discarded padding event which
is smaller than minimum size because it is invisible to reader. Then
use rb_page_commit() instead of BUF_PAGE_SIZE at where accounting for
page-based read/remove/overrun.
Also correct the comments of ring_buffer_bytes_cpu() in this patch.
Fixes: c64e148a3be3 ("trace: Add ring buffer stats to measure rate of events")
Signed-off-by: Zheng Yejian
---
kernel/trace/ring_buffer.c | 28 +++-
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index a1651edc48d5..28daf0ce95c5 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -354,6 +354,11 @@ static void rb_init_page(struct buffer_data_page *bpage)
local_set(>commit, 0);
}
+static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
+{
+ return local_read(>page->commit);
+}
+
static void free_buffer_page(struct buffer_page *bpage)
{
free_page((unsigned long)bpage->page);
@@ -2003,7 +2008,7 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer,
unsigned long nr_pages)
* Increment overrun to account for the lost events.
*/
local_add(page_entries, _buffer->overrun);
- local_sub(BUF_PAGE_SIZE, _buffer->entries_bytes);
+ local_sub(rb_page_commit(to_remove_page),
_buffer->entries_bytes);
local_inc(_buffer->pages_lost);
}
@@ -2367,11 +2372,6 @@ rb_reader_event(struct ring_buffer_per_cpu *cpu_buffer)
cpu_buffer->reader_page->read);
}
-static __always_inline unsigned rb_page_commit(struct buffer_page *bpage)
-{
- return local_read(>page->commit);
-}
-
static struct ring_buffer_event *
rb_iter_head_event(struct ring_buffer_iter *iter)
{
@@ -2517,7 +2517,7 @@ rb_handle_head_page(struct ring_buffer_per_cpu
*cpu_buffer,
* the counters.
*/
local_add(entries, _buffer->overrun);
- local_sub(BUF_PAGE_SIZE, _buffer->entries_bytes);
+ local_sub(rb_page_commit(next_page),
_buffer->entries_bytes);
local_inc(_buffer->pages_lost);
/*
@@ -2660,9 +2660,6 @@ rb_reset_tail(struct ring_buffer_per_cpu *cpu_buffer,
event = __rb_page_index(tail_page, tail);
- /* account for padding bytes */
- local_add(BUF_PAGE_SIZE - tail, _buffer->entries_bytes);
-
/*
* Save the original length to the meta data.
* This will be used by the reader to add lost event
@@ -2676,7 +2673,8 @@ rb_reset_tail(struct ring_buffer_per_cpu *cpu_buffer,
* write counter enough to allow another writer to slip
* in on this page.
* We put in a discarded commit instead, to make sure
-* that this space is not used again.
+* that this space is not used again, and this space will
+* not be accounted into 'entries_bytes'.
*
* If we are less than the minimum size, we don't need to
* worry about it.
@@ -2701,6 +2699,9 @@ rb_reset_tail(struct ring_buffer_per_cpu *cpu_buffer,
/* time delta must be non zero */
event->time_delta = 1;
+ /* account for padding bytes */
+ local_add(BUF_PAGE_SIZE - tail, _buffer->entries_bytes);
+
/* Make sure the padding is visible before the tail_page->write update
*/
smp_wmb();
@@ -4215,7 +4216,7 @@ u64 ring_buffer_oldest_event_ts(struct trace_buffer
*buffer, int cpu)
EXPORT_SYMBOL_GPL(ring_buffer_oldest_event_ts);
/**
- * ring_buffer_bytes_cpu - get the number of bytes consumed in a cpu buffer
+ * ring_buffer_bytes_cpu - get the number of bytes unconsumed in a cpu buffer
* @buffer: The ring buffer
* @cpu: The per CPU buffer to read from.
*/
@@ -4723,6 +4724,7 @@ static void rb_advance_reader(struct ring_buffer_per_cpu
*cpu_buffer)
length = rb_event_length(event);
cpu_buffer->reader_page->read += length;
+ cpu_buffer->read_bytes += length;
}
static void rb_advance_iter(struct ring_buffer_iter *iter)
@@ -5816,7 +5818,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
} else {
/* update the entry counter
Re: [PATCH] kallsyms: Fix kallsyms_selftest failure
Adding live-patching list into Cc.
On Fri 2023-08-25 15:19:10, Leizhen (ThunderTown) wrote:
> On 2023/8/25 11:46, Yonghong Song wrote:
> > Kernel test robot reported a kallsyms_test failure when clang lto is
> > enabled (thin or full) and CONFIG_KALLSYMS_SELFTEST is also enabled.
> > I can reproduce in my local environment with the following error message
> > with thin lto:
> > [1.877897] kallsyms_selftest: Test for 1750th symbol failed:
> > (tsc_cs_mark_unstable) addr=81038090
> > [1.877901] kallsyms_selftest: abort
> >
> > It appears that commit 8cc32a9bbf29 ("kallsyms: strip LTO-only suffixes
> > from promoted global functions") caused the failure. Commit 8cc32a9bbf29
> > changed cleanup_symbol_name() based on ".llvm." instead of '.' where
> > ".llvm." is appended to a before-lto-optimization local symbol name.
> > We need to propagate such knowledge in kallsyms_selftest.c as well.
> >
> > Further more, compare_symbol_name() in kallsyms.c needs change as well.
> > In scripts/kallsyms.c, kallsyms_names and kallsyms_seqs_of_names are used
> > to record symbol names themselves and index to symbol names respectively.
> > For example:
> > kallsyms_names:
> > ...
> > __amd_smn_rw._entry <== seq 1000
> > __amd_smn_rw._entry.5 <== seq 1001
> > __amd_smn_rw.llvm. <== seq 1002
> > ...
> >
> > kallsyms_seqs_of_names are sorted based on cleanup_symbol_name() through, so
> > the order in kallsyms_seqs_of_names actually has
> >
> > index 1000: seq 1002 <== __amd_smn_rw.llvm. (actual symbol
> > comparison using '__amd_smn_rw')
> > index 1001: seq 1000 <== __amd_smn_rw._entry
> > index 1002: seq 1001 <== __amd_smn_rw._entry.5
> >
> > Let us say at a particular point, at index 1000, symbol
> > '__amd_smn_rw.llvm.'
> > is comparing to '__amd_smn_rw._entry' where '__amd_smn_rw._entry' is the
> > one to
> > search e.g., with function kallsyms_on_each_match_symbol(). The current
> > implementation
> > will find out '__amd_smn_rw._entry' is less than '__amd_smn_rw.llvm.'
> > and
> > then continue to search e.g., index 999 and never found a match although
> > the actual
> > index 1001 is a match.
> >
> > To fix this issue, let us do cleanup_symbol_name() first and then do
> > comparison.
> > In the above case, comparing '__amd_smn_rw' vs '__amd_smn_rw._entry' and
> > '__amd_smn_rw._entry' being greater than '__amd_smn_rw', the next
> > comparison will
> > be > index 1000 and eventually index 1001 will be hit an a match is found.
> >
> > For any symbols not having '.llvm.' substr, there is no functionality change
> > for compare_symbol_name().
>
> Reviewed-by: Zhen Lei
>
> >
> > Fixes: 8cc32a9bbf29 ("kallsyms: strip LTO-only suffixes from promoted
> > global functions")
> > Reported-by: kernel test robot
> > Closes:
> > https://lore.kernel.org/oe-lkp/202308232200.1c932a90-oliver.s...@intel.com
> > Signed-off-by: Yonghong Song
> > ---
> > kernel/kallsyms.c | 17 +++--
> > kernel/kallsyms_selftest.c | 23 +--
> > 2 files changed, 8 insertions(+), 32 deletions(-)
> >
> > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > index 016d997131d4..e12d26c10dba 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c
> > @@ -188,16 +188,13 @@ static bool cleanup_symbol_name(char *s)
> >
> > static int compare_symbol_name(const char *name, char *namebuf)
> > {
> > - int ret;
> > -
> > - ret = strcmp(name, namebuf);
> > - if (!ret)
> > - return ret;
> > -
> > - if (cleanup_symbol_name(namebuf) && !strcmp(name, namebuf))
> > - return 0;
> > -
> > - return ret;
> > + /* The kallsyms_seqs_of_names is sorted based on names after
> > +* cleanup_symbol_name() (see scripts/kallsyms.c) if clang lto is
> > enabled.
> > +* To ensure correct bisection in kallsyms_lookup_names(), do
> > +* cleanup_symbol_name(namebuf) before comparing name and namebuf.
> > +*/
> > + cleanup_symbol_name(namebuf);
> > + return strcmp(name, namebuf);
> > }
Hmm, I think that this is not the right fix.
The problem is that compare_symbol_name() does not longer allow
to match the full name of the extra .llwm. symbols.
I think that the problem is that the problem is that the symbols
are sorted using cleanup_symbol_name(). They should be sorted
by using the full name.
Note that the original compare_symbol_name() returned return value
when comparing the non-stripped name. It will work correctly when
the non-stripped names are sorted.
I believe that the correct fix is:
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 653b92f6d4c8..da1f8ae68999 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -339,25 +339,6 @@ static int symbol_absolute(const struct sym_entry *s)
return s->percpu_absolute;
}
-static void cleanup_symbol_name(char *s)
-{
- char *p;
-
- /*
-* ASCII[.] = 2e
-* ASCII[0-9] = 30,39
-*
Re: [PATCH v4] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
Hi. Le mercredi 20 septembre 2023, 21:04:42 EEST Alessandro Carminati a écrit : > Hello Francis, > > Thanks a lot for the review. You are welcome. I also tested it and it works well: root@vm-amd64:~# grep ' name_show' /proc/kallsyms | head -6 810fa070 t name_show 810fa070 t name_show@kernel_irq_irqdesc_c_264 815e67c0 t name_show 815e67c0 t name_show@drivers_pnp_card_c_186 81728bb0 t name_show 81728bb0 t name_show@drivers_gpu_drm_i915_gt_sysfs_engines_c_26 > Il giorno mer 20 set 2023 alle ore 12:53 Francis Laniel > > ha scritto: > > Hi. > > > > Le mardi 19 septembre 2023, 22:39:48 EEST Alessandro Carminati (Red Hat) a > > > > écrit : > > > It is not uncommon for drivers or modules related to similar peripherals > > > to have symbols with the exact same name. > > > While this is not a problem for the kernel's binary itself, it becomes > > > an > > > issue when attempting to trace or probe specific functions using > > > infrastructure like ftrace or kprobe. > > > > > > The tracing subsystem relies on the `nm -n vmlinux` output, which > > > provides > > > symbol information from the kernel's ELF binary. However, when multiple > > > symbols share the same name, the standard nm output does not > > > differentiate > > > between them. This can lead to confusion and difficulty when trying to > > > probe the intended symbol. > > > > > > ~ # cat /proc/kallsyms | grep " name_show" > > > 8c4f76d0 t name_show > > > 8c9cccb0 t name_show > > > 8cb0ac20 t name_show > > > 8cc728c0 t name_show > > > 8ce0efd0 t name_show > > > 8ce126c0 t name_show > > > 8ce1dd20 t name_show > > > 8ce24e70 t name_show > > > 8d1104c0 t name_show > > > 8d1fe480 t name_show > > > > > > kas_alias addresses this challenge by enhancing symbol names with > > > meaningful suffixes generated from the source file and line number > > > during the kernel build process. > > > These newly generated aliases provide tracers with the ability to > > > comprehend the symbols they are interacting with when utilizing the > > > ftracefs interface. > > > This approach may also allow for the probing by name of previously > > > inaccessible symbols. > > > > > > ~ # cat /proc/kallsyms | grep gic_mask_irq > > > d15671e505ac t gic_mask_irq > > > d15671e505ac t gic_mask_irq@drivers_irqchip_irq_gic_c_167 > > > d15671e532a4 t gic_mask_irq > > > d15671e532a4 t gic_mask_irq@drivers_irqchip_irq_gic_v3_c_407 > > > ~ # > > > > > > Changes from v1: > > > - Integrated changes requested by Masami to exclude symbols with > > > prefixes > > > > > > "_cfi" and "_pfx". > > > > > > - Introduced a small framework to handle patterns that need to be > > > excluded > > > > > > from the alias production. > > > > > > - Excluded other symbols using the framework. > > > - Introduced the ability to discriminate between text and data symbols. > > > - Added two new config symbols in this version: > > > CONFIG_KALLSYMS_ALIAS_DATA, > > > > > > which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which > > > excludes all filters and provides an alias for each duplicated symbol. > > > > > > https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carminat > > > i@gm ail.com/ > > > > > > Changes from v2: > > > - Alias tags are created by querying DWARF information from the vmlinux. > > > - The filename + line number is normalized and appended to the original > > > > > > name. > > > > > > - The tag begins with '@' to indicate the symbol source. > > > - Not a change, but worth mentioning, since the alias is added to the > > > > > > existing list, the old duplicated name is preserved, and the livepatch > > > way of dealing with duplicates is maintained. > > > > > > - Acknowledging the existence of scenarios where inlined functions > > > > > > declared in header files may result in multiple copies due to compiler > > > behavior, though it is not actionable as it does not pose an > > > operational > > > issue. > > > > > > - Highlighting a single exception where the same name refers to > > > different > > > > > > functions: the case of "compat_binfmt_elf.c," which directly includes > > > "binfmt_elf.c" producing identical function copies in two separate > > > modules. > > > > > > https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carminat > > > i@gm ail.com/ > > > > > > Changes from v3: > > > - kas_alias was rewritten in Python to create a more concise and > > > > > > maintainable codebase. > > > > > > - The previous automation process used by kas_alias to locate the > > > vmlinux > > > > > > and the addr2line has been replaced with an explicit command-line > > > switch > > > for specifying these requirements. > > > > > > - addr2line has been added into the main Makefile. > > > - A new command-line switch has been introduced, enabling users to > > >
[PATCH 0/4] tracing: improve symbolic printing
So I was frustrated with not seeing the names of SKB dropreasons for all but the core reasons, and then while looking into this all, realized, that the current __print_symbolic() is pretty bad anyway. So I came up with a new approach, using a separate declaration of the symbols, and __print_sym() in there, but to userspace it all doesn't matter, it shows it the same way, just dyamically instead of munging with the strings all the time. This is a huge .data savings as far as I can tell, with a modest amount (~4k) of .text addition, while making it all dynamic and in the SKB dropreason case even reusing the existing list that dropmonitor uses today. Surely patch 3 isn't needed here, but it felt right. Anyway, I think it's a pretty reasonable approach overall, and it does works. I've listed a number of open questions in the first patch since that's where the real changes for this are. johannes
[RFC PATCH 1/4] tracing: add __print_sym() to replace __print_symbolic()
From: Johannes Berg
The way __print_symbolic() works is limited and inefficient
in multiple ways:
- you can only use it with a static list of symbols, but
e.g. the SKB dropreasons are now a dynamic list
- it builds the list in memory _three_ times, so it takes
a lot of memory:
- The print_fmt contains the list (since it's passed to
the macro there). This actually contains the names
_twice_, which is fixed up at runtime.
- TRACE_DEFINE_ENUM() puts a 24-byte struct trace_eval_map
for every entry, plus the string pointed to by it, which
cannot be deduplicated with the strings in the print_fmt
- The in-kernel symbolic printing creates yet another list
of struct trace_print_flags for trace_print_symbols_seq()
- it also requires runtime fixup during init, which is a lot
of string parsing due to the print_fmt fixup
Introduce __print_sym() to - over time - replace the old one.
We can easily extend this also to __print_flags later, but I
cared more about the SKB dropreasons for now.
This new __print_sym() requires only a single list of items,
created by TRACE_DEFINE_SYM_LIST(), for can even use another
already existing list by using TRACE_DEFINE_SYM_FNS() with
lookup and show methods.
Then, instead of doing an init-time fixup, just do this at the
time when userspace reads the print_fmt. This way, dynamically
updated lists are possible.
For userspace, nothing actually changes, because the print_fmt
is shown exactly the same way the old __print_symbolic() was.
This adds about 4k .text in my test builds, but that'll be
more than paid for by the actual conversions.
OPEN QUESTIONS:
- do we need TRACE_EVENT_FL_DYNPRINT or should we just always
go through show_print_fmt()? If we do, we can get rid of
set_event_dynprint() which is also a stupid function, and
somewhat inefficient.
- Is the WARN_ON() in show_print_fmt() correct? Can we have a
dynamic event using this infrastructure? I don't know about
dynamic events enough.
- Is the double-slash (//) thing a good approach? I was looking
for something that couldn't happen in C, and that's a comment
so it shouldn't be possible ... It's only internal though so
we could always change it. Parsing to the next , means we'd
have to parse a LOT of C, so that's out of the question.
- Is it correct that we can assume RCU critical section when in
the lookup function? The SKB code currently does, but I may
not ever have actually run this code yet.
- Where should all the functions be implemented :-)
Signed-off-by: Johannes Berg
---
include/asm-generic/vmlinux.lds.h | 3 +-
include/linux/module.h | 2 +
include/linux/trace_events.h | 10 ++
include/linux/tracepoint.h | 20
include/trace/stages/init.h| 54 +
include/trace/stages/stage2_data_offsets.h | 6 +
include/trace/stages/stage3_trace_output.h | 9 ++
include/trace/stages/stage7_class_define.h | 4 +
kernel/module/main.c | 3 +
kernel/trace/trace_events.c| 131 -
kernel/trace/trace_output.c| 43 +++
11 files changed, 282 insertions(+), 3 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 9c59409104f6..b2b45ff0a701 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -263,7 +263,8 @@
#define FTRACE_EVENTS()
\
. = ALIGN(8); \
BOUNDED_SECTION(_ftrace_events) \
- BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps)
+ BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps) \
+ BOUNDED_SECTION(_ftrace_sym_defs)
#else
#define FTRACE_EVENTS()
#endif
diff --git a/include/linux/module.h b/include/linux/module.h
index a98e188cf37b..bf14b74a872c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -524,6 +524,8 @@ struct module {
unsigned int num_trace_events;
struct trace_eval_map **trace_evals;
unsigned int num_trace_evals;
+ struct trace_sym_def **trace_sym_defs;
+ unsigned int num_trace_sym_defs;
#endif
#ifdef CONFIG_FTRACE_MCOUNT_RECORD
unsigned int num_ftrace_callsites;
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index eb5c3add939b..9ab651a8fa0b 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -24,6 +24,13 @@ const char *trace_print_flags_seq(struct trace_seq *p, const
char *delim,
const char *trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
const struct trace_print_flags
*symbol_array);
+const char *trace_print_sym_seq(struct trace_seq *p, unsigned long long val,
+ const char
[RFC PATCH 3/4] net: drop_monitor: use drop_reason_lookup()
From: Johannes Berg
Now that we have drop_reason_lookup(), we can just use it for
drop_monitor as well, rather than exporting the list itself.
Signed-off-by: Johannes Berg
---
include/net/dropreason.h | 4
net/core/drop_monitor.c | 18 +++---
net/core/skbuff.c| 6 +++---
3 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index c157070b5303..0e2195ccf2cd 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -38,10 +38,6 @@ struct drop_reason_list {
size_t n_reasons;
};
-/* Note: due to dynamic registrations, access must be under RCU */
-extern const struct drop_reason_list __rcu *
-drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
-
#ifdef CONFIG_TRACEPOINTS
const char *drop_reason_lookup(unsigned long long value);
void drop_reason_show(struct seq_file *m);
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index aff31cd944c2..d110a16cde46 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -610,9 +610,8 @@ static int net_dm_packet_report_fill(struct sk_buff *msg,
struct sk_buff *skb,
size_t payload_len)
{
struct net_dm_skb_cb *cb = NET_DM_SKB_CB(skb);
- const struct drop_reason_list *list = NULL;
- unsigned int subsys, subsys_reason;
char buf[NET_DM_MAX_SYMBOL_LEN];
+ const char *reason_str;
struct nlattr *attr;
void *hdr;
int rc;
@@ -630,19 +629,8 @@ static int net_dm_packet_report_fill(struct sk_buff *msg,
struct sk_buff *skb,
goto nla_put_failure;
rcu_read_lock();
- subsys = u32_get_bits(cb->reason, SKB_DROP_REASON_SUBSYS_MASK);
- if (subsys < SKB_DROP_REASON_SUBSYS_NUM)
- list = rcu_dereference(drop_reasons_by_subsys[subsys]);
- subsys_reason = cb->reason & ~SKB_DROP_REASON_SUBSYS_MASK;
- if (!list ||
- subsys_reason >= list->n_reasons ||
- !list->reasons[subsys_reason] ||
- strlen(list->reasons[subsys_reason]) > NET_DM_MAX_REASON_LEN) {
- list =
rcu_dereference(drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_CORE]);
- subsys_reason = SKB_DROP_REASON_NOT_SPECIFIED;
- }
- if (nla_put_string(msg, NET_DM_ATTR_REASON,
- list->reasons[subsys_reason])) {
+ reason_str = drop_reason_lookup(cb->reason);
+ if (nla_put_string(msg, NET_DM_ATTR_REASON, reason_str)) {
rcu_read_unlock();
goto nla_put_failure;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 415329b76921..9efde769949d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -126,13 +126,11 @@ static const struct drop_reason_list drop_reasons_core = {
.n_reasons = ARRAY_SIZE(drop_reasons),
};
-const struct drop_reason_list __rcu *
+static const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
[SKB_DROP_REASON_SUBSYS_CORE] = RCU_INITIALIZER(_reasons_core),
};
-EXPORT_SYMBOL(drop_reasons_by_subsys);
-#ifdef CONFIG_TRACEPOINTS
const char *drop_reason_lookup(unsigned long long value)
{
unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
@@ -149,7 +147,9 @@ const char *drop_reason_lookup(unsigned long long value)
return NULL;
return subsys->reasons[reason];
}
+EXPORT_SYMBOL(drop_reason_lookup);
+#ifdef CONFIG_TRACEPOINTS
void drop_reason_show(struct seq_file *m)
{
u32 subsys_id;
--
2.41.0
[RFC PATCH 4/4] tracing/timer: use __print_sym()
From: Johannes Berg
Use the new __print_sym() in the timer tracing, just to show
how to convert something. This adds ~80 bytes of .text for a
saving of ~1.5K of data in my builds.
Note the format changes from
print fmt: "success=%d dependency=%s", REC->success,
__print_symbolic(REC->dependency, { 0, "NONE" }, { (1 << 0), "POSIX_TIMER" }, {
(1 << 1), "PERF_EVENTS" }, { (1 << 2), "SCHED" }, { (1 << 3), "CLOCK_UNSTABLE"
}, { (1 << 4), "RCU" }, { (1 << 5), "RCU_EXP" })
to
print fmt: "success=%d dependency=%s", REC->success,
__print_symbolic(REC->dependency, { 0, "NONE" }, { 1, "POSIX_TIMER" }, { 2,
"PERF_EVENTS" }, { 4, "SCHED" }, { 8, "CLOCK_UNSTABLE" }, { 16, "RCU" }, { 32,
"RCU_EXP" })
Since the values are now just printed in the show function as
pure decimal values.
Signed-off-by: Johannes Berg
---
include/trace/events/timer.h | 22 +++---
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index b4bc2828fa09..cb8294da29d0 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -382,26 +382,18 @@ TRACE_EVENT(itimer_expire,
#undef tick_dep_mask_name
#undef tick_dep_name_end
-/* The MASK will convert to their bits and they need to be processed too */
-#define tick_dep_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-#define tick_dep_name_end(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-/* NONE only has a mask defined for it */
-#define tick_dep_mask_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-
-TICK_DEP_NAMES
-
-#undef tick_dep_name
-#undef tick_dep_mask_name
-#undef tick_dep_name_end
-
#define tick_dep_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
#define tick_dep_mask_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
#define tick_dep_name_end(sdep) { TICK_DEP_MASK_##sdep, #sdep }
+TRACE_DEFINE_SYM_LIST(tick_dep_names, TICK_DEP_NAMES);
+
+#undef tick_dep_name
+#undef tick_dep_mask_name
+#undef tick_dep_name_end
+
#define show_tick_dep_name(val)\
- __print_symbolic(val, TICK_DEP_NAMES)
+ __print_sym(val, tick_dep_names)
TRACE_EVENT(tick_stop,
--
2.41.0
Re: [RFC PATCH 1/4] tracing: add __print_sym() to replace __print_symbolic()
On Thu, 2023-09-21 at 08:51 +, Johannes Berg wrote:
>
> - Is it correct that we can assume RCU critical section when in
>the lookup function? The SKB code currently does, but I may
>not ever have actually run this code yet.
Well, I could easily answer that myself, and no, it's incorrect.
It'd be really useful though for these lookups to be able to do them
under RCU, so I think I'll fold this?
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -34,7 +34,7 @@ struct trace_eval_map {
struct trace_sym_def {
const char *system;
const char *symbol_id;
- /* may return NULL */
+ /* may return NULL, called under rcu_read_lock() */
const char *(*lookup)(unsigned long long);
/*
* Must print the list: ', { val, "name"}, ...'
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -155,11 +155,13 @@ trace_print_sym_seq(struct trace_seq *p, unsigned
long long val,
const char *ret = trace_seq_buffer_ptr(p);
const char *name;
+ rcu_read_lock();
name = lookup(val);
if (name)
trace_seq_puts(p, name);
else
trace_seq_printf(p, "0x%llx", val);
+ rcu_read_unlock();
trace_seq_putc(p, 0);
johannes
[RFC PATCH 2/4] net: dropreason: use new __print_sym() in tracing
From: Johannes Berg
The __print_symbolic() could only ever print the core
drop reasons, since that's the way the infrastructure
works. Now that we have __print_sym() with all the
advantages mentioned in that commit, convert to that
and get all the drop reasons from all subsystems. As
we already have a list of them, that's really easy.
This is a little bit of .text (~100 bytes in my build)
and saves a lot of .data (~17k).
Signed-off-by: Johannes Berg
---
include/net/dropreason.h | 5 +
include/trace/events/skb.h | 16 +++---
net/core/skbuff.c | 43 ++
3 files changed, 51 insertions(+), 13 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index 56cb7be92244..c157070b5303 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -42,6 +42,11 @@ struct drop_reason_list {
extern const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value);
+void drop_reason_show(struct seq_file *m);
+#endif
+
void drop_reasons_register_subsys(enum skb_drop_reason_subsys subsys,
const struct drop_reason_list *list);
void drop_reasons_unregister_subsys(enum skb_drop_reason_subsys subsys);
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 07e0715628ec..8a1a63f9e796 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -8,15 +8,9 @@
#include
#include
#include
+#include
-#undef FN
-#define FN(reason) TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason);
-DEFINE_DROP_REASON(FN, FN)
-
-#undef FN
-#undef FNe
-#define FN(reason) { SKB_DROP_REASON_##reason, #reason },
-#define FNe(reason){ SKB_DROP_REASON_##reason, #reason }
+TRACE_DEFINE_SYM_FNS(drop_reason, drop_reason_lookup, drop_reason_show);
/*
* Tracepoint for free an sk_buff:
@@ -44,13 +38,9 @@ TRACE_EVENT(kfree_skb,
TP_printk("skbaddr=%p protocol=%u location=%pS reason: %s",
__entry->skbaddr, __entry->protocol, __entry->location,
- __print_symbolic(__entry->reason,
- DEFINE_DROP_REASON(FN, FNe)))
+ __print_sym(__entry->reason, drop_reason ))
);
-#undef FN
-#undef FNe
-
TRACE_EVENT(consume_skb,
TP_PROTO(struct sk_buff *skb, void *location),
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4eaf7ed0d1f4..415329b76921 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -132,6 +132,49 @@ drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
};
EXPORT_SYMBOL(drop_reasons_by_subsys);
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value)
+{
+ unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
+ u32 reason = value & ~SKB_DROP_REASON_SUBSYS_MASK;
+ const struct drop_reason_list *subsys;
+
+ if (subsys_id >= SKB_DROP_REASON_SUBSYS_NUM)
+ return NULL;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ return NULL;
+ if (reason >= subsys->n_reasons)
+ return NULL;
+ return subsys->reasons[reason];
+}
+
+void drop_reason_show(struct seq_file *m)
+{
+ u32 subsys_id;
+
+ rcu_read_lock();
+ for (subsys_id = 0; subsys_id < SKB_DROP_REASON_SUBSYS_NUM;
subsys_id++) {
+ const struct drop_reason_list *subsys;
+ u32 i;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ continue;
+
+ for (i = 0; i < subsys->n_reasons; i++) {
+ if (!subsys->reasons[i])
+ continue;
+ seq_printf(m, ", { %u, \"%s\" }",
+ (subsys_id << SKB_DROP_REASON_SUBSYS_SHIFT)
| i,
+ subsys->reasons[i]);
+ }
+ }
+ rcu_read_unlock();
+}
+#endif
+
/**
* drop_reasons_register_subsys - register another drop reason subsystem
* @subsys: the subsystem to register, must not be the core
--
2.41.0
Re: [PATCH v2] vmscan: add trace events for lru_gen
On Thu, 21 Sep 2023 09:12:30 -0700
"T.J. Mercier" wrote:
> > + TP_fast_assign(
> > + __entry->nid = nid;
> > + __entry->nr_reclaimed = nr_reclaimed;
> > + __entry->nr_dirty = stat->nr_dirty;
> > + __entry->nr_writeback = stat->nr_writeback;
> > + __entry->nr_congested = stat->nr_congested;
> > + __entry->nr_immediate = stat->nr_immediate;
> > + __entry->nr_activate0 = stat->nr_activate[0];
> > + __entry->nr_activate1 = stat->nr_activate[1];
> > + __entry->nr_ref_keep = stat->nr_ref_keep;
> > + __entry->nr_unmap_fail = stat->nr_unmap_fail;
> > + __entry->priority = priority;
> > + __entry->reclaim_flags = trace_reclaim_flags(file);
> > + ),
> > +
> > + TP_printk("nid=%d nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld
> > nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d
> > nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
>
> Many of these values are unsigned so I think many of these format
> specifiers should be %lu instead of %ld.
Other than this, from the tracing POV:
Reviewed-by: Steven Rostedt (Google)
-- Steve
Re: [PATCH v3] riscv: Only consider swbp/ss handlers for correct privileged mode
Hello:
This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt :
On Tue, 12 Sep 2023 08:56:19 +0200 you wrote:
> From: Björn Töpel
>
> RISC-V software breakpoint trap handlers are used for {k,u}probes.
>
> When trapping from kernelmode, only the kernelmode handlers should be
> considered. Vice versa, only usermode handlers for usermode
> traps. This is not the case on RISC-V, which can trigger a bug if a
> userspace process uses uprobes, and a WARN() is triggered from
> kernelmode (which is implemented via {c.,}ebreak).
>
> [...]
Here is the summary with links:
- [v3] riscv: Only consider swbp/ss handlers for correct privileged mode
https://git.kernel.org/riscv/c/9f564b92cf6d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
