| Message ID | 20141107103223.6136.57870.stgit@PASHA-ISP |
|---|---|
| State | New |
| Headers | show |
On 07/11/2014 11:32, Pavel Dovgalyuk wrote: > This patch denies crossing the boundary of the pages in the replay mode, > because it can cause an exception. Do it only when boundary is > crossed by the first instruction in the block. > If current instruction already crossed the bound - it's ok, > because an exception hasn't stopped this code. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > --- > target-i386/cpu.h | 7 +++++++ > target-i386/translate.c | 14 ++++++++++++++ > 2 files changed, 21 insertions(+), 0 deletions(-) > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 2968749..bc3f9f5 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -28,6 +28,13 @@ > #define TARGET_LONG_BITS 32 > #endif > > +/* Maximum instruction code size */ > +#ifdef TARGET_X86_64 > +#define TARGET_MAX_INSN_SIZE 16 > +#else > +#define TARGET_MAX_INSN_SIZE 16 > +#endif > + > /* target supports implicit self modifying code */ > #define TARGET_HAS_SMC > /* support for self modifying code even if the modified instruction is > diff --git a/target-i386/translate.c b/target-i386/translate.c > index 4d5dfb3..a264908 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -8035,6 +8035,20 @@ static inline void gen_intermediate_code_internal(X86CPU *cpu, > gen_eob(dc); > break; > } > + /* Do not cross the boundary of the pages in icount mode, > + it can cause an exception. Do it only when boundary is > + crossed by the first instruction in the block. > + If current instruction already crossed the bound - it's ok, > + because an exception hasn't stopped this code. > + */ > + if (use_icount > + && ((pc_ptr & TARGET_PAGE_MASK) > + != ((pc_ptr + TARGET_MAX_INSN_SIZE - 1) & TARGET_PAGE_MASK) > + || (pc_ptr & ~TARGET_PAGE_MASK) == 0)) { > + gen_jmp_im(pc_ptr - dc->cs_base); > + gen_eob(dc); > + break; > + } > /* if too long translation, stop generation too */ > if (tcg_ctx.gen_opc_ptr >= gen_opc_end || > (pc_ptr - pc_start) >= (TARGET_PAGE_SIZE - 32) || > Why only in icount mode? Does it have a sensible performance problem? Paolo
Am 07.11.2014 um 11:32 schrieb Pavel Dovgalyuk: > This patch denies crossing the boundary of the pages in the replay mode, > because it can cause an exception. Do it only when boundary is > crossed by the first instruction in the block. > If current instruction already crossed the bound - it's ok, > because an exception hasn't stopped this code. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > --- > target-i386/cpu.h | 7 +++++++ > target-i386/translate.c | 14 ++++++++++++++ > 2 files changed, 21 insertions(+), 0 deletions(-) > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 2968749..bc3f9f5 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -28,6 +28,13 @@ > #define TARGET_LONG_BITS 32 > #endif > > +/* Maximum instruction code size */ > +#ifdef TARGET_X86_64 > +#define TARGET_MAX_INSN_SIZE 16 > +#else > +#define TARGET_MAX_INSN_SIZE 16 > +#endif Is this a spot-the-difference game? ;) Seriously, if they're the same values, just drop the #ifdef. > + > /* target supports implicit self modifying code */ > #define TARGET_HAS_SMC > /* support for self modifying code even if the modified instruction is [snip] Regards, Andreas
> From: Paolo Bonzini [mailto:pbonzini@redhat.com] > On 07/11/2014 11:32, Pavel Dovgalyuk wrote: > > This patch denies crossing the boundary of the pages in the replay mode, > > because it can cause an exception. Do it only when boundary is > > crossed by the first instruction in the block. > > If current instruction already crossed the bound - it's ok, > > because an exception hasn't stopped this code. > > > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > --- > > target-i386/cpu.h | 7 +++++++ > > target-i386/translate.c | 14 ++++++++++++++ > > 2 files changed, 21 insertions(+), 0 deletions(-) > > > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > > index 2968749..bc3f9f5 100644 > > --- a/target-i386/cpu.h > > +++ b/target-i386/cpu.h > > @@ -28,6 +28,13 @@ > > #define TARGET_LONG_BITS 32 > > #endif > > > > +/* Maximum instruction code size */ > > +#ifdef TARGET_X86_64 > > +#define TARGET_MAX_INSN_SIZE 16 > > +#else > > +#define TARGET_MAX_INSN_SIZE 16 > > +#endif > > + > > /* target supports implicit self modifying code */ > > #define TARGET_HAS_SMC > > /* support for self modifying code even if the modified instruction is > > diff --git a/target-i386/translate.c b/target-i386/translate.c > > index 4d5dfb3..a264908 100644 > > --- a/target-i386/translate.c > > +++ b/target-i386/translate.c > > @@ -8035,6 +8035,20 @@ static inline void gen_intermediate_code_internal(X86CPU *cpu, > > gen_eob(dc); > > break; > > } > > + /* Do not cross the boundary of the pages in icount mode, > > + it can cause an exception. Do it only when boundary is > > + crossed by the first instruction in the block. > > + If current instruction already crossed the bound - it's ok, > > + because an exception hasn't stopped this code. > > + */ > > + if (use_icount > > + && ((pc_ptr & TARGET_PAGE_MASK) > > + != ((pc_ptr + TARGET_MAX_INSN_SIZE - 1) & TARGET_PAGE_MASK) > > + || (pc_ptr & ~TARGET_PAGE_MASK) == 0)) { > > + gen_jmp_im(pc_ptr - dc->cs_base); > > + gen_eob(dc); > > + break; > > + } > > /* if too long translation, stop generation too */ > > if (tcg_ctx.gen_opc_ptr >= gen_opc_end || > > (pc_ptr - pc_start) >= (TARGET_PAGE_SIZE - 32) || > > > > Why only in icount mode? Does it have a sensible performance problem? Maybe. This is the same problem, which was discussed here for ARM: http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg02232.html But I haven't figured out what I have to do with it. Pavel Dovgalyuk
diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 2968749..bc3f9f5 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -28,6 +28,13 @@ #define TARGET_LONG_BITS 32 #endif +/* Maximum instruction code size */ +#ifdef TARGET_X86_64 +#define TARGET_MAX_INSN_SIZE 16 +#else +#define TARGET_MAX_INSN_SIZE 16 +#endif + /* target supports implicit self modifying code */ #define TARGET_HAS_SMC /* support for self modifying code even if the modified instruction is diff --git a/target-i386/translate.c b/target-i386/translate.c index 4d5dfb3..a264908 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -8035,6 +8035,20 @@ static inline void gen_intermediate_code_internal(X86CPU *cpu, gen_eob(dc); break; } + /* Do not cross the boundary of the pages in icount mode, + it can cause an exception. Do it only when boundary is + crossed by the first instruction in the block. + If current instruction already crossed the bound - it's ok, + because an exception hasn't stopped this code. + */ + if (use_icount + && ((pc_ptr & TARGET_PAGE_MASK) + != ((pc_ptr + TARGET_MAX_INSN_SIZE - 1) & TARGET_PAGE_MASK) + || (pc_ptr & ~TARGET_PAGE_MASK) == 0)) { + gen_jmp_im(pc_ptr - dc->cs_base); + gen_eob(dc); + break; + } /* if too long translation, stop generation too */ if (tcg_ctx.gen_opc_ptr >= gen_opc_end || (pc_ptr - pc_start) >= (TARGET_PAGE_SIZE - 32) ||
This patch denies crossing the boundary of the pages in the replay mode, because it can cause an exception. Do it only when boundary is crossed by the first instruction in the block. If current instruction already crossed the bound - it's ok, because an exception hasn't stopped this code. Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> --- target-i386/cpu.h | 7 +++++++ target-i386/translate.c | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 0 deletions(-)