qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] S390 bios breaks in qemu 2.10.rc3


From: Halil Pasic
Subject: Re: [Qemu-devel] S390 bios breaks in qemu 2.10.rc3
Date: Thu, 24 Aug 2017 17:37:15 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0


On 08/24/2017 05:13 PM, Cornelia Huck wrote:
> On Thu, 24 Aug 2017 11:05:08 -0400
> Farhan Ali <address@hidden> wrote:
> 
>> Hi,
>>
>> There is an issue in QEMU bios which is exposed by commit
>>
>> commit 198c0d1f9df8c429502cb744fc26b6ba6e71db74
>> Author: Halil Pasic <address@hidden>
>> Date:   Thu Jul 27 17:48:42 2017 +0200
>>
>>      s390x/css: check ccw address validity
>>
>>      According to the PoP channel command words (CCW) must be doubleword
>>      aligned and 31 bit addressable for format 1 and 24 bit addressable for
>>      format 0 CCWs.
>>
>>      If the channel subsystem encounters a ccw address which does not 
>> satisfy
>>      this alignment requirement a program-check condition is recognised.
>>
>>      The situation with 31 bit addressable is a bit more complicated: 
>> both the
>>      ORB and a format 1 CCW TIC hold the address of (the rest of) the 
>> channel
>>      program, that is the address of the next CCW in a word, and the PoP
>>      mandates that bit 0 of that word shall be zero -- or a program-check
>>      condition is to be recognized -- and does not belong to the field 
>> holding
>>      the ccw address.
>>
>>      Since in code the corresponding fields span across the whole word 
>> (unlike
>>      in PoP where these are defined as 31 bit wide) we can check this by
>>      applying a mask. The 24 addressable case isn't affecting TIC 
>> because the
>>      address is composed of a halfword and a byte portion (no additional 
>> zero
>>      bit requirements) and just slightly complicates the ORB case where also
>>      bits 1-7 need to be zero.
>>
>>      The same requirements (especially n-bit addressability) apply to the
>>      ccw addresses generated while chaining.
>>
>>      Let's make our CSS implementation follow the AR more closely.
>>
>>      Signed-off-by: Halil Pasic <address@hidden>
>>      Message-Id: <address@hidden>
>>      Reviewed-by: Dong Jia Shi <address@hidden>
>>      Signed-off-by: Cornelia Huck <address@hidden>
>>
>>
>> It looks like the bios does not create a double word aligned CCW. 
>> Looking at the bios code we the CCW1 struct is not aligned
>>
>> /* channel command word (type 1) */
>> struct ccw1 {
>>      __u8 cmd_code;
>>      __u8 flags;
>>      __u16 count;
>>      __u32 cda;
>> } __attribute__ ((packed));
>>
>> and it looks like the compiler does not guarantee a doubleword alignment.
> 
> :(
> 
>>
>> The weird thing about it is I see it break in one of my system and works 
>> fine in another system. Trying a simple fix of aligning the struct also 
>> doesn't seem to work all the time.
> 
> I have not seen this problem on any of the systems I tested on (well, I
> would not have merged this if I did...) - RHEL 7 and F26.

Same here, otherwise I would have mane it a series and posted a fix
first.

> Do we need a
> dynamic allocation to guarantee alignment?
> 

I think a dynamic allocation is one way to get around the
problem but it should be possible to keep the stuff on the
stack. Telling the compiler about the alignment requirement
should do the trick IMHO.

@Farhan:
Since it's your find you have precedence when it comes to
fixing. Are you willing to submit a fix?

Halil




reply via email to

[Prev in Thread] Current Thread [Next in Thread]