WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] x86: miscellaneous emulator adjustments

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH] x86: miscellaneous emulator adjustments
From: Christoph Egger <Christoph.Egger@xxxxxxx>
Date: Tue, 18 Aug 2009 16:12:37 +0200
Cc: Jan Beulich <JBeulich@xxxxxxxxxx>
Delivery-date: Tue, 18 Aug 2009 07:13:47 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4A8AC65902000078000104BF@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4A8AC65902000078000104BF@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.7
How did you test this patch ?

Christoph


On Tuesday 18 August 2009 15:18:49 Jan Beulich wrote:
> Defer fail_if()-s as much as possible (in favor of possibly generating
> exceptions), and avoid generating exceptions when not strictly
> necessary.
>
> Avoid fail_if()-s for simple return code checks (making the code that
> used them consistent with other, longer existing code).
>
> Eliminate redundant generate_exception_if()-s checking lock_prefix
> (which is already covered by the general check prior to decoding
> operands).
>
> Also fix the testing code to add PROT_EXEC for the mapping that is
> intended to have instruction executed from.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
>
> --- 2009-08-18.orig/tools/tests/test_x86_emulator.c   2008-07-18
> 16:19:34.000000000 +0200 +++
> 2009-08-18/tools/tests/test_x86_emulator.c    2009-08-18 14:18:20.000000000
> +0200 @@ -76,7 +76,7 @@ int main(int argc, char **argv)
>      ctxt.addr_size = 32;
>      ctxt.sp_size   = 32;
>
> -    res = mmap((void *)0x100000, MMAP_SZ, PROT_READ|PROT_WRITE,
> +    res = mmap((void *)0x100000, MMAP_SZ, PROT_READ|PROT_WRITE|PROT_EXEC,
>                 MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS, 0, 0);
>      if ( res == MAP_FAILED )
>      {
> --- 2009-08-18.orig/xen/arch/x86/x86_emulate/x86_emulate.c    2009-08-18
> 14:18:15.000000000 +0200 +++
> 2009-08-18/xen/arch/x86/x86_emulate/x86_emulate.c     2009-08-18
> 14:18:20.000000000 +0200 @@ -3583,21 +3583,17 @@ x86_emulate(
>          struct segment_register cs = { 0 }, ss = { 0 };
>          int rc;
>
> -        fail_if(ops->read_msr == NULL);
> -        fail_if(ops->read_segment == NULL);
> -        fail_if(ops->write_segment == NULL);
> -
>          generate_exception_if(in_realmode(ctxt, ops), EXC_UD, 0);
>          generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, 0);
> -        generate_exception_if(lock_prefix, EXC_UD, 0);
>
>          /* Inject #UD if syscall/sysret are disabled. */
> -        rc = ops->read_msr(MSR_EFER, &msr_content, ctxt);
> -        fail_if(rc != 0);
> +        fail_if(ops->read_msr == NULL);
> +        if ( (rc = ops->read_msr(MSR_EFER, &msr_content, ctxt)) != 0 )
> +            goto done;
>          generate_exception_if((msr_content & EFER_SCE) == 0, EXC_UD, 0);
>
> -        rc = ops->read_msr(MSR_STAR, &msr_content, ctxt);
> -        fail_if(rc != 0);
> +        if ( (rc = ops->read_msr(MSR_STAR, &msr_content, ctxt)) != 0 )
> +            goto done;
>
>          msr_content >>= 32;
>          cs.sel = (uint16_t)(msr_content & 0xfffc);
> @@ -3617,27 +3613,27 @@ x86_emulate(
>              _regs.rcx = _regs.rip;
>              _regs.r11 = _regs.eflags & ~EFLG_RF;
>
> -            rc = ops->read_msr(mode_64bit() ? MSR_LSTAR : MSR_CSTAR,
> -                               &msr_content, ctxt);
> -            fail_if(rc != 0);
> -
> +            if ( (rc = ops->read_msr(mode_64bit() ? MSR_LSTAR : MSR_CSTAR,
> +                                     &msr_content, ctxt)) != 0 )
> +                goto done;
>              _regs.rip = msr_content;
>
> -            rc = ops->read_msr(MSR_FMASK, &msr_content, ctxt);
> -            fail_if(rc != 0);
> +            if ( (rc = ops->read_msr(MSR_FMASK, &msr_content, ctxt)) != 0
> ) +                goto done;
>              _regs.eflags &= ~(msr_content | EFLG_RF);
>          }
>          else
>  #endif
>          {
> -            rc = ops->read_msr(MSR_STAR, &msr_content, ctxt);
> -            fail_if(rc != 0);
> +            if ( (rc = ops->read_msr(MSR_STAR, &msr_content, ctxt)) != 0 )
> +                goto done;
>
>              _regs.ecx = _regs.eip;
>              _regs.eip = (uint32_t)msr_content;
>              _regs.eflags &= ~(EFLG_VM | EFLG_IF | EFLG_RF);
>          }
>
> +        fail_if(ops->write_segment == NULL);
>          if ( (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) ||
>               (rc = ops->write_segment(x86_seg_ss, &ss, ctxt)) )
>              goto done;
> @@ -3717,10 +3713,13 @@ x86_emulate(
>      case 0x31: /* rdtsc */ {
>          unsigned long cr4;
>          uint64_t val;
> -        fail_if(ops->read_cr == NULL);
> -        if ( (rc = ops->read_cr(4, &cr4, ctxt)) )
> -            goto done;
> -        generate_exception_if((cr4 & CR4_TSD) && !mode_ring0(), EXC_GP,
> 0); +        if ( !mode_ring0() )
> +        {
> +            fail_if(ops->read_cr == NULL);
> +            if ( (rc = ops->read_cr(4, &cr4, ctxt)) )
> +                goto done;
> +            generate_exception_if(cr4 & CR4_TSD, EXC_GP, 0);
> +        }
>          fail_if(ops->read_msr == NULL);
>          if ( (rc = ops->read_msr(MSR_TSC, &val, ctxt)) != 0 )
>              goto done;
> @@ -3751,17 +3750,13 @@ x86_emulate(
>          struct segment_register cs, ss;
>          int rc;
>
> -        fail_if(ops->read_msr == NULL);
> -        fail_if(ops->read_segment == NULL);
> -        fail_if(ops->write_segment == NULL);
> -
>          generate_exception_if(mode_ring0(), EXC_GP, 0);
>          generate_exception_if(in_realmode(ctxt, ops), EXC_GP, 0);
>          generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
> -        generate_exception_if(lock_prefix, EXC_UD, 0);
>
> -        rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt);
> -        fail_if(rc != 0);
> +        fail_if(ops->read_msr == NULL);
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt)) !=
> 0 ) +            goto done;
>
>          if ( mode_64bit() )
>              generate_exception_if(msr_content == 0, EXC_GP, 0);
> @@ -3770,6 +3765,7 @@ x86_emulate(
>
>          _regs.eflags &= ~(EFLG_VM | EFLG_IF | EFLG_RF);
>
> +        fail_if(ops->read_segment == NULL);
>          ops->read_segment(x86_seg_cs, &cs, ctxt);
>          cs.sel = (uint16_t)msr_content & ~3; /* SELECTOR_RPL_MASK */
>          cs.base = 0;   /* flat segment */
> @@ -3787,17 +3783,17 @@ x86_emulate(
>              cs.attr.fields.l = 1;
>          }
>
> -        rc = ops->write_segment(x86_seg_cs, &cs, ctxt);
> -        fail_if(rc != 0);
> -        rc = ops->write_segment(x86_seg_ss, &ss, ctxt);
> -        fail_if(rc != 0);
> +        fail_if(ops->write_segment == NULL);
> +        if ( (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) != 0 ||
> +             (rc = ops->write_segment(x86_seg_ss, &ss, ctxt)) != 0 )
> +            goto done;
>
> -        rc = ops->read_msr(MSR_SYSENTER_EIP, &msr_content, ctxt);
> -        fail_if(rc != 0);
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_EIP, &msr_content, ctxt)) !=
> 0 ) +            goto done;
>          _regs.eip = msr_content;
>
> -        rc = ops->read_msr(MSR_SYSENTER_ESP, &msr_content, ctxt);
> -        fail_if(rc != 0);
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_ESP, &msr_content, ctxt)) !=
> 0 ) +            goto done;
>          _regs.esp = msr_content;
>
>          break;
> @@ -3809,19 +3805,13 @@ x86_emulate(
>          int user64 = !!(rex_prefix & 8); /* REX.W */
>          int rc;
>
> -        fail_if(ops->read_msr == NULL);
> -        fail_if(ops->read_segment == NULL);
> -        fail_if(ops->write_segment == NULL);
> -
>          generate_exception_if(!mode_ring0(), EXC_GP, 0);
>          generate_exception_if(in_realmode(ctxt, ops), EXC_GP, 0);
>          generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
> -        generate_exception_if(lock_prefix, EXC_UD, 0);
>
> -        rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt);
> -        fail_if(rc != 0);
> -        rc = ops->read_segment(x86_seg_cs, &cs, ctxt);
> -        fail_if(rc != 0);
> +        fail_if(ops->read_msr == NULL);
> +        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt)) !=
> 0 ) +            goto done;
>
>          if ( user64 )
>          {
> @@ -3852,10 +3842,10 @@ x86_emulate(
>              cs.attr.fields.l = 1;
>          }
>
> -        rc = ops->write_segment(x86_seg_cs, &cs, ctxt);
> -        fail_if(rc != 0);
> -        rc = ops->write_segment(x86_seg_ss, &ss, ctxt);
> -        fail_if(rc != 0);
> +        fail_if(ops->write_segment == NULL);
> +        if ( (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) != 0 ||
> +             (rc = ops->write_segment(x86_seg_ss, &ss, ctxt)) != 0 )
> +            goto done;
>
>          _regs.eip = _regs.edx;
>          _regs.esp = _regs.ecx;
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>