* [PATCH v4 1/4] xen/save: pass a size parameter to the HVM compat functions
2015-11-25 15:18 [PATCH v4 0/4] Introduce a flags field to HVM CPU context Roger Pau Monne
@ 2015-11-25 15:18 ` Roger Pau Monne
2015-11-25 15:18 ` [PATCH v4 2/4] xen/save: allow the usage of zeroextend and a fixup function Roger Pau Monne
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Roger Pau Monne @ 2015-11-25 15:18 UTC (permalink / raw)
To: xen-devel
Cc: Tim Deegan, Ian Jackson, Ian Campbell, Jan Beulich,
Roger Pau Monne
In order to cope with types having multiple compat versions pass a size
parameter to the fixup function so we can identify which compat version
Xen is dealing with.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>
---
Changes since v3:
- Add Andrew Cooper Reviewed-by.
- s/d/desc/ in the _hvm_load_entry macro.
Changes since v2:
- Size is uint32_t not int.
- Pass the actual size of the record and not the size of the whole stream.
---
xen/include/public/arch-x86/hvm/save.h | 2 +-
xen/include/public/hvm/save.h | 10 ++++++----
xen/include/xen/hvm/save.h | 4 +++-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index efb0b62..29d513c 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -268,7 +268,7 @@ struct hvm_hw_cpu_compat {
uint32_t error_code;
};
-static inline int _hvm_hw_fix_cpu(void *h) {
+static inline int _hvm_hw_fix_cpu(void *h, uint32_t size) {
union hvm_hw_cpu_union {
struct hvm_hw_cpu nat;
diff --git a/xen/include/public/hvm/save.h b/xen/include/public/hvm/save.h
index cc8b5fd..0bd240d 100644
--- a/xen/include/public/hvm/save.h
+++ b/xen/include/public/hvm/save.h
@@ -63,13 +63,15 @@ struct hvm_save_descriptor {
#ifdef __XEN__
# define DECLARE_HVM_SAVE_TYPE_COMPAT(_x, _code, _type, _ctype, _fix) \
- static inline int __HVM_SAVE_FIX_COMPAT_##_x(void *h) { return _fix(h); } \
- struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[2];}; \
+ static inline int __HVM_SAVE_FIX_COMPAT_##_x(void *h, uint32_t size) \
+ { return _fix(h, size); } \
+ struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[2];}; \
struct __HVM_SAVE_TYPE_COMPAT_##_x { _ctype t; }
# include <xen/lib.h> /* BUG() */
# define DECLARE_HVM_SAVE_TYPE(_x, _code, _type) \
- static inline int __HVM_SAVE_FIX_COMPAT_##_x(void *h) { BUG(); return -1; } \
+ static inline int __HVM_SAVE_FIX_COMPAT_##_x(void *h, uint32_t size) \
+ { BUG(); return -1; } \
struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; char cpt[1];}; \
struct __HVM_SAVE_TYPE_COMPAT_##_x { _type t; }
#else
@@ -89,7 +91,7 @@ struct hvm_save_descriptor {
# define HVM_SAVE_LENGTH_COMPAT(_x) (sizeof (HVM_SAVE_TYPE_COMPAT(_x)))
# define HVM_SAVE_HAS_COMPAT(_x) (sizeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->cpt)-1)
-# define HVM_SAVE_FIX_COMPAT(_x, _dst) __HVM_SAVE_FIX_COMPAT_##_x(_dst)
+# define HVM_SAVE_FIX_COMPAT(_x, _dst, _size) __HVM_SAVE_FIX_COMPAT_##_x(_dst, _size)
#endif
/*
diff --git a/xen/include/xen/hvm/save.h b/xen/include/xen/hvm/save.h
index aa27a50..51bc7d5 100644
--- a/xen/include/xen/hvm/save.h
+++ b/xen/include/xen/hvm/save.h
@@ -60,6 +60,8 @@ void _hvm_read_entry(struct hvm_domain_context *h,
*/
#define _hvm_load_entry(_x, _h, _dst, _strict) ({ \
int r; \
+ struct hvm_save_descriptor *desc \
+ = (struct hvm_save_descriptor *)&(_h)->data[(_h)->cur]; \
if ( (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), \
HVM_SAVE_LENGTH(_x), (_strict))) == 0 ) \
_hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x)); \
@@ -67,7 +69,7 @@ void _hvm_read_entry(struct hvm_domain_context *h,
&& (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), \
HVM_SAVE_LENGTH_COMPAT(_x), (_strict))) == 0 ) { \
_hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH_COMPAT(_x)); \
- r=HVM_SAVE_FIX_COMPAT(_x, (_dst)); \
+ r = HVM_SAVE_FIX_COMPAT(_x, (_dst), desc->length); \
} \
r; })
--
1.9.5 (Apple Git-50.3)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v4 2/4] xen/save: allow the usage of zeroextend and a fixup function
2015-11-25 15:18 [PATCH v4 0/4] Introduce a flags field to HVM CPU context Roger Pau Monne
2015-11-25 15:18 ` [PATCH v4 1/4] xen/save: pass a size parameter to the HVM compat functions Roger Pau Monne
@ 2015-11-25 15:18 ` Roger Pau Monne
2015-11-25 15:18 ` [PATCH v4 3/4] xen/hvm: introduce a flags field in the CPU save record Roger Pau Monne
2015-11-25 15:18 ` [PATCH v4 4/4] Revert "libxc: create an initial FPU state for HVM guests" Roger Pau Monne
3 siblings, 0 replies; 8+ messages in thread
From: Roger Pau Monne @ 2015-11-25 15:18 UTC (permalink / raw)
To: xen-devel
Cc: Tim Deegan, Ian Jackson, Ian Campbell, Jan Beulich,
Roger Pau Monne
With the current compat implementation in the save/restore context handling,
only one compat structure is allowed, and using _zeroextend prevents the
fixup function from being called.
In order to allow for the compat handling layer to be able to handle
different compat versions allow calling the fixup function with
hvm_load_entry_zeroextend.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>
---
Changes since v3:
- Split the if condition in order to avoid changing the '\' delimiters.
- Add Andrew Cooper Reviewed by.
---
xen/include/xen/hvm/save.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/xen/include/xen/hvm/save.h b/xen/include/xen/hvm/save.h
index 51bc7d5..a9a78f9 100644
--- a/xen/include/xen/hvm/save.h
+++ b/xen/include/xen/hvm/save.h
@@ -64,7 +64,12 @@ void _hvm_read_entry(struct hvm_domain_context *h,
= (struct hvm_save_descriptor *)&(_h)->data[(_h)->cur]; \
if ( (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), \
HVM_SAVE_LENGTH(_x), (_strict))) == 0 ) \
+ { \
_hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x)); \
+ if ( HVM_SAVE_HAS_COMPAT(_x) && \
+ desc->length != HVM_SAVE_LENGTH(_x) ) \
+ r = HVM_SAVE_FIX_COMPAT(_x, (_dst), desc->length); \
+ } \
else if (HVM_SAVE_HAS_COMPAT(_x) \
&& (r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), \
HVM_SAVE_LENGTH_COMPAT(_x), (_strict))) == 0 ) { \
--
1.9.5 (Apple Git-50.3)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v4 3/4] xen/hvm: introduce a flags field in the CPU save record
2015-11-25 15:18 [PATCH v4 0/4] Introduce a flags field to HVM CPU context Roger Pau Monne
2015-11-25 15:18 ` [PATCH v4 1/4] xen/save: pass a size parameter to the HVM compat functions Roger Pau Monne
2015-11-25 15:18 ` [PATCH v4 2/4] xen/save: allow the usage of zeroextend and a fixup function Roger Pau Monne
@ 2015-11-25 15:18 ` Roger Pau Monne
2015-11-26 14:32 ` Jan Beulich
2015-11-25 15:18 ` [PATCH v4 4/4] Revert "libxc: create an initial FPU state for HVM guests" Roger Pau Monne
3 siblings, 1 reply; 8+ messages in thread
From: Roger Pau Monne @ 2015-11-25 15:18 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monne
Introduce a new flags field and use bit 0 to signal if the FPU has been
initialised or not. Previously Xen always wrongly assumed the FPU was
initialised on restore.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v3:
- Don't add a comment in the compat structure regaring the fpu_initialised
field.
- Rename fpu_initialised to flags and use it as a bit field. Bit 0 will be
used to signal whether the fpu is initialised.
- Only save the fpu context if it's initialised.
- Only restore the fpu context from the save record if the fpu is
initialised.
- Check that unused bits in the flags field are 0.
Changes since v1:
- Don't add yet another compat structure, new fields should always be added
to the end of the existing structure and offsetof should be used to
compare sizes.
- Leave the previous compat structure as-is, since the field was not added
to the end we cannot remove it and use offsetof in this case.
- Set xstate_bv based on fpu_initialised value instead of unconditionally
setting it to XSTATE_FP_SSE.
---
xen/arch/x86/hvm/hvm.c | 20 +++++++++++++-------
xen/include/public/arch-x86/hvm/save.h | 27 ++++++++++++++++++++-------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 141a130..d966074 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1798,8 +1798,7 @@ static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
if ( v->fpu_initialised )
memcpy(ctxt.fpu_regs, v->arch.fpu_ctxt, sizeof(ctxt.fpu_regs));
- else
- memset(ctxt.fpu_regs, 0, sizeof(ctxt.fpu_regs));
+ ctxt.flags = v->fpu_initialised ? XEN_X86_FPU_INITIALISED : 0;
ctxt.rax = v->arch.user_regs.eax;
ctxt.rbx = v->arch.user_regs.ebx;
@@ -1979,7 +1978,7 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
return -EINVAL;
}
- if ( hvm_load_entry(CPU, h, &ctxt) != 0 )
+ if ( hvm_load_entry_zeroextend(CPU, h, &ctxt) != 0 )
return -EINVAL;
/* Sanity check some control registers. */
@@ -2007,6 +2006,13 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
return -EINVAL;
}
+ if ( (ctxt.flags & ~XEN_X86_FPU_INITIALISED) != 0 )
+ {
+ gprintk(XENLOG_ERR, "bad flags value in CPU context: %#x\n",
+ ctxt.flags);
+ return -EINVAL;
+ }
+
/* Older Xen versions used to save the segment arbytes directly
* from the VMCS on Intel hosts. Detect this and rearrange them
* into the struct segment_register format. */
@@ -2085,16 +2091,17 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
seg.attr.bytes = ctxt.ldtr_arbytes;
hvm_set_segment_register(v, x86_seg_ldtr, &seg);
+ v->fpu_initialised = !!(ctxt.flags & XEN_X86_FPU_INITIALISED);
/* In case xsave-absent save file is restored on a xsave-capable host */
- if ( cpu_has_xsave && !xsave_enabled(v) )
+ if ( cpu_has_xsave && !xsave_enabled(v) && v->fpu_initialised )
{
struct xsave_struct *xsave_area = v->arch.xsave_area;
memcpy(v->arch.xsave_area, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE;
}
- else
- memcpy(v->arch.fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
+ else if ( v->fpu_initialised )
+ memcpy(v->arch.fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
v->arch.user_regs.eax = ctxt.rax;
v->arch.user_regs.ebx = ctxt.rbx;
@@ -2122,7 +2129,6 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
v->arch.debugreg[7] = ctxt.dr7;
v->arch.vgc_flags = VGCF_online;
- v->fpu_initialised = 1;
/* Auxiliary processors should be woken immediately. */
v->is_initialised = 1;
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 29d513c..b6b1bf8 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -47,7 +47,9 @@ DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
/*
* Processor
*
- * Compat: Pre-3.4 didn't have msr_tsc_aux
+ * Compat:
+ * - Pre-3.4 didn't have msr_tsc_aux
+ * - Pre-4.7 didn't have fpu_initialised
*/
struct hvm_hw_cpu {
@@ -157,6 +159,10 @@ struct hvm_hw_cpu {
};
/* error code for pending event */
uint32_t error_code;
+
+#define _XEN_X86_FPU_INITIALISED 0
+#define XEN_X86_FPU_INITIALISED (1U<<_XEN_X86_FPU_INITIALISED)
+ uint32_t flags;
};
struct hvm_hw_cpu_compat {
@@ -275,12 +281,19 @@ static inline int _hvm_hw_fix_cpu(void *h, uint32_t size) {
struct hvm_hw_cpu_compat cmp;
} *ucpu = (union hvm_hw_cpu_union *)h;
- /* If we copy from the end backwards, we should
- * be able to do the modification in-place */
- ucpu->nat.error_code = ucpu->cmp.error_code;
- ucpu->nat.pending_event = ucpu->cmp.pending_event;
- ucpu->nat.tsc = ucpu->cmp.tsc;
- ucpu->nat.msr_tsc_aux = 0;
+ if ( size == sizeof(struct hvm_hw_cpu_compat) )
+ {
+ /*
+ * If we copy from the end backwards, we should
+ * be able to do the modification in-place.
+ */
+ ucpu->nat.error_code = ucpu->cmp.error_code;
+ ucpu->nat.pending_event = ucpu->cmp.pending_event;
+ ucpu->nat.tsc = ucpu->cmp.tsc;
+ ucpu->nat.msr_tsc_aux = 0;
+ }
+ /* Mimic the old behaviour by unconditionally setting fpu_initialised. */
+ ucpu->nat.flags = XEN_X86_FPU_INITIALISED;
return 0;
}
--
1.9.5 (Apple Git-50.3)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v4 3/4] xen/hvm: introduce a flags field in the CPU save record
2015-11-25 15:18 ` [PATCH v4 3/4] xen/hvm: introduce a flags field in the CPU save record Roger Pau Monne
@ 2015-11-26 14:32 ` Jan Beulich
2015-11-27 16:15 ` Roger Pau Monné
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2015-11-26 14:32 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: Andrew Cooper, xen-devel
>>> On 25.11.15 at 16:18, <roger.pau@citrix.com> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1798,8 +1798,7 @@ static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
>
> if ( v->fpu_initialised )
> memcpy(ctxt.fpu_regs, v->arch.fpu_ctxt, sizeof(ctxt.fpu_regs));
> - else
> - memset(ctxt.fpu_regs, 0, sizeof(ctxt.fpu_regs));
> + ctxt.flags = v->fpu_initialised ? XEN_X86_FPU_INITIALISED : 0;
By dropping the memset() you'll leak hypervisor stack contents to
the tool stack / into the save file. Also I think two conditionals
using the same expression would better be combined.
> @@ -2085,16 +2091,17 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> seg.attr.bytes = ctxt.ldtr_arbytes;
> hvm_set_segment_register(v, x86_seg_ldtr, &seg);
>
> + v->fpu_initialised = !!(ctxt.flags & XEN_X86_FPU_INITIALISED);
> /* In case xsave-absent save file is restored on a xsave-capable host */
> - if ( cpu_has_xsave && !xsave_enabled(v) )
> + if ( cpu_has_xsave && !xsave_enabled(v) && v->fpu_initialised )
Hmm, didn't I pretty explicitly ask for this to become
if ( !v->fpu_initialised )
memset();
else if ( ... ) ...
else ...
> {
> struct xsave_struct *xsave_area = v->arch.xsave_area;
>
> memcpy(v->arch.xsave_area, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
> xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE;
> }
> - else
> - memcpy(v->arch.fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
> + else if ( v->fpu_initialised )
> + memcpy(v->arch.fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
And in no case should you break indentation here.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v4 3/4] xen/hvm: introduce a flags field in the CPU save record
2015-11-26 14:32 ` Jan Beulich
@ 2015-11-27 16:15 ` Roger Pau Monné
2015-11-30 10:08 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: Roger Pau Monné @ 2015-11-27 16:15 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
El 26/11/15 a les 15.32, Jan Beulich ha escrit:
>>>> On 25.11.15 at 16:18, <roger.pau@citrix.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -2085,16 +2091,17 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
>> seg.attr.bytes = ctxt.ldtr_arbytes;
>> hvm_set_segment_register(v, x86_seg_ldtr, &seg);
>>
>> + v->fpu_initialised = !!(ctxt.flags & XEN_X86_FPU_INITIALISED);
>> /* In case xsave-absent save file is restored on a xsave-capable host */
>> - if ( cpu_has_xsave && !xsave_enabled(v) )
>> + if ( cpu_has_xsave && !xsave_enabled(v) && v->fpu_initialised )
>
> Hmm, didn't I pretty explicitly ask for this to become
>
> if ( !v->fpu_initialised )
> memset();
I don't think this is possible with the current code.
Sadly the XSTATE stuff is kind of messy IMHO. vcpu_init_fpu calls
xstate_alloc_save_area which on a XSAVE capable CPUs allocates _and_
initializes the FPU registers, while on non-XSAVE capable CPUs
vcpu_init_fpu just allocates the FPU memory, but doesn't initialize the
registers.
So either xstate_alloc_save_area also sets v->fpu_initialised = 1 (this
is the simplest solution), or xstate_alloc_save_area is reworked so it
only allocates the XSAVE area, but doesn't initialize it. Then XSAVE
area initialization should be done in vcpu_restore_fpu_lazy.
Roger.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 3/4] xen/hvm: introduce a flags field in the CPU save record
2015-11-27 16:15 ` Roger Pau Monné
@ 2015-11-30 10:08 ` Jan Beulich
0 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2015-11-30 10:08 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Andrew Cooper, xen-devel
>>> On 27.11.15 at 17:15, <roger.pau@citrix.com> wrote:
> El 26/11/15 a les 15.32, Jan Beulich ha escrit:
>>>>> On 25.11.15 at 16:18, <roger.pau@citrix.com> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2085,16 +2091,17 @@ static int hvm_load_cpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
>>> seg.attr.bytes = ctxt.ldtr_arbytes;
>>> hvm_set_segment_register(v, x86_seg_ldtr, &seg);
>>>
>>> + v->fpu_initialised = !!(ctxt.flags & XEN_X86_FPU_INITIALISED);
>>> /* In case xsave-absent save file is restored on a xsave-capable host */
>>> - if ( cpu_has_xsave && !xsave_enabled(v) )
>>> + if ( cpu_has_xsave && !xsave_enabled(v) && v->fpu_initialised )
>>
>> Hmm, didn't I pretty explicitly ask for this to become
>>
>> if ( !v->fpu_initialised )
>> memset();
>
> I don't think this is possible with the current code.
>
> Sadly the XSTATE stuff is kind of messy IMHO.
Agreed.
> vcpu_init_fpu calls
> xstate_alloc_save_area which on a XSAVE capable CPUs allocates _and_
> initializes the FPU registers, while on non-XSAVE capable CPUs
> vcpu_init_fpu just allocates the FPU memory, but doesn't initialize the
> registers.
Well, I can't see registers being initialized. All I can see is MXCSR
and FCW getting values set in the memory image, which I suppose
is what you meant. But that can be easily mirrored here (which
then would also get closer to what hvm_vcpu_reset_state() does).
> So either xstate_alloc_save_area also sets v->fpu_initialised = 1 (this
> is the simplest solution), or xstate_alloc_save_area is reworked so it
> only allocates the XSAVE area, but doesn't initialize it. Then XSAVE
> area initialization should be done in vcpu_restore_fpu_lazy.
Re-working along those lines certainly is also an option.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 4/4] Revert "libxc: create an initial FPU state for HVM guests"
2015-11-25 15:18 [PATCH v4 0/4] Introduce a flags field to HVM CPU context Roger Pau Monne
` (2 preceding siblings ...)
2015-11-25 15:18 ` [PATCH v4 3/4] xen/hvm: introduce a flags field in the CPU save record Roger Pau Monne
@ 2015-11-25 15:18 ` Roger Pau Monne
3 siblings, 0 replies; 8+ messages in thread
From: Roger Pau Monne @ 2015-11-25 15:18 UTC (permalink / raw)
To: xen-devel
Cc: Wei Liu, Ian Campbell, Stefano Stabellini, Andrew Cooper,
Ian Jackson, Jan Beulich, Roger Pau Monne
This reverts commit d64dbbcc7c9934a46126c59d78536235908377ad:
Xen always set the FPU as initialized when loading a HVM context, so libxc
has to provide a valid FPU context when setting the CPU registers.
This was a stop-gap measure in order to unblock OSSTest Windows 7 failures
while a proper fix for the HVM CPU save/restore is being worked on.
This can now be reverted because a proper fix is in place and we can signal
in the save record whether the FPU is initialized or not.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
tools/libxc/xc_dom_x86.c | 38 --------------------------------------
1 file changed, 38 deletions(-)
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 5ff33ca..50cceee 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -910,27 +910,6 @@ static int vcpu_hvm(struct xc_dom_image *dom)
struct hvm_save_descriptor end_d;
HVM_SAVE_TYPE(END) end;
} bsp_ctx;
- /*
- * The layout of the fpu context structure is the same for
- * both 32 and 64 bits.
- */
- struct {
- uint16_t fcw;
- uint16_t fsw;
- uint8_t ftw;
- uint8_t rsvd1;
- uint16_t fop;
- union {
- uint64_t addr;
- struct {
- uint32_t offs;
- uint16_t sel;
- uint16_t rsvd;
- };
- } fip, fdp;
- uint32_t mxcsr;
- uint32_t mxcsr_mask;
- } *fpu_ctxt;
uint8_t *full_ctx = NULL;
int rc;
@@ -998,23 +977,6 @@ static int vcpu_hvm(struct xc_dom_image *dom)
/* Set the control registers. */
bsp_ctx.cpu.cr0 = X86_CR0_PE | X86_CR0_ET;
- /*
- * XXX: Set initial FPU state.
- *
- * This should be removed once Xen is able to know if the
- * FPU state saved is valid or not, now Xen always sets
- * fpu_initialised to true regardless of the FPU state.
- *
- * The code below mimics the FPU sate after executing
- * fninit
- * ldmxcsr 0x1f80
- */
- fpu_ctxt = (typeof(fpu_ctxt))bsp_ctx.cpu.fpu_regs;
-
- fpu_ctxt->fcw = 0x37f;
- fpu_ctxt->ftw = 0xff;
- fpu_ctxt->mxcsr = 0x1f80;
-
/* Set the IP. */
bsp_ctx.cpu.rip = dom->parms.phys_entry;
--
1.9.5 (Apple Git-50.3)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread