Re: [qemu-s390x] [Qemu-devel] [PATCH v3 3/7] s390x: improve error handling for SSCH and RSCH

[Thomas Huth]

On 18.10.2017 11:30, Thomas Huth wrote:
> On 17.10.2017 16:04, Halil Pasic wrote:
>> Simplify the error handling of the SSCH and RSCH handler avoiding
>> arbitrary and cryptic error codes being used to tell how the instruction
>> is supposed to end.  Let the code detecting the condition tell how it's
>> to be handled in a less ambiguous way.  It's best to handle SSCH and RSCH
>> in one go as the emulation of the two shares a lot of code.
>>
>> For passthrough this change isn't pure refactoring, but changes the way
>> kernel reported EFAULT is handled. After clarifying the kernel interface
>> we decided that EFAULT shall be mapped to unit exception.  Same goes for
>> unexpected error codes and absence of required ORB flags.
>>
>> Signed-off-by: Halil Pasic <address@hidden>
[...]
>> @@ -71,10 +71,24 @@ again:
>>              goto again;
>>          }
>>          error_report("vfio-ccw: wirte I/O region failed with errno=%d", 
>> errno);
>> -        return -errno;
>> +        ret = -errno;
>> +    } else {
>> +        ret = region->ret_code;
>> +    }
>> +    switch (-ret) {
>> +    case 0:
>> +        return IOINST_CC_EXPECTED;
>> +    case EBUSY:
>> +        return IOINST_CC_BUSY;
>> +    case ENODEV:
>> +    case EACCES:
>> +        return IOINST_CC_NOT_OPERATIONAL;
>> +    case EFAULT:
>> +    default:
>> +        sch_gen_unit_exception(sch);
>> +        css_inject_io_interrupt(sch);
>> +        return IOINST_CC_EXPECTED;
> 
> Do we feel really confident that it is OK to do the setcc() in case of
> an exception here later? ... otherwise it might be necessery to
> introduce something like IOINST_EXCEPTION to the enum to signal the
> ioinst_handle_xxx() callers that they should not do the setcc() anymore...

... or maybe rather at least return IOINST_CC_STATUS_PRESENT instead?
IOINST_CC_EXPECTED sounds somewhat wrong to me here.

 Thomas