qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
@ 2007-09-18 20:54 J. Mayer
  2007-09-18 23:05 ` J. Mayer
  0 siblings, 1 reply; 10+ messages in thread
From: J. Mayer @ 2007-09-18 20:54 UTC (permalink / raw)
  To: qemu-devel

Forwarded, as requested.

-------- Forwarded Message --------
> From: Alexander Graf <agraf@suse.de>
> To: J.Mayer <l_indien@magic.fr>
> Subject: Re: [Qemu-devel] [PATCH] SVM support
> Date: Tue, 18 Sep 2007 13:51:26 +0200
> 
> On Sep 18, 2007, at 12:09 PM, J. Mayer wrote:
> 
> > On Tue, 2007-09-18 at 03:02 +0200, Alexander Graf wrote:
> >> Jocelyn Mayer wrote:
> >>> On Mon, 2007-09-17 at 12:02 +0200, Alexander Graf wrote:
> >>>
> >>>> J. Mayer wrote:
> >>>>
> >>>>> On Thu, 2007-09-13 at 17:27 +0200, Alexander Graf wrote:
> >>>>>
> >>>>>
> >>>>>> Thiemo Seufer wrote:
> >>>>>>
> >>>>>>
> >>>>>>> Alexander Graf wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>> Thanks to Michael Peter who tried the patch on an x86 host I  
> >>>>>>>> found some
> >>>>>>>> compilation problems.
> >>>>>>>> This updated patch addresses these issues and thus should  
> >>>>>>>> compile on
> >>>>>>>> every platform for every target available.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>> [...]
> >>>>>
> >>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>> Wow sorry about that, looks like I missed these.
> >>>>>>
> >>>>>>
> >>>>> Index: qemu-0.9.0.cvs/exec-all.h
> >>>>> ================================================================== 
> >>>>> =
> >>>>> --- qemu-0.9.0.cvs.orig/exec-all.h
> >>>>> +++ qemu-0.9.0.cvs/exec-all.h
> >>>>> @@ -166,6 +166,7 @@ static inline int tlb_set_page(CPUState
> >>>>>  typedef struct TranslationBlock {
> >>>>>      target_ulong pc;   /* simulated PC corresponding to this  
> >>>>> block (EIP
> >>>>> + CS base) */
> >>>>>      target_ulong cs_base; /* CS base for this block */
> >>>>> +    uint64_t intercept; /* SVM intercept vector */
> >>>>>      unsigned int flags; /* flags defining in which context the  
> >>>>> code was
> >>>>> generated */
> >>>>>      uint16_t size;      /* size of target code for this block  
> >>>>> (1 <=
> >>>>>                             size <= TARGET_PAGE_SIZE) */
> >>>>> Index: qemu-0.9.0.cvs/cpu-all.h
> >>>>> ================================================================== 
> >>>>> =
> >>>>> --- qemu-0.9.0.cvs.orig/cpu-all.h
> >>>>> +++ qemu-0.9.0.cvs/cpu-all.h
> >>>>> @@ -715,6 +715,7 @@ extern int code_copy_enabled;
> >>>>>  #define CPU_INTERRUPT_HALT   0x20 /* CPU halt wanted */
> >>>>>  #define CPU_INTERRUPT_SMI    0x40 /* (x86 only) SMI interrupt  
> >>>>> pending
> >>>>> */
> >>>>>  #define CPU_INTERRUPT_DEBUG  0x80 /* Debug event occured.  */
> >>>>> +#define CPU_INTERRUPT_VIRQ   0x100 /* virtual interrupt  
> >>>>> pending.  */
> >>>>>
> >>>>>  void cpu_interrupt(CPUState *s, int mask);
> >>>>>  void cpu_reset_interrupt(CPUState *env, int mask);
> >>>>>
> >>>>> Those two patches look ugly to me as target specific features  
> >>>>> should
> >>>>> never go in generic code or structures.
> >>>>> The CPU_INTERRUPT flags should just contain information about the
> >>>>> emulator behavior, thus CPU_INTERRUPT_TIMER, CPU_INTERRUPT_SMI  
> >>>>> are to be
> >>>>> removed. Target specific informations about the exception  
> >>>>> nature should
> >>>>> go in the CPUState structure... Then, adding a  
> >>>>> CPU_INTERRUPT_VIRQ seems
> >>>>> not a good idea at all: it's outside of the generic emulator  
> >>>>> scope and
> >>>>> pointless for most targets.
> >>>>>
> >>>>>
> >>>> I don't see any practical reason not to do it this way. We could  
> >>>> define
> >>>> a CPU_INTERRUPT_TARGET interrupt, that stores the information in  
> >>>> a the
> >>>> target specific CPUState, but this would hit performance  
> >>>> (marginally
> >>>> though) and does not improve the situation. I don't think that  
> >>>> we should
> >>>> should forcefully try to seperate targets where we are not even  
> >>>> close to
> >>>> running out of constants. If everyone on this list sees the  
> >>>> situation as
> >>>> Jocelyn does, I would be fine with writing a patch that puts target
> >>>> specific interrupts to the specific targets.
> >>>>
> >>>
> >>> I was to do the same to add some functionality to the PowerPC  
> >>> target,
> >>> long time ago, and Fabrice Bellard convinced me not to do and  
> >>> agreed it
> >>> has been a bad idea to add the target specific CPU_INTERRUPT_xxx  
> >>> flags.
> >>> Then I did manage the PowerPC target to use only generic
> >>> CPU_INTERRUPT_xxx and put all other target specific informations  
> >>> in the
> >>> CPUState structure. It absolutelly changes nothing in terms of
> >>> functionality nor performance. The only thing is that it makes the
> >>> generic parts simpler and could even allow the cpu_exec function to
> >>> contain almost no target specific code, which would really great  
> >>> imho.
> >>>
> >>
> >> I can give that a try :-). Sounds reasonable for me.
> >
> >> [Next message]
> >> Oh well I just thought about this a bit more and while stumbling  
> >> across
> >> CPU_INTERRUPT_FIQ which does the same thing one major problem came  
> >> to me
> >> on that one: Priority. Real interrupts have priority over virtual
> >> interrupts so the VIRQs have to be processed after HARD IRQs, whereas
> >> SMIs and NMIs have to be taken care of before the HARD IRQs. It  
> >> simply
> >> wouldn't work out to generalize the IRQs, just as it would not  
> >> work with
> >> the VIRQ, as it has to be handled as a real IRQ but without accessing
> >> the APIC which has to be done for HARD IRQs.
> >>
> >> I guess we're stuck with what's there now.
> >
> > Priorities are not an issue, you can take a look to the  
> > ppc_hw_interrupt function
> > to see how the PowerPC emulation code manages priorities and  
> > masking between
> > 10 different hardware interruption sources. The code could be  
> > better (I wanted
> > to preserve existing code) but it's a solution that actually works...
> > But looking better the x86 emulation code, I agree that if you  
> > don't want to change it
> > too much, you have to add a flag. The other solution (and best,  
> > imho) would be
> > to do the same as what has been done for PowerPC: just let the  
> > generic code in
> > the cpu_exec loop and move the x86 specific code somewhere in  
> > target-i386 code.
> > Going this way, I see no reason why the interruption code in the  
> > cpu_exec loop
> > could not finally become:
> >                 interrupt_request = env->interrupt_request;
> >                 if (__builtin_expect(interrupt_request, 0)) {
> >                     if (interrupt_request & CPU_INTERRUPT_DEBUG) {
> >                         env->interrupt_request &=  
> > ~CPU_INTERRUPT_DEBUG;
> >                         env->exception_index = EXCP_DEBUG;
> >                         cpu_loop_exit();
> >                     }
> >                     if (interrupt_request & CPU_INTERRUPT_HALT) {
> >                         env->interrupt_request &= ~CPU_INTERRUPT_HALT;
> >                         env->halted = 1;
> >                         env->exception_index = EXCP_HLT;
> >                         cpu_loop_exit();
> >                     }
> >                     if (interrupt_request & CPU_INTERRUPT_HARD) {
> >                         hw_interrupt(env);
> >                         if (env->pending_interrupts == 0)
> >                             env->interrupt_request &=  
> > ~CPU_INTERRUPT_HARD;
> > #if defined(__sparc__) && !defined(HOST_SOLARIS)
> >                         tmp_T0 = 0;
> > #else
> >                         T0 = 0;
> > #endif
> >                     }
> >                    /* Don't use the cached interupt_request value,
> >                       do_interrupt may have updated the EXITTB  
> > flag. */
> >                     if (interrupt_request & CPU_INTERRUPT_EXITTB) {
> >                         env->interrupt_request &=  
> > ~CPU_INTERRUPT_EXITTB;
> >                         /* ensure that no TB jump will be modified as
> >                            the program flow was changed */
> > #if defined(__sparc__) && !defined(HOST_SOLARIS)
> >                         tmp_T0 = 0;
> > #else
> >                         T0 = 0;
> > #endif
> >                     }
> >                     if (interrupt_request & CPU_INTERRUPT_EXIT) {
> >                         env->interrupt_request &= ~CPU_INTERRUPT_EXIT;
> >                         env->exception_index = EXCP_INTERRUPT;
> >                         cpu_loop_exit();
> >                     }
> >                 }
> > All the targets specific tricks could be done in the hw_interrupt  
> > routine. And the generic code
> > would become much more readable. But this needs some works (not so  
> > much) and intensive tests...
> > And I guess nobody feels like taking this risk right now ;-)
> > But I think this will have to be done someday...
> 
> I definitely agree with you on that one. The current interrupt  
> handlers are filled with #ifdefs and not easily readable. This is a  
> different task though and maybe I will get the time to do this. I  
> believe it should be done "all archs at once" though, so either we  
> use the current method (adding non-gerneric interrupts) for now and  
> tidy up everything later on or tidy up the code now and port the svm  
> patch on that. As I don't see the cleanup coming in the near future  
> I'd vote for including it now and have it reworked at once when  
> someone feels dare enough to clean up the whole generic interrupt  
> handler.
> 
> Nevertheless I really do like the idea of having a cleaned up  
> interrupt handler. This is a different patch though and is rather a  
> TODO than anything that should be mangled with the svm patch.
> 
> >
> >>>
> >>>>> For the same reason, the intercept field in the TB structure  
> >>>>> seems not
> >>>>> acceptable, as TB specific target informations are already to  
> >>>>> be stored
> >>>>> in the flags field. As intercept seems only to be a bit field,  
> >>>>> it should
> >>>>> go, in a way or another, in tb flags. And as it seems that some
> >>>>>
> >>>>>
> >>>> TB flags is a 32-bit bitfield, where several bits are already used.
> >>>> Currently SVM supports 45 intercepts and each combination can  
> >>>> generate a
> >>>> different TB, as we do not want to reduce performance for the
> >>>> non-intercepted case. Think of the intercept field as flags2,  
> >>>> that could
> >>>> be used by other virtualization implementations on other  
> >>>> architectures
> >>>> too (though I do not know of any)
> >>>>
> >>>
> >>> So, why not make it generic and extend the flag field to as many  
> >>> bits as
> >>> you need ? Intercept is x86 specific and has no meaning outside  
> >>> of this
> >>> scope. Having more bits for flags is generic and may be useful  
> >>> for some
> >>> other targets... The easy way is to convert flag to 64 bits, if  
> >>> you have
> >>> enough bits. Another way is to make it a table, but this may need
> >>> changes in all targets code...
> >>>
> >>
> >> I think it might be quite a bad idea to extend flags until everything
> >> fits in there. I'm planning on implementing VMX as well and there are
> >> intercepts too so the meaning of the different fields change and I  
> >> would
> >> not want that happening with the normal flags vector. What would you
> >> think of an #ifdef TARGET_I386? This way intercepts are x86 only and
> >> don't interfere with any other target architecture, but are still
> >> seperated from the flags (which is semantically correct as far as it
> >> goes for me).
> >
> > I don't consider as a problem that one bit in the TB flags field can
> > have different meanings.
> > This field is something opaque from the generic code. I actually have
> > the same thing in
> > the PowerPC emulation (even if it's not fully done for now): TB flags
> > are just an
> > selection of the Machine State Register bits. Some of those bits can
> > have different
> > meanings depending on the PowerPC implementation (ie CPU model).  
> > But the
> > only thing important is to know if the current CPUState flags are  
> > equal
> > to the
> > TB flags, not the actual meaning of the bits. As the MSR is 32 bits on
> > 32 bits
> > PowerPC implementations, I have no hesitation to use those bits  
> > directly
> > even if their actual meaning, from the CPU "point of view" changes.
> > Then, imho, you should not hesitate to put those intercept bits in a
> > generic field...
> 
> 
> Ok, I will try to shift the intercepts in an uint_64 flags variable  
> in the TB.
> 
> >
> >>>
> >>>>> interceptions are related with functions implemented in helpers  
> >>>>> (not
> >>>>> micro-ops), you'd better check the interception in the helper at
> >>>>> runtime, which would add no visible overhead (as calling a  
> >>>>> helper is
> >>>>> slow compared to direct micro-ops execution), then you would  
> >>>>> not need to
> >>>>> store those infos in the TB structure. This may even make the  
> >>>>> emulation
> >>>>>
> >>>>>
> >>>> This was the case in an earlier version of the patch. The current
> >>>> implementation you see is an optimization to make it running  
> >>>> faster, as
> >>>> there is (nearly) no need for runtime detection.
> >>>>
> >>>
> >>> For all interceptions related to operations in helpers, it's not a
> >>> actual optimisation. Things done in helpers are slow and not  
> >>> widely used
> >>> (compared to the translated code, I mean), then adding a single  
> >>> test for
> >>> interception in a helper may not hurt global performances. You  
> >>> may need
> >>> TB flags only for operations you want to intercept that would be  
> >>> inlined
> >>> in the translated code, then maybe reduce the number of bits needed
> >>> without decreasing performanced. But, not knowing a lot about x86 VM
> >>> specification I may be wrong...
> >>>
> >>>
> >>
> >> I feel really uncomfortable with supporting hundreds of different  
> >> ways
> >> of interception. It took me days to take all the intercept checks  
> >> out of
> >> the helpers into the translate.c, as it is way easier to maintain and
> >> read that way. I do not like the idea of converting the intercept  
> >> vector
> >> to something internal. It's quite nice to be able to use the very  
> >> same
> >> constants as kvm does. This makes debugging really easy and you don't
> >> have to define everything twice. Additionally this way you can  
> >> just set
> >> a variable without calculating anything, which does come in handy  
> >> as well.
> >>
> >> Basically the reason I really want to have everyting in  
> >> translate.c is
> >> simplicity though.
> >
> > OK, I understand that using the same code for all interceptions can  
> > make
> > your coding
> > far easier...
> >
> >>>>> run faster has you won't fill the TB cache with multiple  
> >>>>> translation of
> >>>>> the same code each time the env->intercept changes, thus have  
> >>>>> chance to
> >>>>> avoid many TB caches flushes.
> >>>>>
> >>>>>
> >>>> This is actually intended, as I believe the emulated machine  
> >>>> should run
> >>>> as fast as before when not using any interceptions
> >>>>
> >>>
> >>> It depends if those flags are to be changed often or not, which  
> >>> depends
> >>> on the hypervisor software in use I guess. If the flags never  
> >>> change,
> >>> there'll be no TB flush, I agree...
> >>>
> >>
> >> Usually (at least that's the case with kvm) you have two different  
> >> sets
> >> of intercept flags. One for the real machine and one for the  
> >> virtualized
> >> one. There is only one minor situation where that might change but
> >> that's not worth mentioning here. Nevertheless TBs point to physical
> >> memory  (afaik) so since the virtual kernel is on a different page  
> >> than
> >> the real one, we could not reuse TBs anyway so having the intercept
> >> information in the TB only helps in finding out which intercepts to
> >> check for as only a minority of the possible intercepts actually  
> >> get used.
> >
> > So as far as the hypervisor do not provide shared routines to be  
> > used by
> > the OS,
> > you should never have any "collisison" between translated blocks in
> > different environments ?
> > I guess you need a complete TB flush when switching between the
> > virtualized environments.
> > Then, OK, the interception flags should never lead to fill the TB  
> > cache
> > with multiple translations
> > of the same blocks...
> 
> Exactly. The only possible "collision" would be two virtual machines  
> that share code on the same physical page somehow.
> 
> Cheers,
> 
> Alex
-- 
J. Mayer <l_indien@magic.fr>
Never organized

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-18 20:54 [Fwd: Re: [Qemu-devel] [PATCH] SVM support] J. Mayer
@ 2007-09-18 23:05 ` J. Mayer
  2007-09-18 23:28   ` Paul Brook
  2007-09-19 10:59   ` Alexander Graf
  0 siblings, 2 replies; 10+ messages in thread
From: J. Mayer @ 2007-09-18 23:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexander Graf

