linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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	[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	[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).