Re: [qemu-s390x] [PATCH v7 11/12] s390-ccw: set cp_receive mask only when needed and consume pending service irqs

[Christian Borntraeger]

On 02/19/2018 03:15 PM, Christian Borntraeger wrote:
> 
> 
> On 02/16/2018 11:07 PM, 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-type disabling. Those interrupts
>> could potentially propagate to the guest after IPL completes
>> and cause unwanted behavior.
>>
>> As it is today, the SCLP will only recognize write events that
>> are enabled by the control program's send and receive masks. To
>> limit the window for, and prevent further irqs from, ASCII
>> console events (specifically keystrokes), we should only enable
>> the control program's receive mask when we need it.
>>
>> As an additional measure, once we've disabled/cleared the control
>> program's receive mask, we will print an empty string in order
>> to consume any pending service interrupt.
>>
>> Signed-off-by: Collin L. Walling <address@hidden>
> 
> This should work some comments below:
> 
>> ---
>>  pc-bios/s390-ccw/menu.c     |  5 +++++
>>  pc-bios/s390-ccw/s390-ccw.h |  1 +
>>  pc-bios/s390-ccw/sclp.c     | 10 ++++------
>>  3 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
>> index 9601043..14410a8 100644
>> --- a/pc-bios/s390-ccw/menu.c
>> +++ b/pc-bios/s390-ccw/menu.c
>> @@ -11,6 +11,7 @@
>>
>>  #include "menu.h"
>>  #include "s390-ccw.h"
>> +#include "sclp.h"
>>
>>  #define KEYCODE_NO_INP '\0'
>>  #define KEYCODE_ESCAPE '\033'
>> @@ -117,8 +118,12 @@ static int get_index(void)
>>
>>      memset(buf, 0, sizeof(buf));
>>
>> +    sclp_set_write_mask(SCLP_EVENT_MASK_MSG_ASCII, 
>> SCLP_EVENT_MASK_MSG_ASCII);
>>      len = read_prompt(buf, sizeof(buf));
>>
>> +    sclp_set_write_mask(0, SCLP_EVENT_MASK_MSG_ASCII);
>> +    sclp_print(""); /* Clear any pending service int */
>> +
> 
> Why cant you use consume_sclp_int from start.S and not do any printing?
> Shouldnt sclp_set_write_mask always make an interrupt pending?

In fact sclp_set_write_mask should already call consume_sclp_int. Have you seen 
spurious
interrupts?