On 08/24/2017 11:50 AM, Thomas Huth wrote:
On 24.08.2017 17:47, Halil Pasic wrote:
On 08/24/2017 05:35 PM, Thomas Huth wrote:
On 24.08.2017 17:13, Cornelia Huck wrote:
On Thu, 24 Aug 2017 11:05:08 -0400
Farhan Ali <al...@linux.vnet.ibm.com> wrote:
Hi,
There is an issue in QEMU bios which is exposed by commit
commit 198c0d1f9df8c429502cb744fc26b6ba6e71db74
Author: Halil Pasic <pa...@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 <pa...@linux.vnet.ibm.com>
Message-Id: <20170727154842.23427-1-pa...@linux.vnet.ibm.com>
Reviewed-by: Dong Jia Shi <bjsdj...@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <coh...@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?
I guess the problem is the __attribute__((packed)) here - AFAIK GCC then
sometimes assumes that these structs can be byte-aligned. Does it work
if you remove the __attribute__((packed)) here? If yes, I think that
would be a valid fix, since there should not be any padding in this
struct at all (and if you're afraid, you could add an
assert(sizeof(struct ccw1) == 8) somewhere).
I don't think this packed is doing us any good. But even
with the packed removed I not sure we would end up being
8 byte aligned (dobuleword). Wouldn't it be just 4 byte
aligned (according to the ELF ABI supplement for s390)
as the most strictly aligned member is __u32?
True, so that could still be an issue. Looking at the cio.h in the
kernel, they define the struct like this:
struct ccw1 {
__u8 cmd_code;
__u8 flags;
__u16 count;
__u32 cda;
} __attribute__ ((packed,aligned(8)));
So I guess adding the aligned(8) is the right way to go?
Thomas
This was my initial fix and it works on my system. But for some reason
this fix does not work on my colleague's system. So I am hesitant about
submitting this fix
