Re: [Xen-devel] [PATCH] x86/vLAPIC: adjust types in internal read/write handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.06.15 at 14:15, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel