Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions

2013-05-23 Thread li guang 
ping ... again.

在 2013-04-22一的 11:44 +0800，liguang写道：
 for helper_{lsl, lar, verr, verw}, there are
 common parts, so move them outside, and then
 call this new helper-helper function.
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  target-i386/seg_helper.c |  179 ++---
  1 files changed, 56 insertions(+), 123 deletions(-)
 
 diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
 index 906e4f3..419efd8 100644
 --- a/target-i386/seg_helper.c
 +++ b/target-i386/seg_helper.c
 @@ -2292,9 +2292,10 @@ void helper_sysexit(CPUX86State *env, int dflag)
  EIP = EDX;
  }
  
 -target_ulong helper_lsl(CPUX86State *env, target_ulong selector1)
 +
 +static target_ulong misc_check_helper(CPUX86State *env, target_ulong 
 selector1,
 +  int inst)
  {
 -unsigned int limit;
  uint32_t e1, e2, eflags, selector;
  int rpl, dpl, cpl, type;
  
 @@ -2306,14 +2307,30 @@ target_ulong helper_lsl(CPUX86State *env, 
 target_ulong selector1)
  if (load_segment(env, e1, e2, selector) != 0) {
  goto fail;
  }
 +
 +CC_SRC = eflags  ~CC_Z;
 +
  rpl = selector  3;
  dpl = (e2  DESC_DPL_SHIFT)  3;
  cpl = env-hflags  HF_CPL_MASK;
 +
  if (e2  DESC_S_MASK) {
 -if ((e2  DESC_CS_MASK)  (e2  DESC_C_MASK)) {
 -/* conforming */
 -} else {
 -if (dpl  cpl || dpl  rpl) {
 +if (e2  DESC_CS_MASK) {
 +switch (inst) {
 +case 1:
 +goto fail;
 +case 2:
 +if (!(e2  (DESC_R_MASK | DESC_C_MASK))) {
 +goto fail;
 +}
 +break;
 +case 3:
 +case 4:
 +if (!(e2  DESC_C_MASK)) {
 +goto check_pl;
 +}
 +break;
 +default:
  goto fail;
  }
  }
 @@ -2325,140 +2342,56 @@ target_ulong helper_lsl(CPUX86State *env, 
 target_ulong selector1)
  case 3:
  case 9:
  case 11:
 -break;
 +if (inst == 3) {
 +break;
 +}
 +case 5:
 +case 12:
 +if (inst == 4) {
 +break;
 +}
  default:
  goto fail;
  }
 -if (dpl  cpl || dpl  rpl) {
 -fail:
 -CC_SRC = eflags  ~CC_Z;
 -return 0;
 -}
 +goto check_pl;
 +}
 +
 +if (inst == 3) {
 +e2 = 0x00f0ff00;
  }
 -limit = get_seg_limit(e1, e2);
 +if (inst == 4) {
 +e2 = get_seg_limit(e1, e2);
 +}
 +
  CC_SRC = eflags | CC_Z;
 -return limit;
 +
 +check_pl:
 +if (dpl  cpl || dpl  rpl) {
 +goto fail;
 +}
 +
 +fail:
 +return e2;
  }
  
 -target_ulong helper_lar(CPUX86State *env, target_ulong selector1)
 +target_ulong helper_lsl(CPUX86State *env, target_ulong selector1)
  {
 -uint32_t e1, e2, eflags, selector;
 -int rpl, dpl, cpl, type;
 +return misc_check_helper(env, selector1, 4);
 +}
  
 -selector = selector1  0x;
 -eflags = cpu_cc_compute_all(env, CC_OP);
 -if ((selector  0xfffc) == 0) {
 -goto fail;
 -}
 -if (load_segment(env, e1, e2, selector) != 0) {
 -goto fail;
 -}
 -rpl = selector  3;
 -dpl = (e2  DESC_DPL_SHIFT)  3;
 -cpl = env-hflags  HF_CPL_MASK;
 -if (e2  DESC_S_MASK) {
 -if ((e2  DESC_CS_MASK)  (e2  DESC_C_MASK)) {
 -/* conforming */
 -} else {
 -if (dpl  cpl || dpl  rpl) {
 -goto fail;
 -}
 -}
 -} else {
 -type = (e2  DESC_TYPE_SHIFT)  0xf;
 -switch (type) {
 -case 1:
 -case 2:
 -case 3:
 -case 4:
 -case 5:
 -case 9:
 -case 11:
 -case 12:
 -break;
 -default:
 -goto fail;
 -}
 -if (dpl  cpl || dpl  rpl) {
 -fail:
 -CC_SRC = eflags  ~CC_Z;
 -return 0;
 -}
 -}
 -CC_SRC = eflags | CC_Z;
 -return e2  0x00f0ff00;
 +target_ulong helper_lar(CPUX86State *env, target_ulong selector1)
 +{
 +return misc_check_helper(env, selector1, 3);
  }
  
  void helper_verr(CPUX86State *env, target_ulong selector1)
  {
 -uint32_t e1, e2, eflags, selector;
 -int rpl, dpl, cpl;
 -
 -selector = selector1  0x;
 -eflags = cpu_cc_compute_all(env, CC_OP);
 -if ((selector  0xfffc) == 0) {
 -goto fail;
 -}
 -if (load_segment(env, e1, e2, selector) != 0) {
 -goto fail;
 -}
 -if (!(e2  DESC_S_MASK)) {
 -goto fail;
 -}
 -rpl = selector  3;
 -dpl = (e2  DESC_DPL_SHIFT)  3;
 -cpl = env-hflags  HF_CPL_MASK;
 -if (e2  DESC_CS_MASK) {
 -if (!(e2  DESC_R_MASK)) {
 -goto fail;
 -}
 -if (!(e2  DESC_C_MASK)) {
 -

Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions

2013-05-23 Thread Peter Maydell 
On 23 May 2013 09:35, li guang lig.f...@cn.fujitsu.com wrote:
 ping ... again.

misc_check_helper is still a terrible function name.

-- PMM

Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions

2013-05-23 Thread li guang 
在 2013-05-23四的 10:24 +0100，Peter Maydell写道：
 On 23 May 2013 09:35, li guang lig.f...@cn.fujitsu.com wrote:
  ping ... again.
 
 misc_check_helper is still a terrible function name.
 

Oh, sorry, I ping the obsolete one,
I've sent a v2 for your comment:
v2: change misc_check_helper to privilege_check
http://lists.gnu.org/archive/html/qemu-devel/2013-05/msg00554.html

I'll ping it.

Thanks!

Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions

2013-04-30 Thread Peter Maydell 
On 22 April 2013 04:44, liguang lig.f...@cn.fujitsu.com wrote:
 for helper_{lsl, lar, verr, verw}, there are
 common parts, so move them outside, and then
 call this new helper-helper function.

 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  target-i386/seg_helper.c |  179 ++---
  1 files changed, 56 insertions(+), 123 deletions(-)

 diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
 index 906e4f3..419efd8 100644
 --- a/target-i386/seg_helper.c
 +++ b/target-i386/seg_helper.c
 @@ -2292,9 +2292,10 @@ void helper_sysexit(CPUX86State *env, int dflag)
  EIP = EDX;
  }

 -target_ulong helper_lsl(CPUX86State *env, target_ulong selector1)
 +
 +static target_ulong misc_check_helper(CPUX86State *env, target_ulong 
 selector1,
 +  int inst)

This function really needs a better (more specific) name...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions

2013-04-30 Thread li guang 
在 2013-04-30二的 08:16 +0100，Peter Maydell写道：
 On 22 April 2013 04:44, liguang lig.f...@cn.fujitsu.com wrote:
  for helper_{lsl, lar, verr, verw}, there are
  common parts, so move them outside, and then
  call this new helper-helper function.
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   target-i386/seg_helper.c |  179 
  ++---
   1 files changed, 56 insertions(+), 123 deletions(-)
 
  diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
  index 906e4f3..419efd8 100644
  --- a/target-i386/seg_helper.c
  +++ b/target-i386/seg_helper.c
  @@ -2292,9 +2292,10 @@ void helper_sysexit(CPUX86State *env, int dflag)
   EIP = EDX;
   }
 
  -target_ulong helper_lsl(CPUX86State *env, target_ulong selector1)
  +
  +static target_ulong misc_check_helper(CPUX86State *env, target_ulong 
  selector1,
  +  int inst)
 
 This function really needs a better (more specific) name...
 

Sorry, I'm hesitating to ask what will be the proper name?
this function is just a helper of other 4 functions,
seems its function can hardly be described by a name.

Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions

2013-04-29 Thread li guang 
ping ...

在 2013-04-22一的 11:44 +0800，liguang写道：
 for helper_{lsl, lar, verr, verw}, there are
 common parts, so move them outside, and then
 call this new helper-helper function.
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  target-i386/seg_helper.c |  179 ++---
  1 files changed, 56 insertions(+), 123 deletions(-)
 
 diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
 index 906e4f3..419efd8 100644
 --- a/target-i386/seg_helper.c
 +++ b/target-i386/seg_helper.c
 @@ -2292,9 +2292,10 @@ void helper_sysexit(CPUX86State *env, int dflag)
  EIP = EDX;
  }
  
 -target_ulong helper_lsl(CPUX86State *env, target_ulong selector1)
 +
 +static target_ulong misc_check_helper(CPUX86State *env, target_ulong 
 selector1,
 +  int inst)
  {
 -unsigned int limit;
  uint32_t e1, e2, eflags, selector;
  int rpl, dpl, cpl, type;
  
 @@ -2306,14 +2307,30 @@ target_ulong helper_lsl(CPUX86State *env, 
 target_ulong selector1)
  if (load_segment(env, e1, e2, selector) != 0) {
  goto fail;
  }
 +
 +CC_SRC = eflags  ~CC_Z;
 +
  rpl = selector  3;
  dpl = (e2  DESC_DPL_SHIFT)  3;
  cpl = env-hflags  HF_CPL_MASK;
 +
  if (e2  DESC_S_MASK) {
 -if ((e2  DESC_CS_MASK)  (e2  DESC_C_MASK)) {
 -/* conforming */
 -} else {
 -if (dpl  cpl || dpl  rpl) {
 +if (e2  DESC_CS_MASK) {
 +switch (inst) {
 +case 1:
 +goto fail;
 +case 2:
 +if (!(e2  (DESC_R_MASK | DESC_C_MASK))) {
 +goto fail;
 +}
 +break;
 +case 3:
 +case 4:
 +if (!(e2  DESC_C_MASK)) {
 +goto check_pl;
 +}
 +break;
 +default:
  goto fail;
  }
  }
 @@ -2325,140 +2342,56 @@ target_ulong helper_lsl(CPUX86State *env, 
 target_ulong selector1)
  case 3:
  case 9:
  case 11:
 -break;
 +if (inst == 3) {
 +break;
 +}
 +case 5:
 +case 12:
 +if (inst == 4) {
 +break;
 +}
  default:
  goto fail;
  }
 -if (dpl  cpl || dpl  rpl) {
 -fail:
 -CC_SRC = eflags  ~CC_Z;
 -return 0;
 -}
 +goto check_pl;
 +}
 +
 +if (inst == 3) {
 +e2 = 0x00f0ff00;
  }
 -limit = get_seg_limit(e1, e2);
 +if (inst == 4) {
 +e2 = get_seg_limit(e1, e2);
 +}
 +
  CC_SRC = eflags | CC_Z;
 -return limit;
 +
 +check_pl:
 +if (dpl  cpl || dpl  rpl) {
 +goto fail;
 +}
 +
 +fail:
 +return e2;
  }
  
 -target_ulong helper_lar(CPUX86State *env, target_ulong selector1)
 +target_ulong helper_lsl(CPUX86State *env, target_ulong selector1)
  {
 -uint32_t e1, e2, eflags, selector;
 -int rpl, dpl, cpl, type;
 +return misc_check_helper(env, selector1, 4);
 +}
  
 -selector = selector1  0x;
 -eflags = cpu_cc_compute_all(env, CC_OP);
 -if ((selector  0xfffc) == 0) {
 -goto fail;
 -}
 -if (load_segment(env, e1, e2, selector) != 0) {
 -goto fail;
 -}
 -rpl = selector  3;
 -dpl = (e2  DESC_DPL_SHIFT)  3;
 -cpl = env-hflags  HF_CPL_MASK;
 -if (e2  DESC_S_MASK) {
 -if ((e2  DESC_CS_MASK)  (e2  DESC_C_MASK)) {
 -/* conforming */
 -} else {
 -if (dpl  cpl || dpl  rpl) {
 -goto fail;
 -}
 -}
 -} else {
 -type = (e2  DESC_TYPE_SHIFT)  0xf;
 -switch (type) {
 -case 1:
 -case 2:
 -case 3:
 -case 4:
 -case 5:
 -case 9:
 -case 11:
 -case 12:
 -break;
 -default:
 -goto fail;
 -}
 -if (dpl  cpl || dpl  rpl) {
 -fail:
 -CC_SRC = eflags  ~CC_Z;
 -return 0;
 -}
 -}
 -CC_SRC = eflags | CC_Z;
 -return e2  0x00f0ff00;
 +target_ulong helper_lar(CPUX86State *env, target_ulong selector1)
 +{
 +return misc_check_helper(env, selector1, 3);
  }
  
  void helper_verr(CPUX86State *env, target_ulong selector1)
  {
 -uint32_t e1, e2, eflags, selector;
 -int rpl, dpl, cpl;
 -
 -selector = selector1  0x;
 -eflags = cpu_cc_compute_all(env, CC_OP);
 -if ((selector  0xfffc) == 0) {
 -goto fail;
 -}
 -if (load_segment(env, e1, e2, selector) != 0) {
 -goto fail;
 -}
 -if (!(e2  DESC_S_MASK)) {
 -goto fail;
 -}
 -rpl = selector  3;
 -dpl = (e2  DESC_DPL_SHIFT)  3;
 -cpl = env-hflags  HF_CPL_MASK;
 -if (e2  DESC_CS_MASK) {
 -if (!(e2  DESC_R_MASK)) {
 -goto fail;
 -}
 -if (!(e2  DESC_C_MASK)) {
 -