Re: [PATCH v1] s390x/tcg: Implement MONITOR CALL

[Richard Henderson <richard.henderson@linaro.org>]

On 9/17/20 4:54 AM, David Hildenbrand wrote:
> Recent upstream Linux uses the MONITOR CALL instruction for things like
> BUG_ON() and WARN_ON(). We currently inject an operation exception when
> we hit a MONITOR CALL instruction - which is wrong, as the instruction
> is not glued to specific CPU features.
> 
> Doing a simple WARN_ON_ONCE() currently results in a panic:
>   [   18.162801] illegal operation: 0001 ilc:2 [#1] SMP
>   [   18.162889] Modules linked in:
>   [...]
>   [   18.165476] Kernel panic - not syncing: Fatal exception: panic_on_oops
> 
> With a proper implementation, we now get:
>   [   18.242754] ------------[ cut here ]------------
>   [   18.242855] WARNING: CPU: 7 PID: 1 at init/main.c:1534 [...]
>   [   18.242919] Modules linked in:
>   [...]
>   [   18.246262] ---[ end trace a420477d71dc97b4 ]---
>   [   18.259014] Freeing unused kernel memory: 4220K
> 
> To be able to translate it to a NOP easily, mangle the 16 monitor masks
> bits from the cr8 into the TB flags.

This is a rare situation that does not warrant the use of TB flags.  Better to
unconditionally call helper_monitor_event, and have the helper function test
the runtime value of cr8.  If the event is disabled, the helper simply returns.

It should be simpler to write, as well, not having to do this:

> +    /* Copy over the monitor mask bits (16) as two separate bytes. */
> +    byte = (env->cregs[8] & CR8_MONITOR_MASK) >> 8;
> +    *flags |= (uint32_t)byte << FLAG_SHIFT_MM0_7;
> +    byte = env->cregs[8] & CR8_MONITOR_MASK;
> +    *flags |= (uint32_t)byte << FLAG_SHIFT_MM8_15;
> +
> +    QEMU_BUILD_BUG_ON((FLAG_MASK_AFP | FLAG_MASK_VECTOR | FLAG_MASK_MM0_7 |
> +                       FLAG_MASK_MM8_15) & FLAG_MASK_PSW);


r~