On 05/03/2018 11:48 AM, Eric Blake wrote:
On 05/03/2018 10:25 AM, Farhan Ali wrote:
On 05/02/2018 08:52 AM, Cornelia Huck wrote:
We currently pass an integer as the subcode parameter. However,
the upper bits of the register containing the subcode need to
be 0, which is not guaranteed unless we explicitly specify the
subcode to be an unsigned long value.
Fixes: d046c51dad3 ("pc-bios/s390-ccw: Get device address via diag
308/6")
Cc:qemu-sta...@nongnu.org
Signed-off-by: Cornelia Huck<coh...@redhat.com>
Sorry for my ignorance, but is there a C standard that says upper bits
of an int is not guaranteed to be 0?
We're outside the bounds of the C standard because of the use of asm().
The problem here is that the compiler assigning a 32-bit int into a
64-bit register uses the shortest sequence possible (leaving the upper
64 bits garbage), because the compiler assumes you correctly wrote the
assembly to only use 32-bit operations on that register (which don't
care about the upper bits). By using an unsigned long (a 64-bit value),
the compiler instead emits assembly to write the full 64-bit register
value, rather than leaving the upper bits as garbage; and this matters
because we are subsequently using all 64 bits of the register in a later
operation. We could also use a signed long, even long long, or written
it as: (store ? 6ULL : 5ULL) instead of using a temporary variable. The
crux of the fix is that you have to tell asm() that you want a 64-bit
value written (the unpatched (store ? 6 : 5) is only a 32-bit value),
and not whether that value is signed or unsigned (since the
representation of both 6 and 5 are the same regardless of whether the
type being written into the register is signed or not).
Thank you so much for the detailed explanation :).
I did not think about the instruction that will be used by the compiler
to handle the values. Definitely learned something new!