On Tue, 2007-09-18 at 22:54 +0200, J. Mayer wrote:
> Forwarded, as requested.
> 
> -------- Forwarded Message --------
> > From: Alexander Graf <agraf@suse.de>
> > To: J.Mayer <l_indien@magic.fr>
> > Subject: Re: [Qemu-devel] [PATCH] SVM support
> > Date: Tue, 18 Sep 2007 13:51:26 +0200
> > 
> > On Sep 18, 2007, at 12:09 PM, J. Mayer wrote:
> > 
> > > On Tue, 2007-09-18 at 03:02 +0200, Alexander Graf wrote:
> > >> Jocelyn Mayer wrote:
> > >>> On Mon, 2007-09-17 at 12:02 +0200, Alexander Graf wrote:
> > >>>
> > >>>> J. Mayer wrote:
> > >>>>
> > >>>>> On Thu, 2007-09-13 at 17:27 +0200, Alexander Graf wrote:
> > >>>>>
> > >>>>>
> > >>>>>> Thiemo Seufer wrote:
> > >>>>>>
> > >>>>>>
> > >>>>>>> Alexander Graf wrote:
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>> Thanks to Michael Peter who tried the patch on an x86 host I  
> > >>>>>>>> found some
> > >>>>>>>> compilation problems.
> > >>>>>>>> This updated patch addresses these issues and thus should  
> > >>>>>>>> compile on
> > >>>>>>>> every platform for every target available.
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>>
> > >>>>> [...]
> > >>>>>
> > >>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>> Wow sorry about that, looks like I missed these.
> > >>>>>>
> > >>>>>>
> > >>>>> Index: qemu-0.9.0.cvs/exec-all.h
> > >>>>> ================================================================== 
> > >>>>> =
> > >>>>> --- qemu-0.9.0.cvs.orig/exec-all.h
> > >>>>> +++ qemu-0.9.0.cvs/exec-all.h
> > >>>>> @@ -166,6 +166,7 @@ static inline int tlb_set_page(CPUState
> > >>>>>  typedef struct TranslationBlock {
> > >>>>>      target_ulong pc;   /* simulated PC corresponding to this  
> > >>>>> block (EIP
> > >>>>> + CS base) */
> > >>>>>      target_ulong cs_base; /* CS base for this block */
> > >>>>> +    uint64_t intercept; /* SVM intercept vector */
> > >>>>>      unsigned int flags; /* flags defining in which context the  
> > >>>>> code was
> > >>>>> generated */
> > >>>>>      uint16_t size;      /* size of target code for this block  
> > >>>>> (1 <=
> > >>>>>                             size <= TARGET_PAGE_SIZE) */
> > >>>>> Index: qemu-0.9.0.cvs/cpu-all.h
> > >>>>> ================================================================== 
> > >>>>> =
> > >>>>> --- qemu-0.9.0.cvs.orig/cpu-all.h
> > >>>>> +++ qemu-0.9.0.cvs/cpu-all.h
> > >>>>> @@ -715,6 +715,7 @@ extern int code_copy_enabled;
> > >>>>>  #define CPU_INTERRUPT_HALT   0x20 /* CPU halt wanted */
> > >>>>>  #define CPU_INTERRUPT_SMI    0x40 /* (x86 only) SMI interrupt  
> > >>>>> pending
> > >>>>> */
> > >>>>>  #define CPU_INTERRUPT_DEBUG  0x80 /* Debug event occured.  */
> > >>>>> +#define CPU_INTERRUPT_VIRQ   0x100 /* virtual interrupt  
> > >>>>> pending.  */
> > >>>>>
> > >>>>>  void cpu_interrupt(CPUState *s, int mask);
> > >>>>>  void cpu_reset_interrupt(CPUState *env, int mask);
> > >>>>>
> > >>>>> Those two patches look ugly to me as target specific features  
> > >>>>> should
> > >>>>> never go in generic code or structures.
> > >>>>> The CPU_INTERRUPT flags should just contain information about the
> > >>>>> emulator behavior, thus CPU_INTERRUPT_TIMER, CPU_INTERRUPT_SMI  
> > >>>>> are to be
> > >>>>> removed. Target specific informations about the exception  
> > >>>>> nature should
> > >>>>> go in the CPUState structure... Then, adding a  
> > >>>>> CPU_INTERRUPT_VIRQ seems
> > >>>>> not a good idea at all: it's outside of the generic emulator  
> > >>>>> scope and
> > >>>>> pointless for most targets.
> > >>>>>
> > >>>>>
> > >>>> I don't see any practical reason not to do it this way. We could  
> > >>>> define
> > >>>> a CPU_INTERRUPT_TARGET interrupt, that stores the information in  
> > >>>> a the
> > >>>> target specific CPUState, but this would hit performance  
> > >>>> (marginally
> > >>>> though) and does not improve the situation. I don't think that  
> > >>>> we should
> > >>>> should forcefully try to seperate targets where we are not even  
> > >>>> close to
> > >>>> running out of constants. If everyone on this list sees the  
> > >>>> situation as
> > >>>> Jocelyn does, I would be fine with writing a patch that puts target
> > >>>> specific interrupts to the specific targets.
> > >>>>
> > >>>
> > >>> I was to do the same to add some functionality to the PowerPC  
> > >>> target,
> > >>> long time ago, and Fabrice Bellard convinced me not to do and  
> > >>> agreed it
> > >>> has been a bad idea to add the target specific CPU_INTERRUPT_xxx  
> > >>> flags.
> > >>> Then I did manage the PowerPC target to use only generic
> > >>> CPU_INTERRUPT_xxx and put all other target specific informations  
> > >>> in the
> > >>> CPUState structure. It absolutelly changes nothing in terms of
> > >>> functionality nor performance. The only thing is that it makes the
> > >>> generic parts simpler and could even allow the cpu_exec function to
> > >>> contain almost no target specific code, which would really great  
> > >>> imho.
> > >>>
> > >>
> > >> I can give that a try :-). Sounds reasonable for me.
> > >
> > >> [Next message]
> > >> Oh well I just thought about this a bit more and while stumbling  
> > >> across
> > >> CPU_INTERRUPT_FIQ which does the same thing one major problem came  
> > >> to me
> > >> on that one: Priority. Real interrupts have priority over virtual
> > >> interrupts so the VIRQs have to be processed after HARD IRQs, whereas
> > >> SMIs and NMIs have to be taken care of before the HARD IRQs. It  
> > >> simply
> > >> wouldn't work out to generalize the IRQs, just as it would not  
> > >> work with
> > >> the VIRQ, as it has to be handled as a real IRQ but without accessing
> > >> the APIC which has to be done for HARD IRQs.
> > >>
> > >> I guess we're stuck with what's there now.
> > >
> > > Priorities are not an issue, you can take a look to the  
> > > ppc_hw_interrupt function
> > > to see how the PowerPC emulation code manages priorities and  
> > > masking between
> > > 10 different hardware interruption sources. The code could be  
> > > better (I wanted
> > > to preserve existing code) but it's a solution that actually works...
> > > But looking better the x86 emulation code, I agree that if you  
> > > don't want to change it
> > > too much, you have to add a flag. The other solution (and best,  
> > > imho) would be
> > > to do the same as what has been done for PowerPC: just let the  
> > > generic code in
> > > the cpu_exec loop and move the x86 specific code somewhere in  
> > > target-i386 code.
> > > Going this way, I see no reason why the interruption code in the  
> > > cpu_exec loop
> > > could not finally become:
> > >                 interrupt_request = env->interrupt_request;
> > >                 if (__builtin_expect(interrupt_request, 0)) {
> > >                     if (interrupt_request & CPU_INTERRUPT_DEBUG) {
> > >                         env->interrupt_request &=  
> > > ~CPU_INTERRUPT_DEBUG;
> > >                         env->exception_index = EXCP_DEBUG;
> > >                         cpu_loop_exit();
> > >                     }
> > >                     if (interrupt_request & CPU_INTERRUPT_HALT) {
> > >                         env->interrupt_request &= ~CPU_INTERRUPT_HALT;
> > >                         env->halted = 1;
> > >                         env->exception_index = EXCP_HLT;
> > >                         cpu_loop_exit();
> > >                     }
> > >                     if (interrupt_request & CPU_INTERRUPT_HARD) {
> > >                         hw_interrupt(env);
> > >                         if (env->pending_interrupts == 0)
> > >                             env->interrupt_request &=  
> > > ~CPU_INTERRUPT_HARD;
> > > #if defined(__sparc__) && !defined(HOST_SOLARIS)
> > >                         tmp_T0 = 0;
> > > #else
> > >                         T0 = 0;
> > > #endif
> > >                     }
> > >                    /* Don't use the cached interupt_request value,
> > >                       do_interrupt may have updated the EXITTB  
> > > flag. */
> > >                     if (interrupt_request & CPU_INTERRUPT_EXITTB) {
> > >                         env->interrupt_request &=  
> > > ~CPU_INTERRUPT_EXITTB;
> > >                         /* ensure that no TB jump will be modified as
> > >                            the program flow was changed */
> > > #if defined(__sparc__) && !defined(HOST_SOLARIS)
> > >                         tmp_T0 = 0;
> > > #else
> > >                         T0 = 0;
> > > #endif
> > >                     }
> > >                     if (interrupt_request & CPU_INTERRUPT_EXIT) {
> > >                         env->interrupt_request &= ~CPU_INTERRUPT_EXIT;
> > >                         env->exception_index = EXCP_INTERRUPT;
> > >                         cpu_loop_exit();
> > >                     }
> > >                 }
> > > All the targets specific tricks could be done in the hw_interrupt  
> > > routine. And the generic code
> > > would become much more readable. But this needs some works (not so  
> > > much) and intensive tests...
> > > And I guess nobody feels like taking this risk right now ;-)
> > > But I think this will have to be done someday...
> > 
> > I definitely agree with you on that one. The current interrupt  
> > handlers are filled with #ifdefs and not easily readable. This is a  
> > different task though and maybe I will get the time to do this. I  
> > believe it should be done "all archs at once" though, so either we  
> > use the current method (adding non-gerneric interrupts) for now and  
> > tidy up everything later on or tidy up the code now and port the svm  
> > patch on that. As I don't see the cleanup coming in the near future  
> > I'd vote for including it now and have it reworked at once when  
> > someone feels dare enough to clean up the whole generic interrupt  
> > handler.

The danger doing the patch "all archs at once" is that it's difficult
for one to know and test all archs... But it would be a great idea to
fix this at once...
If your patch is ready to be applied, then apply it as it is, especially
if it helps you to go on developing new features to have it commited.

> > 
> > Nevertheless I really do like the idea of having a cleaned up  
> > interrupt handler. This is a different patch though and is rather a  
> > TODO than anything that should be mangled with the svm patch.

yes, those are separate works, but it may help if you can have this in
mind (if it does not break too much of your code too !)...

