qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Farhan Ali <alifm@linux.vnet.ibm.com>
Cc: qemu-devel@nongnu.org, Halil Pasic <pasic@linux.vnet.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	"Collin L. Walling" <walling@linux.vnet.ibm.com>,
	Thomas Huth <thuth@redhat.com>,
	David Hildenbrand <david@redhat.com>
Subject: Re: [Qemu-devel] S390 bios breaks in qemu 2.10.rc3
Date: Thu, 24 Aug 2017 17:13:55 +0200	[thread overview]
Message-ID: <20170824171355.29d1ec32.cohuck@redhat.com> (raw)
In-Reply-To: <e2a1b182-83a9-2259-27f9-68b61b5061d5@linux.vnet.ibm.com>

On Thu, 24 Aug 2017 11:05:08 -0400
Farhan Ali <alifm@linux.vnet.ibm.com> wrote:

> Hi,
> 
> There is an issue in QEMU bios which is exposed by commit
> 
> commit 198c0d1f9df8c429502cb744fc26b6ba6e71db74
> Author: Halil Pasic <pasic@linux.vnet.ibm.com>
> 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 <pasic@linux.vnet.ibm.com>
>      Message-Id: <20170727154842.23427-1-pasic@linux.vnet.ibm.com>
>      Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>      Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> 
> 
> 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. Do we need a
dynamic allocation to guarantee alignment?

  reply	other threads:[~2017-08-24 15:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-24 15:05 [Qemu-devel] S390 bios breaks in qemu 2.10.rc3 Farhan Ali
2017-08-24 15:13 ` Cornelia Huck [this message]
2017-08-24 15:35   ` Thomas Huth
2017-08-24 15:47     ` Halil Pasic
2017-08-24 15:50       ` Thomas Huth
2017-08-24 15:53         ` Farhan Ali
2017-08-24 16:02           ` Halil Pasic
2017-08-24 18:15             ` Christian Borntraeger
2017-08-24 16:07           ` Peter Maydell
2017-08-24 17:38             ` Farhan Ali
2017-08-24 18:14               ` Christian Borntraeger
2017-08-25  7:20                 ` Cornelia Huck
2017-08-25  8:21                   ` Christian Borntraeger
2017-08-25  8:29                     ` Cornelia Huck
2017-08-28  7:18                       ` Christian Borntraeger
2017-08-29  9:35                         ` Thomas Huth
2017-08-29 10:28                           ` Christian Borntraeger
2017-08-31 17:44                           ` Michael Roth
2017-09-01  7:06                             ` Christian Ehrhardt
2017-09-01 14:03                               ` Michael Roth
2017-08-25 14:38                   ` Philippe Mathieu-Daudé
2017-08-24 15:37   ` Halil Pasic
2017-08-24 18:33 ` Christian Borntraeger
2017-08-24 19:56   ` Farhan Ali

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170824171355.29d1ec32.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=alifm@linux.vnet.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=david@redhat.com \
    --cc=pasic@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    --cc=walling@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).