Re: [PATCH 7/7] s390/cio: Remove vfio-ccw checks of command codes

[Eric Farman <farman@linux.ibm.com>]

On 5/14/19 10:29 AM, Cornelia Huck wrote:
> On Fri, 10 May 2019 10:24:31 -0400
> Eric Farman <farman@linux.ibm.com> wrote:
> 
>> On 5/10/19 7:47 AM, Cornelia Huck wrote:
>>> On Wed, 8 May 2019 11:22:07 +0200
>>> Pierre Morel <pmorel@linux.ibm.com> wrote:
>>>    
>>>> For the NOOP its clearly stated that it does not start a data transfer.
>>>> If we pin the CDA, it could then eventually be the cause of errors if
>>>> the address indicated by the CDA is not accessible.
>>>>
>>>> The NOOP is a particular CONTROL operation for which no data is transfered.
>>>> Other CONTROL operation may start a data transfer.
>>>
>>> I've just looked at the documentation again.
>>>
>>> The Olde Common I/O Device Commands document indicates that a NOOP
>>> simply causes channel end/device end.
>>>
>>> The PoP seems to indicate that the cda is always checked (i.e. does it
>>> point to a valid memory area?), but I'm not sure whether the area that
>>> is pointed to is checked for accessibility etc. as well, even if the
>>> command does not transfer any data.
>>>
>>> Has somebody tried to find out what happens on Real Hardware(tm) if you
>>> send a command that is not supposed to transfer any data where the cda
>>> points to a valid, but not accessible area?
>>
>> Hrm...  The CDA itself?  I don't think so.  Since every CCW is converted
>> to an IDAL in vfio-ccw, we guarantee that it's pointing to something
>> valid at that point.
>>
>> So, I hacked ccwchain_fetch_direct() to NOT set the IDAL flag in a NOP
>> CCW, and to leave the CDA alone.  This means it will still contain the
>> guest address, which is risky but hey it's a test system.  :)  (I
>> offline'd a bunch of host memory too, to make sure I had some
>> unavailable addresses.)
>>
>> In my traces, the non-IDA NOP CCWs were issued to the host with and
>> without the skip flag, with zero and non-zero counts, and with zero and
>> non-zero CDAs.  All of them work just fine, including the ones who's
>> addresses fall into the offline space.  Even the combination of no skip,
>> non-zero count, and zero cda.
>>
>> I modified that hack to do the same for a known invalid control opcode,
>> and it seemed to be okay too.  We got an (expected) invalid command
>> before we noticed any problem with the provided address.
> 
> That's interesting; I would not have arrived at this by interpreting
> the PoP...
> 
>>>
>>> In general, I think doing the translation (and probably already hitting
>>> errors there) is better than sending down a guest address.
>>>    
>>
>> I mostly agree, but I have one test program that generates invalid GUEST
>> addresses with its NOP CCWs, since it doesn't seem to care about whether
>> they're valid or not.  So any attempt to pin them will end badly, which
>> is why I call that opcode out in ccw_does_data_transfer(), and just send
>> invalid IDAWs with it.
> 
> So, without the attempt to pin they do not fail? 

Correct.

> Maybe the right
> approach would be to rewrite the cda before sending the ccws?
> 

That would be my vote.  (And it's what this series does.  :)