Re: [Qemu-devel] [PATCH v5 08/11] target-i386: exception handling for seg_helper functions

2015-07-07 Thread Pavel Dovgaluk 
 From: Richard Henderson [mailto:rth7...@gmail.com] On Behalf Of Richard 
 Henderson
 On 07/06/2015 09:26 AM, Pavel Dovgalyuk wrote:
  This patch fixes exception handling for seg_helper functions.
 
  Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru
 
 
 No, you don't want to discriminately change every call.  That was my original
 point about not needing to change seg_helper.c or smm_helper.c.
 
 Further, any such changes would go along with the changes in translate.c to
 remove the state saving there.
 
 I would only change those that are normal memory operations, like fp loads
 etc.  The segmentation changes are rare.  The task state helpers require state
 saving anyway, so requiring a TCG search is a pessimization.

I can refine the patch, but the most of the changes should remain.
E.g., lcall helpers can cause an exception or not. TB ends in both cases.
But icount and PC values in these two situations should be different.
And lcall helpers use most of the seg functions I changed in the patch.

Pavel Dovgalyuk

[Qemu-devel] [PATCH v5 08/11] target-i386: exception handling for seg_helper functions

2015-07-06 Thread Pavel Dovgalyuk 
This patch fixes exception handling for seg_helper functions.

Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru
---
 target-i386/seg_helper.c |  646 +++---
 1 files changed, 330 insertions(+), 316 deletions(-)

diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index a99475c..315caab 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -68,7 +68,8 @@
 
 /* return non zero if error */
 static inline int load_segment(CPUX86State *env, uint32_t *e1_ptr,
-   uint32_t *e2_ptr, int selector)
+   uint32_t *e2_ptr, int selector,
+   uintptr_t retaddr)
 {
 SegmentCache *dt;
 int index;
@@ -84,8 +85,8 @@ static inline int load_segment(CPUX86State *env, uint32_t 
*e1_ptr,
 return -1;
 }
 ptr = dt-base + index;
-*e1_ptr = cpu_ldl_kernel(env, ptr);
-*e2_ptr = cpu_ldl_kernel(env, ptr + 4);
+*e1_ptr = cpu_ldl_kernel_ra(env, ptr, retaddr);
+*e2_ptr = cpu_ldl_kernel_ra(env, ptr + 4, retaddr);
 return 0;
 }
 
@@ -124,7 +125,8 @@ static inline void load_seg_vm(CPUX86State *env, int seg, 
int selector)
 }
 
 static inline void get_ss_esp_from_tss(CPUX86State *env, uint32_t *ss_ptr,
-   uint32_t *esp_ptr, int dpl)
+   uint32_t *esp_ptr, int dpl,
+   uintptr_t retaddr)
 {
 X86CPU *cpu = x86_env_get_cpu(env);
 int type, index, shift;
@@ -153,60 +155,61 @@ static inline void get_ss_esp_from_tss(CPUX86State *env, 
uint32_t *ss_ptr,
 shift = type  3;
 index = (dpl * 4 + 2)  shift;
 if (index + (4  shift) - 1  env-tr.limit) {
-raise_exception_err(env, EXCP0A_TSS, env-tr.selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, env-tr.selector  0xfffc, 
retaddr);
 }
 if (shift == 0) {
-*esp_ptr = cpu_lduw_kernel(env, env-tr.base + index);
-*ss_ptr = cpu_lduw_kernel(env, env-tr.base + index + 2);
+*esp_ptr = cpu_lduw_kernel_ra(env, env-tr.base + index, retaddr);
+*ss_ptr = cpu_lduw_kernel_ra(env, env-tr.base + index + 2, retaddr);
 } else {
-*esp_ptr = cpu_ldl_kernel(env, env-tr.base + index);
-*ss_ptr = cpu_lduw_kernel(env, env-tr.base + index + 4);
+*esp_ptr = cpu_ldl_kernel_ra(env, env-tr.base + index, retaddr);
+*ss_ptr = cpu_lduw_kernel_ra(env, env-tr.base + index + 4, retaddr);
 }
 }
 
-static void tss_load_seg(CPUX86State *env, int seg_reg, int selector, int cpl)
+static void tss_load_seg(CPUX86State *env, int seg_reg, int selector, int cpl,
+ uintptr_t retaddr)
 {
 uint32_t e1, e2;
 int rpl, dpl;
 
 if ((selector  0xfffc) != 0) {
-if (load_segment(env, e1, e2, selector) != 0) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+if (load_segment(env, e1, e2, selector, retaddr) != 0) {
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);
 }
 if (!(e2  DESC_S_MASK)) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);
 }
 rpl = selector  3;
 dpl = (e2  DESC_DPL_SHIFT)  3;
 if (seg_reg == R_CS) {
 if (!(e2  DESC_CS_MASK)) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);
 }
 if (dpl != rpl) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);
 }
 } else if (seg_reg == R_SS) {
 /* SS must be writable data */
 if ((e2  DESC_CS_MASK) || !(e2  DESC_W_MASK)) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);
 }
 if (dpl != cpl || dpl != rpl) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);
 }
 } else {
 /* not readable code */
 if ((e2  DESC_CS_MASK)  !(e2  DESC_R_MASK)) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);
 }
 /* if data or non conforming code, checks the rights */
 if (((e2  DESC_TYPE_SHIFT)  0xf)  12) {
 if (dpl  cpl || dpl  rpl) {
-raise_exception_err(env, EXCP0A_TSS, selector  0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector  0xfffc, 
retaddr);

Re: [Qemu-devel] [PATCH v5 08/11] target-i386: exception handling for seg_helper functions

2015-07-06 Thread Richard Henderson 

On 07/06/2015 09:26 AM, Pavel Dovgalyuk wrote:

This patch fixes exception handling for seg_helper functions.

Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru



No, you don't want to discriminately change every call.  That was my original 
point about not needing to change seg_helper.c or smm_helper.c.


Further, any such changes would go along with the changes in translate.c to 
remove the state saving there.


I would only change those that are normal memory operations, like fp loads 
etc.  The segmentation changes are rare.  The task state helpers require state 
saving anyway, so requiring a TCG search is a pessimization.



r~

Re: [Qemu-devel] [PATCH v5 08/11] target-i386: exception handling for seg_helper functions

2015-07-06 Thread Pavel Dovgaluk 
 From: Richard Henderson [mailto:rth7...@gmail.com] On Behalf Of Richard 
 Henderson
 On 07/06/2015 09:26 AM, Pavel Dovgalyuk wrote:
  This patch fixes exception handling for seg_helper functions.
 
  Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru
 
 
 No, you don't want to discriminately change every call.  That was my original
 point about not needing to change seg_helper.c or smm_helper.c.
 
 Further, any such changes would go along with the changes in translate.c to
 remove the state saving there.
 
 I would only change those that are normal memory operations, like fp loads
 etc.  The segmentation changes are rare.  The task state helpers require state
 saving anyway, so requiring a TCG search is a pessimization.

Then we'll have to stop the translation after every instruction that uses these 
function.
Is this a limited subset?

Pavel Dovgalyuk