Re: [Xen-devel] [PATCH v6 06/11] x86/vmce: emulate MSR_IA32_MCG_EXT_CTL

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On 07/05/17 04:36 -0600, Jan Beulich wrote:
> >>> On 05.07.17 at 05:12, <haozhong.zhang@xxxxxxxxx> wrote:
> > @@ -878,6 +879,8 @@ long arch_do_domctl(
[..] 
> > @@ -917,9 +920,21 @@ long arch_do_domctl(
> >                           offsetof(struct xen_domctl_ext_vcpucontext,
> >                                    vmce.caps));
> >              BUILD_BUG_ON(sizeof(evc->mcg_cap) != sizeof(evc->vmce.caps));
> > -            if ( evc->size >= offsetof(typeof(*evc), vmce) +
> > -                              sizeof(evc->vmce) )
> > +            if ( evc->size >= vmce_off + sizeof(evc->vmce) )
> >                  ret = vmce_restore_vcpu(v, &evc->vmce);
> > +            else if ( evc->size >= vmce_off +
> > +                                   offsetof(typeof(evc->vmce), 
> > mcg_ext_ctl) )
> > +            {
> > +                /*
> > +                 * If migrating from old Xen that uses a smaller 'struct
> > +                 * hvm_vmce_vcpu', only restore the components in the
> > +                 * old version.
> > +                 */
> > +                struct hvm_vmce_vcpu vmce = { 0 };
> 
> Just { } would suffice.
> 
> > +                memcpy(&vmce, &evc->vmce, evc->size - vmce_off);
> > +                ret = vmce_restore_vcpu(v, &vmce);
> > +            }
> >              else if ( evc->size >= offsetof(typeof(*evc), mcg_cap) +
> >                                     sizeof(evc->mcg_cap) )
> >              {
> 
> So you really have two choices here: Either mirror/clone what's
> already there, or (preferable imo) carefully generalize the existing
> code. But please don't use yet another slightly different model.
> 

How is about generalize above two else cases by

            else if ( evc->size >= offsetof(typeof(*evc), mcg_cap) +
                                   sizeof(evc->mcg_cap) )
            {
                struct hvm_vmce_vcpu vmce = { };

                memcpy(&vmce, &evc->vmce,
                       evc->size - offsetof(typeof(*evc), mcg_cap));
                ret = vmce_restore_vcpu(v, &vmce);
            }

However, I'm not sure whether it's safe to blindly copy what is passed
in 'evc->vmce'. Before this patch, the else-if branch only copies
mcg_cap if the size of evc->vmce is larger than the size of mcg_cap,
so it implies no?

Haozhong

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel