Re: [Qemu-devel] [PATCH] s390-ccw: force diag 308 subcode to unsigned long

[Eric Blake <eblake@redhat.com>]

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-stable@nongnu.org
>> Signed-off-by: Cornelia Huck<cohuck@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).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org