* [GIT PULL] s390 updates for 5.15 merge window
@ 2021-08-30 13:11 Heiko Carstens
2021-08-30 20:17 ` pr-tracker-bot
2021-08-31 2:19 ` Nathan Chancellor
0 siblings, 2 replies; 15+ messages in thread
From: Heiko Carstens @ 2021-08-30 13:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Vasily Gorbik, Christian Borntraeger, linux-s390, linux-kernel
Hi Linus,
please pull s390 updates for the 5.15 merge window. There are some small
changes outside of s390, but everything has been agreed on (see provided
links below).
Thanks,
Heiko
The following changes since commit ff1176468d368232b684f75e82563369208bc371:
Linux 5.14-rc3 (2021-07-25 15:35:14 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git tags/s390-5.15-1
for you to fetch changes up to 927932240aa1739ac8c92b142a5e2dcc490f36e0:
s390: remove SCHED_CORE from defconfigs (2021-08-30 12:55:05 +0200)
----------------------------------------------------------------
s390 updates for 5.15 merge window
- Improve ftrace code patching so that stop_machine is not required anymore.
This requires a small common code patch acked by Steven Rostedt:
https://lore.kernel.org/linux-s390/20210730220741.4da6fdf6@oasis.local.home/
- Enable KCSAN for s390. This comes with a small common code change to fix a
compile warning. Acked by Marco Elver:
https://lore.kernel.org/r/20210729142811.1309391-1-hca@linux.ibm.com/
- Add KFENCE support for s390. This also comes with a minimal x86 patch from
Marco Elver who said also this can be carried via the s390 tree:
https://lore.kernel.org/linux-s390/YQJdarx6XSUQ1tFZ@elver.google.com/
- More changes to prepare the decompressor for relocation.
- Enable DAT also for CPU restart path.
- Final set of register asm removal patches; leaving only three locations where
needed and sane.
- Add NNPA, Vector-Packed-Decimal-Enhancement Facility 2, PCI MIO support to
hwcaps flags.
- Cleanup hwcaps implementation.
- Add new instructions to in-kernel disassembler.
- Various QDIO cleanups.
- Add SCLP debug feature.
- Various other cleanups and improvements all over the place.
----------------------------------------------------------------
Alexander Egorenkov (19):
s390/boot: move all linker symbol declarations from c to h files
s390/boot: make stacks part of the decompressor's image
s390/cio: remove unused include linux/spinlock.h from cio.h
s390/sclp: use only one sclp early buffer to send commands
s390/boot: move uv function declarations to boot/uv.h
s390/boot: disable Secure Execution in dump mode
s390/uv: de-duplicate checks for Protected Host Virtualization
s390/boot: get rid of magic numbers for startup offsets
s390/boot: move sclp early buffer from fixed address in asm to C
s390/boot: introduce boot data 'initrd_data'
s390/dump: introduce boot data 'oldmem_data'
s390/setup: remove unused symbolic constants for C code from setup.h
s390/setup: drop _OFFSET macros
s390/setup: generate asm offsets from struct parmarea
s390/boot: move EP_OFFSET and EP_STRING to head.S
s390/boot: make _diag308_reset_dma() position-independent
s390/boot: move dma sections from decompressor to decompressed kernel
s390/setup: don't reserve memory that occupied decompressor's head
s390/sclp: reserve memory occupied by sclp early buffer
Alexander Gordeev (4):
s390/kasan: fix large PMD pages address alignment check
s390/boot: factor out offset_vmlinux_info() function
s390/smp: enable DAT before CPU restart callback is called
s390/smp: do not use nodat_stack for secondary CPU start
David Hildenbrand (1):
s390/mm: remove unused cmma functions
Harald Freudenberger (2):
s390/zcrypt: fix wrong offset index for APKA master key valid state
s390/ap: fix state machine hang after failure to enable irq
Heiko Carstens (34):
s390/mm: use pr_err() instead of printk() for pte_ERROR & friends
s390/mm: don't print hashed values for pte_ERROR() & friends
s390/jump_label: print real address in a case of a jump label bug
s390/dasd: remove debug printk
s390/debug: remove unused print defines
s390/cpacf: get rid of register asm
s390/syscall: provide generic system call functions
s390/vdso: use system call functions
s390/ctl_reg: add ctlreg5 and ctlreg15 unions
s390: report more CPU capabilities
s390/disassembler: add instructions
s390/hwcaps: shorten HWCAP defines
s390/hwcaps: introduce HWCAP bit numbers
s390/hwcaps: use named initializers for hwcap string arrays
s390/hwcaps: add sanity checks
s390/hwcaps: move setup_hwcaps()
s390/hwcaps: split setup_hwcaps()
s390/hwcaps: open code initialization of first six hwcap bits
s390/hwcaps: use consistent coding style / remove comments
s390/hwcaps: remove z/Architecture mode active check
s390/hwcaps: remove hwcap stfle check
s390/hwcaps: make sie capability regular hwcap
s390/boot: get rid of arithmetics on function pointers
s390/delay: get rid of not needed header includes
s390/mm: implement set_memory_4k()
kcsan: use u64 instead of cycles_t
s390/mm: use page_to_virt() in __kernel_map_pages()
s390: rename dma section to amode31
s390: fix typo in linker script
s390/mm,pageattr: fix walk_pte_level() early exit
s390/diag: make restart_part2 a local label
KVM: s390: generate kvm hypercall functions
s390: update defconfigs
s390: remove SCHED_CORE from defconfigs
Ilya Leoshkevich (4):
s390/headers: fix code style in module.h
s390: enable KCSAN
ftrace: Introduce ftrace_need_init_nop()
s390/ftrace: implement hotpatching
Julian Wiedmann (18):
s390/qdio: fix roll-back after timeout on ESTABLISH ccw
s390/qdio: cancel the ESTABLISH ccw after timeout
s390/qdio: improve roll-back after error on ESTABLISH ccw
s390/qdio: propagate error when cancelling a ccw fails
s390/qdio: remove remaining tasklet & timer code
s390/qdio: remove unneeded siga-sync for Output Queue
s390/qdio: clarify reporting of errors to the drivers
s390/qdio: remove unused macros
s390/qdio: use absolute data address in ESTABLISH ccw
s390/qdio: remove unused sync-after-IRQ infrastructure
s390/qdio: clean up SIGA capability tracking
s390/qdio: fine-tune the queue sync
s390/qdio: use dev_info() in qdio_print_subchannel_info()
s390/qdio: consolidate QIB code
s390/qdio: remove unused support for SLIB parameters
s390/ap: use the common device_driver pointer
s390/ap: use the common driver-data pointer
s390/zcrypt: remove gratuitious NULL check in .remove() callbacks
Marco Elver (1):
kfence, x86: only define helpers if !MODULE
Masahiro Yamada (1):
s390: move the install rule to arch/s390/Makefile
Niklas Schnelle (8):
s390: make PCI mio support a machine flag
s390: add HWCAP_S390_PCI_MIO to ELF hwcaps
s390/pci: cleanup resources only if necessary
s390/pci: reset zdev->zbus on registration failure
s390/pci: fix misleading rc in clp_set_pci_fn()
s390/pci: handle FH state mismatch only on disable
s390/pci: simplify CLP List PCI handling
s390/pci: improve DMA translation init and exit
Peter Oberparleiter (4):
s390/debug: keep debug data on resize
s390/debug: fix debug area life cycle
s390/debug: add early tracing support
s390/sclp: add tracing of SCLP interactions
Randy Dunlap (1):
s390/crypto: fix all kernel-doc warnings in vfio_ap_ops.c
Sebastian Andrzej Siewior (2):
s390: replace deprecated CPU-hotplug functions
s390/sclp: replace deprecated CPU-hotplug functions
Sven Schnelle (4):
kfence: add function to mask address bits
s390: add support for KFENCE
s390: add kfence region to pagetable dumper
s390: remove do_signal() prototype and do_notify_resume() function
Vineeth Vijayan (2):
s390/cio: add rescan functionality on channel subsystem
s390/cio: add dev_busid sysfs entry for each subchannel
arch/s390/Kconfig | 2 +
arch/s390/Makefile | 3 +-
arch/s390/boot/Makefile | 7 +-
arch/s390/boot/boot.h | 14 +-
arch/s390/boot/compressed/Makefile | 1 +
arch/s390/boot/compressed/decompressor.c | 5 -
arch/s390/boot/compressed/decompressor.h | 5 +
arch/s390/boot/compressed/vmlinux.lds.S | 35 +--
arch/s390/boot/head.S | 56 +---
arch/s390/boot/ipl_report.c | 6 +-
arch/s390/boot/kaslr.c | 6 +-
arch/s390/boot/mem_detect.c | 8 +-
arch/s390/boot/pgm_check_info.c | 5 +-
arch/s390/boot/sclp_early_core.c | 9 +
arch/s390/boot/startup.c | 78 ++---
arch/s390/boot/uv.c | 40 ++-
arch/s390/boot/uv.h | 19 ++
arch/s390/configs/debug_defconfig | 3 +-
arch/s390/configs/defconfig | 1 -
arch/s390/hypfs/hypfs_diag0c.c | 12 +-
arch/s390/include/asm/cio.h | 1 -
arch/s390/include/asm/cpacf.h | 208 +++++++------
arch/s390/include/asm/cpufeature.h | 2 +-
arch/s390/include/asm/ctl_reg.h | 17 ++
arch/s390/include/asm/debug.h | 122 ++++++--
arch/s390/include/asm/diag.h | 15 +-
arch/s390/include/asm/elf.h | 76 +++--
arch/s390/include/asm/extable.h | 4 +-
arch/s390/include/asm/ftrace.h | 46 +--
arch/s390/include/asm/ftrace.lds.h | 21 ++
arch/s390/include/asm/ipl.h | 1 +
arch/s390/include/asm/kfence.h | 42 +++
arch/s390/include/asm/kvm_para.h | 229 +++++---------
arch/s390/include/asm/linkage.h | 4 +-
arch/s390/include/asm/lowcore.h | 3 +-
arch/s390/include/asm/module.h | 14 +-
arch/s390/include/asm/page.h | 3 -
arch/s390/include/asm/pci.h | 7 +-
arch/s390/include/asm/pci_dma.h | 2 -
arch/s390/include/asm/pgtable.h | 10 +-
arch/s390/include/asm/processor.h | 2 +
arch/s390/include/asm/qdio.h | 19 +-
arch/s390/include/asm/sclp.h | 10 +-
arch/s390/include/asm/sections.h | 4 +-
arch/s390/include/asm/set_memory.h | 6 +
arch/s390/include/asm/setup.h | 46 +--
arch/s390/include/asm/syscall.h | 59 ++++
arch/s390/include/asm/uv.h | 8 -
arch/s390/include/asm/vdso/gettimeofday.h | 22 +-
arch/s390/kernel/Makefile | 2 +-
arch/s390/kernel/asm-offsets.c | 8 +
arch/s390/kernel/crash_dump.c | 46 +--
arch/s390/kernel/debug.c | 247 ++++++++++-----
arch/s390/kernel/diag.c | 27 +-
arch/s390/kernel/dis.c | 2 +
arch/s390/kernel/early.c | 4 +
arch/s390/kernel/entry.S | 11 +-
arch/s390/kernel/entry.h | 11 +-
arch/s390/kernel/ftrace.c | 222 +++++++++++++-
arch/s390/kernel/ftrace.h | 26 ++
arch/s390/kernel/head64.S | 17 ++
arch/s390/kernel/ipl.c | 5 +-
arch/s390/kernel/ipl_vmparm.c | 2 +
arch/s390/kernel/irq.c | 4 +-
arch/s390/kernel/jump_label.c | 2 +-
arch/s390/kernel/machine_kexec.c | 5 +-
arch/s390/kernel/module.c | 45 +++
arch/s390/kernel/os_info.c | 2 +-
arch/s390/kernel/perf_cpum_cf.c | 4 +-
arch/s390/kernel/processor.c | 177 ++++++++++-
arch/s390/kernel/setup.c | 286 ++++++++----------
arch/s390/kernel/signal.c | 6 -
arch/s390/kernel/smp.c | 64 ++--
.../{boot/text_dma.S => kernel/text_amode31.S} | 60 ++--
arch/s390/kernel/topology.c | 4 +-
arch/s390/kernel/traps.c | 2 +-
arch/s390/kernel/uv.c | 15 -
arch/s390/kernel/vdso32/Makefile | 1 +
arch/s390/kernel/vdso64/Makefile | 1 +
arch/s390/kernel/vmlinux.lds.S | 35 +++
arch/s390/lib/delay.c | 11 +-
arch/s390/mm/dump_pagetables.c | 16 +
arch/s390/mm/fault.c | 13 +-
arch/s390/mm/init.c | 3 +-
arch/s390/mm/kasan_init.c | 43 ++-
arch/s390/mm/maccess.c | 4 +-
arch/s390/mm/page-states.c | 43 ---
arch/s390/mm/pageattr.c | 19 +-
arch/s390/mm/vmem.c | 2 +-
arch/s390/pci/pci.c | 73 +++--
arch/s390/pci/pci_bus.c | 8 +-
arch/s390/pci/pci_clp.c | 186 ++++++------
arch/s390/pci/pci_dma.c | 25 +-
arch/s390/pci/pci_event.c | 5 +-
arch/s390/pci/pci_sysfs.c | 19 +-
arch/s390/purgatory/Makefile | 1 +
arch/s390/tools/opcodes.txt | 18 ++
arch/x86/include/asm/kfence.h | 4 +
drivers/iommu/s390-iommu.c | 18 +-
drivers/s390/block/dasd_ioctl.c | 4 +-
drivers/s390/char/sclp.c | 230 +++++++++++++-
drivers/s390/char/sclp.h | 2 -
drivers/s390/char/sclp_cmd.c | 2 +-
drivers/s390/char/sclp_config.c | 4 +-
drivers/s390/char/sclp_early_core.c | 19 +-
drivers/s390/char/zcore.c | 2 +-
drivers/s390/cio/css.c | 30 ++
drivers/s390/cio/qdio.h | 40 +--
drivers/s390/cio/qdio_debug.c | 3 -
drivers/s390/cio/qdio_main.c | 331 +++++----------------
drivers/s390/cio/qdio_setup.c | 114 ++-----
drivers/s390/crypto/ap_bus.c | 32 +-
drivers/s390/crypto/ap_bus.h | 13 +-
drivers/s390/crypto/ap_queue.c | 20 +-
drivers/s390/crypto/vfio_ap_ops.c | 116 ++++----
drivers/s390/crypto/zcrypt_api.c | 4 +-
drivers/s390/crypto/zcrypt_card.c | 8 +-
drivers/s390/crypto/zcrypt_ccamisc.c | 8 +-
drivers/s390/crypto/zcrypt_cex2a.c | 17 +-
drivers/s390/crypto/zcrypt_cex2c.c | 24 +-
drivers/s390/crypto/zcrypt_cex4.c | 38 +--
drivers/s390/crypto/zcrypt_queue.c | 8 +-
drivers/s390/net/qeth_core_main.c | 10 +-
drivers/s390/scsi/zfcp_qdio.c | 5 +-
include/linux/ftrace.h | 16 +
kernel/kcsan/debugfs.c | 2 +-
kernel/trace/ftrace.c | 4 +-
mm/kfence/kfence_test.c | 13 +-
128 files changed, 2467 insertions(+), 1824 deletions(-)
create mode 100644 arch/s390/boot/uv.h
create mode 100644 arch/s390/include/asm/ftrace.lds.h
create mode 100644 arch/s390/include/asm/kfence.h
create mode 100644 arch/s390/kernel/ftrace.h
rename arch/s390/{boot/text_dma.S => kernel/text_amode31.S} (69%)
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-30 13:11 [GIT PULL] s390 updates for 5.15 merge window Heiko Carstens @ 2021-08-30 20:17 ` pr-tracker-bot 2021-08-31 2:19 ` Nathan Chancellor 1 sibling, 0 replies; 15+ messages in thread From: pr-tracker-bot @ 2021-08-30 20:17 UTC (permalink / raw) To: Heiko Carstens Cc: Linus Torvalds, Vasily Gorbik, Christian Borntraeger, linux-s390, linux-kernel The pull request you sent on Mon, 30 Aug 2021 15:11:50 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git tags/s390-5.15-1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/c7a5238ef68b98130fe36716bb3fa44502f56001 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-30 13:11 [GIT PULL] s390 updates for 5.15 merge window Heiko Carstens 2021-08-30 20:17 ` pr-tracker-bot @ 2021-08-31 2:19 ` Nathan Chancellor 2021-08-31 7:09 ` Christian Borntraeger 1 sibling, 1 reply; 15+ messages in thread From: Nathan Chancellor @ 2021-08-31 2:19 UTC (permalink / raw) To: Heiko Carstens Cc: Linus Torvalds, Vasily Gorbik, Christian Borntraeger, linux-s390, linux-kernel, llvm Hi Heiko, On Mon, Aug 30, 2021 at 03:11:50PM +0200, Heiko Carstens wrote: > - Enable KCSAN for s390. This comes with a small common code change to fix a > compile warning. Acked by Marco Elver: > https://lore.kernel.org/r/20210729142811.1309391-1-hca@linux.ibm.com/ This caught my eye, as we are boot testing KCSAN + KCSAN_KUNIT_TEST in our CI [1] for x86_64 so it would be nice to enable this for s390 as well. However, it does not seem like the unit tests pass when booting up in QEMU, is this expected or am I doing something wrong? The results and compiler versions are below (the results are the same), they should both have the commits that are mentioned in the KCSAN message. GCC 12.0.0 @ d904008df267cbcc01bd6edf98fa0789fb6e94da [ 131.813482] not ok 1 - test_basic [ 135.321137] not ok 2 - test_concurrent_races [ 138.830648] ok 3 - test_novalue_change [ 142.342562] not ok 4 - test_novalue_change_exception [ 145.851008] not ok 5 - test_unknown_origin [ 149.361416] ok 6 - test_write_write_assume_atomic [ 152.872013] not ok 7 - test_write_write_struct [ 156.382960] not ok 8 - test_write_write_struct_part [ 159.890222] ok 9 - test_read_atomic_write_atomic [ 163.402919] not ok 10 - test_read_plain_atomic_write [ 166.912931] not ok 11 - test_read_plain_atomic_rmw [ 170.431915] not ok 12 - test_zero_size_access [ 173.940959] ok 13 - test_data_race [ 177.452028] not ok 14 - test_assert_exclusive_writer [ 180.962840] not ok 15 - test_assert_exclusive_access [ 184.474686] not ok 16 - test_assert_exclusive_access_writer [ 187.992282] not ok 17 - test_assert_exclusive_bits_change [ 191.501869] ok 18 - test_assert_exclusive_bits_nochange [ 195.013138] not ok 19 - test_assert_exclusive_writer_scoped [ 199.534212] not ok 20 - test_assert_exclusive_access_scoped [ 203.053361] ok 21 - test_jiffies_noreport [ 206.573803] ok 22 - test_seqlock_noreport [ 210.093508] ok 23 - test_atomic_builtins [ 210.094014] not ok 1 - kcsan clang 14.0.0 @ 657bb7262d4a53e903e702d46fdcab57b7085128: [ 10.341427] not ok 1 - test_basic [ 13.848960] not ok 2 - test_concurrent_races [ 17.359671] ok 3 - test_novalue_change [ 20.869202] not ok 4 - test_novalue_change_exception [ 24.379067] not ok 5 - test_unknown_origin [ 27.889492] ok 6 - test_write_write_assume_atomic [ 31.399572] not ok 7 - test_write_write_struct [ 34.910833] not ok 8 - test_write_write_struct_part [ 38.419473] ok 9 - test_read_atomic_write_atomic [ 41.929642] not ok 10 - test_read_plain_atomic_write [ 45.439644] not ok 11 - test_read_plain_atomic_rmw [ 48.950048] not ok 12 - test_zero_size_access [ 52.459026] ok 13 - test_data_race [ 55.969806] not ok 14 - test_assert_exclusive_writer [ 59.480436] not ok 15 - test_assert_exclusive_access [ 62.990164] not ok 16 - test_assert_exclusive_access_writer [ 66.499199] not ok 17 - test_assert_exclusive_bits_change [ 70.009481] ok 18 - test_assert_exclusive_bits_nochange [ 73.522184] not ok 19 - test_assert_exclusive_writer_scoped [ 78.030448] not ok 20 - test_assert_exclusive_access_scoped [ 81.539059] ok 21 - test_jiffies_noreport [ 85.051769] ok 22 - test_seqlock_noreport [ 88.572048] ok 23 - test_atomic_builtins [ 88.572279] not ok 1 - kcsan [1]: https://github.com/ClangBuiltLinux/continuous-integration2 Cheers, Nathan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 2:19 ` Nathan Chancellor @ 2021-08-31 7:09 ` Christian Borntraeger 2021-08-31 10:13 ` Heiko Carstens 0 siblings, 1 reply; 15+ messages in thread From: Christian Borntraeger @ 2021-08-31 7:09 UTC (permalink / raw) To: Nathan Chancellor, Heiko Carstens Cc: Linus Torvalds, Vasily Gorbik, linux-s390, linux-kernel, llvm, qemu-s390x On 31.08.21 04:19, Nathan Chancellor wrote: > Hi Heiko, > > On Mon, Aug 30, 2021 at 03:11:50PM +0200, Heiko Carstens wrote: >> - Enable KCSAN for s390. This comes with a small common code change to fix a >> compile warning. Acked by Marco Elver: >> https://lore.kernel.org/r/20210729142811.1309391-1-hca@linux.ibm.com/ > > This caught my eye, as we are boot testing KCSAN + KCSAN_KUNIT_TEST in > our CI [1] for x86_64 so it would be nice to enable this for s390 as > well. However, it does not seem like the unit tests pass when booting up > in QEMU, is this expected or am I doing something wrong? The results and > compiler versions are below (the results are the same), they should both > have the commits that are mentioned in the KCSAN message. Do you have a branch somewhere where you have the s390 build rules as well as the qemu command line? Maybe its a QEMU TCG issue, dont know. CC qemu-s390x just in case. FWIW, if you want access to a virtual s390x let me know. > > GCC 12.0.0 @ d904008df267cbcc01bd6edf98fa0789fb6e94da > > [ 131.813482] not ok 1 - test_basic > [ 135.321137] not ok 2 - test_concurrent_races > [ 138.830648] ok 3 - test_novalue_change > [ 142.342562] not ok 4 - test_novalue_change_exception > [ 145.851008] not ok 5 - test_unknown_origin > [ 149.361416] ok 6 - test_write_write_assume_atomic > [ 152.872013] not ok 7 - test_write_write_struct > [ 156.382960] not ok 8 - test_write_write_struct_part > [ 159.890222] ok 9 - test_read_atomic_write_atomic > [ 163.402919] not ok 10 - test_read_plain_atomic_write > [ 166.912931] not ok 11 - test_read_plain_atomic_rmw > [ 170.431915] not ok 12 - test_zero_size_access > [ 173.940959] ok 13 - test_data_race > [ 177.452028] not ok 14 - test_assert_exclusive_writer > [ 180.962840] not ok 15 - test_assert_exclusive_access > [ 184.474686] not ok 16 - test_assert_exclusive_access_writer > [ 187.992282] not ok 17 - test_assert_exclusive_bits_change > [ 191.501869] ok 18 - test_assert_exclusive_bits_nochange > [ 195.013138] not ok 19 - test_assert_exclusive_writer_scoped > [ 199.534212] not ok 20 - test_assert_exclusive_access_scoped > [ 203.053361] ok 21 - test_jiffies_noreport > [ 206.573803] ok 22 - test_seqlock_noreport > [ 210.093508] ok 23 - test_atomic_builtins > [ 210.094014] not ok 1 - kcsan > > clang 14.0.0 @ 657bb7262d4a53e903e702d46fdcab57b7085128: > > [ 10.341427] not ok 1 - test_basic > [ 13.848960] not ok 2 - test_concurrent_races > [ 17.359671] ok 3 - test_novalue_change > [ 20.869202] not ok 4 - test_novalue_change_exception > [ 24.379067] not ok 5 - test_unknown_origin > [ 27.889492] ok 6 - test_write_write_assume_atomic > [ 31.399572] not ok 7 - test_write_write_struct > [ 34.910833] not ok 8 - test_write_write_struct_part > [ 38.419473] ok 9 - test_read_atomic_write_atomic > [ 41.929642] not ok 10 - test_read_plain_atomic_write > [ 45.439644] not ok 11 - test_read_plain_atomic_rmw > [ 48.950048] not ok 12 - test_zero_size_access > [ 52.459026] ok 13 - test_data_race > [ 55.969806] not ok 14 - test_assert_exclusive_writer > [ 59.480436] not ok 15 - test_assert_exclusive_access > [ 62.990164] not ok 16 - test_assert_exclusive_access_writer > [ 66.499199] not ok 17 - test_assert_exclusive_bits_change > [ 70.009481] ok 18 - test_assert_exclusive_bits_nochange > [ 73.522184] not ok 19 - test_assert_exclusive_writer_scoped > [ 78.030448] not ok 20 - test_assert_exclusive_access_scoped > [ 81.539059] ok 21 - test_jiffies_noreport > [ 85.051769] ok 22 - test_seqlock_noreport > [ 88.572048] ok 23 - test_atomic_builtins > [ 88.572279] not ok 1 - kcsan > > [1]: https://github.com/ClangBuiltLinux/continuous-integration2 > > Cheers, > Nathan > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 7:09 ` Christian Borntraeger @ 2021-08-31 10:13 ` Heiko Carstens 2021-08-31 10:46 ` Marco Elver 0 siblings, 1 reply; 15+ messages in thread From: Heiko Carstens @ 2021-08-31 10:13 UTC (permalink / raw) To: Christian Borntraeger Cc: Nathan Chancellor, Linus Torvalds, Vasily Gorbik, linux-s390, linux-kernel, llvm, qemu-s390x On Tue, Aug 31, 2021 at 09:09:10AM +0200, Christian Borntraeger wrote: > > > On 31.08.21 04:19, Nathan Chancellor wrote: > > Hi Heiko, > > > > On Mon, Aug 30, 2021 at 03:11:50PM +0200, Heiko Carstens wrote: > > > - Enable KCSAN for s390. This comes with a small common code change to fix a > > > compile warning. Acked by Marco Elver: > > > https://lore.kernel.org/r/20210729142811.1309391-1-hca@linux.ibm.com/ > > > > This caught my eye, as we are boot testing KCSAN + KCSAN_KUNIT_TEST in > > our CI [1] for x86_64 so it would be nice to enable this for s390 as > > well. However, it does not seem like the unit tests pass when booting up > > in QEMU, is this expected or am I doing something wrong? The results and > > compiler versions are below (the results are the same), they should both > > have the commits that are mentioned in the KCSAN message. > > Do you have a branch somewhere where you have the s390 build rules as well as > the qemu command line? Maybe its a QEMU TCG issue, dont know. CC qemu-s390x > just in case. I really don't think this is QEMU related. The test fails are sort of expected: we've seen KCSAN reports when the kernel boots and wanted to fix them later. However I have to admit that I wasn't aware of the KCSAN KUNIT tests, and wouldn't have sent the s390 KCSAN enablement upstream if I would have been aware of failing self tests. We'll fix them, and I let you know if things are supposed to work. Thanks a lot for making aware of this! ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 10:13 ` Heiko Carstens @ 2021-08-31 10:46 ` Marco Elver 2021-08-31 15:02 ` Marco Elver 0 siblings, 1 reply; 15+ messages in thread From: Marco Elver @ 2021-08-31 10:46 UTC (permalink / raw) To: Heiko Carstens Cc: Christian Borntraeger, Nathan Chancellor, Linus Torvalds, Vasily Gorbik, linux-s390, linux-kernel, llvm, qemu-s390x On Tue, 31 Aug 2021 at 12:13, Heiko Carstens <hca@linux.ibm.com> wrote: [...] > I really don't think this is QEMU related. The test fails are sort of > expected: we've seen KCSAN reports when the kernel boots and wanted to > fix them later. > However I have to admit that I wasn't aware of the KCSAN KUNIT tests, > and wouldn't have sent the s390 KCSAN enablement upstream if I would > have been aware of failing self tests. > > We'll fix them, and I let you know if things are supposed to work. > > Thanks a lot for making aware of this! Note: Set `CONFIG_KCSAN_REPORT_ONCE_IN_MS=100` (or smaller) instead of the default to make the test complete faster. The pattern I see from what Nathan reported is that all test cases that expect race reports don't observe them ("not ok" cases), and all those where no races are meant to be reported are fine ("ok" cases). Without actually seeing the log, I'm guessing that no races are reported at all, which is certainly not working as intended. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 10:46 ` Marco Elver @ 2021-08-31 15:02 ` Marco Elver 2021-08-31 15:18 ` Heiko Carstens ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Marco Elver @ 2021-08-31 15:02 UTC (permalink / raw) To: Heiko Carstens Cc: Christian Borntraeger, Nathan Chancellor, Linus Torvalds, Vasily Gorbik, linux-s390, linux-kernel, llvm, qemu-s390x On Tue, Aug 31, 2021 at 12:46PM +0200, Marco Elver wrote: > On Tue, 31 Aug 2021 at 12:13, Heiko Carstens <hca@linux.ibm.com> wrote: > [...] > > I really don't think this is QEMU related. The test fails are sort of > > expected: we've seen KCSAN reports when the kernel boots and wanted to > > fix them later. > > However I have to admit that I wasn't aware of the KCSAN KUNIT tests, > > and wouldn't have sent the s390 KCSAN enablement upstream if I would > > have been aware of failing self tests. > > > > We'll fix them, and I let you know if things are supposed to work. > > > > Thanks a lot for making aware of this! > > Note: Set `CONFIG_KCSAN_REPORT_ONCE_IN_MS=100` (or smaller) instead of > the default to make the test complete faster. > > The pattern I see from what Nathan reported is that all test cases > that expect race reports don't observe them ("not ok" cases), and all > those where no races are meant to be reported are fine ("ok" cases). > Without actually seeing the log, I'm guessing that no races are > reported at all, which is certainly not working as intended. I repro'd, and the problem is part QEMU TCG and a minor problem with stack_trace_save() on s390: 1. QEMU TCG doesn't seem to want to execute threads concurrently, resulting in no "value changes" being observed. This is probably just a limitation of TCG, and if run on a real CPU, shouldn't be a problem. On QEMU, most test cases will pass with CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=n (There's one left that requires value changes to be observable) 2. stack_trace_save() is subtly broken on s390: it starts the trace in stack_trace_save() itself. This is incorrect, as the trace should start with the caller. We reported something similar to arm64, also because one of our sanitizer tests failed: https://lkml.kernel.org/r/20210319184106.5688-1-mark.rutland@arm.com I noticed because stack traces like this: | read to 0x0000000001309128 of 8 bytes by task 49 on cpu 1: | print_report+0x48/0x6c0 | kcsan_report_known_origin+0x112/0x200 | kcsan_setup_watchpoint+0x464/0x500 | test_kernel_read+0x2a/0x40 | access_thread+0x84/0xb0 | kthread+0x3aa/0x3d0 | __ret_from_fork+0x58/0x90 | ret_from_fork+0xa/0x30 , which should not be generated because KCSAN uses stack_trace_save(..., 1) in print_report(). I fixed it with the below, and now most tests pass. Note that, other debugging tools may also report misleading stack traces without the stack_trace_save() fix (e.g. certain KFENCE reports). If you have a better solution for how to fix stack_trace_save() on s390, please discard my patch. Thanks, -- Marco ------ >8 ------ From: Marco Elver <elver@google.com> Date: Tue, 31 Aug 2021 16:00:03 +0200 Subject: [PATCH] s390/stacktrace: do not include arch_stack_walk() in stack trace Callers of stack_trace_save() expect that it does not include itself, which attempts to exclude itself by skipping + 1. This contract is broken if arch_stack_walk() still includes itself. Fix it by skipping the initial entry in s390's arch_stack_walk(). Signed-off-by: Marco Elver <elver@google.com> --- arch/s390/kernel/stacktrace.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/s390/kernel/stacktrace.c b/arch/s390/kernel/stacktrace.c index 101477b3e263..47d1841af03e 100644 --- a/arch/s390/kernel/stacktrace.c +++ b/arch/s390/kernel/stacktrace.c @@ -16,11 +16,16 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, { struct unwind_state state; unsigned long addr; + bool init = true; unwind_for_each_frame(&state, task, regs, 0) { addr = unwind_get_return_address(&state); - if (!addr || !consume_entry(cookie, addr)) + if (!addr) + break; + + if (!init && !consume_entry(cookie, addr)) break; + init = false; } } @@ -29,6 +34,7 @@ int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry, { struct unwind_state state; unsigned long addr; + bool init = true; unwind_for_each_frame(&state, task, NULL, 0) { if (state.stack_info.type != STACK_TYPE_TASK) @@ -50,8 +56,9 @@ int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry, return -EINVAL; #endif - if (!consume_entry(cookie, addr)) + if (!init && !consume_entry(cookie, addr)) return -EINVAL; + init = false; } /* Check for stack corruption */ -- 2.33.0.259.gc128427fd7-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 15:02 ` Marco Elver @ 2021-08-31 15:18 ` Heiko Carstens 2021-08-31 17:48 ` Nathan Chancellor 2021-09-01 14:03 ` Vasily Gorbik 2 siblings, 0 replies; 15+ messages in thread From: Heiko Carstens @ 2021-08-31 15:18 UTC (permalink / raw) To: Marco Elver Cc: Christian Borntraeger, Nathan Chancellor, Linus Torvalds, Vasily Gorbik, linux-s390, linux-kernel, llvm, qemu-s390x On Tue, Aug 31, 2021 at 05:02:15PM +0200, Marco Elver wrote: > On Tue, Aug 31, 2021 at 12:46PM +0200, Marco Elver wrote: > > On Tue, 31 Aug 2021 at 12:13, Heiko Carstens <hca@linux.ibm.com> wrote: > I repro'd, and the problem is part QEMU TCG and a minor problem with > stack_trace_save() on s390: > > 1. QEMU TCG doesn't seem to want to execute threads concurrently, > resulting in no "value changes" being observed. This is probably just > a limitation of TCG, and if run on a real CPU, shouldn't be a problem. > On QEMU, most test cases will pass with CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=n > (There's one left that requires value changes to be observable) > > 2. stack_trace_save() is subtly broken on s390: it starts the trace in > stack_trace_save() itself. This is incorrect, as the trace should > start with the caller. We reported something similar to arm64, also > because one of our sanitizer tests failed: > https://lkml.kernel.org/r/20210319184106.5688-1-mark.rutland@arm.com ... > I fixed it with the below, and now most tests pass. Note that, other > debugging tools may also report misleading stack traces without the > stack_trace_save() fix (e.g. certain KFENCE reports). > > If you have a better solution for how to fix stack_trace_save() on s390, > please discard my patch. Ah, with your patch I get the below on a real machine; so it looks like this really was the only problem for the failing self tests. Thanks a lot for the patch! Will look into it later and apply if everything else still works! [ 5.138547] ok 1 - test_basic [ 7.538799] ok 2 - test_concurrent_races [ 9.938524] ok 3 - test_novalue_change [ 11.620063] ok 4 - test_novalue_change_exception [ 11.952404] ok 5 - test_unknown_origin [ 14.349852] ok 6 - test_write_write_assume_atomic [ 14.681937] ok 7 - test_write_write_struct [ 15.031046] ok 8 - test_write_write_struct_part [ 17.429869] ok 9 - test_read_atomic_write_atomic [ 17.760862] ok 10 - test_read_plain_atomic_write [ 18.093500] ok 11 - test_read_plain_atomic_rmw [ 20.490570] ok 12 - test_zero_size_access [ 22.939270] ok 13 - test_data_race [ 23.271104] ok 14 - test_assert_exclusive_writer [ 23.601940] ok 15 - test_assert_exclusive_access [ 26.000281] ok 16 - test_assert_exclusive_access_writer [ 26.330718] ok 17 - test_assert_exclusive_bits_change [ 28.739480] ok 18 - test_assert_exclusive_bits_nochange [ 28.962218] ok 19 - test_assert_exclusive_writer_scoped [ 29.071385] ok 20 - test_assert_exclusive_access_scoped [ 31.479706] ok 21 - test_jiffies_noreport [ 33.879581] ok 22 - test_seqlock_noreport [ 36.279853] ok 23 - test_atomic_builtins [ 36.279893] ok 1 - kcsan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 15:02 ` Marco Elver 2021-08-31 15:18 ` Heiko Carstens @ 2021-08-31 17:48 ` Nathan Chancellor 2021-08-31 17:49 ` Christian Borntraeger 2021-09-01 14:03 ` Vasily Gorbik 2 siblings, 1 reply; 15+ messages in thread From: Nathan Chancellor @ 2021-08-31 17:48 UTC (permalink / raw) To: Marco Elver Cc: Heiko Carstens, Christian Borntraeger, Linus Torvalds, Vasily Gorbik, linux-s390, linux-kernel, llvm, qemu-s390x On Tue, Aug 31, 2021 at 05:02:15PM +0200, Marco Elver wrote: > On Tue, Aug 31, 2021 at 12:46PM +0200, Marco Elver wrote: > > On Tue, 31 Aug 2021 at 12:13, Heiko Carstens <hca@linux.ibm.com> wrote: > > [...] > > > I really don't think this is QEMU related. The test fails are sort of > > > expected: we've seen KCSAN reports when the kernel boots and wanted to > > > fix them later. > > > However I have to admit that I wasn't aware of the KCSAN KUNIT tests, > > > and wouldn't have sent the s390 KCSAN enablement upstream if I would > > > have been aware of failing self tests. > > > > > > We'll fix them, and I let you know if things are supposed to work. > > > > > > Thanks a lot for making aware of this! > > > > Note: Set `CONFIG_KCSAN_REPORT_ONCE_IN_MS=100` (or smaller) instead of > > the default to make the test complete faster. > > > > The pattern I see from what Nathan reported is that all test cases > > that expect race reports don't observe them ("not ok" cases), and all > > those where no races are meant to be reported are fine ("ok" cases). > > Without actually seeing the log, I'm guessing that no races are > > reported at all, which is certainly not working as intended. > > I repro'd, and the problem is part QEMU TCG and a minor problem with > stack_trace_save() on s390: > > 1. QEMU TCG doesn't seem to want to execute threads concurrently, > resulting in no "value changes" being observed. This is probably just > a limitation of TCG, and if run on a real CPU, shouldn't be a problem. > On QEMU, most test cases will pass with CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=n > (There's one left that requires value changes to be observable) Is this just a limitation of s390's TCG implementation or in general? Our CI runs on GitHub Actions, which does not support virtualization so I believe that all of our tests are being done with TCG and x86_64 passes just fine: https://github.com/ClangBuiltLinux/continuous-integration2/runs/3473222334?check_suite_focus=true Good to hear that it is working on bare metal now though, we could still enable build testing of it at a minimum but it would be nice to see the tests pass even in QEMU :) Cheers, Nathan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 17:48 ` Nathan Chancellor @ 2021-08-31 17:49 ` Christian Borntraeger 0 siblings, 0 replies; 15+ messages in thread From: Christian Borntraeger @ 2021-08-31 17:49 UTC (permalink / raw) To: Nathan Chancellor, Marco Elver, Richard Henderson Cc: Heiko Carstens, Linus Torvalds, Vasily Gorbik, linux-s390, linux-kernel, llvm, qemu-s390x On 31.08.21 19:48, Nathan Chancellor wrote: > On Tue, Aug 31, 2021 at 05:02:15PM +0200, Marco Elver wrote: >> On Tue, Aug 31, 2021 at 12:46PM +0200, Marco Elver wrote: >>> On Tue, 31 Aug 2021 at 12:13, Heiko Carstens <hca@linux.ibm.com> wrote: >>> [...] >>>> I really don't think this is QEMU related. The test fails are sort of >>>> expected: we've seen KCSAN reports when the kernel boots and wanted to >>>> fix them later. >>>> However I have to admit that I wasn't aware of the KCSAN KUNIT tests, >>>> and wouldn't have sent the s390 KCSAN enablement upstream if I would >>>> have been aware of failing self tests. >>>> >>>> We'll fix them, and I let you know if things are supposed to work. >>>> >>>> Thanks a lot for making aware of this! >>> >>> Note: Set `CONFIG_KCSAN_REPORT_ONCE_IN_MS=100` (or smaller) instead of >>> the default to make the test complete faster. >>> >>> The pattern I see from what Nathan reported is that all test cases >>> that expect race reports don't observe them ("not ok" cases), and all >>> those where no races are meant to be reported are fine ("ok" cases). >>> Without actually seeing the log, I'm guessing that no races are >>> reported at all, which is certainly not working as intended. >> >> I repro'd, and the problem is part QEMU TCG and a minor problem with >> stack_trace_save() on s390: >> >> 1. QEMU TCG doesn't seem to want to execute threads concurrently, >> resulting in no "value changes" being observed. This is probably just >> a limitation of TCG, and if run on a real CPU, shouldn't be a problem. >> On QEMU, most test cases will pass with CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=n >> (There's one left that requires value changes to be observable) > > Is this just a limitation of s390's TCG implementation or in general? > Our CI runs on GitHub Actions, which does not support virtualization so > I believe that all of our tests are being done with TCG and x86_64 > passes just fine: Maybe Richard knows? > > https://github.com/ClangBuiltLinux/continuous-integration2/runs/3473222334?check_suite_focus=true > > Good to hear that it is working on bare metal now though, we could still > enable build testing of it at a minimum but it would be nice to see the > tests pass even in QEMU :) > > Cheers, > Nathan > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [GIT PULL] s390 updates for 5.15 merge window 2021-08-31 15:02 ` Marco Elver 2021-08-31 15:18 ` Heiko Carstens 2021-08-31 17:48 ` Nathan Chancellor @ 2021-09-01 14:03 ` Vasily Gorbik 2021-09-01 14:05 ` [PATCH] s390/unwind: use current_frame_address() to unwind current task Vasily Gorbik 2 siblings, 1 reply; 15+ messages in thread From: Vasily Gorbik @ 2021-09-01 14:03 UTC (permalink / raw) To: Marco Elver Cc: Heiko Carstens, Christian Borntraeger, Nathan Chancellor, Linus Torvalds, linux-s390, linux-kernel, llvm, qemu-s390x On Tue, Aug 31, 2021 at 05:02:15PM +0200, Marco Elver wrote: > 2. stack_trace_save() is subtly broken on s390: it starts the trace in > stack_trace_save() itself. This is incorrect, as the trace should > start with the caller. We reported something similar to arm64, also > because one of our sanitizer tests failed: > https://lkml.kernel.org/r/20210319184106.5688-1-mark.rutland@arm.com Thanks a lot for looking into it and debugging it! > Fix it by skipping the initial entry in s390's arch_stack_walk(). ... > diff --git a/arch/s390/kernel/stacktrace.c b/arch/s390/kernel/stacktrace.c > index 101477b3e263..47d1841af03e 100644 > --- a/arch/s390/kernel/stacktrace.c > +++ b/arch/s390/kernel/stacktrace.c > @@ -16,11 +16,16 @@ void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > { > struct unwind_state state; > unsigned long addr; > + bool init = true; > > unwind_for_each_frame(&state, task, regs, 0) { > addr = unwind_get_return_address(&state); > - if (!addr || !consume_entry(cookie, addr)) > + if (!addr) > + break; > + > + if (!init && !consume_entry(cookie, addr)) > break; > + init = false; > } I believe we don't need to skip the first unwinder result if task != current or regs != NULL. Same for arch_stack_walk_reliable. But after you pinpointed the problem I see that the actual difference with x86 implementation comes from get_stack_pointer(). I'll send a patch as reply. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] s390/unwind: use current_frame_address() to unwind current task 2021-09-01 14:03 ` Vasily Gorbik @ 2021-09-01 14:05 ` Vasily Gorbik 2021-09-01 17:51 ` Marco Elver 2021-09-03 23:23 ` Nathan Chancellor 0 siblings, 2 replies; 15+ messages in thread From: Vasily Gorbik @ 2021-09-01 14:05 UTC (permalink / raw) To: Marco Elver, Heiko Carstens, Christian Borntraeger Cc: Nathan Chancellor, Linus Torvalds, linux-s390, linux-kernel, llvm, qemu-s390x current_stack_pointer() simply returns current value of %r15. If current_stack_pointer() caller allocates stack (which is the case in unwind code) %r15 points to a stack frame allocated for callees, meaning current_stack_pointer() caller (e.g. stack_trace_save) will end up in the stacktrace. This is not expected by stack_trace_save*() callers and causes problems. current_frame_address() on the other hand returns function stack frame address, which matches %r15 upon function invocation. Using it in get_stack_pointer() makes it more aligned with x86 implementation (according to BACKTRACE_SELF_TEST output) and meets stack_trace_save*() caller's expectations, notably KCSAN. Also make sure unwind_start is always inlined. Reported-by: Nathan Chancellor <nathan@kernel.org> Suggested-by: Marco Elver <elver@google.com> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com> --- arch/s390/include/asm/stacktrace.h | 20 ++++++++++---------- arch/s390/include/asm/unwind.h | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/arch/s390/include/asm/stacktrace.h b/arch/s390/include/asm/stacktrace.h index 3d8a4b94c620..22c41d7fd95c 100644 --- a/arch/s390/include/asm/stacktrace.h +++ b/arch/s390/include/asm/stacktrace.h @@ -34,16 +34,6 @@ static inline bool on_stack(struct stack_info *info, return addr >= info->begin && addr + len <= info->end; } -static __always_inline unsigned long get_stack_pointer(struct task_struct *task, - struct pt_regs *regs) -{ - if (regs) - return (unsigned long) kernel_stack_pointer(regs); - if (task == current) - return current_stack_pointer(); - return (unsigned long) task->thread.ksp; -} - /* * Stack layout of a C stack frame. */ @@ -74,6 +64,16 @@ struct stack_frame { ((unsigned long)__builtin_frame_address(0) - \ offsetof(struct stack_frame, back_chain)) +static __always_inline unsigned long get_stack_pointer(struct task_struct *task, + struct pt_regs *regs) +{ + if (regs) + return (unsigned long) kernel_stack_pointer(regs); + if (task == current) + return current_frame_address(); + return (unsigned long) task->thread.ksp; +} + /* * To keep this simple mark register 2-6 as being changed (volatile) * by the called function, even though register 6 is saved/nonvolatile. diff --git a/arch/s390/include/asm/unwind.h b/arch/s390/include/asm/unwind.h index de9006b0cfeb..5ebf534ef753 100644 --- a/arch/s390/include/asm/unwind.h +++ b/arch/s390/include/asm/unwind.h @@ -55,10 +55,10 @@ static inline bool unwind_error(struct unwind_state *state) return state->error; } -static inline void unwind_start(struct unwind_state *state, - struct task_struct *task, - struct pt_regs *regs, - unsigned long first_frame) +static __always_inline void unwind_start(struct unwind_state *state, + struct task_struct *task, + struct pt_regs *regs, + unsigned long first_frame) { task = task ?: current; first_frame = first_frame ?: get_stack_pointer(task, regs); -- 2.25.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] s390/unwind: use current_frame_address() to unwind current task 2021-09-01 14:05 ` [PATCH] s390/unwind: use current_frame_address() to unwind current task Vasily Gorbik @ 2021-09-01 17:51 ` Marco Elver 2021-09-01 18:07 ` Heiko Carstens 2021-09-03 23:23 ` Nathan Chancellor 1 sibling, 1 reply; 15+ messages in thread From: Marco Elver @ 2021-09-01 17:51 UTC (permalink / raw) To: Vasily Gorbik Cc: Heiko Carstens, Christian Borntraeger, Nathan Chancellor, Linus Torvalds, linux-s390, linux-kernel, llvm, qemu-s390x On Wed, 1 Sept 2021 at 16:06, Vasily Gorbik <gor@linux.ibm.com> wrote: > current_stack_pointer() simply returns current value of %r15. If > current_stack_pointer() caller allocates stack (which is the case in > unwind code) %r15 points to a stack frame allocated for callees, meaning > current_stack_pointer() caller (e.g. stack_trace_save) will end up in > the stacktrace. This is not expected by stack_trace_save*() callers and > causes problems. > > current_frame_address() on the other hand returns function stack frame > address, which matches %r15 upon function invocation. Using it in > get_stack_pointer() makes it more aligned with x86 implementation > (according to BACKTRACE_SELF_TEST output) and meets stack_trace_save*() > caller's expectations, notably KCSAN. > > Also make sure unwind_start is always inlined. > > Reported-by: Nathan Chancellor <nathan@kernel.org> > Suggested-by: Marco Elver <elver@google.com> > Signed-off-by: Vasily Gorbik <gor@linux.ibm.com> Tested-by: Marco Elver <elver@google.com> Thanks! > --- > arch/s390/include/asm/stacktrace.h | 20 ++++++++++---------- > arch/s390/include/asm/unwind.h | 8 ++++---- > 2 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/arch/s390/include/asm/stacktrace.h b/arch/s390/include/asm/stacktrace.h > index 3d8a4b94c620..22c41d7fd95c 100644 > --- a/arch/s390/include/asm/stacktrace.h > +++ b/arch/s390/include/asm/stacktrace.h > @@ -34,16 +34,6 @@ static inline bool on_stack(struct stack_info *info, > return addr >= info->begin && addr + len <= info->end; > } > > -static __always_inline unsigned long get_stack_pointer(struct task_struct *task, > - struct pt_regs *regs) > -{ > - if (regs) > - return (unsigned long) kernel_stack_pointer(regs); > - if (task == current) > - return current_stack_pointer(); > - return (unsigned long) task->thread.ksp; > -} > - > /* > * Stack layout of a C stack frame. > */ > @@ -74,6 +64,16 @@ struct stack_frame { > ((unsigned long)__builtin_frame_address(0) - \ > offsetof(struct stack_frame, back_chain)) > > +static __always_inline unsigned long get_stack_pointer(struct task_struct *task, > + struct pt_regs *regs) > +{ > + if (regs) > + return (unsigned long) kernel_stack_pointer(regs); > + if (task == current) > + return current_frame_address(); > + return (unsigned long) task->thread.ksp; > +} > + > /* > * To keep this simple mark register 2-6 as being changed (volatile) > * by the called function, even though register 6 is saved/nonvolatile. > diff --git a/arch/s390/include/asm/unwind.h b/arch/s390/include/asm/unwind.h > index de9006b0cfeb..5ebf534ef753 100644 > --- a/arch/s390/include/asm/unwind.h > +++ b/arch/s390/include/asm/unwind.h > @@ -55,10 +55,10 @@ static inline bool unwind_error(struct unwind_state *state) > return state->error; > } > > -static inline void unwind_start(struct unwind_state *state, > - struct task_struct *task, > - struct pt_regs *regs, > - unsigned long first_frame) > +static __always_inline void unwind_start(struct unwind_state *state, > + struct task_struct *task, > + struct pt_regs *regs, > + unsigned long first_frame) > { > task = task ?: current; > first_frame = first_frame ?: get_stack_pointer(task, regs); > -- > 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] s390/unwind: use current_frame_address() to unwind current task 2021-09-01 17:51 ` Marco Elver @ 2021-09-01 18:07 ` Heiko Carstens 0 siblings, 0 replies; 15+ messages in thread From: Heiko Carstens @ 2021-09-01 18:07 UTC (permalink / raw) To: Marco Elver Cc: Vasily Gorbik, Christian Borntraeger, Nathan Chancellor, Linus Torvalds, linux-s390, linux-kernel, llvm, qemu-s390x On Wed, Sep 01, 2021 at 07:51:06PM +0200, Marco Elver wrote: > On Wed, 1 Sept 2021 at 16:06, Vasily Gorbik <gor@linux.ibm.com> wrote: > > current_stack_pointer() simply returns current value of %r15. If > > current_stack_pointer() caller allocates stack (which is the case in > > unwind code) %r15 points to a stack frame allocated for callees, meaning > > current_stack_pointer() caller (e.g. stack_trace_save) will end up in > > the stacktrace. This is not expected by stack_trace_save*() callers and > > causes problems. > > > > current_frame_address() on the other hand returns function stack frame > > address, which matches %r15 upon function invocation. Using it in > > get_stack_pointer() makes it more aligned with x86 implementation > > (according to BACKTRACE_SELF_TEST output) and meets stack_trace_save*() > > caller's expectations, notably KCSAN. > > > > Also make sure unwind_start is always inlined. > > > > Reported-by: Nathan Chancellor <nathan@kernel.org> > > Suggested-by: Marco Elver <elver@google.com> > > Signed-off-by: Vasily Gorbik <gor@linux.ibm.com> > > Tested-by: Marco Elver <elver@google.com> > > Thanks! > > > --- > > arch/s390/include/asm/stacktrace.h | 20 ++++++++++---------- > > arch/s390/include/asm/unwind.h | 8 ++++---- > > 2 files changed, 14 insertions(+), 14 deletions(-) Applied, thanks! ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] s390/unwind: use current_frame_address() to unwind current task 2021-09-01 14:05 ` [PATCH] s390/unwind: use current_frame_address() to unwind current task Vasily Gorbik 2021-09-01 17:51 ` Marco Elver @ 2021-09-03 23:23 ` Nathan Chancellor 1 sibling, 0 replies; 15+ messages in thread From: Nathan Chancellor @ 2021-09-03 23:23 UTC (permalink / raw) To: Vasily Gorbik Cc: Marco Elver, Heiko Carstens, Christian Borntraeger, Linus Torvalds, linux-s390, linux-kernel, llvm, qemu-s390x On Wed, Sep 01, 2021 at 04:05:59PM +0200, Vasily Gorbik wrote: > current_stack_pointer() simply returns current value of %r15. If > current_stack_pointer() caller allocates stack (which is the case in > unwind code) %r15 points to a stack frame allocated for callees, meaning > current_stack_pointer() caller (e.g. stack_trace_save) will end up in > the stacktrace. This is not expected by stack_trace_save*() callers and > causes problems. > > current_frame_address() on the other hand returns function stack frame > address, which matches %r15 upon function invocation. Using it in > get_stack_pointer() makes it more aligned with x86 implementation > (according to BACKTRACE_SELF_TEST output) and meets stack_trace_save*() > caller's expectations, notably KCSAN. > > Also make sure unwind_start is always inlined. > > Reported-by: Nathan Chancellor <nathan@kernel.org> > Suggested-by: Marco Elver <elver@google.com> > Signed-off-by: Vasily Gorbik <gor@linux.ibm.com> Sorry for the late response and I see that this has already been applied but I took this for a spin and all of the tests pass with clang-14 in QEMU. Thank you for the quick fix so that we can get this turned on in CI :) [ 10.362073] ok 1 - test_basic [ 13.870386] ok 2 - test_concurrent_races [ 17.379643] ok 3 - test_novalue_change [ 17.393315] ok 4 - test_novalue_change_exception [ 17.409815] ok 5 - test_unknown_origin [ 20.914289] ok 6 - test_write_write_assume_atomic [ 20.982545] ok 7 - test_write_write_struct [ 21.106135] ok 8 - test_write_write_struct_part [ 24.622205] ok 9 - test_read_atomic_write_atomic [ 24.662048] ok 10 - test_read_plain_atomic_write [ 24.775291] ok 11 - test_read_plain_atomic_rmw [ 28.294457] ok 12 - test_zero_size_access [ 31.829529] ok 13 - test_data_race [ 31.867174] ok 14 - test_assert_exclusive_writer [ 31.929184] ok 15 - test_assert_exclusive_access [ 35.446281] ok 16 - test_assert_exclusive_access_writer [ 35.540228] ok 17 - test_assert_exclusive_bits_change [ 39.052271] ok 18 - test_assert_exclusive_bits_nochange [ 39.097020] ok 19 - test_assert_exclusive_writer_scoped [ 39.152914] ok 20 - test_assert_exclusive_access_scoped [ 42.675158] ok 21 - test_jiffies_noreport [ 46.192453] ok 22 - test_seqlock_noreport [ 49.712712] ok 23 - test_atomic_builtins [ 49.746428] ok 24 - test_1bit_value_change [ 49.753316] ok 1 - kcsan Tested-by: Nathan Chancellor <nathan@kernel.org> > --- > arch/s390/include/asm/stacktrace.h | 20 ++++++++++---------- > arch/s390/include/asm/unwind.h | 8 ++++---- > 2 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/arch/s390/include/asm/stacktrace.h b/arch/s390/include/asm/stacktrace.h > index 3d8a4b94c620..22c41d7fd95c 100644 > --- a/arch/s390/include/asm/stacktrace.h > +++ b/arch/s390/include/asm/stacktrace.h > @@ -34,16 +34,6 @@ static inline bool on_stack(struct stack_info *info, > return addr >= info->begin && addr + len <= info->end; > } > > -static __always_inline unsigned long get_stack_pointer(struct task_struct *task, > - struct pt_regs *regs) > -{ > - if (regs) > - return (unsigned long) kernel_stack_pointer(regs); > - if (task == current) > - return current_stack_pointer(); > - return (unsigned long) task->thread.ksp; > -} > - > /* > * Stack layout of a C stack frame. > */ > @@ -74,6 +64,16 @@ struct stack_frame { > ((unsigned long)__builtin_frame_address(0) - \ > offsetof(struct stack_frame, back_chain)) > > +static __always_inline unsigned long get_stack_pointer(struct task_struct *task, > + struct pt_regs *regs) > +{ > + if (regs) > + return (unsigned long) kernel_stack_pointer(regs); > + if (task == current) > + return current_frame_address(); > + return (unsigned long) task->thread.ksp; > +} > + > /* > * To keep this simple mark register 2-6 as being changed (volatile) > * by the called function, even though register 6 is saved/nonvolatile. > diff --git a/arch/s390/include/asm/unwind.h b/arch/s390/include/asm/unwind.h > index de9006b0cfeb..5ebf534ef753 100644 > --- a/arch/s390/include/asm/unwind.h > +++ b/arch/s390/include/asm/unwind.h > @@ -55,10 +55,10 @@ static inline bool unwind_error(struct unwind_state *state) > return state->error; > } > > -static inline void unwind_start(struct unwind_state *state, > - struct task_struct *task, > - struct pt_regs *regs, > - unsigned long first_frame) > +static __always_inline void unwind_start(struct unwind_state *state, > + struct task_struct *task, > + struct pt_regs *regs, > + unsigned long first_frame) > { > task = task ?: current; > first_frame = first_frame ?: get_stack_pointer(task, regs); > -- > 2.25.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-09-03 23:24 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-30 13:11 [GIT PULL] s390 updates for 5.15 merge window Heiko Carstens 2021-08-30 20:17 ` pr-tracker-bot 2021-08-31 2:19 ` Nathan Chancellor 2021-08-31 7:09 ` Christian Borntraeger 2021-08-31 10:13 ` Heiko Carstens 2021-08-31 10:46 ` Marco Elver 2021-08-31 15:02 ` Marco Elver 2021-08-31 15:18 ` Heiko Carstens 2021-08-31 17:48 ` Nathan Chancellor 2021-08-31 17:49 ` Christian Borntraeger 2021-09-01 14:03 ` Vasily Gorbik 2021-09-01 14:05 ` [PATCH] s390/unwind: use current_frame_address() to unwind current task Vasily Gorbik 2021-09-01 17:51 ` Marco Elver 2021-09-01 18:07 ` Heiko Carstens 2021-09-03 23:23 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).