On Wed, 8 Jun 2022 14:29:51 +0200
Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> wrote:
> gcc 12 warns if a memory operand to inline asm points to memory in the
> first 4k bytes. However, in our case, these operands are fine, either
> because we actually want to use that memory, or expect and handle the
> resulting exception.
thanks, queued
>
> v2 -> v3
> * extend commit msg
> * pick up r-b
> * use macro instead of pointer to address memory
>
> v1 -> v2
> * replace mechanism, don't use pragmas, instead use an extern symbol so
> gcc cannot conclude that the pointer is <4k
>
> This new extern symbol refers to the lowcore. As a result, code
> generation for lowcore accesses becomes worse.
>
> Alternatives:
> * Don't use extern symbol for lowcore, just for problematic pointers
> * Hide value from gcc via inline asm
> * Disable the warning globally
> * Use memory clobber instead of memory output
> Use address in register input instead of memory input
> (may require WRITE_ONCE)
> * Don't use gcc 12.0, with newer versions --param=min-pagesize=0 might
> avoid the problem
>
> Janis Schoetterl-Glausch (2):
> s390x: Introduce symbol for lowcore and use it
> s390x: Fix gcc 12 warning about array bounds
>
> lib/s390x/asm/arch_def.h | 2 ++
> lib/s390x/asm/facility.h | 4 +--
> lib/s390x/asm/mem.h | 4 +++
> lib/s390x/css.h | 2 --
> lib/s390x/css_lib.c | 12 ++++----
> lib/s390x/fault.c | 10 +++----
> lib/s390x/interrupt.c | 61 +++++++++++++++++++-------------------
> lib/s390x/mmu.c | 3 +-
> s390x/flat.lds | 1 +
> s390x/snippets/c/flat.lds | 1 +
> s390x/css.c | 4 +--
> s390x/diag288.c | 4 +--
> s390x/edat.c | 5 ++--
> s390x/emulator.c | 15 +++++-----
> s390x/mvpg.c | 7 ++---
> s390x/sclp.c | 3 +-
> s390x/skey.c | 2 +-
> s390x/skrf.c | 11 +++----
> s390x/smp.c | 23 +++++++-------
> s390x/snippets/c/spec_ex.c | 5 ++--
> 20 files changed, 83 insertions(+), 96 deletions(-)
>
> Range-diff against v2:
> 1: 412a9962 ! 1: 44b10d41 s390x: Introduce symbol for lowcore and use it
> @@ Commit message
> The new symbol is not a pointer. While this will lead to worse code
> generation (cannot use register 0 for addressing), that should not
> matter too much for kvm unit tests.
> + Since the lowcore is located per definition at address 0, the symbol is
> + defined via the linker scripts.
> The symbol also will be used to create pointers that the compiler cannot
> warn about as being outside the bounds of an array.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
> + Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
>
> ## lib/s390x/asm/arch_def.h ##
> @@ lib/s390x/asm/arch_def.h: struct lowcore {
> 2: 9b2eeee3 ! 2: e9e88996 s390x: Fix gcc 12 warning about array bounds
> @@ lib/s390x/asm/mem.h
> #define _ASMS390X_MEM_H_
> +#include <asm/arch_def.h>
> +
> -+/* pointer to 0 used to avoid compiler warnings */
> -+uint8_t *mem_all = (uint8_t *)&lowcore;
> ++/* create pointer while avoiding compiler warnings */
> ++#define OPAQUE_PTR(x) ((void *)(((uint64_t)&lowcore) + (x)))
>
> #define SKEY_ACC 0xf0
> #define SKEY_FP 0x08
> @@ s390x/emulator.c: static __always_inline void __test_cpacf_invalid_parm(unsigned
> report_prefix_push("invalid parm address");
> expect_pgm_int();
> - __cpacf_query(opcode, (void *) -1);
> -+ __cpacf_query(opcode, (cpacf_mask_t *)&mem_all[-1]);
> ++ __cpacf_query(opcode, OPAQUE_PTR(-1));
> check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
> report_prefix_pop();
> }
> @@ s390x/emulator.c: static __always_inline void __test_cpacf_protected_parm(unsign
> expect_pgm_int();
> low_prot_enable();
> - __cpacf_query(opcode, (void *) 8);
> -+ __cpacf_query(opcode, (cpacf_mask_t *)&mem_all[8]);
> ++ __cpacf_query(opcode, OPAQUE_PTR(8));
> low_prot_disable();
> check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> report_prefix_pop();
> @@ s390x/skey.c: static void test_set_prefix(void)
> expect_pgm_int();
> install_page(root, virt_to_pte_phys(root, pagebuf), 0);
> - set_prefix_key_1((uint32_t *)2048);
> -+ set_prefix_key_1((uint32_t *)&mem_all[2048]);
> ++ set_prefix_key_1(OPAQUE_PTR(2048));
> install_page(root, 0, 0);
> check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> report(get_prefix() == old_prefix, "did not set prefix");
>
> base-commit: 2eed0bf1096077144cc3a0dd9974689487f9511a
