Re: [Xen-devel] [PATCHv1 4/5] x86/viridian: set x87 FIP width to 4 for Windows guests

[Jan Beulich]

>>> On 18.02.16 at 19:52,  wrote:
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -148,6 +148,30 @@ static void dump_guest_os_id(const struct domain *d)
> goi->fields.service_pack, goi->fields.build_number);
>  }
>  
> +static void set_guest_os_id(struct domain *d, uint64_t val)
> +{
> +const union viridian_guest_os_id *goi;
> +
> +d->arch.hvm_domain.viridian.guest_os_id.raw = val;
> +goi = >arch.hvm_domain.viridian.guest_os_id;

Please flip the two lines and use goi also for the assignment
of val.

Jan


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

Re: [Xen-devel] [PATCHv1 4/5] x86/viridian: set x87 FIP width to 4 for Windows guests

[Andrew Cooper]

On 18/02/16 18:52, David Vrabel wrote:
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index 6bd844b..fb9f044 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -148,6 +148,30 @@ static void dump_guest_os_id(const struct domain *d)
> goi->fields.service_pack, goi->fields.build_number);
>  }
>  
> +static void set_guest_os_id(struct domain *d, uint64_t val)
> +{
> +const union viridian_guest_os_id *goi;
> +
> +d->arch.hvm_domain.viridian.guest_os_id.raw = val;
> +goi = >arch.hvm_domain.viridian.guest_os_id;
> +
> +/*
> + * Microsoft Windows only saves the lower 32-bits of FIP/FDP and
> + * can get upset if the selectors are not saved/restored by the
> + * hypervisor.
> + *
> + * Only do this if the FIP width is not in auto-mode, so this
> + * heuristic can be overriden by the toolstack.
> + */
> +if ( !d->arch.x87_fip_width )
> +{
> +if ( goi->fields.vendor == 1 && goi->fields.os == 4 )

Are there any named parameters we could use here?

In principle, Reviewed-by: Andrew Cooper 
(allowing for the potential change of x86_fip_width per the discussion
on patch 3).

> +d->arch.x87_fip_width = 4;
> +}
> +
> +dump_guest_os_id(d);
> +}
> +
>  static void dump_hypercall(const struct domain *d)
>  {
>  const union viridian_hypercall_gpa *hg;


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

[Xen-devel] [PATCHv1 4/5] x86/viridian: set x87 FIP width to 4 for Windows guests

[David Vrabel]

Microsoft Windows always uses a 32-bit FPU state save/restore and expects
the FCS/FDS to be saved/restored.  Ensure that for these guests, the
hypervisor does 32-bit save/restore to preserve FCS/FDS.

These guests are identified by the write to the Guest OS ID MSR.

This fixes an 0x3D BugCheck when running the Driver Verifier in 64-bit
Windows.  This BugCheck occurs because a context switch would clear
FCS/FDS and Driver Verifier would assert because the FPU state changed.

We only set FIP width if it is still in auto-mode, to allow the toolstack
to override if necessary.

Signed-off-by: David Vrabel 
---
 xen/arch/x86/hvm/viridian.c | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 6bd844b..fb9f044 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -148,6 +148,30 @@ static void dump_guest_os_id(const struct domain *d)
goi->fields.service_pack, goi->fields.build_number);
 }
 
+static void set_guest_os_id(struct domain *d, uint64_t val)
+{
+const union viridian_guest_os_id *goi;
+
+d->arch.hvm_domain.viridian.guest_os_id.raw = val;
+goi = >arch.hvm_domain.viridian.guest_os_id;
+
+/*
+ * Microsoft Windows only saves the lower 32-bits of FIP/FDP and
+ * can get upset if the selectors are not saved/restored by the
+ * hypervisor.
+ *
+ * Only do this if the FIP width is not in auto-mode, so this
+ * heuristic can be overriden by the toolstack.
+ */
+if ( !d->arch.x87_fip_width )
+{
+if ( goi->fields.vendor == 1 && goi->fields.os == 4 )
+d->arch.x87_fip_width = 4;
+}
+
+dump_guest_os_id(d);
+}
+
 static void dump_hypercall(const struct domain *d)
 {
 const union viridian_hypercall_gpa *hg;
@@ -334,8 +358,7 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t val)
 {
 case VIRIDIAN_MSR_GUEST_OS_ID:
 perfc_incr(mshv_wrmsr_osid);
-d->arch.hvm_domain.viridian.guest_os_id.raw = val;
-dump_guest_os_id(d);
+set_guest_os_id(d, val);
 break;
 
 case VIRIDIAN_MSR_HYPERCALL:
-- 
2.1.4


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