On 18.10.2017 11:58, Cornelia Huck wrote:
> On Wed, 18 Oct 2017 11:52:03 +0200
> Thomas Huth <th...@redhat.com> wrote:
>
>> 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 <pa...@linux.vnet.ibm.com>
>> [...]
>>>> @@ -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.
>
> But the ssch did conclude as expected :)
>
> Keep in mind that QEMU performs the start function synchronously (i.e.,
> before the condition code is set). On real hardware, you get a cc 0 for
> the ssch if the subchannel is basically in a status that's ok for
> triggering the start function. A unit exception is a possible result of
> the start function (and therefore generating an I/O interrupt, which
> you only get if ssch set cc 0.)
OK, I'm not that familiar with the I/O sub-system. If you say that it is
ok, then simply ignore my comment, please :-)
Thomas
