* [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions
@ 2013-04-22 3:44 liguang
2013-04-22 3:44 ` [Qemu-devel] [PATCH 2/2] target-i386/seg_helper: define names for code/data segment types liguang
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: liguang @ 2013-04-22 3:44 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, pbonzini, afaerber, lig.fnst
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.fnst@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 & 0xffff;
- 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 & 0xffff;
- 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)) {
- if (dpl < cpl || dpl < rpl) {
- goto fail;
- }
- }
- } else {
- if (dpl < cpl || dpl < rpl) {
- fail:
- CC_SRC = eflags & ~CC_Z;
- return;
- }
- }
- CC_SRC = eflags | CC_Z;
+ misc_check_helper(env, selector1, 2);
}
void helper_verw(CPUX86State *env, target_ulong selector1)
{
- uint32_t e1, e2, eflags, selector;
- int rpl, dpl, cpl;
-
- selector = selector1 & 0xffff;
- 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) {
- goto fail;
- } else {
- if (dpl < cpl || dpl < rpl) {
- goto fail;
- }
- if (!(e2 & DESC_W_MASK)) {
- fail:
- CC_SRC = eflags & ~CC_Z;
- return;
- }
- }
- CC_SRC = eflags | CC_Z;
+ misc_check_helper(env, selector1, 1);
}
#if defined(CONFIG_USER_ONLY)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] target-i386/seg_helper: define names for code/data segment types
2013-04-22 3:44 [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions liguang
@ 2013-04-22 3:44 ` liguang
2013-04-30 3:21 ` [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions li guang
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: liguang @ 2013-04-22 3:44 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, pbonzini, afaerber, lig.fnst
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
target-i386/seg_helper.c | 71 +++++++++++++++++++++++++++++++++------------
1 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 419efd8..6b35b7c 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -2292,6 +2292,39 @@ void helper_sysexit(CPUX86State *env, int dflag)
EIP = EDX;
}
+/*
+ * for data segment types
+ * RO -- read-only, A -- accessed,
+ * W -- write, E -- expand-down
+ */
+#define DESC_DSEG_RO 0
+#define DESC_DSEG_ROA 1
+#define DESC_DSEG_RW 2
+#define DESC_DSEG_RWA 3
+#define DESC_DSEG_ROE 4
+#define DESC_DSEG_ROEA 5
+#define DESC_DSEG_RWE 6
+#define DESC_DSEG_RWEA 7
+
+/*
+ * for code segment types
+ * EO -- execute-only, A -- accessed,
+ * C -- conforming, E -- expand-down, R -- read
+ */
+
+#define DESC_CSEG_EO 8
+#define DESC_CSEG_EOA 9
+#define DESC_CSEG_ER 10
+#define DESC_CSEG_ERA 11
+#define DESC_CSEG_EOC 12
+#define DESC_CSEG_EOCA 13
+#define DESC_CSEG_ERC 14
+#define DESC_CSEG_ERCA 15
+
+#define FUNC_LSL 4
+#define FUNC_LAR 3
+#define FUNC_VERR 2
+#define FUNC_VERW 1
static target_ulong misc_check_helper(CPUX86State *env, target_ulong selector1,
int inst)
@@ -2317,15 +2350,15 @@ static target_ulong misc_check_helper(CPUX86State *env, target_ulong selector1,
if (e2 & DESC_S_MASK) {
if (e2 & DESC_CS_MASK) {
switch (inst) {
- case 1:
+ case FUNC_VERW:
goto fail;
- case 2:
+ case FUNC_VERR:
if (!(e2 & (DESC_R_MASK | DESC_C_MASK))) {
goto fail;
}
break;
- case 3:
- case 4:
+ case FUNC_LAR:
+ case FUNC_LSL:
if (!(e2 & DESC_C_MASK)) {
goto check_pl;
}
@@ -2337,17 +2370,17 @@ static target_ulong misc_check_helper(CPUX86State *env, target_ulong selector1,
} else {
type = (e2 >> DESC_TYPE_SHIFT) & 0xf;
switch (type) {
- case 1:
- case 2:
- case 3:
- case 9:
- case 11:
- if (inst == 3) {
+ case DESC_DSEG_ROA:
+ case DESC_DSEG_RW:
+ case DESC_DSEG_RWA:
+ case DESC_CSEG_EOA:
+ case DESC_CSEG_ERA:
+ if (inst == FUNC_LAR) {
break;
}
- case 5:
- case 12:
- if (inst == 4) {
+ case DESC_DSEG_ROEA:
+ case DESC_CSEG_EOC:
+ if (inst == FUNC_LSL) {
break;
}
default:
@@ -2356,10 +2389,10 @@ static target_ulong misc_check_helper(CPUX86State *env, target_ulong selector1,
goto check_pl;
}
- if (inst == 3) {
+ if (inst == FUNC_LAR) {
e2 &= 0x00f0ff00;
}
- if (inst == 4) {
+ if (inst == FUNC_LSL) {
e2 = get_seg_limit(e1, e2);
}
@@ -2376,22 +2409,22 @@ fail:
target_ulong helper_lsl(CPUX86State *env, target_ulong selector1)
{
- return misc_check_helper(env, selector1, 4);
+ return misc_check_helper(env, selector1, FUNC_LSL);
}
target_ulong helper_lar(CPUX86State *env, target_ulong selector1)
{
- return misc_check_helper(env, selector1, 3);
+ return misc_check_helper(env, selector1, FUNC_LAR);
}
void helper_verr(CPUX86State *env, target_ulong selector1)
{
- misc_check_helper(env, selector1, 2);
+ misc_check_helper(env, selector1, FUNC_VERR);
}
void helper_verw(CPUX86State *env, target_ulong selector1)
{
- misc_check_helper(env, selector1, 1);
+ misc_check_helper(env, selector1, FUNC_VERW);
}
#if defined(CONFIG_USER_ONLY)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions
2013-04-22 3:44 [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions liguang
2013-04-22 3:44 ` [Qemu-devel] [PATCH 2/2] target-i386/seg_helper: define names for code/data segment types liguang
@ 2013-04-30 3:21 ` li guang
2013-04-30 7:16 ` Peter Maydell
2013-05-23 8:35 ` li guang
3 siblings, 0 replies; 8+ messages in thread
From: li guang @ 2013-04-30 3:21 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, pbonzini, afaerber
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.fnst@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 & 0xffff;
> - 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 & 0xffff;
> - 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)) {
> - if (dpl < cpl || dpl < rpl) {
> - goto fail;
> - }
> - }
> - } else {
> - if (dpl < cpl || dpl < rpl) {
> - fail:
> - CC_SRC = eflags & ~CC_Z;
> - return;
> - }
> - }
> - CC_SRC = eflags | CC_Z;
> + misc_check_helper(env, selector1, 2);
> }
>
> void helper_verw(CPUX86State *env, target_ulong selector1)
> {
> - uint32_t e1, e2, eflags, selector;
> - int rpl, dpl, cpl;
> -
> - selector = selector1 & 0xffff;
> - 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) {
> - goto fail;
> - } else {
> - if (dpl < cpl || dpl < rpl) {
> - goto fail;
> - }
> - if (!(e2 & DESC_W_MASK)) {
> - fail:
> - CC_SRC = eflags & ~CC_Z;
> - return;
> - }
> - }
> - CC_SRC = eflags | CC_Z;
> + misc_check_helper(env, selector1, 1);
> }
>
> #if defined(CONFIG_USER_ONLY)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions
2013-04-22 3:44 [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions liguang
2013-04-22 3:44 ` [Qemu-devel] [PATCH 2/2] target-i386/seg_helper: define names for code/data segment types liguang
2013-04-30 3:21 ` [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions li guang
@ 2013-04-30 7:16 ` Peter Maydell
2013-04-30 7:27 ` li guang
2013-05-23 8:35 ` li guang
3 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2013-04-30 7:16 UTC (permalink / raw)
To: liguang; +Cc: blauwirbel, pbonzini, qemu-devel, afaerber
On 22 April 2013 04:44, liguang <lig.fnst@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.fnst@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions
2013-04-30 7:16 ` Peter Maydell
@ 2013-04-30 7:27 ` li guang
0 siblings, 0 replies; 8+ messages in thread
From: li guang @ 2013-04-30 7:27 UTC (permalink / raw)
To: Peter Maydell; +Cc: blauwirbel, pbonzini, qemu-devel, afaerber
在 2013-04-30二的 08:16 +0100，Peter Maydell写道：
> On 22 April 2013 04:44, liguang <lig.fnst@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.fnst@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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions
2013-04-22 3:44 [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions liguang
` (2 preceding siblings ...)
2013-04-30 7:16 ` Peter Maydell
@ 2013-05-23 8:35 ` li guang
2013-05-23 9:24 ` Peter Maydell
3 siblings, 1 reply; 8+ messages in thread
From: li guang @ 2013-05-23 8:35 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, pbonzini, afaerber
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.fnst@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 & 0xffff;
> - 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 & 0xffff;
> - 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)) {
> - if (dpl < cpl || dpl < rpl) {
> - goto fail;
> - }
> - }
> - } else {
> - if (dpl < cpl || dpl < rpl) {
> - fail:
> - CC_SRC = eflags & ~CC_Z;
> - return;
> - }
> - }
> - CC_SRC = eflags | CC_Z;
> + misc_check_helper(env, selector1, 2);
> }
>
> void helper_verw(CPUX86State *env, target_ulong selector1)
> {
> - uint32_t e1, e2, eflags, selector;
> - int rpl, dpl, cpl;
> -
> - selector = selector1 & 0xffff;
> - 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) {
> - goto fail;
> - } else {
> - if (dpl < cpl || dpl < rpl) {
> - goto fail;
> - }
> - if (!(e2 & DESC_W_MASK)) {
> - fail:
> - CC_SRC = eflags & ~CC_Z;
> - return;
> - }
> - }
> - CC_SRC = eflags | CC_Z;
> + misc_check_helper(env, selector1, 1);
> }
>
> #if defined(CONFIG_USER_ONLY)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions
2013-05-23 8:35 ` li guang
@ 2013-05-23 9:24 ` Peter Maydell
2013-05-24 0:17 ` li guang
0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2013-05-23 9:24 UTC (permalink / raw)
To: li guang; +Cc: blauwirbel, pbonzini, qemu-devel, afaerber
On 23 May 2013 09:35, li guang <lig.fnst@cn.fujitsu.com> wrote:
> ping ... again.
misc_check_helper is still a terrible function name.
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions
2013-05-23 9:24 ` Peter Maydell
@ 2013-05-24 0:17 ` li guang
0 siblings, 0 replies; 8+ messages in thread
From: li guang @ 2013-05-24 0:17 UTC (permalink / raw)
To: Peter Maydell; +Cc: blauwirbel, pbonzini, qemu-devel, afaerber
在 2013-05-23四的 10:24 +0100，Peter Maydell写道：
> On 23 May 2013 09:35, li guang <lig.fnst@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!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-05-24 0:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-22 3:44 [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions liguang
2013-04-22 3:44 ` [Qemu-devel] [PATCH 2/2] target-i386/seg_helper: define names for code/data segment types liguang
2013-04-30 3:21 ` [Qemu-devel] [PATCH 1/2] target-i386/seg_helper: refactor 4 helper functions li guang
2013-04-30 7:16 ` Peter Maydell
2013-04-30 7:27 ` li guang
2013-05-23 8:35 ` li guang
2013-05-23 9:24 ` Peter Maydell
2013-05-24 0:17 ` li guang
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).