Re: [Xen-devel] [PATCH v4] Sanity check xsave area when migrating or restoring from older Xen verions

[Don Koch <dkoch@xxxxxxxxxxx>]

On Wed, 22 Oct 2014 15:03:23 +0100
Jan Beulich <JBeulich@xxxxxxxx> wrote:

> >>> On 22.10.14 at 15:19, <dkoch@xxxxxxxxxxx> wrote:
> > On Wed, 22 Oct 2014 11:00:52 +0100
> > Jan Beulich <JBeulich@xxxxxxxx> wrote:
> > 
> >> >>> On 21.10.14 at 21:25, <dkoch@xxxxxxxxxxx> wrote:
> >> > On Tue, 21 Oct 2014 20:00:53 +0100
> >> > Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> >> > 
> >> >> On 21/10/14 19:40, Don Koch wrote:
[...]
> >> >> > +                printk(XENLOG_G_WARNING
> >> >> > +                       "HVM%u.%u restore mismatch: xsave length %u > 
> > %u\n",
> >> >> > +                       d->domain_id, vcpuid, desc->length, size);
> >> >> > +                printk(XENLOG_G_WARNING
> >> >> > +                       "HVM%u.%u restore mismatch: xsave has 
> >> >> > non-zero data 
> > starting at %#x\n",
> >> >> > +                       d->domain_id, vcpuid, i);
> >> >> 
> >> >> This should be one message.  Also note that, while a lot of code gets it
> >> >> wrong, domain_id is signed while vcpuid is unsigned.
> >> > 
> >> > I had suggested one message. Jan said it should be two.
> >> 
> >> Right, and I still think it should be two. Just not the way you did it.
> >> I specifically said in the reply to the previous version " just add your
> >> new check ahead of the existing printk()". In case this was ambiguous
> >> to you - I think the pre-existing printk() should continue to get
> >> issued (even if not being on an error path anymore) so that we have
> >> some kind of indication that the truncating path was taken. After all
> >> this shouldn't happen frequently, considering that the most recent
> >> stable releases of the older branches already don't do this anymore.
> > 
> > I thought that's where I had it. If the block size mismatch is detected,
> > issue the first message then go into the loop to check for non-zero data
> > and, if any is found, then issue the second and exit.
> > 
> > Andrew, IIUC, didn't want the first one issued unless the non-zero data
> > case was found, i.e. issue no message unless both conditions were met.
> > 
> > So, which should I do?
> 
> I'm really getting tired of this; I don't think it's that difficult:
> 
> if size too large
>       loop over extra data
>               if non-zero
>                       issue error message
>                       return
>       issue warning message

My only issue with this is seeing just the error:
   HVM1.1 restore mismatch: xsave has non-zero data starting at 0x232
would make one wonder, "What's wrong with non-zero data? If it's supposed
to be zero, why is it being sent in the first place?"

> 
> Jan

-d

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