On 22/06/15 12:49, Jan Beulich wrote:
> - use 32-bit types where possible (produces slightly better code)
> - drop (now) unnecessary casts
> - avoid indirection where not needed
> - avoid duplicate log messages in vlapic_write()
> - minor other cleanup
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>, with two
suggestions.
>
> @@ -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 can't quite decide whether 0xff
will be interpreted as signed or unsigned, given the integer promotion
rules.)
> break;
>
> case 2:
> if ( alignment & 1 )
> goto unaligned_exit_and_crash;
> - val = ((tmp & ~(0xffff << (8*alignment))) |
> - ((val & 0xffff) << (8*alignment)));
> + val = ((tmp & ~(0xffff << alignment)) |
> + ((val & 0xffff) << alignment));
> break;
>
> default:
> - gdprintk(XENLOG_ERR, "Local APIC write with len = %lx, "
> - "should be 4 instead\n", len);
> + gprintk(XENLOG_ERR, "LAPIC write with len %lx\n", len);
> goto exit_and_crash;
> }
> +
> + gdprintk(XENLOG_INFO, "Notice: LAPIC write with len %lx\n", len);
Probably better using %lu.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
