From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] x86/vLAPIC: adjust types in internal read/write handling Date: Mon, 22 Jun 2015 12:49:35 +0100 Message-ID: <55881270020000780008778A@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part1327CC5F.1__=" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z70EP-00023E-Cu for xen-devel@lists.xenproject.org; Mon, 22 Jun 2015 11:49:41 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel Cc: Andrew Cooper , Keir Fraser List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part1327CC5F.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline - 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 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -556,52 +556,42 @@ static void vlapic_set_tdcr(struct vlapi "timer_divisor: %d", vlapic->hw.timer_divisor); } =20 -static void vlapic_read_aligned( - struct vlapic *vlapic, unsigned int offset, unsigned int *result) +static uint32_t vlapic_read_aligned(struct vlapic *vlapic, unsigned int = offset) { switch ( offset ) { case APIC_PROCPRI: - *result =3D vlapic_get_ppr(vlapic); - break; + return vlapic_get_ppr(vlapic); =20 case APIC_TMCCT: /* Timer CCR */ if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) = ) - { - *result =3D 0; break; - } - *result =3D vlapic_get_tmcct(vlapic); - break; + return vlapic_get_tmcct(vlapic); =20 case APIC_TMICT: /* Timer ICR */ if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) = ) - { - *result =3D 0; break; - } + /* fall through */ default: - *result =3D vlapic_get_reg(vlapic, offset); - break; + return vlapic_get_reg(vlapic, offset); } + + return 0; } =20 static int vlapic_read( struct vcpu *v, unsigned long address, unsigned long len, unsigned long *pval) { - unsigned int alignment; - unsigned int tmp; - unsigned long result =3D 0; struct vlapic *vlapic =3D vcpu_vlapic(v); unsigned int offset =3D address - vlapic_base_address(vlapic); + unsigned int alignment =3D offset & 3, tmp, result =3D 0; =20 if ( offset > (APIC_TDCR + 0x3) ) goto out; =20 - alignment =3D offset & 0x3; + tmp =3D vlapic_read_aligned(vlapic, offset & ~3); =20 - vlapic_read_aligned(vlapic, offset & ~0x3, &tmp); switch ( len ) { case 1: @@ -627,7 +617,7 @@ static int vlapic_read( } =20 HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset %#x with length %#lx, " - "and the result is %#lx", offset, len, result); + "and the result is %#x", offset, len, result); =20 out: *pval =3D result; @@ -657,19 +647,17 @@ int hvm_x2apic_msr_read(struct vcpu *v,=20 #undef REGBLOCK }; struct vlapic *vlapic =3D vcpu_vlapic(v); - uint32_t low, high =3D 0, reg =3D msr - MSR_IA32_APICBASE_MSR, - offset =3D reg << 4; + uint32_t high =3D 0, reg =3D msr - MSR_IA32_APICBASE_MSR, offset =3D = reg << 4; =20 if ( !vlapic_x2apic_mode(vlapic) || (reg >=3D sizeof(readable) * 8) || !test_bit(reg, readable) ) return X86EMUL_UNHANDLEABLE; =20 if ( offset =3D=3D APIC_ICR ) - vlapic_read_aligned(vlapic, APIC_ICR2, &high); - - vlapic_read_aligned(vlapic, offset, &low); + high =3D vlapic_read_aligned(vlapic, APIC_ICR2); =20 - *msr_content =3D (((uint64_t)high) << 32) | low; + *msr_content =3D ((uint64_t)high << 32) | + vlapic_read_aligned(vlapic, offset); =20 return X86EMUL_OKAY; } @@ -687,7 +675,7 @@ static void vlapic_tdt_pt_cb(struct vcpu } =20 static int vlapic_reg_write(struct vcpu *v, - unsigned int offset, unsigned long val) + unsigned int offset, uint32_t val) { struct vlapic *vlapic =3D vcpu_vlapic(v); int rc =3D X86EMUL_OKAY; @@ -797,8 +785,7 @@ static int vlapic_reg_write(struct vcpu=20 break; } =20 - period =3D ((uint64_t)APIC_BUS_CYCLE_NS * - (uint32_t)val * vlapic->hw.timer_divisor); + period =3D (uint64_t)APIC_BUS_CYCLE_NS * val * vlapic->hw.timer_di= visor; TRACE_2_LONG_3D(TRC_HVM_EMUL_LAPIC_START_TIMER, TRC_PAR_LONG(perio= d), TRC_PAR_LONG(vlapic_lvtt_period(vlapic) ? period : 0LL), vlapic->pt.irq); @@ -811,7 +798,7 @@ static int vlapic_reg_write(struct vcpu=20 =20 HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "bus cycle is %uns, " - "initial count %lu, period %"PRIu64"ns", + "initial count %u, period %"PRIu64"ns", APIC_BUS_CYCLE_NS, val, period); } break; @@ -847,47 +834,41 @@ static int vlapic_write(struct vcpu *v,=20 * According to the IA32 Manual, all accesses should be 32 bits. * Some OSes do 8- or 16-byte accesses, however. */ - val =3D (uint32_t)val; - if ( len !=3D 4 ) + if ( unlikely(len !=3D 4) ) { - unsigned int tmp; - unsigned char alignment; - - gdprintk(XENLOG_INFO, "Notice: Local APIC write with len =3D = %lx\n",len); - - alignment =3D offset & 0x3; - (void)vlapic_read_aligned(vlapic, offset & ~0x3, &tmp); + unsigned int tmp =3D vlapic_read_aligned(vlapic, offset & ~3); + unsigned char alignment =3D (offset & 3) * 8; =20 switch ( len ) { case 1: - val =3D ((tmp & ~(0xff << (8*alignment))) | - ((val & 0xff) << (8*alignment))); + val =3D ((tmp & ~(0xff << alignment)) | + ((val & 0xff) << alignment)); break; =20 case 2: if ( alignment & 1 ) goto unaligned_exit_and_crash; - val =3D ((tmp & ~(0xffff << (8*alignment))) | - ((val & 0xffff) << (8*alignment))); + val =3D ((tmp & ~(0xffff << alignment)) | + ((val & 0xffff) << alignment)); break; =20 default: - gdprintk(XENLOG_ERR, "Local APIC write with len =3D %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); + offset &=3D ~3; } - else if ( (offset & 0x3) !=3D 0 ) + else if ( unlikely(offset & 3) ) goto unaligned_exit_and_crash; =20 - offset &=3D ~0x3; - return vlapic_reg_write(v, offset, val); =20 unaligned_exit_and_crash: - gdprintk(XENLOG_ERR, "Unaligned LAPIC write len=3D%#lx at offset=3D%#x= .\n", - len, offset); + gprintk(XENLOG_ERR, "Unaligned LAPIC write: len=3D%#lx offset=3D%#x.\n= ", + len, offset); exit_and_crash: domain_crash(v->domain); return rc; @@ -983,7 +964,7 @@ int hvm_x2apic_msr_write(struct vcpu *v, return X86EMUL_UNHANDLEABLE; } =20 - return vlapic_reg_write(v, offset, (uint32_t)msr_content); + return vlapic_reg_write(v, offset, msr_content); } =20 static int vlapic_range(struct vcpu *v, unsigned long addr) --=__Part1327CC5F.1__= Content-Type: text/plain; name="x86-vLAPIC-read-write-types.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86-vLAPIC-read-write-types.patch" x86/vLAPIC: adjust types in internal read/write handling=0A=0A- use 32-bit = types where possible (produces slightly better code)=0A- drop (now) = unnecessary casts=0A- avoid indirection where not needed=0A- avoid = duplicate log messages in vlapic_write()=0A- minor other cleanup=0A=0ASigne= d-off-by: Jan Beulich =0A=0A--- a/xen/arch/x86/hvm/vlapi= c.c=0A+++ b/xen/arch/x86/hvm/vlapic.c=0A@@ -556,52 +556,42 @@ static void = vlapic_set_tdcr(struct vlapi=0A "timer_divisor: %d", = vlapic->hw.timer_divisor);=0A }=0A =0A-static void vlapic_read_aligned(=0A-= struct vlapic *vlapic, unsigned int offset, unsigned int *result)=0A+st= atic uint32_t vlapic_read_aligned(struct vlapic *vlapic, unsigned int = offset)=0A {=0A switch ( offset )=0A {=0A case APIC_PROCPRI:=0A= - *result =3D vlapic_get_ppr(vlapic);=0A- break;=0A+ = return vlapic_get_ppr(vlapic);=0A =0A case APIC_TMCCT: /* Timer CCR = */=0A if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlap= ic) )=0A- {=0A- *result =3D 0;=0A break;=0A- = }=0A- *result =3D vlapic_get_tmcct(vlapic);=0A- = break;=0A+ return vlapic_get_tmcct(vlapic);=0A =0A case = APIC_TMICT: /* Timer ICR */=0A if ( !vlapic_lvtt_oneshot(vlapic) = && !vlapic_lvtt_period(vlapic) )=0A- {=0A- *result =3D = 0;=0A break;=0A- }=0A+ /* fall through */=0A = default:=0A- *result =3D vlapic_get_reg(vlapic, offset);=0A- = break;=0A+ return vlapic_get_reg(vlapic, offset);=0A }=0A+=0A+ = return 0;=0A }=0A =0A static int vlapic_read(=0A struct vcpu *v, = unsigned long address,=0A unsigned long len, unsigned long *pval)=0A = {=0A- unsigned int alignment;=0A- unsigned int tmp;=0A- unsigned = long result =3D 0;=0A struct vlapic *vlapic =3D vcpu_vlapic(v);=0A = unsigned int offset =3D address - vlapic_base_address(vlapic);=0A+ = unsigned int alignment =3D offset & 3, tmp, result =3D 0;=0A =0A if ( = offset > (APIC_TDCR + 0x3) )=0A goto out;=0A =0A- alignment =3D = offset & 0x3;=0A+ tmp =3D vlapic_read_aligned(vlapic, offset & ~3);=0A = =0A- vlapic_read_aligned(vlapic, offset & ~0x3, &tmp);=0A switch ( = len )=0A {=0A case 1:=0A@@ -627,7 +617,7 @@ static int vlapic_read(= =0A }=0A =0A HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset %#x with length = %#lx, "=0A- "and the result is %#lx", offset, len, = result);=0A+ "and the result is %#x", offset, len, = result);=0A =0A out:=0A *pval =3D result;=0A@@ -657,19 +647,17 @@ int = hvm_x2apic_msr_read(struct vcpu *v, =0A #undef REGBLOCK=0A };=0A = struct vlapic *vlapic =3D vcpu_vlapic(v);=0A- uint32_t low, high =3D = 0, reg =3D msr - MSR_IA32_APICBASE_MSR,=0A- offset =3D reg << = 4;=0A+ uint32_t high =3D 0, reg =3D msr - MSR_IA32_APICBASE_MSR, offset = =3D reg << 4;=0A =0A if ( !vlapic_x2apic_mode(vlapic) ||=0A = (reg >=3D sizeof(readable) * 8) || !test_bit(reg, readable) )=0A = return X86EMUL_UNHANDLEABLE;=0A =0A if ( offset =3D=3D APIC_ICR )=0A- = vlapic_read_aligned(vlapic, APIC_ICR2, &high);=0A-=0A- vlapic_read= _aligned(vlapic, offset, &low);=0A+ high =3D vlapic_read_aligned(vla= pic, APIC_ICR2);=0A =0A- *msr_content =3D (((uint64_t)high) << 32) | = low;=0A+ *msr_content =3D ((uint64_t)high << 32) |=0A+ = vlapic_read_aligned(vlapic, offset);=0A =0A return X86EMUL_OKAY;=0A = }=0A@@ -687,7 +675,7 @@ static void vlapic_tdt_pt_cb(struct vcpu=0A }=0A = =0A static int vlapic_reg_write(struct vcpu *v,=0A- = unsigned int offset, unsigned long val)=0A+ = unsigned int offset, uint32_t val)=0A {=0A struct vlapic *vlapic =3D = vcpu_vlapic(v);=0A int rc =3D X86EMUL_OKAY;=0A@@ -797,8 +785,7 @@ = static int vlapic_reg_write(struct vcpu =0A break;=0A = }=0A =0A- period =3D ((uint64_t)APIC_BUS_CYCLE_NS *=0A- = (uint32_t)val * vlapic->hw.timer_divisor);=0A+ period =3D = (uint64_t)APIC_BUS_CYCLE_NS * val * vlapic->hw.timer_divisor;=0A = TRACE_2_LONG_3D(TRC_HVM_EMUL_LAPIC_START_TIMER, TRC_PAR_LONG(period),=0A = TRC_PAR_LONG(vlapic_lvtt_period(vlapic) ? period : 0LL),=0A = vlapic->pt.irq);=0A@@ -811,7 +798,7 @@ static int = vlapic_reg_write(struct vcpu =0A =0A HVM_DBG_LOG(DBG_LEVEL_VLAPIC,= =0A "bus cycle is %uns, "=0A- = "initial count %lu, period %"PRIu64"ns",=0A+ "initial = count %u, period %"PRIu64"ns",=0A APIC_BUS_CYCLE_NS, = val, period);=0A }=0A break;=0A@@ -847,47 +834,41 @@ static int = vlapic_write(struct vcpu *v, =0A * According to the IA32 Manual, all = accesses should be 32 bits.=0A * Some OSes do 8- or 16-byte accesses, = however.=0A */=0A- val =3D (uint32_t)val;=0A- if ( len !=3D 4 = )=0A+ if ( unlikely(len !=3D 4) )=0A {=0A- unsigned int = tmp;=0A- unsigned char alignment;=0A-=0A- gdprintk(XENLOG_INF= O, "Notice: Local APIC write with len =3D %lx\n",len);=0A-=0A- = alignment =3D offset & 0x3;=0A- (void)vlapic_read_aligned(vlapic, = offset & ~0x3, &tmp);=0A+ unsigned int tmp =3D vlapic_read_aligned(v= lapic, offset & ~3);=0A+ unsigned char alignment =3D (offset & 3) * = 8;=0A =0A switch ( len )=0A {=0A case 1:=0A- = val =3D ((tmp & ~(0xff << (8*alignment))) |=0A- = ((val & 0xff) << (8*alignment)));=0A+ val =3D ((tmp & ~(0xff << = alignment)) |=0A+ ((val & 0xff) << alignment));=0A = break;=0A =0A case 2:=0A if ( alignment & 1 )=0A = goto unaligned_exit_and_crash;=0A- val =3D = ((tmp & ~(0xffff << (8*alignment))) |=0A- ((val & = 0xffff) << (8*alignment)));=0A+ val =3D ((tmp & ~(0xffff << = alignment)) |=0A+ ((val & 0xffff) << alignment));=0A = break;=0A =0A default:=0A- gdprintk(XENLOG_ERR, = "Local APIC write with len =3D %lx, "=0A- "should be 4 = instead\n", len);=0A+ gprintk(XENLOG_ERR, "LAPIC write with len = %lx\n", len);=0A goto exit_and_crash;=0A }=0A+=0A+ = gdprintk(XENLOG_INFO, "Notice: LAPIC write with len %lx\n", len);=0A+ = offset &=3D ~3;=0A }=0A- else if ( (offset & 0x3) !=3D 0 )=0A+ = else if ( unlikely(offset & 3) )=0A goto unaligned_exit_and_cras= h;=0A =0A- offset &=3D ~0x3;=0A-=0A return vlapic_reg_write(v, = offset, val);=0A =0A unaligned_exit_and_crash:=0A- gdprintk(XENLOG_ERR,= "Unaligned LAPIC write len=3D%#lx at offset=3D%#x.\n",=0A- = len, offset);=0A+ gprintk(XENLOG_ERR, "Unaligned LAPIC write: len=3D%#lx= offset=3D%#x.\n",=0A+ len, offset);=0A exit_and_crash:=0A = domain_crash(v->domain);=0A return rc;=0A@@ -983,7 +964,7 @@ int = hvm_x2apic_msr_write(struct vcpu *v,=0A return X86EMUL_UNHANDLE= ABLE;=0A }=0A =0A- return vlapic_reg_write(v, offset, (uint32_t)msr_= content);=0A+ return vlapic_reg_write(v, offset, msr_content);=0A }=0A = =0A static int vlapic_range(struct vcpu *v, unsigned long addr)=0A --=__Part1327CC5F.1__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --=__Part1327CC5F.1__=--