Re: [Qemu-devel] [qemu-s390x] [PATCH v5 11/12] s390-ccw: clear pending irqs

[Thomas Huth <thuth@redhat.com>]

On 05.02.2018 21:57, Collin L. Walling wrote:
> It is possible while waiting for multiple types of external
> interrupts that we might have pending irqs remaining between
> irq consumption and irq disabling. Those interrupts could
> propagate to the guest after IPL completes and cause unwanted
> behavior.
> 
> To avoid this, we clear the write event mask to prevent further
> service interrupts from ASCII events and then consume all pending
> irqs for a miniscule duration. Once finished, we reset the write
> event mask and resume business as usual.
> 
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> ---
>  pc-bios/s390-ccw/menu.c | 16 ++++++++++++++++
>  pc-bios/s390-ccw/sclp.c | 12 ++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
> index 85d285f..971f6b6 100644
> --- a/pc-bios/s390-ccw/menu.c
> +++ b/pc-bios/s390-ccw/menu.c
> @@ -64,6 +64,20 @@ static inline bool check_clock_int(void)
>      return *code == 0x1004;
>  }
>  
> +static void clear_pending_irqs(void)
> +{
> +    uint64_t time = 50 * TOD_CLOCK_SECOND / 0x3e8;

s/0x3e8/1000/ please.

> +    sclp_clear_write_mask();
> +
> +    set_clock_comparator(get_clock() + time);
> +    enable_clock_int();
> +    consume_sclp_int();
> +    disable_clock_int();
> +
> +    sclp_setup(); /* re-enable write mask */
> +}

I'm pretty much confused by this code. First, isn't there a small chance
that there is a clock int between consume_sclp_int() and
disable_clock_int() (if consume_sclp_int() has consumed an SCLP
interrupt instead of a clock interrupt) ?
Second, if you finally enable the SCLP write mask again, doesn't that
mean that there could be interrupts pending again afterwards?

 Thomas