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

["Collin L. Walling" <walling@linux.vnet.ibm.com>]

On 02/19/2018 09:17 AM, Christian Borntraeger wrote:
>
> 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 <walling@linux.vnet.ibm.com>
>> 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?
>
>
Correct.  We don't need that print there.  Setting the mask kills two 
birds with one stone.

-- 
- Collin L Walling