Re: [kvm-unit-tests PATCH v3 0/2] s390x: Avoid gcc 12 warnings

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Wed,  8 Jun 2022 14:29:51 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> 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@linux.ibm.com>
>     +    Reviewed-by: Thomas Huth <thuth@redhat.com>
>      
>       ## 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