[Qemu-devel] Re: [PATCH] kvm: x86: Save/restore error_code

[Jason Wang]

Juan Quintela writes:
 > Jason Wang <address@hidden> wrote:
 > > Juan Quintela writes:
 > >  > Jason Wang <address@hidden> wrote:
 > >  > > The saving and restoring of error_code seems lost and convert the
 > >  > > error_code to uint32_t.
 > >  > >
 > >  > > Signed-off-by: Jason Wang <address@hidden>
 > >  > > ---
 > >  > >  target-i386/cpu.h     |    4 ++--
 > >  > >  target-i386/machine.c |    2 ++
 > >  > >  2 files changed, 4 insertions(+), 2 deletions(-)
 > >  > 
 > >  > It should be a new subsection.  The test is if has_error_code != 0
 > >  > according to gleb.
 > >  > 
 > >  > Later, Juan.
 > >  > 
 > >
 > > Thanks for reminding, and maybe we can just use VMSTATE_UINT32_TEST() 
 > > which is
 > > simpler than subsection to do the check, isn't it?
 > 
 > we need the subsection, that way we don't need to bump the section
 > version.
 > 
 > Later, Juan.
 > 

Have tried the subsection, but there's an issue I find:

When we use subsections with the structure who have an nested
VMStateDescription at the end of fields, the subsections could not be loaded
correctly becuase qemu always try to match the subsection with the nested one.

So when we use subsections for vmstate_cpu, the subsection always fail as it try
to load the cpu's subsection for vmstate_ymmh_reg which is an nested
VMStateDescription on the end.

Maybe we need to modify vmstate_subsection_load() to handle this condition.

Any thought about this?

 > >
 > >  > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
 > >  > > index 06e40f3..c990db9 100644
 > >  > > --- a/target-i386/cpu.h
 > >  > > +++ b/target-i386/cpu.h
 > >  > > @@ -688,7 +688,7 @@ typedef struct CPUX86State {
 > >  > >      uint64_t pat;
 > >  > >  
 > >  > >      /* exception/interrupt handling */
 > >  > > -    int error_code;
 > >  > > +    uint32_t error_code;
 > >  > >      int exception_is_int;
 > >  > >      target_ulong exception_next_eip;
 > >  > >      target_ulong dr[8]; /* debug registers */
 > >  > > @@ -933,7 +933,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 > >  > >  #define cpu_list_id x86_cpu_list
 > >  > >  #define cpudef_setup x86_cpudef_setup
 > >  > >  
 > >  > > -#define CPU_SAVE_VERSION 12
 > >  > > +#define CPU_SAVE_VERSION 13
 > >  > >  
 > >  > >  /* MMU modes definitions */
 > >  > >  #define MMU_MODE0_SUFFIX _kernel
 > >  > > diff --git a/target-i386/machine.c b/target-i386/machine.c
 > >  > > index d78eceb..0e467da 100644
 > >  > > --- a/target-i386/machine.c
 > >  > > +++ b/target-i386/machine.c
 > >  > > @@ -491,6 +491,8 @@ static const VMStateDescription vmstate_cpu = {
 > >  > >          VMSTATE_UINT64_V(xcr0, CPUState, 12),
 > >  > >          VMSTATE_UINT64_V(xstate_bv, CPUState, 12),
 > >  > >          VMSTATE_YMMH_REGS_VARS(ymmh_regs, CPUState, CPU_NB_REGS, 12),
 > >  > > +
 > >  > > +        VMSTATE_UINT32_V(error_code, CPUState, 13),
 > >  > >          VMSTATE_END_OF_LIST()
 > >  > >          /* The above list is not sorted /wrt version numbers, watch 
 > > out! */
 > >  > >      },
 > --
 > To unsubscribe from this list: send the line "unsubscribe kvm" in
 > the body of a message to address@hidden
 > More majordomo info at  http://vger.kernel.org/majordomo-info.html