> > >
> > >>>
> > >>>>> For the same reason, the intercept field in the TB structure  
> > >>>>> seems not
> > >>>>> acceptable, as TB specific target informations are already to  
> > >>>>> be stored
> > >>>>> in the flags field. As intercept seems only to be a bit field,  
> > >>>>> it should
> > >>>>> go, in a way or another, in tb flags. And as it seems that some
> > >>>>>
> > >>>>>
> > >>>> TB flags is a 32-bit bitfield, where several bits are already used.
> > >>>> Currently SVM supports 45 intercepts and each combination can  
> > >>>> generate a
> > >>>> different TB, as we do not want to reduce performance for the
> > >>>> non-intercepted case. Think of the intercept field as flags2,  
> > >>>> that could
> > >>>> be used by other virtualization implementations on other  
> > >>>> architectures
> > >>>> too (though I do not know of any)
> > >>>>
> > >>>
> > >>> So, why not make it generic and extend the flag field to as many  
> > >>> bits as
> > >>> you need ? Intercept is x86 specific and has no meaning outside  
> > >>> of this
> > >>> scope. Having more bits for flags is generic and may be useful  
> > >>> for some
> > >>> other targets... The easy way is to convert flag to 64 bits, if  
> > >>> you have
> > >>> enough bits. Another way is to make it a table, but this may need
> > >>> changes in all targets code...
> > >>>
> > >>
> > >> I think it might be quite a bad idea to extend flags until everything
> > >> fits in there. I'm planning on implementing VMX as well and there are
> > >> intercepts too so the meaning of the different fields change and I  
> > >> would
> > >> not want that happening with the normal flags vector. What would you
> > >> think of an #ifdef TARGET_I386? This way intercepts are x86 only and
> > >> don't interfere with any other target architecture, but are still
> > >> seperated from the flags (which is semantically correct as far as it
> > >> goes for me).
> > >
> > > I don't consider as a problem that one bit in the TB flags field can
> > > have different meanings.
> > > This field is something opaque from the generic code. I actually have
> > > the same thing in
> > > the PowerPC emulation (even if it's not fully done for now): TB flags
> > > are just an
> > > selection of the Machine State Register bits. Some of those bits can
> > > have different
> > > meanings depending on the PowerPC implementation (ie CPU model).  
> > > But the
> > > only thing important is to know if the current CPUState flags are  
> > > equal
> > > to the
> > > TB flags, not the actual meaning of the bits. As the MSR is 32 bits on
> > > 32 bits
> > > PowerPC implementations, I have no hesitation to use those bits  
> > > directly
> > > even if their actual meaning, from the CPU "point of view" changes.
> > > Then, imho, you should not hesitate to put those intercept bits in a
> > > generic field...
> > 
> > 
> > Ok, I will try to shift the intercepts in an uint_64 flags variable  
> > in the TB.

OK, great. Having 64 bits may also help for additional (ie future...)
features in PowerPC 64 emulation.

[...]

-- 
J. Mayer <l_indien@magic.fr>
Never organized

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-18 23:05 ` J. Mayer
@ 2007-09-18 23:28   ` Paul Brook
  2007-09-19 10:55     ` Alexander Graf
  2007-09-19 10:56     ` Alexander Graf
  2007-09-19 10:59   ` Alexander Graf
  1 sibling, 2 replies; 10+ messages in thread
From: Paul Brook @ 2007-09-18 23:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: J. Mayer, Alexander Graf

> > > Ok, I will try to shift the intercepts in an uint_64 flags variable
> > > in the TB.
>
> OK, great. Having 64 bits may also help for additional (ie future...)
> features in PowerPC 64 emulation.

Maybe worth letting the target say whether it needs 32 or 64-bit flags.
The flag lookup is likely to be on a hot path.

Paul

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-18 23:28   ` Paul Brook
@ 2007-09-19 10:55     ` Alexander Graf
  2007-09-19 10:56     ` Alexander Graf
  1 sibling, 0 replies; 10+ messages in thread
From: Alexander Graf @ 2007-09-19 10:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paul Brook


On Sep 19, 2007, at 1:28 AM, Paul Brook wrote:

>>>> Ok, I will try to shift the intercepts in an uint_64 flags variable
>>>> in the TB.
>>
>> OK, great. Having 64 bits may also help for additional (ie future...)
>> features in PowerPC 64 emulation.
>
> Maybe worth letting the target say whether it needs 32 or 64-bit  
> flags.
> The flag lookup is likely to be on a hot path.
>
> Paul

I actually don't see too much of a performance penality in that but  
nevertheless could you please test if the appended patch hits  
performance for you? This changes the tb->flags field to be 64 bits.

If it does not, please apply this to the CVS as my SVM patch now  
relies on it.

Cheers,

Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-18 23:28   ` Paul Brook
  2007-09-19 10:55     ` Alexander Graf
@ 2007-09-19 10:56     ` Alexander Graf
  2007-09-19 14:39       ` Jocelyn Mayer
  1 sibling, 1 reply; 10+ messages in thread
From: Alexander Graf @ 2007-09-19 10:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: J. Mayer, Paul Brook

[-- Attachment #1: flags64.patch --]
[-- Type: application/octet-stream, Size: 2983 bytes --]

Index: qemu/target-i386/translate.c
===================================================================
--- qemu.orig/target-i386/translate.c
+++ qemu/target-i386/translate.c
@@ -95,7 +95,7 @@ typedef struct DisasContext {
     int singlestep_enabled; /* "hardware" single step enabled */
     int jmp_opt; /* use direct block chaining for direct jumps */
     int mem_index; /* select memory access functions */
-    int flags; /* all execution flags */
+    uint64_t flags; /* all execution flags */
     struct TranslationBlock *tb;
     int popl_esp_hack; /* for correct popl with esp base handling */
     int rip_offset; /* only used in x86_64, but left for simplicity */
@@ -6462,7 +6462,8 @@ static inline int gen_intermediate_code_
     DisasContext dc1, *dc = &dc1;
     target_ulong pc_ptr;
     uint16_t *gen_opc_end;
-    int flags, j, lj, cflags;
+    int j, lj, cflags;
+    uint64_t flags;
     target_ulong pc_start;
     target_ulong cs_base;
 
Index: qemu/cpu-exec.c
===================================================================
--- qemu.orig/cpu-exec.c
+++ qemu/cpu-exec.c
@@ -77,7 +77,7 @@ void cpu_resume_from_signal(CPUState *en
 
 static TranslationBlock *tb_find_slow(target_ulong pc,
                                       target_ulong cs_base,
-                                      unsigned int flags)
+                                      uint64_t flags)
 {
     TranslationBlock *tb, **ptb1;
     int code_gen_size;
@@ -155,7 +155,7 @@ static inline TranslationBlock *tb_find_
 {
     TranslationBlock *tb;
     target_ulong cs_base, pc;
-    unsigned int flags;
+    uint64_t flags;
 
     /* we record a subset of the CPU state. It will
        always be the same before a given translated block
Index: qemu/exec-all.h
===================================================================
--- qemu.orig/exec-all.h
+++ qemu/exec-all.h
@@ -166,7 +166,7 @@ static inline int tlb_set_page(CPUState 
 typedef struct TranslationBlock {
     target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS base) */
     target_ulong cs_base; /* CS base for this block */
-    unsigned int flags; /* flags defining in which context the code was generated */
+    uint64_t flags; /* flags defining in which context the code was generated */
     uint16_t size;      /* size of target code for this block (1 <=
                            size <= TARGET_PAGE_SIZE) */
     uint16_t cflags;    /* compile flags */
Index: qemu/target-mips/translate.c
===================================================================
--- qemu.orig/target-mips/translate.c
+++ qemu/target-mips/translate.c
@@ -6403,7 +6403,7 @@ gen_intermediate_code_internal (CPUState
     ctx.tb = tb;
     ctx.bstate = BS_NONE;
     /* Restore delay slot state from the tb context.  */
-    ctx.hflags = tb->flags;
+    ctx.hflags = (uint32_t)tb->flags; /* FIXME: maybe use 64 bits here? */
     restore_cpu_state(env, &ctx);
 #if defined(CONFIG_USER_ONLY)
     ctx.mem_idx = 0;

[-- Attachment #2: Type: text/plain, Size: 411 bytes --]


On Sep 19, 2007, at 1:28 AM, Paul Brook wrote:

>>>> Ok, I will try to shift the intercepts in an uint_64 flags variable
>>>> in the TB.
>>
>> OK, great. Having 64 bits may also help for additional (ie future...)
>> features in PowerPC 64 emulation.
>
> Maybe worth letting the target say whether it needs 32 or 64-bit  
> flags.
> The flag lookup is likely to be on a hot path.
>
> Paul

here comes the patch.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-18 23:05 ` J. Mayer
  2007-09-18 23:28   ` Paul Brook
@ 2007-09-19 10:59   ` Alexander Graf
  1 sibling, 0 replies; 10+ messages in thread
From: Alexander Graf @ 2007-09-19 10:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: J. Mayer

[-- Attachment #1: svm.patch --]
[-- Type: application/octet-stream, Size: 67749 bytes --]

Index: qemu/target-i386/helper2.c
===================================================================
--- qemu.orig/target-i386/helper2.c
+++ qemu/target-i386/helper2.c
@@ -27,6 +27,7 @@
 
 #include "cpu.h"
 #include "exec-all.h"
+#include "svm.h"
 
 //#define DEBUG_MMU
 
@@ -111,10 +112,11 @@ CPUX86State *cpu_x86_init(void)
                                CPUID_CX8 | CPUID_PGE | CPUID_CMOV |
                                CPUID_PAT);
         env->pat = 0x0007040600070406ULL;
+        env->cpuid_ext3_features = CPUID_EXT3_SVM;
         env->cpuid_ext_features = CPUID_EXT_SSE3;
         env->cpuid_features |= CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | CPUID_PAE | CPUID_SEP;
         env->cpuid_features |= CPUID_APIC;
-        env->cpuid_xlevel = 0x80000006;
+        env->cpuid_xlevel = 0x8000000e;
         {
             const char *model_id = "QEMU Virtual CPU version " QEMU_VERSION;
             int c, len, i;
@@ -131,7 +133,6 @@ CPUX86State *cpu_x86_init(void)
         /* currently not enabled for std i386 because not fully tested */
         env->cpuid_ext2_features = (env->cpuid_features & 0x0183F3FF);
         env->cpuid_ext2_features |= CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX;
-        env->cpuid_xlevel = 0x80000008;
 
         /* these features are needed for Win64 and aren't fully implemented */
         env->cpuid_features |= CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA;
@@ -162,6 +163,7 @@ void cpu_reset(CPUX86State *env)
 #ifdef CONFIG_SOFTMMU
     env->hflags |= HF_SOFTMMU_MASK;
 #endif
+    env->hflags |= HF_GIF_MASK;
 
     cpu_x86_update_cr0(env, 0x60000010);
     env->a20_mask = 0xffffffff;
@@ -865,7 +867,6 @@ int cpu_x86_handle_mmu_fault(CPUX86State
  do_fault_protect:
     error_code = PG_ERROR_P_MASK;
  do_fault:
-    env->cr[2] = addr;
     error_code |= (is_write << PG_ERROR_W_BIT);
     if (is_user)
         error_code |= PG_ERROR_U_MASK;
@@ -873,8 +874,16 @@ int cpu_x86_handle_mmu_fault(CPUX86State
         (env->efer & MSR_EFER_NXE) &&
         (env->cr[4] & CR4_PAE_MASK))
         error_code |= PG_ERROR_I_D_MASK;
+    if (INTERCEPTEDl(_exceptions, 1 << EXCP0E_PAGE)) {
+        stq_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_info_2), addr);
+    } else {
+        env->cr[2] = addr;
+    }
     env->error_code = error_code;
     env->exception_index = EXCP0E_PAGE;
+    /* the VMM will handle this */
+    if (INTERCEPTEDl(_exceptions, 1 << EXCP0E_PAGE))
+        return 2;
     return 1;
 }
 
Index: qemu/target-i386/translate.c
===================================================================
--- qemu.orig/target-i386/translate.c
+++ qemu/target-i386/translate.c
@@ -1995,6 +1995,98 @@ static void gen_movl_seg_T0(DisasContext
     }
 }
 
+#ifdef TARGET_X86_64
+#define SVM_movq_T1_im(x) gen_op_movq_T1_im64((x) >> 32, x)
+#else
+#define SVM_movq_T1_im(x) gen_op_movl_T1_im(x)
+#endif
+
+static inline int
+gen_svm_check_io(DisasContext *s, target_ulong pc_start, uint64_t type)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if(s->flags & (1ULL << INTERCEPT_IOIO_PROT)) {
+        if (s->cc_op != CC_OP_DYNAMIC)
+            gen_op_set_cc_op(s->cc_op);
+        SVM_movq_T1_im(s->pc - s->cs_base);
+        gen_jmp_im(pc_start - s->cs_base);
+        gen_op_geneflags();
+        gen_op_svm_check_intercept_io((uint32_t)(type >> 32), (uint32_t)type);
+        s->cc_op = CC_OP_DYNAMIC;
+        /* FIXME: maybe we could move the io intercept vector to the TB as well
+                  so we know if this is an EOB or not ... let's assume it's not
+                  for now. */
+    }
+#endif
+    return 0;
+}
+
+static inline int svm_is_rep(int prefixes)
+{
+    return ((prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) ? 8 : 0);
+}
+
+static inline int
+gen_svm_check_intercept_param(DisasContext *s, target_ulong pc_start,
+                              uint64_t type, uint64_t param)
+{
+    if(!(s->flags & (INTERCEPT_SVM_MASK)))
+	/* no SVM activated */
+        return 0;
+    switch(type) {
+        /* CRx and DRx reads/writes */
+        case SVM_EXIT_READ_CR0 ... SVM_EXIT_EXCP_BASE - 1:
+            if (s->cc_op != CC_OP_DYNAMIC) {
+                gen_op_set_cc_op(s->cc_op);
+                s->cc_op = CC_OP_DYNAMIC;
+            }
+            gen_jmp_im(pc_start - s->cs_base);
+            SVM_movq_T1_im(param);
+            gen_op_geneflags();
+            gen_op_svm_check_intercept_param((uint32_t)(type >> 32), (uint32_t)type);
+            /* this is a special case as we do not know if the interception occurs
+               so we assume there was none */
+            return 0;
+        case SVM_EXIT_MSR:
+            if(s->flags & (1ULL << INTERCEPT_MSR_PROT)) {
+                if (s->cc_op != CC_OP_DYNAMIC) {
+                    gen_op_set_cc_op(s->cc_op);
+                    s->cc_op = CC_OP_DYNAMIC;
+                }
+                gen_jmp_im(pc_start - s->cs_base);
+                SVM_movq_T1_im(param);
+                gen_op_geneflags();
+                gen_op_svm_check_intercept_param((uint32_t)(type >> 32), (uint32_t)type);
+                /* this is a special case as we do not know if the interception occurs
+                   so we assume there was none */
+                return 0;
+            }
+            break;
+        default:
+            if(s->flags & (1ULL << ((type - SVM_EXIT_INTR) + INTERCEPT_INTR))) {
+                if (s->cc_op != CC_OP_DYNAMIC) {
+                    gen_op_set_cc_op(s->cc_op);
+		    s->cc_op = CC_OP_EFLAGS;
+                }
+                gen_jmp_im(pc_start - s->cs_base);
+                SVM_movq_T1_im(param);
+                gen_op_geneflags();
+                gen_op_svm_vmexit(type >> 32, type);
+                /* we can optimize this one so TBs don't get longer
+                   than up to vmexit */
+                gen_eob(s);
+                return 1;
+            }
+    }
+    return 0;
+}
+
+static inline int
+gen_svm_check_intercept(DisasContext *s, target_ulong pc_start, uint64_t type)
+{
+    return gen_svm_check_intercept_param(s, pc_start, type, 0);
+}
+
 static inline void gen_stack_update(DisasContext *s, int addend)
 {
 #ifdef TARGET_X86_64
@@ -4880,6 +4972,12 @@ static target_ulong disas_insn(DisasCont
         else
             ot = dflag ? OT_LONG : OT_WORD;
         gen_check_io(s, ot, 1, pc_start - s->cs_base);
+        gen_op_mov_TN_reg[OT_WORD][0][R_EDX]();
+        gen_op_andl_T0_ffff();
+        if (gen_svm_check_io(s, pc_start,
+                             SVM_IOIO_TYPE_MASK | (1 << (4+ot)) |
+                             svm_is_rep(prefixes) | 4 | (1 << (7+s->aflag))))
+            break;
         if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) {
             gen_repz_ins(s, ot, pc_start - s->cs_base, s->pc - s->cs_base);
         } else {
@@ -4893,6 +4991,12 @@ static target_ulong disas_insn(DisasCont
         else
             ot = dflag ? OT_LONG : OT_WORD;
         gen_check_io(s, ot, 1, pc_start - s->cs_base);
+        gen_op_mov_TN_reg[OT_WORD][0][R_EDX]();
+        gen_op_andl_T0_ffff();
+        if (gen_svm_check_io(s, pc_start,
+                             (1 << (4+ot)) | svm_is_rep(prefixes) |
+                             4 | (1 << (7+s->aflag))))
+            break;
         if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) {
             gen_repz_outs(s, ot, pc_start - s->cs_base, s->pc - s->cs_base);
         } else {
@@ -4902,6 +5006,7 @@ static target_ulong disas_insn(DisasCont
 
         /************************/
         /* port I/O */
+
     case 0xe4:
     case 0xe5:
         if ((b & 1) == 0)
@@ -4911,6 +5016,10 @@ static target_ulong disas_insn(DisasCont
         val = ldub_code(s->pc++);
         gen_op_movl_T0_im(val);
         gen_check_io(s, ot, 0, pc_start - s->cs_base);
+        if (gen_svm_check_io(s, pc_start,
+                             SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes) |
+                             (1 << (4+ot))))
+            break;
         gen_op_in[ot]();
         gen_op_mov_reg_T1[ot][R_EAX]();
         break;
@@ -4923,6 +5032,9 @@ static target_ulong disas_insn(DisasCont
         val = ldub_code(s->pc++);
         gen_op_movl_T0_im(val);
         gen_check_io(s, ot, 0, pc_start - s->cs_base);
+        if (gen_svm_check_io(s, pc_start, svm_is_rep(prefixes) |
+                             (1 << (4+ot))))
+            break;
         gen_op_mov_TN_reg[ot][1][R_EAX]();
         gen_op_out[ot]();
         break;
@@ -4935,6 +5047,10 @@ static target_ulong disas_insn(DisasCont
         gen_op_mov_TN_reg[OT_WORD][0][R_EDX]();
         gen_op_andl_T0_ffff();
         gen_check_io(s, ot, 0, pc_start - s->cs_base);
+        if (gen_svm_check_io(s, pc_start,
+                             SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes) |
+                             (1 << (4+ot))))
+            break;
         gen_op_in[ot]();
         gen_op_mov_reg_T1[ot][R_EAX]();
         break;
@@ -4947,6 +5063,9 @@ static target_ulong disas_insn(DisasCont
         gen_op_mov_TN_reg[OT_WORD][0][R_EDX]();
         gen_op_andl_T0_ffff();
         gen_check_io(s, ot, 0, pc_start - s->cs_base);
+        if (gen_svm_check_io(s, pc_start,
+                             svm_is_rep(prefixes) | (1 << (4+ot))))
+            break;
         gen_op_mov_TN_reg[ot][1][R_EAX]();
         gen_op_out[ot]();
         break;
@@ -5004,6 +5123,8 @@ static target_ulong disas_insn(DisasCont
         val = 0;
         goto do_lret;
     case 0xcf: /* iret */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_IRET))
+            break;
         if (!s->pe) {
             /* real mode */
             gen_op_iret_real(s->dflag);
@@ -5125,6 +5246,8 @@ static target_ulong disas_insn(DisasCont
         /************************/
         /* flags */
     case 0x9c: /* pushf */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_PUSHF))
+            break;
         if (s->vm86 && s->iopl != 3) {
             gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         } else {
@@ -5135,6 +5258,8 @@ static target_ulong disas_insn(DisasCont
         }
         break;
     case 0x9d: /* popf */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_POPF))
+            break;
         if (s->vm86 && s->iopl != 3) {
             gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         } else {
@@ -5348,6 +5473,9 @@ static target_ulong disas_insn(DisasCont
         /* XXX: correct lock test for all insn */
         if (prefixes & PREFIX_LOCK)
             goto illegal_op;
+        if (prefixes & PREFIX_REPZ) {
+            gen_svm_check_intercept(s, pc_start, SVM_EXIT_PAUSE);
+        }
         break;
     case 0x9b: /* fwait */
         if ((s->flags & (HF_MP_MASK | HF_TS_MASK)) ==
@@ -5361,10 +5489,14 @@ static target_ulong disas_insn(DisasCont
         }
         break;
     case 0xcc: /* int3 */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_SWINT))
+            break;
         gen_interrupt(s, EXCP03_INT3, pc_start - s->cs_base, s->pc - s->cs_base);
         break;
     case 0xcd: /* int N */
         val = ldub_code(s->pc++);
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_SWINT))
+            break;
         if (s->vm86 && s->iopl != 3) {
             gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         } else {
@@ -5374,12 +5506,16 @@ static target_ulong disas_insn(DisasCont
     case 0xce: /* into */
         if (CODE64(s))
             goto illegal_op;
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_SWINT))
+            break;
         if (s->cc_op != CC_OP_DYNAMIC)
             gen_op_set_cc_op(s->cc_op);
         gen_jmp_im(pc_start - s->cs_base);
         gen_op_into(s->pc - pc_start);
         break;
     case 0xf1: /* icebp (undocumented, exits to external debugger) */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_ICEBP))
+            break;
 #if 1
         gen_debug(s, pc_start - s->cs_base);
 #else
@@ -5415,6 +5551,8 @@ static target_ulong disas_insn(DisasCont
                     gen_op_set_inhibit_irq();
                 /* give a chance to handle pending irqs */
                 gen_jmp_im(s->pc - s->cs_base);
+                if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_VINTR))
+                    break;
                 gen_eob(s);
             } else {
                 gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
@@ -5507,13 +5645,21 @@ static target_ulong disas_insn(DisasCont
         if (s->cpl != 0) {
             gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         } else {
-            if (b & 2)
+            int retval = 0;
+            if (b & 2) {
+                retval = gen_svm_check_intercept_param(s, pc_start, SVM_EXIT_MSR, 0);
                 gen_op_rdmsr();
-            else
+            } else {
+                retval = gen_svm_check_intercept_param(s, pc_start, SVM_EXIT_MSR, 1);
                 gen_op_wrmsr();
+            }
+            if(retval)
+                gen_eob(s);
         }
         break;
     case 0x131: /* rdtsc */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_RDTSC))
+            break;
         gen_jmp_im(pc_start - s->cs_base);
         gen_op_rdtsc();
         break;
@@ -5576,12 +5722,16 @@ static target_ulong disas_insn(DisasCont
         break;
 #endif
     case 0x1a2: /* cpuid */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_CPUID))
+            break;
         gen_op_cpuid();
         break;
     case 0xf4: /* hlt */
         if (s->cpl != 0) {
             gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         } else {
+            if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_HLT))
+                break;
             if (s->cc_op != CC_OP_DYNAMIC)
                 gen_op_set_cc_op(s->cc_op);
             gen_jmp_im(s->pc - s->cs_base);
@@ -5597,6 +5747,8 @@ static target_ulong disas_insn(DisasCont
         case 0: /* sldt */
             if (!s->pe || s->vm86)
                 goto illegal_op;
+            if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_LDTR_READ))
+                break;
             gen_op_movl_T0_env(offsetof(CPUX86State,ldt.selector));
             ot = OT_WORD;
             if (mod == 3)
@@ -5609,6 +5761,8 @@ static target_ulong disas_insn(DisasCont
             if (s->cpl != 0) {
                 gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
             } else {
+                if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_LDTR_WRITE))
+                    break;
                 gen_ldst_modrm(s, modrm, OT_WORD, OR_TMP0, 0);
                 gen_jmp_im(pc_start - s->cs_base);
                 gen_op_lldt_T0();
@@ -5617,6 +5771,8 @@ static target_ulong disas_insn(DisasCont
         case 1: /* str */
             if (!s->pe || s->vm86)
                 goto illegal_op;
+            if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_TR_READ))
+                break;
             gen_op_movl_T0_env(offsetof(CPUX86State,tr.selector));
             ot = OT_WORD;
             if (mod == 3)
@@ -5629,6 +5785,8 @@ static target_ulong disas_insn(DisasCont
             if (s->cpl != 0) {
                 gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
             } else {
+                if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_TR_WRITE))
+                    break;
                 gen_ldst_modrm(s, modrm, OT_WORD, OR_TMP0, 0);
                 gen_jmp_im(pc_start - s->cs_base);
                 gen_op_ltr_T0();
@@ -5660,6 +5818,8 @@ static target_ulong disas_insn(DisasCont
         case 0: /* sgdt */
             if (mod == 3)
                 goto illegal_op;
+            if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_GDTR_READ))
+                break;
             gen_lea_modrm(s, modrm, &reg_addr, &offset_addr);
             gen_op_movl_T0_env(offsetof(CPUX86State, gdt.limit));
             gen_op_st_T0_A0[OT_WORD + s->mem_index]();
@@ -5676,6 +5836,8 @@ static target_ulong disas_insn(DisasCont
                     if (!(s->cpuid_ext_features & CPUID_EXT_MONITOR) ||
                         s->cpl != 0)
                         goto illegal_op;
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_MONITOR))
+                        break;
                     gen_jmp_im(pc_start - s->cs_base);
 #ifdef TARGET_X86_64
                     if (s->aflag == 2) {
@@ -5700,6 +5862,8 @@ static target_ulong disas_insn(DisasCont
                         gen_op_set_cc_op(s->cc_op);
                         s->cc_op = CC_OP_DYNAMIC;
                     }
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_MWAIT))
+                        break;
                     gen_jmp_im(s->pc - s->cs_base);
                     gen_op_mwait();
                     gen_eob(s);
@@ -5708,6 +5872,8 @@ static target_ulong disas_insn(DisasCont
                     goto illegal_op;
                 }
             } else { /* sidt */
+                if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_IDTR_READ))
+                    break;
                 gen_lea_modrm(s, modrm, &reg_addr, &offset_addr);
                 gen_op_movl_T0_env(offsetof(CPUX86State, idt.limit));
                 gen_op_st_T0_A0[OT_WORD + s->mem_index]();
@@ -5720,11 +5886,63 @@ static target_ulong disas_insn(DisasCont
             break;
         case 2: /* lgdt */
         case 3: /* lidt */
-            if (mod == 3)
-                goto illegal_op;
-            if (s->cpl != 0) {
+            if (mod == 3) {
+                switch(rm) {
+                case 0: /* VMRUN */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_VMRUN))
+                        break;
+                    if (s->cc_op != CC_OP_DYNAMIC)
+                        gen_op_set_cc_op(s->cc_op);
+                    gen_jmp_im(s->pc - s->cs_base);
+                    gen_op_vmrun();
+                    s->cc_op = CC_OP_EFLAGS;
+                    gen_eob(s);
+                    break;
+                case 1: /* VMMCALL */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_VMMCALL))
+                         break;
+                    /* FIXME: cause #UD if hflags & SVM */
+                    gen_op_vmmcall();
+                    break;
+                case 2: /* VMLOAD */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_VMLOAD))
+                         break;
+                    gen_op_vmload();
+                    break;
+                case 3: /* VMSAVE */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_VMSAVE))
+                         break;
+                    gen_op_vmsave();
+                    break;
+                case 4: /* STGI */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_STGI))
+                         break;
+                    gen_op_stgi();
+                    break;
+                case 5: /* CLGI */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_CLGI))
+                         break;
+                    gen_op_clgi();
+                    break;
+                case 6: /* SKINIT */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_SKINIT))
+                         break;
+                    gen_op_skinit();
+                    break;
+                case 7: /* INVLPGA */
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_INVLPGA))
+                         break;
+                    gen_op_invlpga();
+                    break;
+                default:
+                    goto illegal_op;
+                }
+            } else if (s->cpl != 0) {
                 gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
             } else {
+                if (gen_svm_check_intercept(s, pc_start,
+                                            op==2 ? SVM_EXIT_GDTR_WRITE : SVM_EXIT_IDTR_WRITE))
+                    break;
                 gen_lea_modrm(s, modrm, &reg_addr, &offset_addr);
                 gen_op_ld_T1_A0[OT_WORD + s->mem_index]();
                 gen_add_A0_im(s, 2);
@@ -5741,6 +5959,8 @@ static target_ulong disas_insn(DisasCont
             }
             break;
         case 4: /* smsw */
+            if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_READ_CR0))
+                break;
             gen_op_movl_T0_env(offsetof(CPUX86State,cr[0]));
             gen_ldst_modrm(s, modrm, OT_WORD, OR_TMP0, 1);
             break;
@@ -5748,6 +5968,8 @@ static target_ulong disas_insn(DisasCont
             if (s->cpl != 0) {
                 gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
             } else {
+                if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_CR0))
+                    break;
                 gen_ldst_modrm(s, modrm, OT_WORD, OR_TMP0, 0);
                 gen_op_lmsw_T0();
                 gen_jmp_im(s->pc - s->cs_base);
@@ -5772,6 +5994,8 @@ static target_ulong disas_insn(DisasCont
                         goto illegal_op;
                     }
                 } else {
+                    if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_INVLPG))
+                        break;
                     gen_lea_modrm(s, modrm, &reg_addr, &offset_addr);
                     gen_op_invlpg_A0();
                     gen_jmp_im(s->pc - s->cs_base);
@@ -5788,6 +6012,8 @@ static target_ulong disas_insn(DisasCont
         if (s->cpl != 0) {
             gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         } else {
+            if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_INVD))
+                break;
             /* nothing to do */
         }
         break;
@@ -5908,11 +6134,13 @@ static target_ulong disas_insn(DisasCont
             case 4:
             case 8:
                 if (b & 2) {
+                    gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_CR0 + reg);
                     gen_op_mov_TN_reg[ot][0][rm]();
                     gen_op_movl_crN_T0(reg);
                     gen_jmp_im(s->pc - s->cs_base);
                     gen_eob(s);
                 } else {
+                    gen_svm_check_intercept(s, pc_start, SVM_EXIT_READ_CR0 + reg);
 #if !defined(CONFIG_USER_ONLY)
                     if (reg == 8)
                         gen_op_movtl_T0_cr8();
@@ -5945,11 +6173,13 @@ static target_ulong disas_insn(DisasCont
             if (reg == 4 || reg == 5 || reg >= 8)
                 goto illegal_op;
             if (b & 2) {
+                gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_DR0 + reg);
                 gen_op_mov_TN_reg[ot][0][rm]();
                 gen_op_movl_drN_T0(reg);
                 gen_jmp_im(s->pc - s->cs_base);
                 gen_eob(s);
             } else {
+                gen_svm_check_intercept(s, pc_start, SVM_EXIT_READ_DR0 + reg);
                 gen_op_movtl_T0_env(offsetof(CPUX86State,dr[reg]));
                 gen_op_mov_reg_T0[ot][rm]();
             }
@@ -5959,6 +6189,7 @@ static target_ulong disas_insn(DisasCont
         if (s->cpl != 0) {
             gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
         } else {
+            gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_CR0);
             gen_op_clts();
             /* abort block because static cpu state changed */
             gen_jmp_im(s->pc - s->cs_base);
@@ -6050,6 +6281,8 @@ static target_ulong disas_insn(DisasCont
         /* ignore for now */
         break;
     case 0x1aa: /* rsm */
+        if (gen_svm_check_intercept(s, pc_start, SVM_EXIT_RSM))
+            break;
         if (!(s->flags & HF_SMM_MASK))
             goto illegal_op;
         if (s->cc_op != CC_OP_DYNAMIC) {
Index: qemu/target-i386/cpu.h
===================================================================
--- qemu.orig/target-i386/cpu.h
+++ qemu/target-i386/cpu.h
@@ -84,6 +84,7 @@
 #define DESC_AVL_MASK   (1 << 20)
 #define DESC_P_MASK     (1 << 15)
 #define DESC_DPL_SHIFT  13
+#define DESC_DPL_MASK   (1 << DESC_DPL_SHIFT)
 #define DESC_S_MASK     (1 << 12)
 #define DESC_TYPE_SHIFT 8
 #define DESC_A_MASK     (1 << 8)
@@ -149,6 +150,8 @@
 #define HF_VM_SHIFT         17 /* must be same as eflags */
 #define HF_HALTED_SHIFT     18 /* CPU halted */
 #define HF_SMM_SHIFT        19 /* CPU in SMM mode */
+#define HF_GIF_SHIFT        20 /* if set CPU takes interrupts */
+#define HF_HIF_SHIFT        21 /* shadow copy of IF_MASK when in SVM */
 
 #define HF_CPL_MASK          (3 << HF_CPL_SHIFT)
 #define HF_SOFTMMU_MASK      (1 << HF_SOFTMMU_SHIFT)
@@ -166,6 +169,8 @@
 #define HF_OSFXSR_MASK       (1 << HF_OSFXSR_SHIFT)
 #define HF_HALTED_MASK       (1 << HF_HALTED_SHIFT)
 #define HF_SMM_MASK          (1 << HF_SMM_SHIFT)
+#define HF_GIF_MASK          (1 << HF_GIF_SHIFT)
+#define HF_HIF_MASK          (1 << HF_HIF_SHIFT)
 
 #define CR0_PE_MASK  (1 << 0)
 #define CR0_MP_MASK  (1 << 1)
@@ -249,6 +254,8 @@
 #define MSR_GSBASE                      0xc0000101
 #define MSR_KERNELGSBASE                0xc0000102
 
+#define MSR_VM_HSAVE_PA                 0xc0010117
+
 /* cpuid_features bits */
 #define CPUID_FP87 (1 << 0)
 #define CPUID_VME  (1 << 1)
@@ -283,6 +290,8 @@
 #define CPUID_EXT2_FFXSR   (1 << 25)
 #define CPUID_EXT2_LM      (1 << 29)
 
+#define CPUID_EXT3_SVM     (1 << 2)
+
 #define EXCP00_DIVZ	0
 #define EXCP01_SSTP	1
 #define EXCP02_NMI	2
@@ -491,6 +500,16 @@ typedef struct CPUX86State {
     uint32_t sysenter_eip;
     uint64_t efer;
     uint64_t star;
+
+    target_phys_addr_t vm_hsave;
+    target_phys_addr_t vm_vmcb;
+    uint64_t intercept;
+    uint16_t intercept_cr_read;
+    uint16_t intercept_cr_write;
+    uint16_t intercept_dr_read;
+    uint16_t intercept_dr_write;
+    uint32_t intercept_exceptions;
+
 #ifdef TARGET_X86_64
     target_ulong lstar;
     target_ulong cstar;
@@ -532,6 +551,7 @@ typedef struct CPUX86State {
     uint32_t cpuid_xlevel;
     uint32_t cpuid_model[12];
     uint32_t cpuid_ext2_features;
+    uint32_t cpuid_ext3_features;
     uint32_t cpuid_apic_id;
 
 #ifdef USE_KQEMU
@@ -672,4 +692,6 @@ static inline int cpu_get_time_fast(void
 
 #include "cpu-all.h"
 
+#include "svm.h"
+
 #endif /* CPU_I386_H */
Index: qemu/target-i386/op.c
===================================================================
--- qemu.orig/target-i386/op.c
+++ qemu/target-i386/op.c
@@ -513,8 +513,6 @@ typedef union UREG64 {
 } UREG64;
 #endif
 
-#ifdef TARGET_X86_64
-
 #define PARAMQ1 \
 ({\
     UREG64 __p;\
@@ -523,6 +521,8 @@ typedef union UREG64 {
     __p.q;\
 })
 
+#ifdef TARGET_X86_64
+
 void OPPROTO op_movq_T0_im64(void)
 {
     T0 = PARAMQ1;
@@ -1242,12 +1242,52 @@ void OPPROTO op_ltr_T0(void)
     helper_ltr_T0();
 }
 
-/* CR registers access */
+/* CR registers access. */
 void OPPROTO op_movl_crN_T0(void)
 {
     helper_movl_crN_T0(PARAM1);
 }
 
+/* These pseudo-opcodes check for SVM intercepts. */
+void OPPROTO op_svm_check_intercept(void)
+{
+    A0 = PARAM1 & PARAM2;
+    svm_check_intercept(PARAMQ1);
+}
+
+void OPPROTO op_svm_check_intercept_param(void)
+{
+    A0 = PARAM1 & PARAM2;
+    svm_check_intercept_param(PARAMQ1, T1);
+}
+
+void OPPROTO op_svm_vmexit(void)
+{
+    A0 = PARAM1 & PARAM2;
+    vmexit(PARAMQ1, T1);
+}
+
+void OPPROTO op_geneflags(void)
+{
+    CC_SRC = cc_table[CC_OP].compute_all();
+}
+
+/* This pseudo-opcode checks for IO intercepts. */
+#if !defined(CONFIG_USER_ONLY)
+void OPPROTO op_svm_check_intercept_io(void)
+{
+    A0 = PARAM1 & PARAM2;
+    /* PARAMQ1 = TYPE (0 = OUT, 1 = IN; 4 = STRING; 8 = REP)
+       T0      = PORT
+       T1      = next eip */
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_info_2), T1);
+    /* ASIZE does not appear on real hw */
+    svm_check_intercept_param(SVM_EXIT_IOIO,
+                              (PARAMQ1 & ~SVM_IOIO_ASIZE_MASK) |
+                              ((T0 & 0xffff) << 16));
+}
+#endif
+
 #if !defined(CONFIG_USER_ONLY)
 void OPPROTO op_movtl_T0_cr8(void)
 {
@@ -2452,3 +2492,45 @@ void OPPROTO op_emms(void)
 
 #define SHIFT 1
 #include "ops_sse.h"
+
+/* Secure Virtual Machine ops */
+
+void OPPROTO op_vmrun(void)
+{
+    helper_vmrun(EAX);
+}
+
+void OPPROTO op_vmmcall(void)
+{
+    helper_vmmcall();
+}
+
+void OPPROTO op_vmload(void)
+{
+    helper_vmload(EAX);
+}
+
+void OPPROTO op_vmsave(void)
+{
+    helper_vmsave(EAX);
+}
+
+void OPPROTO op_stgi(void)
+{
+    helper_stgi();
+}
+
+void OPPROTO op_clgi(void)
+{
+    helper_clgi();
+}
+
+void OPPROTO op_skinit(void)
+{
+    helper_skinit();
+}
+
+void OPPROTO op_invlpga(void)
+{
+    helper_invlpga();
+}
Index: qemu/target-i386/helper.c
===================================================================
--- qemu.orig/target-i386/helper.c
+++ qemu/target-i386/helper.c
@@ -594,7 +594,18 @@ static void do_interrupt_protected(int i
     int has_error_code, new_stack, shift;
     uint32_t e1, e2, offset, ss, esp, ss_e1, ss_e2;
     uint32_t old_eip, sp_mask;
+    int svm_should_check = 1;
 
+    if ((env->intercept & INTERCEPT_SVM_MASK) && !is_int && next_eip==-1) {
+        next_eip = EIP;
+        svm_should_check = 0;
+    }
+
+    if (svm_should_check
+        && (INTERCEPTEDl(_exceptions, 1 << intno)
+        && !is_int)) {
+        raise_interrupt(intno, is_int, error_code, 0);
+    }
     has_error_code = 0;
     if (!is_int && !is_hw) {
         switch(intno) {
@@ -830,7 +841,17 @@ static void do_interrupt64(int intno, in
     int has_error_code, new_stack;
     uint32_t e1, e2, e3, ss;
     target_ulong old_eip, esp, offset;
+    int svm_should_check = 1;
 
+    if ((env->intercept & INTERCEPT_SVM_MASK) && !is_int && next_eip==-1) {
+        next_eip = EIP;
+        svm_should_check = 0;
+    }
+    if (svm_should_check
+        && INTERCEPTEDl(_exceptions, 1 << intno)
+        && !is_int) {
+        raise_interrupt(intno, is_int, error_code, 0);
+    }
     has_error_code = 0;
     if (!is_int && !is_hw) {
         switch(intno) {
@@ -1077,7 +1098,17 @@ static void do_interrupt_real(int intno,
     int selector;
     uint32_t offset, esp;
     uint32_t old_cs, old_eip;
+    int svm_should_check = 1;
 
+    if ((env->intercept & INTERCEPT_SVM_MASK) && !is_int && next_eip==-1) {
+        next_eip = EIP;
+        svm_should_check = 0;
+    }
+    if (svm_should_check
+        && INTERCEPTEDl(_exceptions, 1 << intno)
+        && !is_int) {
+        raise_interrupt(intno, is_int, error_code, 0);
+    }
     /* real mode (simpler !) */
     dt = &env->idt;
     if (intno * 4 + 3 > dt->limit)
@@ -1227,8 +1258,10 @@ int check_exception(int intno, int *erro
 void raise_interrupt(int intno, int is_int, int error_code,
                      int next_eip_addend)
 {
-    if (!is_int)
+    if (!is_int) {
+        svm_check_intercept_param(SVM_EXIT_EXCP_BASE + intno, error_code);
         intno = check_exception(intno, &error_code);
+    }
 
     env->exception_index = intno;
     env->error_code = error_code;
@@ -1671,7 +1704,7 @@ void helper_cpuid(void)
     case 0x80000001:
         EAX = env->cpuid_features;
         EBX = 0;
-        ECX = 0;
+        ECX = env->cpuid_ext3_features;
         EDX = env->cpuid_ext2_features;
         break;
     case 0x80000002:
@@ -2745,6 +2778,9 @@ void helper_wrmsr(void)
     case MSR_PAT:
         env->pat = val;
         break;
+    case MSR_VM_HSAVE_PA:
+        env->vm_hsave = val;
+        break;
 #ifdef TARGET_X86_64
     case MSR_LSTAR:
         env->lstar = val;
@@ -2796,6 +2832,9 @@ void helper_rdmsr(void)
     case MSR_PAT:
         val = env->pat;
         break;
+    case MSR_VM_HSAVE_PA:
+        val = env->vm_hsave;
+        break;
 #ifdef TARGET_X86_64
     case MSR_LSTAR:
         val = env->lstar;
@@ -3877,3 +3916,483 @@ void tlb_fill(target_ulong addr, int is_
     }
     env = saved_env;
 }
+
+
+/* Secure Virtual Machine helpers */
+
+void helper_stgi(void)
+{
+    env->hflags |= HF_GIF_MASK;
+}
+
+void helper_clgi(void)
+{
+    env->hflags &= ~HF_GIF_MASK;
+}
+
+#if defined(CONFIG_USER_ONLY)
+
+void helper_vmrun(target_ulong addr) { }
+void helper_vmmcall(void) { }
+void helper_vmload(target_ulong addr) { }
+void helper_vmsave(target_ulong addr) { }
+void helper_skinit(void) { }
+void helper_invlpga(void) { }
+void vmexit(uint64_t exit_code, uint64_t exit_info_1) { }
+int svm_check_intercept_param(uint32_t type, uint64_t param)
+{
+    return 0;
+}
+
+#else
+
+static inline uint32_t
+vmcb2cpu_attrib(uint16_t vmcb_attrib, uint32_t vmcb_base, uint32_t vmcb_limit)
+{
+    return    ((vmcb_attrib & 0x00ff) << 8)          /* Type, S, DPL, P */
+	    | ((vmcb_attrib & 0x0f00) << 12)         /* AVL, L, DB, G */
+	    | ((vmcb_base >> 16) & 0xff)             /* Base 23-16 */
+	    | (vmcb_base & 0xff000000)               /* Base 31-24 */
+	    | (vmcb_limit & 0xf0000);                /* Limit 19-16 */
+}
+
+static inline uint16_t cpu2vmcb_attrib(uint32_t cpu_attrib)
+{
+    return    ((cpu_attrib >> 8) & 0xff)             /* Type, S, DPL, P */
+	    | ((cpu_attrib & 0xf00000) >> 12);       /* AVL, L, DB, G */
+}
+
+extern uint8_t *phys_ram_base;
+void helper_vmrun(target_ulong addr)
+{
+    uint32_t event_inj;
+    uint32_t int_ctl;
+
+    if (loglevel & CPU_LOG_TB_IN_ASM)
+        fprintf(logfile,"vmrun! " TARGET_FMT_lx "\n", addr);
+
+    env->vm_vmcb = addr;
+    regs_to_env();
+
+    /* save the current CPU state in the hsave page */
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.gdtr.base), env->gdt.base);
+    stl_phys(env->vm_hsave + offsetof(struct vmcb, save.gdtr.limit), env->gdt.limit);
+
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.idtr.base), env->idt.base);
+    stl_phys(env->vm_hsave + offsetof(struct vmcb, save.idtr.limit), env->idt.limit);
+
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr0), env->cr[0]);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr2), env->cr[2]);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr3), env->cr[3]);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr4), env->cr[4]);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr8), env->cr[8]);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.dr6), env->dr[6]);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.dr7), env->dr[7]);
+
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.efer), env->efer);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.rflags), compute_eflags());
+
+    SVM_SAVE_SEG(env->vm_hsave, segs[R_ES], es);
+    SVM_SAVE_SEG(env->vm_hsave, segs[R_CS], cs);
+    SVM_SAVE_SEG(env->vm_hsave, segs[R_SS], ss);
+    SVM_SAVE_SEG(env->vm_hsave, segs[R_DS], ds);
+
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.rip), EIP);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.rsp), ESP);
+    stq_phys(env->vm_hsave + offsetof(struct vmcb, save.rax), EAX);
+
+    /* load the interception bitmaps so we do not need to access the
+       vmcb in svm mode */
+    /* We shift all the intercept bits so we can OR them with the TB
+       flags later on */
+    env->intercept            = (ldq_phys(env->vm_vmcb + offsetof(struct vmcb, control.intercept)) << INTERCEPT_INTR) | INTERCEPT_SVM_MASK;
+    env->intercept_cr_read    = lduw_phys(env->vm_vmcb + offsetof(struct vmcb, control.intercept_cr_read));
+    env->intercept_cr_write   = lduw_phys(env->vm_vmcb + offsetof(struct vmcb, control.intercept_cr_write));
+    env->intercept_dr_read    = lduw_phys(env->vm_vmcb + offsetof(struct vmcb, control.intercept_dr_read));
+    env->intercept_dr_write   = lduw_phys(env->vm_vmcb + offsetof(struct vmcb, control.intercept_dr_write));
+    env->intercept_exceptions = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.intercept_exceptions));
+
+    env->gdt.base  = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.gdtr.base));
+    env->gdt.limit = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, save.gdtr.limit));
+
+    env->idt.base  = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.idtr.base));
+    env->idt.limit = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, save.idtr.limit));
+
+    /* clear exit_info_2 so we behave like the real hardware */
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_info_2), 0);
+
+    cpu_x86_update_cr0(env, ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr0)));
+    cpu_x86_update_cr4(env, ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr4)));
+    cpu_x86_update_cr3(env, ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr3)));
+    env->cr[2] = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr2));
+    int_ctl = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl));
+    if (int_ctl & V_INTR_MASKING_MASK) {
+        env->cr[8] = int_ctl & V_TPR_MASK;
+        if (env->eflags & IF_MASK)
+            env->hflags |= HF_HIF_MASK;
+    }
+
+#ifdef TARGET_X86_64
+    env->efer = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.efer));
+    env->hflags &= ~HF_LMA_MASK;
+    if (env->efer & MSR_EFER_LMA)
+       env->hflags |= HF_LMA_MASK;
+#endif
+    env->eflags = 0;
+    load_eflags(ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rflags)),
+                ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C | DF_MASK));
+    CC_OP = CC_OP_EFLAGS;
+    CC_DST = 0xffffffff;
+
+    SVM_LOAD_SEG(env->vm_vmcb, ES, es);
+    SVM_LOAD_SEG(env->vm_vmcb, CS, cs);
+    SVM_LOAD_SEG(env->vm_vmcb, SS, ss);
+    SVM_LOAD_SEG(env->vm_vmcb, DS, ds);
+
+    EIP = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rip));
+    env->eip = EIP;
+    ESP = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rsp));
+    EAX = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rax));
+    env->dr[7] = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.dr7));
+    env->dr[6] = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, save.dr6));
+    cpu_x86_set_cpl(env, ldub_phys(env->vm_vmcb + offsetof(struct vmcb, save.cpl)));
+
+    /* FIXME: guest state consistency checks */
+
+    switch(ldub_phys(env->vm_vmcb + offsetof(struct vmcb, control.tlb_ctl))) {
+        case TLB_CONTROL_DO_NOTHING:
+            break;
+        case TLB_CONTROL_FLUSH_ALL_ASID:
+            /* FIXME: this is not 100% correct but should work for now */
+            tlb_flush(env, 1);
+        break;
+    }
+
+    helper_stgi();
+
+    regs_to_env();
+
+    /* maybe we need to inject an event */
+    event_inj = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj));
+    if (event_inj & SVM_EVTINJ_VALID) {
+        uint8_t vector = event_inj & SVM_EVTINJ_VEC_MASK;
+        uint16_t valid_err = event_inj & SVM_EVTINJ_VALID_ERR;
+        uint32_t event_inj_err = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj_err));
+        stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), event_inj & ~SVM_EVTINJ_VALID);
+
+        if (loglevel & CPU_LOG_TB_IN_ASM)
+            fprintf(logfile, "Injecting(%#hx): ", valid_err);
+        /* FIXME: need to implement valid_err */
+        switch (event_inj & SVM_EVTINJ_TYPE_MASK) {
+        case SVM_EVTINJ_TYPE_INTR:
+                env->exception_index = vector;
+                env->error_code = event_inj_err;
+                env->exception_is_int = 1;
+                env->exception_next_eip = -1;
+                if (loglevel & CPU_LOG_TB_IN_ASM)
+                    fprintf(logfile, "INTR");
+                break;
+        case SVM_EVTINJ_TYPE_NMI:
+                env->exception_index = vector;
+                env->error_code = event_inj_err;
+                env->exception_is_int = 1;
+                env->exception_next_eip = EIP;
+                if (loglevel & CPU_LOG_TB_IN_ASM)
+                    fprintf(logfile, "NMI");
+                break;
+        case SVM_EVTINJ_TYPE_EXEPT:
+                env->exception_index = vector;
+                env->error_code = event_inj_err;
+                env->exception_is_int = 0;
+                env->exception_next_eip = -1;
+                if (loglevel & CPU_LOG_TB_IN_ASM)
+                    fprintf(logfile, "EXEPT");
+                break;
+        case SVM_EVTINJ_TYPE_SOFT:
+                env->exception_index = vector;
+                env->error_code = event_inj_err;
+                env->exception_is_int = 1;
+                env->exception_next_eip = EIP;
+                if (loglevel & CPU_LOG_TB_IN_ASM)
+                    fprintf(logfile, "SOFT");
+                break;
+        }
+        if (loglevel & CPU_LOG_TB_IN_ASM)
+            fprintf(logfile, " %#x %#x\n", env->exception_index, env->error_code);
+    }
+    if (int_ctl & V_IRQ_MASK)
+        env->interrupt_request |= CPU_INTERRUPT_VIRQ;
+
+    cpu_loop_exit();
+}
+
+void helper_vmmcall(void)
+{
+    if (loglevel & CPU_LOG_TB_IN_ASM)
+        fprintf(logfile,"vmmcall!\n");
+}
+
+void helper_vmload(target_ulong addr)
+{
+    if (loglevel & CPU_LOG_TB_IN_ASM)
+        fprintf(logfile,"vmload! " TARGET_FMT_lx "\nFS: %016" PRIx64 " | " TARGET_FMT_lx "\n",
+                addr, ldq_phys(addr + offsetof(struct vmcb, save.fs.base)),
+                env->segs[R_FS].base);
+
+    SVM_LOAD_SEG2(addr, segs[R_FS], fs);
+    SVM_LOAD_SEG2(addr, segs[R_GS], gs);
+    SVM_LOAD_SEG2(addr, tr, tr);
+    SVM_LOAD_SEG2(addr, ldt, ldtr);
+
+#ifdef TARGET_X86_64
+    env->kernelgsbase = ldq_phys(addr + offsetof(struct vmcb, save.kernel_gs_base));
+    env->lstar = ldq_phys(addr + offsetof(struct vmcb, save.lstar));
+    env->cstar = ldq_phys(addr + offsetof(struct vmcb, save.cstar));
+    env->fmask = ldq_phys(addr + offsetof(struct vmcb, save.sfmask));
+#endif
+    env->star = ldq_phys(addr + offsetof(struct vmcb, save.star));
+    env->sysenter_cs = ldq_phys(addr + offsetof(struct vmcb, save.sysenter_cs));
+    env->sysenter_esp = ldq_phys(addr + offsetof(struct vmcb, save.sysenter_esp));
+    env->sysenter_eip = ldq_phys(addr + offsetof(struct vmcb, save.sysenter_eip));
+}
+
+void helper_vmsave(target_ulong addr)
+{
+    if (loglevel & CPU_LOG_TB_IN_ASM)
+        fprintf(logfile,"vmsave! " TARGET_FMT_lx "\nFS: %016" PRIx64 " | " TARGET_FMT_lx "\n",
+                addr, ldq_phys(addr + offsetof(struct vmcb, save.fs.base)),
+                env->segs[R_FS].base);
+
+    SVM_SAVE_SEG(addr, segs[R_FS], fs);
+    SVM_SAVE_SEG(addr, segs[R_GS], gs);
+    SVM_SAVE_SEG(addr, tr, tr);
+    SVM_SAVE_SEG(addr, ldt, ldtr);
+
+#ifdef TARGET_X86_64
+    stq_phys(addr + offsetof(struct vmcb, save.kernel_gs_base), env->kernelgsbase);
+    stq_phys(addr + offsetof(struct vmcb, save.lstar), env->lstar);
+    stq_phys(addr + offsetof(struct vmcb, save.cstar), env->cstar);
+    stq_phys(addr + offsetof(struct vmcb, save.sfmask), env->fmask);
+#endif
+    stq_phys(addr + offsetof(struct vmcb, save.star), env->star);
+    stq_phys(addr + offsetof(struct vmcb, save.sysenter_cs), env->sysenter_cs);
+    stq_phys(addr + offsetof(struct vmcb, save.sysenter_esp), env->sysenter_esp);
+    stq_phys(addr + offsetof(struct vmcb, save.sysenter_eip), env->sysenter_eip);
+}
+
+void helper_skinit(void)
+{
+    if (loglevel & CPU_LOG_TB_IN_ASM)
+        fprintf(logfile,"skinit!\n");
+}
+
+void helper_invlpga(void)
+{
+    tlb_flush(env, 0);
+}
+
+int svm_check_intercept_param(uint32_t type, uint64_t param)
+{
+    switch(type) {
+    case SVM_EXIT_READ_CR0 ... SVM_EXIT_READ_CR0 + 8:
+        if (INTERCEPTEDw(_cr_read, (1 << (type - SVM_EXIT_READ_CR0)))) {
+            vmexit(type, param);
+            return 1;
+        }
+        break;
+    case SVM_EXIT_READ_DR0 ... SVM_EXIT_READ_DR0 + 8:
+        if (INTERCEPTEDw(_dr_read, (1 << (type - SVM_EXIT_READ_DR0)))) {
+            vmexit(type, param);
+            return 1;
+        }
+        break;
+    case SVM_EXIT_WRITE_CR0 ... SVM_EXIT_WRITE_CR0 + 8:
+        if (INTERCEPTEDw(_cr_write, (1 << (type - SVM_EXIT_WRITE_CR0)))) {
+            vmexit(type, param);
+            return 1;
+        }
+        break;
+    case SVM_EXIT_WRITE_DR0 ... SVM_EXIT_WRITE_DR0 + 8:
+        if (INTERCEPTEDw(_dr_write, (1 << (type - SVM_EXIT_WRITE_DR0)))) {
+            vmexit(type, param);
+            return 1;
+        }
+        break;
+    case SVM_EXIT_EXCP_BASE ... SVM_EXIT_EXCP_BASE + 16:
+        if (INTERCEPTEDl(_exceptions, (1 << (type - SVM_EXIT_EXCP_BASE)))) {
+            vmexit(type, param);
+            return 1;
+        }
+        break;
+    case SVM_EXIT_IOIO:
+        if (INTERCEPTED(1ULL << INTERCEPT_IOIO_PROT)) {
+            /* FIXME: this should be read in at vmrun (faster this way?) */
+            uint64_t addr = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, control.iopm_base_pa));
+            uint16_t port = (uint16_t) (param >> 16);
+
+            if(ldub_phys(addr + port / 8) & (1 << (port % 8)))
+                vmexit(type, param);
+        }
+        break;
+
+    case SVM_EXIT_MSR:
+        if (INTERCEPTED(1ULL << INTERCEPT_MSR_PROT)) {
+            /* FIXME: this should be read in at vmrun (faster this way?) */
+            uint64_t addr = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, control.msrpm_base_pa));
+            switch((uint32_t)ECX) {
+            case 0 ... 0x1fff:
+                T0 = (ECX * 2) % 8;
+                T1 = ECX / 8;
+                break;
+            case 0xc0000000 ... 0xc0001fff:
+                T0 = (8192 + ECX - 0xc0000000) * 2;
+                T1 = (T0 / 8);
+                T0 %= 8;
+                break;
+            case 0xc0010000 ... 0xc0011fff:
+                T0 = (16384 + ECX - 0xc0010000) * 2;
+                T1 = (T0 / 8);
+                T0 %= 8;
+                break;
+            default:
+                vmexit(type, param);
+                return 1;
+            }
+            if (ldub_phys(addr + T1) & ((1 << param) << T0))
+                vmexit(type, param);
+            return 1;
+        }
+        break;
+    default:
+        if (INTERCEPTED((1ULL << ((type - SVM_EXIT_INTR) + INTERCEPT_INTR)))) {
+            vmexit(type, param);
+            return 1;
+        }
+        break;
+    }
+    return 0;
+}
+
+void vmexit(uint64_t exit_code, uint64_t exit_info_1)
+{
+    uint32_t int_ctl;
+
+    if (loglevel & CPU_LOG_TB_IN_ASM)
+        fprintf(logfile,"vmexit(%016" PRIx64 ", %016" PRIx64 ", %016" PRIx64 ", " TARGET_FMT_lx ")!\n",
+                exit_code, exit_info_1,
+                ldq_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_info_2)),
+                EIP);
+
+    /* Save the VM state in the vmcb */
+    SVM_SAVE_SEG(env->vm_vmcb, segs[R_ES], es);
+    SVM_SAVE_SEG(env->vm_vmcb, segs[R_CS], cs);
+    SVM_SAVE_SEG(env->vm_vmcb, segs[R_SS], ss);
+    SVM_SAVE_SEG(env->vm_vmcb, segs[R_DS], ds);
+
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.gdtr.base), env->gdt.base);
+    stl_phys(env->vm_vmcb + offsetof(struct vmcb, save.gdtr.limit), env->gdt.limit);
+
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.idtr.base), env->idt.base);
+    stl_phys(env->vm_vmcb + offsetof(struct vmcb, save.idtr.limit), env->idt.limit);
+
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.efer), env->efer);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr0), env->cr[0]);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr2), env->cr[2]);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr3), env->cr[3]);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.cr4), env->cr[4]);
+
+    if ((int_ctl = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl))) & V_INTR_MASKING_MASK) {
+        int_ctl &= ~V_TPR_MASK;
+        int_ctl |= env->cr[8] & V_TPR_MASK;
+        stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl), int_ctl);
+    }
+
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rflags), compute_eflags());
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rip), env->eip);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rsp), ESP);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.rax), EAX);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.dr7), env->dr[7]);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, save.dr6), env->dr[6]);
+    stb_phys(env->vm_vmcb + offsetof(struct vmcb, save.cpl), env->hflags & HF_CPL_MASK);
+
+    /* Reload the host state from vm_hsave */
+    env->hflags &= ~HF_HIF_MASK;
+    env->intercept = 0;
+    env->intercept_exceptions = 0;
+    env->interrupt_request &= ~CPU_INTERRUPT_VIRQ;
+
+    env->gdt.base  = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.gdtr.base));
+    env->gdt.limit = ldl_phys(env->vm_hsave + offsetof(struct vmcb, save.gdtr.limit));
+
+    env->idt.base  = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.idtr.base));
+    env->idt.limit = ldl_phys(env->vm_hsave + offsetof(struct vmcb, save.idtr.limit));
+
+    cpu_x86_update_cr0(env, ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr0)) | CR0_PE_MASK);
+    cpu_x86_update_cr4(env, ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr4)));
+    cpu_x86_update_cr3(env, ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr3)));
+    if (int_ctl & V_INTR_MASKING_MASK)
+        env->cr[8] = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.cr8));
+    /* we need to set the efer after the crs so the hidden flags get set properly */
+#ifdef TARGET_X86_64
+    env->efer  = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.efer));
+    env->hflags &= ~HF_LMA_MASK;
+    if (env->efer & MSR_EFER_LMA)
+       env->hflags |= HF_LMA_MASK;
+#endif
+
+    env->eflags = 0;
+    load_eflags(ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.rflags)),
+                ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C | DF_MASK));
+    CC_OP = CC_OP_EFLAGS;
+
+    SVM_LOAD_SEG(env->vm_hsave, ES, es);
+    SVM_LOAD_SEG(env->vm_hsave, CS, cs);
+    SVM_LOAD_SEG(env->vm_hsave, SS, ss);
+    SVM_LOAD_SEG(env->vm_hsave, DS, ds);
+
+    EIP = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.rip));
+    ESP = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.rsp));
+    EAX = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.rax));
+
+    env->dr[6] = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.dr6));
+    env->dr[7] = ldq_phys(env->vm_hsave + offsetof(struct vmcb, save.dr7));
+
+    /* other setups */
+    cpu_x86_set_cpl(env, 0);
+    stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_code_hi), (uint32_t)(exit_code >> 32));
+    stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_code), exit_code);
+    stq_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_info_1), exit_info_1);
+
+    helper_clgi();
+    /* FIXME: Resets the current ASID register to zero (host ASID). */
+
+    /* Clears the V_IRQ and V_INTR_MASKING bits inside the processor. */
+
+    /* Clears the TSC_OFFSET inside the processor. */
+
+    /* If the host is in PAE mode, the processor reloads the host's PDPEs
+       from the page table indicated the host's CR3. If the PDPEs contain
+       illegal state, the processor causes a shutdown. */
+
+    /* Forces CR0.PE = 1, RFLAGS.VM = 0. */
+    env->cr[0] |= CR0_PE_MASK;
+    env->eflags &= ~VM_MASK;
+
+    /* Disables all breakpoints in the host DR7 register. */
+
+    /* Checks the reloaded host state for consistency. */
+
+    /* If the host's rIP reloaded by #VMEXIT is outside the limit of the
+       host's code segment or non-canonical (in the case of long mode), a
+       #GP fault is delivered inside the host.) */
+
+    /* remove any pending exception */
+    env->exception_index = -1;
+    env->error_code = 0;
+    env->old_exception = -1;
+
+    regs_to_env();
+    cpu_loop_exit();
+}
+
+#endif
Index: qemu/target-i386/svm.h
===================================================================
--- /dev/null
+++ qemu/target-i386/svm.h
@@ -0,0 +1,358 @@
+#ifndef __SVM_H
+#define __SVM_H
+
+enum {
+        /* We shift all the intercept bits so we can OR them with the
+           TB flags later on */
+	INTERCEPT_INTR = HF_HIF_SHIFT,
+	INTERCEPT_NMI,
+	INTERCEPT_SMI,
+	INTERCEPT_INIT,
+	INTERCEPT_VINTR,
+	INTERCEPT_SELECTIVE_CR0,
+	INTERCEPT_STORE_IDTR,
+	INTERCEPT_STORE_GDTR,
+	INTERCEPT_STORE_LDTR,
+	INTERCEPT_STORE_TR,
+	INTERCEPT_LOAD_IDTR,
+	INTERCEPT_LOAD_GDTR,
+	INTERCEPT_LOAD_LDTR,
+	INTERCEPT_LOAD_TR,
+	INTERCEPT_RDTSC,
+	INTERCEPT_RDPMC,
+	INTERCEPT_PUSHF,
+	INTERCEPT_POPF,
+	INTERCEPT_CPUID,
+	INTERCEPT_RSM,
+	INTERCEPT_IRET,
+	INTERCEPT_INTn,
+	INTERCEPT_INVD,
+	INTERCEPT_PAUSE,
+	INTERCEPT_HLT,
+	INTERCEPT_INVLPG,
+	INTERCEPT_INVLPGA,
+	INTERCEPT_IOIO_PROT,
+	INTERCEPT_MSR_PROT,
+	INTERCEPT_TASK_SWITCH,
+	INTERCEPT_FERR_FREEZE,
+	INTERCEPT_SHUTDOWN,
+	INTERCEPT_VMRUN,
+	INTERCEPT_VMMCALL,
+	INTERCEPT_VMLOAD,
+	INTERCEPT_VMSAVE,
+	INTERCEPT_STGI,
+	INTERCEPT_CLGI,
+	INTERCEPT_SKINIT,
+	INTERCEPT_RDTSCP,
+	INTERCEPT_ICEBP,
+	INTERCEPT_WBINVD,
+};
+/* This is not really an intercept but rather a placeholder to
+   show that we are in an SVM (just like a hidden flag, but keeps the
+   TBs clean) */
+#define INTERCEPT_SVM 63
+#define INTERCEPT_SVM_MASK (1ULL << INTERCEPT_SVM)
+
+struct __attribute__ ((__packed__)) vmcb_control_area {
+	uint16_t intercept_cr_read;
+	uint16_t intercept_cr_write;
+	uint16_t intercept_dr_read;
+	uint16_t intercept_dr_write;
+	uint32_t intercept_exceptions;
+	uint64_t intercept;
+	uint8_t reserved_1[44];
+	uint64_t iopm_base_pa;
+	uint64_t msrpm_base_pa;
+	uint64_t tsc_offset;
+	uint32_t asid;
+	uint8_t tlb_ctl;
+	uint8_t reserved_2[3];
+	uint32_t int_ctl;
+	uint32_t int_vector;
+	uint32_t int_state;
+	uint8_t reserved_3[4];
+	uint32_t exit_code;
+	uint32_t exit_code_hi;
+	uint64_t exit_info_1;
+	uint64_t exit_info_2;
+	uint32_t exit_int_info;
+	uint32_t exit_int_info_err;
+	uint64_t nested_ctl;
+	uint8_t reserved_4[16];
+	uint32_t event_inj;
+	uint32_t event_inj_err;
+	uint64_t nested_cr3;
+	uint64_t lbr_ctl;
+	uint8_t reserved_5[832];
+};
+
+
+#define TLB_CONTROL_DO_NOTHING 0
+#define TLB_CONTROL_FLUSH_ALL_ASID 1
+
+#define V_TPR_MASK 0x0f
+
+#define V_IRQ_SHIFT 8
+#define V_IRQ_MASK (1 << V_IRQ_SHIFT)
+
+#define V_INTR_PRIO_SHIFT 16
+#define V_INTR_PRIO_MASK (0x0f << V_INTR_PRIO_SHIFT)
+
+#define V_IGN_TPR_SHIFT 20
+#define V_IGN_TPR_MASK (1 << V_IGN_TPR_SHIFT)
+
+#define V_INTR_MASKING_SHIFT 24
+#define V_INTR_MASKING_MASK (1 << V_INTR_MASKING_SHIFT)
+
+#define SVM_INTERRUPT_SHADOW_MASK 1
+
+#define SVM_IOIO_STR_SHIFT 2
+#define SVM_IOIO_REP_SHIFT 3
+#define SVM_IOIO_SIZE_SHIFT 4
+#define SVM_IOIO_ASIZE_SHIFT 7
+
+#define SVM_IOIO_TYPE_MASK 1
+#define SVM_IOIO_STR_MASK (1 << SVM_IOIO_STR_SHIFT)
+#define SVM_IOIO_REP_MASK (1 << SVM_IOIO_REP_SHIFT)
+#define SVM_IOIO_SIZE_MASK (7 << SVM_IOIO_SIZE_SHIFT)
+#define SVM_IOIO_ASIZE_MASK (7 << SVM_IOIO_ASIZE_SHIFT)
+
+struct __attribute__ ((__packed__)) vmcb_seg {
+	uint16_t selector;
+	uint16_t attrib;
+	uint32_t limit;
+	uint64_t base;
+};
+
+struct __attribute__ ((__packed__)) vmcb_save_area {
+	struct vmcb_seg es;
+	struct vmcb_seg cs;
+	struct vmcb_seg ss;
+	struct vmcb_seg ds;
+	struct vmcb_seg fs;
+	struct vmcb_seg gs;
+	struct vmcb_seg gdtr;
+	struct vmcb_seg ldtr;
+	struct vmcb_seg idtr;
+	struct vmcb_seg tr;
+	uint8_t reserved_1[43];
+	uint8_t cpl;
+	uint8_t reserved_2[4];
+	uint64_t efer;
+	uint8_t reserved_3[112];
+	uint64_t cr4;
+	uint64_t cr3;
+	uint64_t cr0;
+	uint64_t dr7;
+	uint64_t dr6;
+	uint64_t rflags;
+	uint64_t rip;
+	uint8_t reserved_4[88];
+	uint64_t rsp;
+	uint8_t reserved_5[24];
+	uint64_t rax;
+	uint64_t star;
+	uint64_t lstar;
+	uint64_t cstar;
+	uint64_t sfmask;
+	uint64_t kernel_gs_base;
+	uint64_t sysenter_cs;
+	uint64_t sysenter_esp;
+	uint64_t sysenter_eip;
+	uint64_t cr2;
+	/* qemu: cr8 added to reuse this as hsave */
+	uint64_t cr8;
+	uint8_t reserved_6[32 - 8]; /* originally 32 */
+	uint64_t g_pat;
+	uint64_t dbgctl;
+	uint64_t br_from;
+	uint64_t br_to;
+	uint64_t last_excp_from;
+	uint64_t last_excp_to;
+};
+
+struct __attribute__ ((__packed__)) vmcb {
+	struct vmcb_control_area control;
+	struct vmcb_save_area save;
+};
+
+#define SVM_CPUID_FEATURE_SHIFT 2
+#define SVM_CPUID_FUNC 0x8000000a
+
+#define MSR_EFER_SVME_MASK (1ULL << 12)
+
+#define SVM_SELECTOR_S_SHIFT 4
+#define SVM_SELECTOR_DPL_SHIFT 5
+#define SVM_SELECTOR_P_SHIFT 7
+#define SVM_SELECTOR_AVL_SHIFT 8
+#define SVM_SELECTOR_L_SHIFT 9
+#define SVM_SELECTOR_DB_SHIFT 10
+#define SVM_SELECTOR_G_SHIFT 11
+
+#define SVM_SELECTOR_TYPE_MASK (0xf)
+#define SVM_SELECTOR_S_MASK (1 << SVM_SELECTOR_S_SHIFT)
+#define SVM_SELECTOR_DPL_MASK (3 << SVM_SELECTOR_DPL_SHIFT)
+#define SVM_SELECTOR_P_MASK (1 << SVM_SELECTOR_P_SHIFT)
+#define SVM_SELECTOR_AVL_MASK (1 << SVM_SELECTOR_AVL_SHIFT)
+#define SVM_SELECTOR_L_MASK (1 << SVM_SELECTOR_L_SHIFT)
+#define SVM_SELECTOR_DB_MASK (1 << SVM_SELECTOR_DB_SHIFT)
+#define SVM_SELECTOR_G_MASK (1 << SVM_SELECTOR_G_SHIFT)
+
+#define SVM_SELECTOR_WRITE_MASK (1 << 1)
+#define SVM_SELECTOR_READ_MASK SVM_SELECTOR_WRITE_MASK
+#define SVM_SELECTOR_CODE_MASK (1 << 3)
+
+#define INTERCEPT_CR0_MASK 1
+#define INTERCEPT_CR3_MASK (1 << 3)
+#define INTERCEPT_CR4_MASK (1 << 4)
+
+#define INTERCEPT_DR0_MASK 1
+#define INTERCEPT_DR1_MASK (1 << 1)
+#define INTERCEPT_DR2_MASK (1 << 2)
+#define INTERCEPT_DR3_MASK (1 << 3)
+#define INTERCEPT_DR4_MASK (1 << 4)
+#define INTERCEPT_DR5_MASK (1 << 5)
+#define INTERCEPT_DR6_MASK (1 << 6)
+#define INTERCEPT_DR7_MASK (1 << 7)
+
+#define SVM_EVTINJ_VEC_MASK 0xff
+
+#define SVM_EVTINJ_TYPE_SHIFT 8
+#define SVM_EVTINJ_TYPE_MASK (7 << SVM_EVTINJ_TYPE_SHIFT)
+
+#define SVM_EVTINJ_TYPE_INTR (0 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_NMI (2 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_EXEPT (3 << SVM_EVTINJ_TYPE_SHIFT)
+#define SVM_EVTINJ_TYPE_SOFT (4 << SVM_EVTINJ_TYPE_SHIFT)
+
+#define SVM_EVTINJ_VALID (1 << 31)
+#define SVM_EVTINJ_VALID_ERR (1 << 11)
+
+#define SVM_EXITINTINFO_VEC_MASK SVM_EVTINJ_VEC_MASK
+
+#define	SVM_EXITINTINFO_TYPE_INTR SVM_EVTINJ_TYPE_INTR
+#define	SVM_EXITINTINFO_TYPE_NMI SVM_EVTINJ_TYPE_NMI
+#define	SVM_EXITINTINFO_TYPE_EXEPT SVM_EVTINJ_TYPE_EXEPT
+#define	SVM_EXITINTINFO_TYPE_SOFT SVM_EVTINJ_TYPE_SOFT
+
+#define SVM_EXITINTINFO_VALID SVM_EVTINJ_VALID
+#define SVM_EXITINTINFO_VALID_ERR SVM_EVTINJ_VALID_ERR
+
+#define	SVM_EXIT_READ_CR0 	0x000
+#define	SVM_EXIT_READ_CR3 	0x003
+#define	SVM_EXIT_READ_CR4 	0x004
+#define	SVM_EXIT_READ_CR8 	0x008
+#define	SVM_EXIT_WRITE_CR0 	0x010
+#define	SVM_EXIT_WRITE_CR3 	0x013
+#define	SVM_EXIT_WRITE_CR4 	0x014
+#define	SVM_EXIT_WRITE_CR8 	0x018
+#define	SVM_EXIT_READ_DR0 	0x020
+#define	SVM_EXIT_READ_DR1 	0x021
+#define	SVM_EXIT_READ_DR2 	0x022
+#define	SVM_EXIT_READ_DR3 	0x023
+#define	SVM_EXIT_READ_DR4 	0x024
+#define	SVM_EXIT_READ_DR5 	0x025
+#define	SVM_EXIT_READ_DR6 	0x026
+#define	SVM_EXIT_READ_DR7 	0x027
+#define	SVM_EXIT_WRITE_DR0 	0x030
+#define	SVM_EXIT_WRITE_DR1 	0x031
+#define	SVM_EXIT_WRITE_DR2 	0x032
+#define	SVM_EXIT_WRITE_DR3 	0x033
+#define	SVM_EXIT_WRITE_DR4 	0x034
+#define	SVM_EXIT_WRITE_DR5 	0x035
+#define	SVM_EXIT_WRITE_DR6 	0x036
+#define	SVM_EXIT_WRITE_DR7 	0x037
+#define SVM_EXIT_EXCP_BASE      0x040
+#define SVM_EXIT_INTR		0x060
+#define SVM_EXIT_NMI		0x061
+#define SVM_EXIT_SMI		0x062
+#define SVM_EXIT_INIT		0x063
+#define SVM_EXIT_VINTR		0x064
+#define SVM_EXIT_CR0_SEL_WRITE	0x065
+#define SVM_EXIT_IDTR_READ	0x066
+#define SVM_EXIT_GDTR_READ	0x067
+#define SVM_EXIT_LDTR_READ	0x068
+#define SVM_EXIT_TR_READ	0x069
+#define SVM_EXIT_IDTR_WRITE	0x06a
+#define SVM_EXIT_GDTR_WRITE	0x06b
+#define SVM_EXIT_LDTR_WRITE	0x06c
+#define SVM_EXIT_TR_WRITE	0x06d
+#define SVM_EXIT_RDTSC		0x06e
+#define SVM_EXIT_RDPMC		0x06f
+#define SVM_EXIT_PUSHF		0x070
+#define SVM_EXIT_POPF		0x071
+#define SVM_EXIT_CPUID		0x072
+#define SVM_EXIT_RSM		0x073
+#define SVM_EXIT_IRET		0x074
+#define SVM_EXIT_SWINT		0x075
+#define SVM_EXIT_INVD		0x076
+#define SVM_EXIT_PAUSE		0x077
+#define SVM_EXIT_HLT		0x078
+#define SVM_EXIT_INVLPG		0x079
+#define SVM_EXIT_INVLPGA	0x07a
+#define SVM_EXIT_IOIO		0x07b
+#define SVM_EXIT_MSR		0x07c
+#define SVM_EXIT_TASK_SWITCH	0x07d
+#define SVM_EXIT_FERR_FREEZE	0x07e
+#define SVM_EXIT_SHUTDOWN	0x07f
+#define SVM_EXIT_VMRUN		0x080
+#define SVM_EXIT_VMMCALL	0x081
+#define SVM_EXIT_VMLOAD		0x082
+#define SVM_EXIT_VMSAVE		0x083
+#define SVM_EXIT_STGI		0x084
+#define SVM_EXIT_CLGI		0x085
+#define SVM_EXIT_SKINIT		0x086
+#define SVM_EXIT_RDTSCP		0x087
+#define SVM_EXIT_ICEBP		0x088
+#define SVM_EXIT_WBINVD		0x089
+/* only included in documentation, maybe wrong */
+#define SVM_EXIT_MONITOR	0x08a
+#define SVM_EXIT_MWAIT		0x08b
+#define SVM_EXIT_NPF  		0x400
+
+#define SVM_EXIT_ERR		-1
+
+#define SVM_CR0_SELECTIVE_MASK (1 << 3 | 1) /* TS and MP */
+
+#define SVM_VMLOAD ".byte 0x0f, 0x01, 0xda"
+#define SVM_VMRUN  ".byte 0x0f, 0x01, 0xd8"
+#define SVM_VMSAVE ".byte 0x0f, 0x01, 0xdb"
+#define SVM_CLGI   ".byte 0x0f, 0x01, 0xdd"
+#define SVM_STGI   ".byte 0x0f, 0x01, 0xdc"
+#define SVM_INVLPGA ".byte 0x0f, 0x01, 0xdf"
+
+/* function references */
+
+void helper_stgi();
+void vmexit(uint64_t exit_code, uint64_t exit_info_1);
+int svm_check_intercept_param(uint32_t type, uint64_t param);
+static inline int svm_check_intercept(unsigned int type) {
+    return svm_check_intercept_param(type, 0);
+}
+
+
+#define INTERCEPTED(mask) (env->intercept & mask)
+#define INTERCEPTEDw(var, mask) (env->intercept ## var & mask)
+#define INTERCEPTEDl(var, mask) (env->intercept ## var & mask)
+
+#define SVM_LOAD_SEG(addr, seg_index, seg) \
+    cpu_x86_load_seg_cache(env, \
+                    R_##seg_index, \
+                    lduw_phys(addr + offsetof(struct vmcb, save.seg.selector)),\
+                    ldq_phys(addr + offsetof(struct vmcb, save.seg.base)),\
+                    ldl_phys(addr + offsetof(struct vmcb, save.seg.limit)),\
+                    vmcb2cpu_attrib(lduw_phys(addr + offsetof(struct vmcb, save.seg.attrib)), ldq_phys(addr + offsetof(struct vmcb, save.seg.base)), ldl_phys(addr + offsetof(struct vmcb, save.seg.limit))))
+
+#define SVM_LOAD_SEG2(addr, seg_qemu, seg_vmcb) \
+    env->seg_qemu.selector  = lduw_phys(addr + offsetof(struct vmcb, save.seg_vmcb.selector)); \
+    env->seg_qemu.base      = ldq_phys(addr + offsetof(struct vmcb, save.seg_vmcb.base)); \
+    env->seg_qemu.limit     = ldl_phys(addr + offsetof(struct vmcb, save.seg_vmcb.limit)); \
+    env->seg_qemu.flags     = vmcb2cpu_attrib(lduw_phys(addr + offsetof(struct vmcb, save.seg_vmcb.attrib)), env->seg_qemu.base, env->seg_qemu.limit)
+
+#define SVM_SAVE_SEG(addr, seg_qemu, seg_vmcb) \
+    stw_phys(addr + offsetof(struct vmcb, save.seg_vmcb.selector), env->seg_qemu.selector); \
+    stq_phys(addr + offsetof(struct vmcb, save.seg_vmcb.base), env->seg_qemu.base); \
+    stl_phys(addr + offsetof(struct vmcb, save.seg_vmcb.limit), env->seg_qemu.limit); \
+    stw_phys(addr + offsetof(struct vmcb, save.seg_vmcb.attrib), cpu2vmcb_attrib(env->seg_qemu.flags))
+
+#endif
Index: qemu/cpu-exec.c
===================================================================
--- qemu.orig/cpu-exec.c
+++ qemu/cpu-exec.c
@@ -163,6 +163,7 @@ static inline TranslationBlock *tb_find_
 #if defined(TARGET_I386)
     flags = env->hflags;
     flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
+    flags |= env->intercept;
     cs_base = env->segs[R_CS].base;
     pc = cs_base + env->eip;
 #elif defined(TARGET_ARM)
@@ -373,7 +374,11 @@ int cpu_exec(CPUState *env1)
                 tmp_T0 = T0;
 #endif
                 interrupt_request = env->interrupt_request;
-                if (__builtin_expect(interrupt_request, 0)) {
+                if (__builtin_expect(interrupt_request, 0)
+#if defined(TARGET_I386)
+			&& env->hflags & HF_GIF_MASK
+#endif
+				) {
                     if (interrupt_request & CPU_INTERRUPT_DEBUG) {
                         env->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
                         env->exception_index = EXCP_DEBUG;
@@ -391,6 +396,7 @@ int cpu_exec(CPUState *env1)
 #if defined(TARGET_I386)
                     if ((interrupt_request & CPU_INTERRUPT_SMI) &&
                         !(env->hflags & HF_SMM_MASK)) {
+                        svm_check_intercept(SVM_EXIT_SMI);
                         env->interrupt_request &= ~CPU_INTERRUPT_SMI;
                         do_smm_enter();
 #if defined(__sparc__) && !defined(HOST_SOLARIS)
@@ -399,9 +405,10 @@ int cpu_exec(CPUState *env1)
                         T0 = 0;
 #endif
                     } else if ((interrupt_request & CPU_INTERRUPT_HARD) &&
-                        (env->eflags & IF_MASK) &&
+                        (env->eflags & IF_MASK || env->hflags & HF_HIF_MASK) &&
                         !(env->hflags & HF_INHIBIT_IRQ_MASK)) {
                         int intno;
+                        svm_check_intercept(SVM_EXIT_INTR);
                         env->interrupt_request &= ~CPU_INTERRUPT_HARD;
                         intno = cpu_get_pic_interrupt(env);
                         if (loglevel & CPU_LOG_TB_IN_ASM) {
@@ -415,6 +422,24 @@ int cpu_exec(CPUState *env1)
 #else
                         T0 = 0;
 #endif
+#if !defined(CONFIG_USER_ONLY)
+                    } else if ((interrupt_request & CPU_INTERRUPT_VIRQ) &&
+                        (env->eflags & IF_MASK) && !(env->hflags & HF_INHIBIT_IRQ_MASK)) {
+                         int intno;
+                         /* FIXME: this should respect TPR */
+                         env->interrupt_request &= ~CPU_INTERRUPT_VIRQ;
+                         stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl),
+                                  ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl)) & ~V_IRQ_MASK);
+                         intno = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_vector));
+                         if (loglevel & CPU_LOG_TB_IN_ASM)
+                             fprintf(logfile, "Servicing virtual hardware INT=0x%02x\n", intno);
+	                 do_interrupt(intno, 0, 0, -1, 1);
+#if defined(__sparc__) && !defined(HOST_SOLARIS)
+                         tmp_T0 = 0;
+#else
+                         T0 = 0;
+#endif
+#endif
                     }
 #elif defined(TARGET_PPC)
 #if 0
Index: qemu/target-i386/exec.h
===================================================================
--- qemu.orig/target-i386/exec.h
+++ qemu/target-i386/exec.h
@@ -506,6 +506,15 @@ void update_fp_status(void);
 void helper_hlt(void);
 void helper_monitor(void);
 void helper_mwait(void);
+void helper_vmrun(target_ulong addr);
+void helper_vmmcall(void);
+void helper_vmload(target_ulong addr);
+void helper_vmsave(target_ulong addr);
+void helper_stgi(void);
+void helper_clgi(void);
+void helper_skinit(void);
+void helper_invlpga(void);
+void vmexit(uint64_t exit_code, uint64_t exit_info_1);
 
 extern const uint8_t parity_table[256];
 extern const uint8_t rclw_table[32];
@@ -593,3 +602,4 @@ static inline int cpu_halted(CPUState *e
     }
     return EXCP_HALTED;
 }
+
Index: qemu/exec.c
===================================================================
--- qemu.orig/exec.c
+++ qemu/exec.c
@@ -1292,6 +1292,11 @@ void cpu_abort(CPUState *env, const char
     vfprintf(stderr, fmt, ap);
     fprintf(stderr, "\n");
 #ifdef TARGET_I386
+    if(env->intercept & INTERCEPT_SVM_MASK) {
+	/* most probably the virtual machine should not
+	   be shut down but rather caught by the VMM */
+        vmexit(SVM_EXIT_SHUTDOWN, 0);
+    }
     cpu_dump_state(env, stderr, fprintf, X86_DUMP_FPU | X86_DUMP_CCOP);
 #else
     cpu_dump_state(env, stderr, fprintf, 0);
Index: qemu/cpu-all.h
===================================================================
--- qemu.orig/cpu-all.h
+++ qemu/cpu-all.h
@@ -716,6 +716,7 @@ extern int code_copy_enabled;
 #define CPU_INTERRUPT_HALT   0x20 /* CPU halt wanted */
 #define CPU_INTERRUPT_SMI    0x40 /* (x86 only) SMI interrupt pending */
 #define CPU_INTERRUPT_DEBUG  0x80 /* Debug event occured.  */
+#define CPU_INTERRUPT_VIRQ   0x100 /* virtual interrupt pending.  */
 
 void cpu_interrupt(CPUState *s, int mask);
 void cpu_reset_interrupt(CPUState *env, int mask);

[-- Attachment #2: Type: text/plain, Size: 13710 bytes --]


On Sep 19, 2007, at 1:05 AM, J. Mayer wrote:

> On Tue, 2007-09-18 at 22:54 +0200, J. Mayer wrote:
>> Forwarded, as requested.
>>
>> -------- Forwarded Message --------
>>> From: Alexander Graf <agraf@suse.de>
>>> To: J.Mayer <l_indien@magic.fr>
>>> Subject: Re: [Qemu-devel] [PATCH] SVM support
>>> Date: Tue, 18 Sep 2007 13:51:26 +0200
>>>
>>> On Sep 18, 2007, at 12:09 PM, J. Mayer wrote:
>>>
>>>> On Tue, 2007-09-18 at 03:02 +0200, Alexander Graf wrote:
>>>>> Jocelyn Mayer wrote:
>>>>>> On Mon, 2007-09-17 at 12:02 +0200, Alexander Graf wrote:
>>>>>>
>>>>>>> J. Mayer wrote:
>>>>>>>
>>>>>>>> On Thu, 2007-09-13 at 17:27 +0200, Alexander Graf wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> Thiemo Seufer wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Alexander Graf wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Thanks to Michael Peter who tried the patch on an x86 host I
>>>>>>>>>>> found some
>>>>>>>>>>> compilation problems.
>>>>>>>>>>> This updated patch addresses these issues and thus should
>>>>>>>>>>> compile on
>>>>>>>>>>> every platform for every target available.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>> [...]
>>>>>>>>
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Wow sorry about that, looks like I missed these.
>>>>>>>>>
>>>>>>>>>
>>>>>>>> Index: qemu-0.9.0.cvs/exec-all.h
>>>>>>>> =============================================================== 
>>>>>>>> ===
>>>>>>>> =
>>>>>>>> --- qemu-0.9.0.cvs.orig/exec-all.h
>>>>>>>> +++ qemu-0.9.0.cvs/exec-all.h
>>>>>>>> @@ -166,6 +166,7 @@ static inline int tlb_set_page(CPUState
>>>>>>>>  typedef struct TranslationBlock {
>>>>>>>>      target_ulong pc;   /* simulated PC corresponding to this
>>>>>>>> block (EIP
>>>>>>>> + CS base) */
>>>>>>>>      target_ulong cs_base; /* CS base for this block */
>>>>>>>> +    uint64_t intercept; /* SVM intercept vector */
>>>>>>>>      unsigned int flags; /* flags defining in which context the
>>>>>>>> code was
>>>>>>>> generated */
>>>>>>>>      uint16_t size;      /* size of target code for this block
>>>>>>>> (1 <=
>>>>>>>>                             size <= TARGET_PAGE_SIZE) */
>>>>>>>> Index: qemu-0.9.0.cvs/cpu-all.h
>>>>>>>> =============================================================== 
>>>>>>>> ===
>>>>>>>> =
>>>>>>>> --- qemu-0.9.0.cvs.orig/cpu-all.h
>>>>>>>> +++ qemu-0.9.0.cvs/cpu-all.h
>>>>>>>> @@ -715,6 +715,7 @@ extern int code_copy_enabled;
>>>>>>>>  #define CPU_INTERRUPT_HALT   0x20 /* CPU halt wanted */
>>>>>>>>  #define CPU_INTERRUPT_SMI    0x40 /* (x86 only) SMI interrupt
>>>>>>>> pending
>>>>>>>> */
>>>>>>>>  #define CPU_INTERRUPT_DEBUG  0x80 /* Debug event occured.  */
>>>>>>>> +#define CPU_INTERRUPT_VIRQ   0x100 /* virtual interrupt
>>>>>>>> pending.  */
>>>>>>>>
>>>>>>>>  void cpu_interrupt(CPUState *s, int mask);
>>>>>>>>  void cpu_reset_interrupt(CPUState *env, int mask);
>>>>>>>>
>>>>>>>> Those two patches look ugly to me as target specific features
>>>>>>>> should
>>>>>>>> never go in generic code or structures.
>>>>>>>> The CPU_INTERRUPT flags should just contain information  
>>>>>>>> about the
>>>>>>>> emulator behavior, thus CPU_INTERRUPT_TIMER, CPU_INTERRUPT_SMI
>>>>>>>> are to be
>>>>>>>> removed. Target specific informations about the exception
>>>>>>>> nature should
>>>>>>>> go in the CPUState structure... Then, adding a
>>>>>>>> CPU_INTERRUPT_VIRQ seems
>>>>>>>> not a good idea at all: it's outside of the generic emulator
>>>>>>>> scope and
>>>>>>>> pointless for most targets.
>>>>>>>>
>>>>>>>>
>>>>>>> I don't see any practical reason not to do it this way. We could
>>>>>>> define
>>>>>>> a CPU_INTERRUPT_TARGET interrupt, that stores the information in
>>>>>>> a the
>>>>>>> target specific CPUState, but this would hit performance
>>>>>>> (marginally
>>>>>>> though) and does not improve the situation. I don't think that
>>>>>>> we should
>>>>>>> should forcefully try to seperate targets where we are not even
>>>>>>> close to
>>>>>>> running out of constants. If everyone on this list sees the
>>>>>>> situation as
>>>>>>> Jocelyn does, I would be fine with writing a patch that puts  
>>>>>>> target
>>>>>>> specific interrupts to the specific targets.
>>>>>>>
>>>>>>
>>>>>> I was to do the same to add some functionality to the PowerPC
>>>>>> target,
>>>>>> long time ago, and Fabrice Bellard convinced me not to do and
>>>>>> agreed it
>>>>>> has been a bad idea to add the target specific CPU_INTERRUPT_xxx
>>>>>> flags.
>>>>>> Then I did manage the PowerPC target to use only generic
>>>>>> CPU_INTERRUPT_xxx and put all other target specific informations
>>>>>> in the
>>>>>> CPUState structure. It absolutelly changes nothing in terms of
>>>>>> functionality nor performance. The only thing is that it makes  
>>>>>> the
>>>>>> generic parts simpler and could even allow the cpu_exec  
>>>>>> function to
>>>>>> contain almost no target specific code, which would really great
>>>>>> imho.
>>>>>>
>>>>>
>>>>> I can give that a try :-). Sounds reasonable for me.
>>>>
>>>>> [Next message]
>>>>> Oh well I just thought about this a bit more and while stumbling
>>>>> across
>>>>> CPU_INTERRUPT_FIQ which does the same thing one major problem came
>>>>> to me
>>>>> on that one: Priority. Real interrupts have priority over virtual
>>>>> interrupts so the VIRQs have to be processed after HARD IRQs,  
>>>>> whereas
>>>>> SMIs and NMIs have to be taken care of before the HARD IRQs. It
>>>>> simply
>>>>> wouldn't work out to generalize the IRQs, just as it would not
>>>>> work with
>>>>> the VIRQ, as it has to be handled as a real IRQ but without  
>>>>> accessing
>>>>> the APIC which has to be done for HARD IRQs.
>>>>>
>>>>> I guess we're stuck with what's there now.
>>>>
>>>> Priorities are not an issue, you can take a look to the
>>>> ppc_hw_interrupt function
>>>> to see how the PowerPC emulation code manages priorities and
>>>> masking between
>>>> 10 different hardware interruption sources. The code could be
>>>> better (I wanted
>>>> to preserve existing code) but it's a solution that actually  
>>>> works...
>>>> But looking better the x86 emulation code, I agree that if you
>>>> don't want to change it
>>>> too much, you have to add a flag. The other solution (and best,
>>>> imho) would be
>>>> to do the same as what has been done for PowerPC: just let the
>>>> generic code in
>>>> the cpu_exec loop and move the x86 specific code somewhere in
>>>> target-i386 code.
>>>> Going this way, I see no reason why the interruption code in the
>>>> cpu_exec loop
>>>> could not finally become:
>>>>                 interrupt_request = env->interrupt_request;
>>>>                 if (__builtin_expect(interrupt_request, 0)) {
>>>>                     if (interrupt_request & CPU_INTERRUPT_DEBUG) {
>>>>                         env->interrupt_request &=
>>>> ~CPU_INTERRUPT_DEBUG;
>>>>                         env->exception_index = EXCP_DEBUG;
>>>>                         cpu_loop_exit();
>>>>                     }
>>>>                     if (interrupt_request & CPU_INTERRUPT_HALT) {
>>>>                         env->interrupt_request &=  
>>>> ~CPU_INTERRUPT_HALT;
>>>>                         env->halted = 1;
>>>>                         env->exception_index = EXCP_HLT;
>>>>                         cpu_loop_exit();
>>>>                     }
>>>>                     if (interrupt_request & CPU_INTERRUPT_HARD) {
>>>>                         hw_interrupt(env);
>>>>                         if (env->pending_interrupts == 0)
>>>>                             env->interrupt_request &=
>>>> ~CPU_INTERRUPT_HARD;
>>>> #if defined(__sparc__) && !defined(HOST_SOLARIS)
>>>>                         tmp_T0 = 0;
>>>> #else
>>>>                         T0 = 0;
>>>> #endif
>>>>                     }
>>>>                    /* Don't use the cached interupt_request value,
>>>>                       do_interrupt may have updated the EXITTB
>>>> flag. */
>>>>                     if (interrupt_request & CPU_INTERRUPT_EXITTB) {
>>>>                         env->interrupt_request &=
>>>> ~CPU_INTERRUPT_EXITTB;
>>>>                         /* ensure that no TB jump will be  
>>>> modified as
>>>>                            the program flow was changed */
>>>> #if defined(__sparc__) && !defined(HOST_SOLARIS)
>>>>                         tmp_T0 = 0;
>>>> #else
>>>>                         T0 = 0;
>>>> #endif
>>>>                     }
>>>>                     if (interrupt_request & CPU_INTERRUPT_EXIT) {
>>>>                         env->interrupt_request &=  
>>>> ~CPU_INTERRUPT_EXIT;
>>>>                         env->exception_index = EXCP_INTERRUPT;
>>>>                         cpu_loop_exit();
>>>>                     }
>>>>                 }
>>>> All the targets specific tricks could be done in the hw_interrupt
>>>> routine. And the generic code
>>>> would become much more readable. But this needs some works (not so
>>>> much) and intensive tests...
>>>> And I guess nobody feels like taking this risk right now ;-)
>>>> But I think this will have to be done someday...
>>>
>>> I definitely agree with you on that one. The current interrupt
>>> handlers are filled with #ifdefs and not easily readable. This is a
>>> different task though and maybe I will get the time to do this. I
>>> believe it should be done "all archs at once" though, so either we
>>> use the current method (adding non-gerneric interrupts) for now and
>>> tidy up everything later on or tidy up the code now and port the svm
>>> patch on that. As I don't see the cleanup coming in the near future
>>> I'd vote for including it now and have it reworked at once when
>>> someone feels dare enough to clean up the whole generic interrupt
>>> handler.
>
> The danger doing the patch "all archs at once" is that it's difficult
> for one to know and test all archs... But it would be a great idea to
> fix this at once...
> If your patch is ready to be applied, then apply it as it is,  
> especially
> if it helps you to go on developing new features to have it commited.

As I do not have any commit rights, I need someone to apply this for  
me ;-).
I'm not too sure about new features getting in soon though, as I will  
very soon have to write my bachelor thesis and am not allowed to  
write it on anything technical like qemu.

>
>>>
>>> Nevertheless I really do like the idea of having a cleaned up
>>> interrupt handler. This is a different patch though and is rather a
>>> TODO than anything that should be mangled with the svm patch.
>
> yes, those are separate works, but it may help if you can have this in
> mind (if it does not break too much of your code too !)...
>
>>>>
>>>>>>
>>>>>>>> For the same reason, the intercept field in the TB structure
>>>>>>>> seems not
>>>>>>>> acceptable, as TB specific target informations are already to
>>>>>>>> be stored
>>>>>>>> in the flags field. As intercept seems only to be a bit field,
>>>>>>>> it should
>>>>>>>> go, in a way or another, in tb flags. And as it seems that some
>>>>>>>>
>>>>>>>>
>>>>>>> TB flags is a 32-bit bitfield, where several bits are already  
>>>>>>> used.
>>>>>>> Currently SVM supports 45 intercepts and each combination can
>>>>>>> generate a
>>>>>>> different TB, as we do not want to reduce performance for the
>>>>>>> non-intercepted case. Think of the intercept field as flags2,
>>>>>>> that could
>>>>>>> be used by other virtualization implementations on other
>>>>>>> architectures
>>>>>>> too (though I do not know of any)
>>>>>>>
>>>>>>
>>>>>> So, why not make it generic and extend the flag field to as many
>>>>>> bits as
>>>>>> you need ? Intercept is x86 specific and has no meaning outside
>>>>>> of this
>>>>>> scope. Having more bits for flags is generic and may be useful
>>>>>> for some
>>>>>> other targets... The easy way is to convert flag to 64 bits, if
>>>>>> you have
>>>>>> enough bits. Another way is to make it a table, but this may need
>>>>>> changes in all targets code...
>>>>>>
>>>>>
>>>>> I think it might be quite a bad idea to extend flags until  
>>>>> everything
>>>>> fits in there. I'm planning on implementing VMX as well and  
>>>>> there are
>>>>> intercepts too so the meaning of the different fields change and I
>>>>> would
>>>>> not want that happening with the normal flags vector. What  
>>>>> would you
>>>>> think of an #ifdef TARGET_I386? This way intercepts are x86  
>>>>> only and
>>>>> don't interfere with any other target architecture, but are still
>>>>> seperated from the flags (which is semantically correct as far  
>>>>> as it
>>>>> goes for me).
>>>>
>>>> I don't consider as a problem that one bit in the TB flags field  
>>>> can
>>>> have different meanings.
>>>> This field is something opaque from the generic code. I actually  
>>>> have
>>>> the same thing in
>>>> the PowerPC emulation (even if it's not fully done for now): TB  
>>>> flags
>>>> are just an
>>>> selection of the Machine State Register bits. Some of those bits  
>>>> can
>>>> have different
>>>> meanings depending on the PowerPC implementation (ie CPU model).
>>>> But the
>>>> only thing important is to know if the current CPUState flags are
>>>> equal
>>>> to the
>>>> TB flags, not the actual meaning of the bits. As the MSR is 32  
>>>> bits on
>>>> 32 bits
>>>> PowerPC implementations, I have no hesitation to use those bits
>>>> directly
>>>> even if their actual meaning, from the CPU "point of view" changes.
>>>> Then, imho, you should not hesitate to put those intercept bits  
>>>> in a
>>>> generic field...
>>>
>>>
>>> Ok, I will try to shift the intercepts in an uint_64 flags variable
>>> in the TB.
>
> OK, great. Having 64 bits may also help for additional (ie future...)
> features in PowerPC 64 emulation.

Changed the patch and it seems to work for me.

Cheers,

Alex



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-19 10:56     ` Alexander Graf
@ 2007-09-19 14:39       ` Jocelyn Mayer
  2007-09-19 14:55         ` Alexander Graf
  2007-09-19 15:35         ` Paul Brook
  0 siblings, 2 replies; 10+ messages in thread
From: Jocelyn Mayer @ 2007-09-19 14:39 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-devel, Paul Brook

On Wed, 2007-09-19 at 12:56 +0200, Alexander Graf wrote:
> On Sep 19, 2007, at 1:28 AM, Paul Brook wrote:
> 
> >>>> Ok, I will try to shift the intercepts in an uint_64 flags variable
> >>>> in the TB.
> >>
> >> OK, great. Having 64 bits may also help for additional (ie future...)
> >> features in PowerPC 64 emulation.
> >
> > Maybe worth letting the target say whether it needs 32 or 64-bit  
> > flags.
> > The flag lookup is likely to be on a hot path.
> >
> > Paul
> 
> here comes the patch.

The patch looks correct to me, I'm just wondering why a cast to 32 bits
is needed in mips code ?

I may be wrong, but it seems to me that TB flags are only used to be
compared in tb_find_fast and tb_find_slow, which are not a very fast
path (we come here when we cannot jump directly to the next tb and are
not in the most time critical code) then we can afford adding a few asm
instructions (I would say max 2 ?) to replace a 32 bits comparison with
a 64 bits one. My feeling is that the performance loss here won't be
sensible, but that may need to be proved.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-19 14:39       ` Jocelyn Mayer
@ 2007-09-19 14:55         ` Alexander Graf
  2007-09-19 15:35         ` Paul Brook
  1 sibling, 0 replies; 10+ messages in thread
From: Alexander Graf @ 2007-09-19 14:55 UTC (permalink / raw)
  To: J. Mayer; +Cc: qemu-devel, Paul Brook


On Sep 19, 2007, at 4:39 PM, Jocelyn Mayer wrote:

> On Wed, 2007-09-19 at 12:56 +0200, Alexander Graf wrote:
>> On Sep 19, 2007, at 1:28 AM, Paul Brook wrote:
>>
>>>>>> Ok, I will try to shift the intercepts in an uint_64 flags  
>>>>>> variable
>>>>>> in the TB.
>>>>
>>>> OK, great. Having 64 bits may also help for additional (ie  
>>>> future...)
>>>> features in PowerPC 64 emulation.
>>>
>>> Maybe worth letting the target say whether it needs 32 or 64-bit
>>> flags.
>>> The flag lookup is likely to be on a hot path.
>>>
>>> Paul
>>
>> here comes the patch.
>
> The patch looks correct to me, I'm just wondering why a cast to 32  
> bits
> is needed in mips code ?


It's not needed - I just did it so one can see that there is  
potential that is not used right now and someone should think about  
it. I'm not too much into mips so I have no clues if they have any  
use for more that 32 bits of flags.

>
> I may be wrong, but it seems to me that TB flags are only used to be
> compared in tb_find_fast and tb_find_slow, which are not a very fast
> path (we come here when we cannot jump directly to the next tb and are
> not in the most time critical code) then we can afford adding a few  
> asm
> instructions (I would say max 2 ?) to replace a 32 bits comparison  
> with
> a 64 bits one. My feeling is that the performance loss here won't be
> sensible, but that may need to be proved.
>
>

I actually don't even believe the difference can be measured at all,  
which is why I seperated the patches.

Cheers,

Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-19 14:39       ` Jocelyn Mayer
  2007-09-19 14:55         ` Alexander Graf
@ 2007-09-19 15:35         ` Paul Brook
  2007-09-19 18:24           ` J. Mayer
  1 sibling, 1 reply; 10+ messages in thread
From: Paul Brook @ 2007-09-19 15:35 UTC (permalink / raw)
  To: qemu-devel, l_indien; +Cc: Alexander Graf

> > >> OK, great. Having 64 bits may also help for additional (ie future...)
> > >> features in PowerPC 64 emulation.
> > >
> > > Maybe worth letting the target say whether it needs 32 or 64-bit
> > > flags.
> > > The flag lookup is likely to be on a hot path.
> > >
>
> I may be wrong, but it seems to me that TB flags are only used to be
> compared in tb_find_fast and tb_find_slow, which are not a very fast
> path (we come here when we cannot jump directly to the next tb and are
> not in the most time critical code) then we can afford adding a few asm
> instructions (I would say max 2 ?) to replace a 32 bits comparison with
> a 64 bits one. My feeling is that the performance loss here won't be
> sensible, but that may need to be proved.

For some reason I thought the flags were also used in the hash lookup. This is 
not the case, so I withdraw my objection.

Paul

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Fwd: Re: [Qemu-devel] [PATCH] SVM support]
  2007-09-19 15:35         ` Paul Brook
@ 2007-09-19 18:24           ` J. Mayer
  0 siblings, 0 replies; 10+ messages in thread
From: J. Mayer @ 2007-09-19 18:24 UTC (permalink / raw)
  To: Paul Brook; +Cc: qemu-devel, Alexander Graf

On Wed, 2007-09-19 at 16:35 +0100, Paul Brook wrote:
> > > >> OK, great. Having 64 bits may also help for additional (ie future...)
> > > >> features in PowerPC 64 emulation.
> > > >
> > > > Maybe worth letting the target say whether it needs 32 or 64-bit
> > > > flags.
> > > > The flag lookup is likely to be on a hot path.
> > > >
> >
> > I may be wrong, but it seems to me that TB flags are only used to be
> > compared in tb_find_fast and tb_find_slow, which are not a very fast
> > path (we come here when we cannot jump directly to the next tb and are
> > not in the most time critical code) then we can afford adding a few asm
> > instructions (I would say max 2 ?) to replace a 32 bits comparison with
> > a 64 bits one. My feeling is that the performance loss here won't be
> > sensible, but that may need to be proved.
> 
> For some reason I thought the flags were also used in the hash lookup. This is 
> not the case, so I withdraw my objection.

OK, then I feel like the flags size modification patch is acceptable.
If there is no objection (and if no one commits it before ;-)), I'm
ready to integrate it in the days to come.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2007-09-19 18:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-18 20:54 [Fwd: Re: [Qemu-devel] [PATCH] SVM support] J. Mayer
2007-09-18 23:05 ` J. Mayer
2007-09-18 23:28   ` Paul Brook
2007-09-19 10:55     ` Alexander Graf
2007-09-19 10:56     ` Alexander Graf
2007-09-19 14:39       ` Jocelyn Mayer
2007-09-19 14:55         ` Alexander Graf
2007-09-19 15:35         ` Paul Brook
2007-09-19 18:24           ` J. Mayer
2007-09-19 10:59   ` Alexander Graf

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).