From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/traps: consolidate PV RDMSR emulation paths Date: Thu, 26 Feb 2015 14:35:32 +0000 Message-ID: <54EF2F34.4020300@citrix.com> References: <54EF38C302000078000641B1@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YQzY1-00067z-SO for xen-devel@lists.xenproject.org; Thu, 26 Feb 2015 14:36:17 +0000 In-Reply-To: <54EF38C302000078000641B1@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org On 26/02/15 14:16, Jan Beulich wrote: > Settle on just using one variable (val), and move the other into > WRMSR's local scope. Chain up further success paths to the > rdmsr_writeback label rather than open coding them. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper > > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -2008,7 +2008,7 @@ static int emulate_privileged_op(struct > unsigned long code_base, code_limit; > char io_emul_stub[32]; > void (*io_emul)(struct cpu_user_regs *) __attribute__((__regparm__(1))); > - uint64_t val, msr_content; > + uint64_t val; > > if ( !read_descriptor(regs->cs, v, regs, > &code_base, &code_limit, &ar, > @@ -2498,8 +2498,9 @@ static int emulate_privileged_op(struct > case 0x30: /* WRMSR */ { > uint32_t eax = regs->eax; > uint32_t edx = regs->edx; > - msr_content = ((uint64_t)edx << 32) | eax; > - switch ( (u32)regs->ecx ) > + uint64_t msr_content = ((uint64_t)edx << 32) | eax; > + > + switch ( regs->_ecx ) > { > case MSR_FS_BASE: > if ( is_pv_32on64_vcpu(v) ) > @@ -2670,7 +2671,7 @@ static int emulate_privileged_op(struct > break; > > case 0x32: /* RDMSR */ > - switch ( (u32)regs->ecx ) > + switch ( regs->_ecx ) > { > case MSR_FS_BASE: > if ( is_pv_32on64_vcpu(v) ) > @@ -2686,9 +2687,8 @@ static int emulate_privileged_op(struct > case MSR_SHADOW_GS_BASE: > if ( is_pv_32on64_vcpu(v) ) > goto fail; > - regs->eax = v->arch.pv_vcpu.gs_base_user & 0xFFFFFFFFUL; > - regs->edx = v->arch.pv_vcpu.gs_base_user >> 32; > - break; > + val = v->arch.pv_vcpu.gs_base_user; > + goto rdmsr_writeback; > case MSR_K7_FID_VID_CTL: > case MSR_K7_FID_VID_STATUS: > case MSR_K8_PSTATE_LIMIT: > @@ -2720,12 +2720,10 @@ static int emulate_privileged_op(struct > } > goto rdmsr_normal; > case MSR_IA32_MISC_ENABLE: > - if ( rdmsr_safe(regs->ecx, msr_content) ) > + if ( rdmsr_safe(regs->ecx, val) ) > goto fail; > - msr_content = guest_misc_enable(msr_content); > - regs->eax = (uint32_t)msr_content; > - regs->edx = (uint32_t)(msr_content >> 32); > - break; > + val = guest_misc_enable(val); > + goto rdmsr_writeback; > > case MSR_AMD64_DR0_ADDRESS_MASK: > if ( !boot_cpu_has(X86_FEATURE_DBEXT) ) > @@ -2743,12 +2741,7 @@ static int emulate_privileged_op(struct > > default: > if ( rdmsr_hypervisor_regs(regs->ecx, &val) ) > - { > - rdmsr_writeback: > - regs->eax = (uint32_t)val; > - regs->edx = (uint32_t)(val >> 32); > - break; > - } > + goto rdmsr_writeback; > > rc = vmce_rdmsr(regs->ecx, &val); > if ( rc < 0 ) > @@ -2761,10 +2754,11 @@ static int emulate_privileged_op(struct > /* Everyone can read the MSR space. */ > /* gdprintk(XENLOG_WARNING,"Domain attempted RDMSR %p.\n", > _p(regs->ecx));*/ > - if ( rdmsr_safe(regs->ecx, msr_content) ) > + if ( rdmsr_safe(regs->ecx, val) ) > goto fail; > - regs->eax = (uint32_t)msr_content; > - regs->edx = (uint32_t)(msr_content >> 32); > + rdmsr_writeback: > + regs->eax = (uint32_t)val; > + regs->edx = (uint32_t)(val >> 32); > break; > } > break; > > >