Re: [Qemu-devel] [PATCH v6 09/10] target-i386: exception handling for other helper functions

[Pavel Dovgaluk]

> From: Richard Henderson [mailto:address@hidden On Behalf Of Richard Henderson
> On 07/07/2015 02:31 PM, Pavel Dovgalyuk wrote:
> > diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
> > index 52c5d65..c8e7ee9 100644
> > --- a/target-i386/misc_helper.c
> > +++ b/target-i386/misc_helper.c
> > @@ -220,7 +220,7 @@ void helper_rdtsc(CPUX86State *env)
> >       uint64_t val;
> >
> >       if ((env->cr[4] & CR4_TSD_MASK) && ((env->hflags & HF_CPL_MASK) != 
> > 0)) {
> > -        raise_exception(env, EXCP0D_GPF);
> > +        raise_exception_ra(env, EXCP0D_GPF, GETPC());
> >       }
> >       cpu_svm_check_intercept_param(env, SVM_EXIT_RDTSC, 0);
> >
> > @@ -238,13 +238,13 @@ void helper_rdtscp(CPUX86State *env)
> >   void helper_rdpmc(CPUX86State *env)
> >   {
> >       if ((env->cr[4] & CR4_PCE_MASK) && ((env->hflags & HF_CPL_MASK) != 
> > 0)) {
> > -        raise_exception(env, EXCP0D_GPF);
> > +        raise_exception_ra(env, EXCP0D_GPF, GETPC());
> >       }
> >       cpu_svm_check_intercept_param(env, SVM_EXIT_RDPMC, 0);
> >
> >       /* currently unimplemented */
> >       qemu_log_mask(LOG_UNIMP, "x86: unimplemented rdpmc\n");
> > -    raise_exception_err(env, EXCP06_ILLOP, 0);
> > +    raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
> >   }
> >
> >   #if defined(CONFIG_USER_ONLY)
> > @@ -589,7 +589,7 @@ void helper_hlt(CPUX86State *env, int next_eip_addend)
> >   void helper_monitor(CPUX86State *env, target_ulong ptr)
> >   {
> >       if ((uint32_t)env->regs[R_ECX] != 0) {
> > -        raise_exception(env, EXCP0D_GPF);
> > +        raise_exception_ra(env, EXCP0D_GPF, GETPC());
> >       }
> >       /* XXX: store address? */
> >       cpu_svm_check_intercept_param(env, SVM_EXIT_MONITOR, 0);
> 
> Likewise.

Like what?

> > @@ -601,7 +601,7 @@ void helper_mwait(CPUX86State *env, int next_eip_addend)
> >       X86CPU *cpu;
> >
> >       if ((uint32_t)env->regs[R_ECX] != 0) {
> > -        raise_exception(env, EXCP0D_GPF);
> > +        raise_exception_ra(env, EXCP0D_GPF, GETPC());
> >       }
> >       cpu_svm_check_intercept_param(env, SVM_EXIT_MWAIT, 0);
> >       env->eip += next_eip_addend;
> 
> Similar to lcall/ljmp, this can be switched to not use an addend.

Not exactly. cpu_svm_check_intercept_param could call helper_vmexit,
which uses env->eip.

> > diff --git a/target-i386/svm_helper.c b/target-i386/svm_helper.c
> > index f1fabf5..cc8c2ec 100644
> > --- a/target-i386/svm_helper.c
> > +++ b/target-i386/svm_helper.c
> > @@ -354,7 +354,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int 
> > next_eip_addend)
> >   void helper_vmmcall(CPUX86State *env)
> >   {
> >       cpu_svm_check_intercept_param(env, SVM_EXIT_VMMCALL, 0);
> > -    raise_exception(env, EXCP06_ILLOP);
> > +    raise_exception_ra(env, EXCP06_ILLOP, GETPC());
> >   }
> >
> >   void helper_vmload(CPUX86State *env, int aflag)
> > @@ -457,7 +457,7 @@ void helper_skinit(CPUX86State *env)
> >   {
> >       cpu_svm_check_intercept_param(env, SVM_EXIT_SKINIT, 0);
> >       /* XXX: not implemented */
> > -    raise_exception(env, EXCP06_ILLOP);
> > +    raise_exception_ra(env, EXCP06_ILLOP, GETPC());
> >   }
> 
> Either these are missing a change to translate.c, or they're pointless 
> changes.
>   Probably the later.

Then it is better to remove these changes from the patch?


Pavel Dovgalyuk