Xen devel - Oct 2010 - [Patch 4/4] Refining Xsave/Xrestore support - Version 2

Hi, Keir,

This is patch #4, which adds domain save/restore support when
Xsave/Xrestore is supported.

Shan Haitao


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Sorry, Keir, I attached a wrong version of patch file.
Please use this patch instead. Thanks!

Shan Haitao

2010/10/29 Haitao Shan <maillists.shan@gmail.com>:
> Hi, Keir,
>
> This is patch #4, which adds domain save/restore support when
> Xsave/Xrestore is supported.
>
> Shan Haitao
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> On 29.10.10 at 03:49, Haitao Shan <maillists.shan@gmail.com>
wrote:
>+            if ( evc->size != PV_XSAVE_SIZE ||
>+                 evc->xfeature_mask != xfeature_mask )
>+            {
>+                ret = EFAULT;
This ought to be negative, and perhaps another error code would be
better to pick here.
>+                goto vcpuextstate_out;
>+            }
>...
>+            if ( evc->size > PV_XSAVE_SIZE )
>+                goto vcpuextstate_out;
What if evc->size < PV_XSAVE_SIZE? You''re still copying
xsave_cntxt_size bytes in the final copy_from_guest_offset().

Also, you''re copying directly from the user buffer into struct vcpu
fields, so you''ll leave inconsistent state there if the second or
third copy fails but at least the first succeeded. I think you need
to copy the full input structure first, check for validity, and only
then put the fields into the respective struct vcpu ones.
>+#define CPU_XSAVE_CODE  16
I forgot to ask already on your first submission why this isn''t
using DECLARE_HVM_SAVE_TYPE().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel