Re: [Qemu-devel] [PATCH] s390-ccw: Fix alignment for CCW1

[Christian Borntraeger]

On 08/28/2017 03:18 PM, Farhan Ali wrote:
> 
> 
> On 08/28/2017 09:06 AM, Cornelia Huck wrote:
>> On Mon, 28 Aug 2017 08:56:42 -0400
>> Farhan Ali <address@hidden> wrote:
>>
>>> On 08/28/2017 04:22 AM, Cornelia Huck wrote:
>>>> On Fri, 25 Aug 2017 11:05:30 -0400
>>>> Farhan Ali <address@hidden> wrote:
>>>>
>>>>> On 08/25/2017 10:04 AM, Cornelia Huck wrote:
>>>>>> On Fri, 25 Aug 2017 09:24:46 -0400
>>>>>> Farhan Ali <address@hidden> wrote:
>>>>>>
>>>>>>> The commit 198c0d1f9df8c4 s390x/css: check ccw address validity
>>>>>>> exposes an alignment issue in ccw bios.
>>>>>>>
>>>>>>> According to PoP the CCW must be doubleword aligned. Let's fix
>>>>>>> this in the bios.
>>>>>>>
>>>>>>> Cc: address@hidden
>>>>>>> Signed-off-by: Farhan Ali <address@hidden>
>>>>>>> Reviewed-by: Halil Pasic <address@hidden>
>>>>>>> Reviewed-by: Eric Farman <address@hidden>
>>>>>>> Acked-by: Christian Borntraeger <address@hidden>
>>>>>>> ---
>>>>>>>  pc-bios/s390-ccw/cio.h | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
>>>>>>> index f5b4549..55eaeee 100644
>>>>>>> --- a/pc-bios/s390-ccw/cio.h
>>>>>>> +++ b/pc-bios/s390-ccw/cio.h
>>>>>>> @@ -133,7 +133,7 @@ struct ccw1 {
>>>>>>>      __u8 flags;
>>>>>>>      __u16 count;
>>>>>>>      __u32 cda;
>>>>>>> -} __attribute__ ((packed));
>>>>>>> +} __attribute__ ((packed, aligned(8)));
>>>>>>>
>>>>>>>  #define CCW_FLAG_DC              0x80
>>>>>>>  #define CCW_FLAG_CC              0x40
>>>>>>
>>>>>> Currently testing.
>>>>>>
>>>>>> This looks obviously right, but did you figure out what the (probably
>>>>>> unrelated) other failure was?
>>>>>>
>>>>>
>>>>> That is still under investigation, for some reason it only fails for an
>>>>> LDL DASD and it works for SCSIs and CDL DASD.
>>>>
>>>> Which are the symptoms of the failure? I'd like to understand this
>>>> before I update the (currently working by accident) bios with an
>>>> updated version.
>>>>
>>>> I'll just apply the patch for now.
>>>>
>>>
>>> Well it's seems like the failure for LDL DASD could be a disk setup
>>> failure. We tried the test on a different environment with LDL disks and
>>> everything worked fine with the patch applied.
>>
>> Odd that it breaks after this change, though. Do you get command
>> rejects, or what happens?
>>
> 
> It's the alignment of the CCW which causes the problem.
> 
> The exact error message when starting the guest was:
> 
> ! No virtio device found !
> 
> Since it worked for SCSI and CDL, and failed for LDL disks on that particular 
> system, we are not really sure what caused the failure.
> Debugging it further showed the CCW for LDL disks were not aligned at double 
> word boundary.
> 
> Trying the test on a different system with LDL disks worked fine, with the 
> aligned(8) fix.

Do you happen to have an old s390-ccw.img laying around in the test folder? 
QEMU might pick up 
this one (e.g. when calling it without libvirt from the command line).