Re: [Qemu-devel] [PATCH v4 2/3] target-i386: add migration support for Intel LMCE

[Eduardo Habkost]

On Thu, Jun 16, 2016 at 06:55:29PM +0800, Haozhong Zhang wrote:
> On 06/16/16 12:41, Paolo Bonzini wrote:
> > 
> > 
> > On 16/06/2016 12:29, Haozhong Zhang wrote:
> > > On 06/16/16 11:51, Paolo Bonzini wrote:
> > >>
> > >>
> > >> On 16/06/2016 08:06, Haozhong Zhang wrote:
> > >>> Migration is only allowed between VCPUs with the same lmce option.
> > >>>
> > >>> Signed-off-by: Haozhong Zhang <address@hidden>
> > >>> ---
> > >>>  target-i386/machine.c | 25 +++++++++++++++++++++++++
> > >>>  1 file changed, 25 insertions(+)
> > >>>
> > >>> diff --git a/target-i386/machine.c b/target-i386/machine.c
> > >>> index cb9adf2..00375a3 100644
> > >>> --- a/target-i386/machine.c
> > >>> +++ b/target-i386/machine.c
> > >>> @@ -347,6 +347,12 @@ static int cpu_post_load(void *opaque, int 
> > >>> version_id)
> > >>>          return -EINVAL;
> > >>>      }
> > >>>  
> > >>> +    if (!cpu->enable_lmce && (env->mcg_cap & MCG_LMCE_P)) {
> > >>> +        error_report("Config mismatch: VCPU has LMCE enabled, "
> > >>> +                     "but \"lmce\" option is disabled");
> > >>> +        return -EINVAL;
> > >>> +    }
> > >>> +
> > >>
> > >> I think this is unnecessary.  Apart from this, the patch is good and can
> > >> be squashed in patch 1 for v5.
> > >>
> > > 
> > > Without this check, the migration from LMCE enabled QEMU to LMCE
> > > disabled QEMU will not fail. Is such configuration change considered
> > > be error? If not, I will remove the error report and return, but add a
> > > fix to remove MCG_LMCE_P from env->mcg_cap in this check.
> > 
> > It's considered a user error.  You can skip the "if" completely.
> >
> 
> Eduardo said nice for this part in previous version [1], so we may wait
> for his comments?
> 
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg01992.html

I agree we don't need this check, but I still believe it is a
nice thing to have.

In addition to detecting user errors, they don't hurt and are
useful for things like "-cpu host", that don't guarantee
live-migration compatibility but still allow migration if you
ensure host capabilities are the same on both sides.

(I was going to suggest enabling lmce automatically on "-cpu
host" as a follow-up patch, BTW.)

-- 
Eduardo