* A simple clean up of __get_instruction_length & __update_guest_eip
@ 2010-09-15 14:34 Dong, Eddie
2010-09-15 15:25 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Dong, Eddie @ 2010-09-15 14:34 UTC (permalink / raw)
To: xen-devel@lists.xensource.com; +Cc: Dong, Eddie
[-- Attachment #1: Type: text/plain, Size: 4863 bytes --]
Replace so many sparsed __get_instruction_length & __update_guest_eip with one function call.
Signed-off-by: Eddie Dong <eddie.dong@intel.com>
diff -r 221cf46cbf2c xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Wed Sep 15 17:31:53 2010 +0800
+++ b/xen/arch/x86/hvm/vmx/vmx.c Wed Sep 15 17:46:24 2010 +0800
@@ -1494,6 +1494,14 @@
vmx_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE);
}
+static void update_guest_eip(void)
+{
+ unsigned long inst_len;
+
+ inst_len = __get_instruction_length();
+ __update_guest_eip(inst_len);
+}
+
static void vmx_fpu_dirty_intercept(void)
{
struct vcpu *curr = current;
@@ -2244,8 +2252,7 @@
if ( (((exit_qualification >> 12) & 0xf) == 1) &&
((exit_qualification & 0xfff) == APIC_EOI) )
{
- int inst_len = __get_instruction_length(); /* Safe: APIC data write */
- __update_guest_eip(inst_len);
+ update_guest_eip();
vlapic_EOI_set(vcpu_vlapic(current));
return 1;
}
@@ -2424,8 +2431,7 @@
case TRAP_int3:
if ( !v->domain->debugger_attached )
goto exit_and_crash;
- inst_len = __get_instruction_length(); /* Safe: INT3 */
- __update_guest_eip(inst_len);
+ update_guest_eip();
current->arch.gdbsx_vcpu_event = TRAP_int3;
domain_pause_for_debugger();
break;
@@ -2516,18 +2522,15 @@
break;
}
case EXIT_REASON_CPUID:
- inst_len = __get_instruction_length(); /* Safe: CPUID */
- __update_guest_eip(inst_len);
+ update_guest_eip();
vmx_do_cpuid(regs);
break;
case EXIT_REASON_HLT:
- inst_len = __get_instruction_length(); /* Safe: HLT */
- __update_guest_eip(inst_len);
+ update_guest_eip();
hvm_hlt(regs->eflags);
break;
case EXIT_REASON_INVLPG:
- inst_len = __get_instruction_length(); /* Safe: INVLPG */
- __update_guest_eip(inst_len);
+ update_guest_eip();
exit_qualification = __vmread(EXIT_QUALIFICATION);
vmx_invlpg_intercept(exit_qualification);
break;
@@ -2535,19 +2538,17 @@
regs->ecx = hvm_msr_tsc_aux(v);
/* fall through */
case EXIT_REASON_RDTSC:
- inst_len = __get_instruction_length();
- __update_guest_eip(inst_len);
+ update_guest_eip();
hvm_rdtsc_intercept(regs);
break;
case EXIT_REASON_VMCALL:
{
int rc;
HVMTRACE_1D(VMMCALL, regs->eax);
- inst_len = __get_instruction_length(); /* Safe: VMCALL */
rc = hvm_do_hypercall(regs);
if ( rc != HVM_HCALL_preempted )
{
- __update_guest_eip(inst_len);
+ update_guest_eip();
if ( rc == HVM_HCALL_invalidate )
send_invalidate_req();
}
@@ -2556,9 +2557,8 @@
case EXIT_REASON_CR_ACCESS:
{
exit_qualification = __vmread(EXIT_QUALIFICATION);
- inst_len = __get_instruction_length(); /* Safe: MOV Cn, LMSW, CLTS */
if ( vmx_cr_access(exit_qualification, regs) )
- __update_guest_eip(inst_len);
+ update_guest_eip();
break;
}
case EXIT_REASON_DR_ACCESS:
@@ -2568,22 +2568,20 @@
case EXIT_REASON_MSR_READ:
{
uint64_t msr_content;
- inst_len = __get_instruction_length(); /* Safe: RDMSR */
if ( hvm_msr_read_intercept(regs->ecx, &msr_content) == X86EMUL_OKAY )
{
regs->eax = (uint32_t)msr_content;
regs->edx = (uint32_t)(msr_content >> 32);
- __update_guest_eip(inst_len);
+ update_guest_eip();
}
break;
}
case EXIT_REASON_MSR_WRITE:
{
uint64_t msr_content;
- inst_len = __get_instruction_length(); /* Safe: WRMSR */
msr_content = ((uint64_t)regs->edx << 32) | (uint32_t)regs->eax;
if ( hvm_msr_write_intercept(regs->ecx, msr_content) == X86EMUL_OKAY )
- __update_guest_eip(inst_len);
+ update_guest_eip();
break;
}
@@ -2652,8 +2650,7 @@
case EXIT_REASON_INVD:
case EXIT_REASON_WBINVD:
{
- inst_len = __get_instruction_length(); /* Safe: INVD, WBINVD */
- __update_guest_eip(inst_len);
+ update_guest_eip();
vmx_wbinvd_intercept();
break;
}
@@ -2686,8 +2683,7 @@
u64 new_bv = (((u64)regs->edx) << 32) | regs->eax;
if ( vmx_handle_xsetbv(new_bv) == 0 )
{
- inst_len = __get_instruction_length();
- __update_guest_eip(inst_len);
+ update_guest_eip();
}
break;
}
[-- Attachment #2: clean1.patch --]
[-- Type: application/octet-stream, Size: 4722 bytes --]
Replace so many sparsed __get_instruction_length & __update_guest_eip with one function call.
Signed-off-by: Eddie Dong <eddie.dong@intel.com>
diff -r 221cf46cbf2c xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Wed Sep 15 17:31:53 2010 +0800
+++ b/xen/arch/x86/hvm/vmx/vmx.c Wed Sep 15 17:46:24 2010 +0800
@@ -1494,6 +1494,14 @@
vmx_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE);
}
+static void update_guest_eip(void)
+{
+ unsigned long inst_len;
+
+ inst_len = __get_instruction_length();
+ __update_guest_eip(inst_len);
+}
+
static void vmx_fpu_dirty_intercept(void)
{
struct vcpu *curr = current;
@@ -2244,8 +2252,7 @@
if ( (((exit_qualification >> 12) & 0xf) == 1) &&
((exit_qualification & 0xfff) == APIC_EOI) )
{
- int inst_len = __get_instruction_length(); /* Safe: APIC data write */
- __update_guest_eip(inst_len);
+ update_guest_eip();
vlapic_EOI_set(vcpu_vlapic(current));
return 1;
}
@@ -2424,8 +2431,7 @@
case TRAP_int3:
if ( !v->domain->debugger_attached )
goto exit_and_crash;
- inst_len = __get_instruction_length(); /* Safe: INT3 */
- __update_guest_eip(inst_len);
+ update_guest_eip();
current->arch.gdbsx_vcpu_event = TRAP_int3;
domain_pause_for_debugger();
break;
@@ -2516,18 +2522,15 @@
break;
}
case EXIT_REASON_CPUID:
- inst_len = __get_instruction_length(); /* Safe: CPUID */
- __update_guest_eip(inst_len);
+ update_guest_eip();
vmx_do_cpuid(regs);
break;
case EXIT_REASON_HLT:
- inst_len = __get_instruction_length(); /* Safe: HLT */
- __update_guest_eip(inst_len);
+ update_guest_eip();
hvm_hlt(regs->eflags);
break;
case EXIT_REASON_INVLPG:
- inst_len = __get_instruction_length(); /* Safe: INVLPG */
- __update_guest_eip(inst_len);
+ update_guest_eip();
exit_qualification = __vmread(EXIT_QUALIFICATION);
vmx_invlpg_intercept(exit_qualification);
break;
@@ -2535,19 +2538,17 @@
regs->ecx = hvm_msr_tsc_aux(v);
/* fall through */
case EXIT_REASON_RDTSC:
- inst_len = __get_instruction_length();
- __update_guest_eip(inst_len);
+ update_guest_eip();
hvm_rdtsc_intercept(regs);
break;
case EXIT_REASON_VMCALL:
{
int rc;
HVMTRACE_1D(VMMCALL, regs->eax);
- inst_len = __get_instruction_length(); /* Safe: VMCALL */
rc = hvm_do_hypercall(regs);
if ( rc != HVM_HCALL_preempted )
{
- __update_guest_eip(inst_len);
+ update_guest_eip();
if ( rc == HVM_HCALL_invalidate )
send_invalidate_req();
}
@@ -2556,9 +2557,8 @@
case EXIT_REASON_CR_ACCESS:
{
exit_qualification = __vmread(EXIT_QUALIFICATION);
- inst_len = __get_instruction_length(); /* Safe: MOV Cn, LMSW, CLTS */
if ( vmx_cr_access(exit_qualification, regs) )
- __update_guest_eip(inst_len);
+ update_guest_eip();
break;
}
case EXIT_REASON_DR_ACCESS:
@@ -2568,22 +2568,20 @@
case EXIT_REASON_MSR_READ:
{
uint64_t msr_content;
- inst_len = __get_instruction_length(); /* Safe: RDMSR */
if ( hvm_msr_read_intercept(regs->ecx, &msr_content) == X86EMUL_OKAY )
{
regs->eax = (uint32_t)msr_content;
regs->edx = (uint32_t)(msr_content >> 32);
- __update_guest_eip(inst_len);
+ update_guest_eip();
}
break;
}
case EXIT_REASON_MSR_WRITE:
{
uint64_t msr_content;
- inst_len = __get_instruction_length(); /* Safe: WRMSR */
msr_content = ((uint64_t)regs->edx << 32) | (uint32_t)regs->eax;
if ( hvm_msr_write_intercept(regs->ecx, msr_content) == X86EMUL_OKAY )
- __update_guest_eip(inst_len);
+ update_guest_eip();
break;
}
@@ -2652,8 +2650,7 @@
case EXIT_REASON_INVD:
case EXIT_REASON_WBINVD:
{
- inst_len = __get_instruction_length(); /* Safe: INVD, WBINVD */
- __update_guest_eip(inst_len);
+ update_guest_eip();
vmx_wbinvd_intercept();
break;
}
@@ -2686,8 +2683,7 @@
u64 new_bv = (((u64)regs->edx) << 32) | regs->eax;
if ( vmx_handle_xsetbv(new_bv) == 0 )
{
- inst_len = __get_instruction_length();
- __update_guest_eip(inst_len);
+ update_guest_eip();
}
break;
}
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: A simple clean up of __get_instruction_length & __update_guest_eip
2010-09-15 14:34 A simple clean up of __get_instruction_length & __update_guest_eip Dong, Eddie
@ 2010-09-15 15:25 ` Jan Beulich
2010-09-15 15:38 ` Keir Fraser
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2010-09-15 15:25 UTC (permalink / raw)
To: Eddie Dong; +Cc: xen-devel@lists.xensource.com
>>> On 15.09.10 at 16:34, "Dong, Eddie" <eddie.dong@intel.com> wrote:
> Replace so many sparsed __get_instruction_length & __update_guest_eip with
> one function call.
>
> Signed-off-by: Eddie Dong <eddie.dong@intel.com>
>
> diff -r 221cf46cbf2c xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c Wed Sep 15 17:31:53 2010 +0800
> +++ b/xen/arch/x86/hvm/vmx/vmx.c Wed Sep 15 17:46:24 2010 +0800
> @@ -1494,6 +1494,14 @@
> vmx_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE);
> }
>
> +static void update_guest_eip(void)
> +{
> + unsigned long inst_len;
> +
> + inst_len = __get_instruction_length();
> + __update_guest_eip(inst_len);
> +}
> +
> static void vmx_fpu_dirty_intercept(void)
> {
> struct vcpu *curr = current;
> @@ -2244,8 +2252,7 @@
> if ( (((exit_qualification >> 12) & 0xf) == 1) &&
> ((exit_qualification & 0xfff) == APIC_EOI) )
> {
> - int inst_len = __get_instruction_length(); /* Safe: APIC data write
> */
> - __update_guest_eip(inst_len);
> + update_guest_eip();
Is it really a good idea to remove all these "Safe: ..." comments?
Without them, I think it'll be much easier to not remember that
this cannot be done everywhere and hence to add an inappropriate
call to this function.
Jan
> vlapic_EOI_set(vcpu_vlapic(current));
> return 1;
> }
> @@ -2424,8 +2431,7 @@
> case TRAP_int3:
> if ( !v->domain->debugger_attached )
> goto exit_and_crash;
> - inst_len = __get_instruction_length(); /* Safe: INT3 */
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> current->arch.gdbsx_vcpu_event = TRAP_int3;
> domain_pause_for_debugger();
> break;
> @@ -2516,18 +2522,15 @@
> break;
> }
> case EXIT_REASON_CPUID:
> - inst_len = __get_instruction_length(); /* Safe: CPUID */
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> vmx_do_cpuid(regs);
> break;
> case EXIT_REASON_HLT:
> - inst_len = __get_instruction_length(); /* Safe: HLT */
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> hvm_hlt(regs->eflags);
> break;
> case EXIT_REASON_INVLPG:
> - inst_len = __get_instruction_length(); /* Safe: INVLPG */
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> exit_qualification = __vmread(EXIT_QUALIFICATION);
> vmx_invlpg_intercept(exit_qualification);
> break;
> @@ -2535,19 +2538,17 @@
> regs->ecx = hvm_msr_tsc_aux(v);
> /* fall through */
> case EXIT_REASON_RDTSC:
> - inst_len = __get_instruction_length();
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> hvm_rdtsc_intercept(regs);
> break;
> case EXIT_REASON_VMCALL:
> {
> int rc;
> HVMTRACE_1D(VMMCALL, regs->eax);
> - inst_len = __get_instruction_length(); /* Safe: VMCALL */
> rc = hvm_do_hypercall(regs);
> if ( rc != HVM_HCALL_preempted )
> {
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> if ( rc == HVM_HCALL_invalidate )
> send_invalidate_req();
> }
> @@ -2556,9 +2557,8 @@
> case EXIT_REASON_CR_ACCESS:
> {
> exit_qualification = __vmread(EXIT_QUALIFICATION);
> - inst_len = __get_instruction_length(); /* Safe: MOV Cn, LMSW, CLTS
> */
> if ( vmx_cr_access(exit_qualification, regs) )
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> break;
> }
> case EXIT_REASON_DR_ACCESS:
> @@ -2568,22 +2568,20 @@
> case EXIT_REASON_MSR_READ:
> {
> uint64_t msr_content;
> - inst_len = __get_instruction_length(); /* Safe: RDMSR */
> if ( hvm_msr_read_intercept(regs->ecx, &msr_content) == X86EMUL_OKAY )
> {
> regs->eax = (uint32_t)msr_content;
> regs->edx = (uint32_t)(msr_content >> 32);
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> }
> break;
> }
> case EXIT_REASON_MSR_WRITE:
> {
> uint64_t msr_content;
> - inst_len = __get_instruction_length(); /* Safe: WRMSR */
> msr_content = ((uint64_t)regs->edx << 32) | (uint32_t)regs->eax;
> if ( hvm_msr_write_intercept(regs->ecx, msr_content) == X86EMUL_OKAY
> )
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> break;
> }
>
> @@ -2652,8 +2650,7 @@
> case EXIT_REASON_INVD:
> case EXIT_REASON_WBINVD:
> {
> - inst_len = __get_instruction_length(); /* Safe: INVD, WBINVD */
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> vmx_wbinvd_intercept();
> break;
> }
> @@ -2686,8 +2683,7 @@
> u64 new_bv = (((u64)regs->edx) << 32) | regs->eax;
> if ( vmx_handle_xsetbv(new_bv) == 0 )
> {
> - inst_len = __get_instruction_length();
> - __update_guest_eip(inst_len);
> + update_guest_eip();
> }
> break;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: A simple clean up of __get_instruction_length & __update_guest_eip
2010-09-15 15:25 ` Jan Beulich
@ 2010-09-15 15:38 ` Keir Fraser
0 siblings, 0 replies; 3+ messages in thread
From: Keir Fraser @ 2010-09-15 15:38 UTC (permalink / raw)
To: Jan Beulich, Eddie Dong; +Cc: xen-devel@lists.xensource.com
On 15/09/2010 16:25, "Jan Beulich" <JBeulich@novell.com> wrote:
> Is it really a good idea to remove all these "Safe: ..." comments?
> Without them, I think it'll be much easier to not remember that
> this cannot be done everywhere and hence to add an inappropriate
> call to this function.
I meant to check they were kept, and then forgot and applied it anyway. I'll
add them back in.
--Keir
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-09-15 15:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-15 14:34 A simple clean up of __get_instruction_length & __update_guest_eip Dong, Eddie
2010-09-15 15:25 ` Jan Beulich
2010-09-15 15:38 ` Keir Fraser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).