>>> On 22.06.15 at 14:15, <andrew.coop...@citrix.com> wrote:
> On 22/06/15 12:49, Jan Beulich wrote:
>> @@ -847,47 +834,41 @@ static int vlapic_write(struct vcpu *v,
>> * According to the IA32 Manual, all accesses should be 32 bits.
>> * Some OSes do 8- or 16-byte accesses, however.
>> */
>> - val = (uint32_t)val;
>> - if ( len != 4 )
>> + if ( unlikely(len != 4) )
>> {
>> - unsigned int tmp;
>> - unsigned char alignment;
>> -
>> - gdprintk(XENLOG_INFO, "Notice: Local APIC write with len =
>> %lx\n",len);
>> -
>> - alignment = offset & 0x3;
>> - (void)vlapic_read_aligned(vlapic, offset & ~0x3, &tmp);
>> + unsigned int tmp = vlapic_read_aligned(vlapic, offset & ~3);
>> + unsigned char alignment = (offset & 3) * 8;
>>
>> switch ( len )
>> {
>> case 1:
>> - val = ((tmp & ~(0xff << (8*alignment))) |
>> - ((val & 0xff) << (8*alignment)));
>> + val = ((tmp & ~(0xff << alignment)) |
>> + ((val & 0xff) << alignment));
>
> These should probably be explicitly unsigned constants, to avoid issues
> with shifting a 1 into the sign bit.
I don't see what harm the sign bit would do here - even if the shift
operation is one on signed int, the & converts the operand to
unsigned int anyway (and with them being the same size, the
binary representation doesn't change).
> (I can't quite decide whether 0xff
> will be interpreted as signed or unsigned, given the integer promotion
> rules.)
Literal numbers representable as int will always be "promoted to"
int.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
