[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [Qemu-devel] [PATCH v3 2/7] s390x/css: IO instr handler
From: |
Halil Pasic |
Subject: |
Re: [qemu-s390x] [Qemu-devel] [PATCH v3 2/7] s390x/css: IO instr handler ending control |
Date: |
Tue, 17 Oct 2017 18:13:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 10/17/2017 04:58 PM, Cornelia Huck wrote:
> On Tue, 17 Oct 2017 16:04:48 +0200
> Halil Pasic <address@hidden> wrote:
>
>> CSS code needs to tell the IO instruction handlers located in how should
>> the emulated instruction be ended. Currently this is done by returning
>> generic (POSIX) error codes, and mapping them to outcomes like condition
>> codes. This makes bugs easy to create and hard to recognise.
also
s/recognise/recognize/
To on mix American and British.
>>
>> As a preparation for moving a way form (mis)using generic error codes for
>
> s/form/from/
>
Sorry this seems to be a recurring typo of me (not detected by spell check
because both valid).
>> flow control let us introduce a type which tells the instruction
>> handler function how to end the instruction, in a more straight-forward
>> and less ambiguous way.
>>
>> Signed-off-by: Halil Pasic <address@hidden>
>> ---
>> include/hw/s390x/css.h | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
>> index 69b374730e..7e0dbd162f 100644
>> --- a/include/hw/s390x/css.h
>> +++ b/include/hw/s390x/css.h
>> @@ -99,6 +99,22 @@ typedef struct CcwDataStream {
>> hwaddr cda;
>> } CcwDataStream;
>>
>> +/*
>> + * IO instructions conclude according this. Currently we have only
>
> s/this/to this/
>
>> + * cc codes. Valid values are 0,1,2,3 and the generic semantic for
>
> blanks between numbers?
Overrated. Just joking. Definitely blanks between numbers.
>
>> + * IO instructions is described briefly. For more details consult the PoP.
>> + */
>> +typedef enum IOInstEnding {
>> + /* produced expected result */
>> + IOINST_CC_EXPECTED = 0,
>> + /* status conditions were present or produced alternate result */
>> + IOINST_CC_STATUS_PRESENT = 1,
>> + /* inst. ineffective because busy with previously initiated function */
>> + IOINST_CC_BUSY = 2,
>> + /* inst. ineffective because not operational */
>> + IOINST_CC_NOT_OPERATIONAL = 3
>> +} IOInstEnding;
>
> This looks a bit odd for some I/O instructions (STCRW, TPI, TSCH), but
> is fine for the others. But as the PoP also defines the meanings as
> above, it should be fine (and not confusing).
Yeah. The original idea was to keep stuff abstract, but I decided
to go with the names carrying meaning because Thomas seems to
be more reliable than me when it comes to matters of taste, and also
for consistency with SIGP_CC_*. I think, in the end it does not matter
that much.
>
>> +
>> typedef struct SubchDev SubchDev;
>> struct SubchDev {
>> /* channel-subsystem related things: */
>
>