[Xen-devel] [PATCH 0/3] More viridian code cleanup

[Xen-devel] [PATCH 0/3] More viridian code cleanup

[Wei Liu]

This series contains an updated patch from last batch and two new
patches.

Wei Liu (3):
  x86/viridian: drop a wrong invalid value from reference TSC
    implementation
  x86/viridian: drop virdian_sint_msr
  x86/viridian: drop viridian_stimer_config_msr

 xen/arch/x86/hvm/viridian/synic.c       | 20 +++++------
 xen/arch/x86/hvm/viridian/time.c        | 44 ++++++++++---------------
 xen/include/asm-x86/guest/hyperv-tlfs.h |  3 +-
 xen/include/asm-x86/hvm/viridian.h      | 35 ++------------------
 4 files changed, 32 insertions(+), 70 deletions(-)

-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/3] x86/viridian: drop a wrong invalid value from reference TSC implementation

[Wei Liu]

The only invalid value mentioned in Hyper-V TLFS 5.0c is 0. Michael
Kelley confirmed that 0xFFFFFFFF was never used [0].

[0] https://lists.xen.org/archives/html/xen-devel/2019-12/msg01564.html

Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 xen/arch/x86/hvm/viridian/time.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
index 6b2d745f3a..0f1cd9e208 100644
--- a/xen/arch/x86/hvm/viridian/time.c
+++ b/xen/arch/x86/hvm/viridian/time.c
@@ -45,14 +45,9 @@ static void update_reference_tsc(const struct domain *d, bool initialize)
     if ( !host_tsc_is_safe() || d->arch.vtsc )
     {
         /*
-         * The specification states that valid values of TscSequence range
-         * from 0 to 0xFFFFFFFE. The value 0xFFFFFFFF is used to indicate
-         * this mechanism is no longer a reliable source of time and that
-         * the VM should fall back to a different source.
-         *
-         * Server 2012 (6.2 kernel) and 2012 R2 (6.3 kernel) actually
-         * violate the spec. and rely on a value of 0 to indicate that this
-         * enlightenment should no longer be used.
+         * The value 0 is used to indicate this mechanism is no longer a
+         * reliable source of time and that the VM should fall back to a
+         * different source.
          */
         p->tsc_sequence = 0;
 
@@ -77,10 +72,7 @@ static void update_reference_tsc(const struct domain *d, bool initialize)
     smp_wmb();
 
     seq = p->tsc_sequence + 1;
-    if ( seq == 0xFFFFFFFF || seq == 0 ) /* Avoid both 'invalid' values */
-        seq = 1;
-
-    p->tsc_sequence = seq;
+    p->tsc_sequence = seq ? seq : 1; /* Avoid 'invalid' value 0 */
 }
 
 static uint64_t trc_val(const struct domain *d, int64_t offset)
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 2/3] x86/viridian: drop virdian_sint_msr

[Wei Liu]

Use hv_synic_sint in hyperv-tlfs.h instead.

This requires adding the missing "polling" member to hv_synic_sint.

No functional change.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 xen/arch/x86/hvm/viridian/synic.c       | 20 ++++++++++----------
 xen/include/asm-x86/guest/hyperv-tlfs.h |  3 ++-
 xen/include/asm-x86/hvm/viridian.h      | 16 +---------------
 3 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/synic.c b/xen/arch/x86/hvm/viridian/synic.c
index 54c62f843f..94a2b88733 100644
--- a/xen/arch/x86/hvm/viridian/synic.c
+++ b/xen/arch/x86/hvm/viridian/synic.c
@@ -143,7 +143,7 @@ int viridian_synic_wrmsr(struct vcpu *v, uint32_t idx, uint64_t val)
     case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
     {
         unsigned int sintx = idx - HV_X64_MSR_SINT0;
-        union viridian_sint_msr new, *vs =
+        union hv_synic_sint new, *vs =
             &array_access_nospec(vv->sint, sintx);
         uint8_t vector;
 
@@ -151,7 +151,7 @@ int viridian_synic_wrmsr(struct vcpu *v, uint32_t idx, uint64_t val)
             return X86EMUL_EXCEPTION;
 
         /* Vectors must be in the range 0x10-0xff inclusive */
-        new.raw = val;
+        new.as_uint64 = val;
         if ( new.vector < 0x10 )
             return X86EMUL_EXCEPTION;
 
@@ -256,13 +256,13 @@ int viridian_synic_rdmsr(const struct vcpu *v, uint32_t idx, uint64_t *val)
     case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
     {
         unsigned int sintx = idx - HV_X64_MSR_SINT0;
-        const union viridian_sint_msr *vs =
+        const union hv_synic_sint *vs =
             &array_access_nospec(vv->sint, sintx);
 
         if ( !(viridian_feature_mask(d) & HVMPV_synic) )
             return X86EMUL_EXCEPTION;
 
-        *val = vs->raw;
+        *val = vs->as_uint64;
         break;
     }
 
@@ -284,7 +284,7 @@ int viridian_synic_vcpu_init(const struct vcpu *v)
      * initally masked.
      */
     for ( i = 0; i < ARRAY_SIZE(vv->sint); i++ )
-        vv->sint[i].mask = 1;
+        vv->sint[i].masked = 1;
 
     /* Initialize the mapping array with invalid values */
     for ( i = 0; i < ARRAY_SIZE(vv->vector_to_sintx); i++ )
@@ -321,7 +321,7 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
                                       uint64_t delivery)
 {
     struct viridian_vcpu *vv = v->arch.hvm.viridian;
-    const union viridian_sint_msr *vs = &vv->sint[sintx];
+    const union hv_synic_sint *vs = &vv->sint[sintx];
     struct hv_message *msg = vv->simp.ptr;
     struct {
         uint32_t TimerIndex;
@@ -360,7 +360,7 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
     BUILD_BUG_ON(sizeof(payload) > sizeof(msg->u.payload));
     memcpy(msg->u.payload, &payload, sizeof(payload));
 
-    if ( !vs->mask )
+    if ( !vs->masked )
         vlapic_set_irq(vcpu_vlapic(v), vs->vector, 0);
 
     return true;
@@ -371,7 +371,7 @@ bool viridian_synic_is_auto_eoi_sint(const struct vcpu *v,
 {
     const struct viridian_vcpu *vv = v->arch.hvm.viridian;
     unsigned int sintx = vv->vector_to_sintx[vector];
-    const union viridian_sint_msr *vs =
+    const union hv_synic_sint *vs =
         &array_access_nospec(vv->sint, sintx);
 
     if ( sintx >= ARRAY_SIZE(vv->sint) )
@@ -401,7 +401,7 @@ void viridian_synic_save_vcpu_ctxt(const struct vcpu *v,
     BUILD_BUG_ON(ARRAY_SIZE(vv->sint) != ARRAY_SIZE(ctxt->sint_msr));
 
     for ( i = 0; i < ARRAY_SIZE(vv->sint); i++ )
-        ctxt->sint_msr[i] = vv->sint[i].raw;
+        ctxt->sint_msr[i] = vv->sint[i].as_uint64;
 
     ctxt->simp_msr = vv->simp.msr.raw;
 
@@ -430,7 +430,7 @@ void viridian_synic_load_vcpu_ctxt(
     {
         uint8_t vector;
 
-        vv->sint[i].raw = ctxt->sint_msr[i];
+        vv->sint[i].as_uint64 = ctxt->sint_msr[i];
 
         vector = vv->sint[i].vector;
         if ( vector < 0x10 )
diff --git a/xen/include/asm-x86/guest/hyperv-tlfs.h b/xen/include/asm-x86/guest/hyperv-tlfs.h
index 4402854c80..fe9fb232d0 100644
--- a/xen/include/asm-x86/guest/hyperv-tlfs.h
+++ b/xen/include/asm-x86/guest/hyperv-tlfs.h
@@ -819,7 +819,8 @@ union hv_synic_sint {
 		u64 reserved1:8;
 		u64 masked:1;
 		u64 auto_eoi:1;
-		u64 reserved2:46;
+		u64 polling:1;
+		u64 reserved2:45;
 	} __packed;
 };
 
diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-x86/hvm/viridian.h
index cfbaede158..d694d83521 100644
--- a/xen/include/asm-x86/hvm/viridian.h
+++ b/xen/include/asm-x86/hvm/viridian.h
@@ -28,20 +28,6 @@ struct viridian_page
     void *ptr;
 };
 
-union viridian_sint_msr
-{
-    uint64_t raw;
-    struct
-    {
-        uint64_t vector:8;
-        uint64_t reserved_preserved1:8;
-        uint64_t mask:1;
-        uint64_t auto_eoi:1;
-        uint64_t polling:1;
-        uint64_t reserved_preserved2:45;
-    };
-};
-
 union viridian_stimer_config_msr
 {
     uint64_t raw;
@@ -77,7 +63,7 @@ struct viridian_vcpu
     uint64_t scontrol;
     uint64_t siefp;
     struct viridian_page simp;
-    union viridian_sint_msr sint[16];
+    union hv_synic_sint sint[16];
     uint8_t vector_to_sintx[256];
     struct viridian_stimer stimer[4];
     unsigned int stimer_enabled;
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 3/3] x86/viridian: drop viridian_stimer_config_msr

[Wei Liu]

Use hv_stimer_config instead. No functional change.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 xen/arch/x86/hvm/viridian/time.c   | 28 ++++++++++++++--------------
 xen/include/asm-x86/hvm/viridian.h | 19 +------------------
 2 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
index 0f1cd9e208..3de5665c02 100644
--- a/xen/arch/x86/hvm/viridian/time.c
+++ b/xen/arch/x86/hvm/viridian/time.c
@@ -220,7 +220,7 @@ static void poll_stimer(struct vcpu *v, unsigned int stimerx)
      * is disabled make sure the pending bit is cleared to avoid re-
      * polling.
      */
-    if ( !vs->config.enabled )
+    if ( !vs->config.enable )
     {
         clear_bit(stimerx, &vv->stimer_pending);
         return;
@@ -239,7 +239,7 @@ static void poll_stimer(struct vcpu *v, unsigned int stimerx)
     if ( vs->config.periodic )
         start_stimer(vs);
     else
-        vs->config.enabled = 0;
+        vs->config.enable = 0;
 }
 
 void viridian_time_poll_timers(struct vcpu *v)
@@ -285,7 +285,7 @@ static void time_vcpu_thaw(struct vcpu *v)
     {
         struct viridian_stimer *vs = &vv->stimer[i];
 
-        if ( vs->config.enabled )
+        if ( vs->config.enable )
             start_stimer(vs);
     }
 }
@@ -355,12 +355,12 @@ int viridian_time_wrmsr(struct vcpu *v, uint32_t idx, uint64_t val)
 
         stop_stimer(vs);
 
-        vs->config.raw = val;
+        vs->config.as_uint64 = val;
 
         if ( !vs->config.sintx )
-            vs->config.enabled = 0;
+            vs->config.enable = 0;
 
-        if ( vs->config.enabled )
+        if ( vs->config.enable )
             start_stimer(vs);
 
         break;
@@ -383,11 +383,11 @@ int viridian_time_wrmsr(struct vcpu *v, uint32_t idx, uint64_t val)
         vs->count = val;
 
         if ( !vs->count  )
-            vs->config.enabled = 0;
+            vs->config.enable = 0;
         else if ( vs->config.auto_enable )
-            vs->config.enabled = 1;
+            vs->config.enable = 1;
 
-        if ( vs->config.enabled )
+        if ( vs->config.enable )
             start_stimer(vs);
 
         break;
@@ -454,7 +454,7 @@ int viridian_time_rdmsr(const struct vcpu *v, uint32_t idx, uint64_t *val)
         unsigned int stimerx = (idx - HV_X64_MSR_STIMER0_CONFIG) / 2;
         const struct viridian_stimer *vs =
             &array_access_nospec(vv->stimer, stimerx);
-        union viridian_stimer_config_msr config = vs->config;
+        union hv_stimer_config config = vs->config;
 
         if ( !(viridian_feature_mask(d) & HVMPV_stimer) )
             return X86EMUL_EXCEPTION;
@@ -464,9 +464,9 @@ int viridian_time_rdmsr(const struct vcpu *v, uint32_t idx, uint64_t *val)
          * the enabled flag is clear.
          */
         if ( !config.periodic && test_bit(stimerx, &vv->stimer_pending) )
-            config.enabled = 0;
+            config.enable = 0;
 
-        *val = config.raw;
+        *val = config.as_uint64;
         break;
     }
 
@@ -549,7 +549,7 @@ void viridian_time_save_vcpu_ctxt(
     {
         const struct viridian_stimer *vs = &vv->stimer[i];
 
-        ctxt->stimer_config_msr[i] = vs->config.raw;
+        ctxt->stimer_config_msr[i] = vs->config.as_uint64;
         ctxt->stimer_count_msr[i] = vs->count;
     }
 }
@@ -564,7 +564,7 @@ void viridian_time_load_vcpu_ctxt(
     {
         struct viridian_stimer *vs = &vv->stimer[i];
 
-        vs->config.raw = ctxt->stimer_config_msr[i];
+        vs->config.as_uint64 = ctxt->stimer_config_msr[i];
         vs->count = ctxt->stimer_count_msr[i];
     }
 }
diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-x86/hvm/viridian.h
index d694d83521..d9138562e6 100644
--- a/xen/include/asm-x86/hvm/viridian.h
+++ b/xen/include/asm-x86/hvm/viridian.h
@@ -28,27 +28,10 @@ struct viridian_page
     void *ptr;
 };
 
-union viridian_stimer_config_msr
-{
-    uint64_t raw;
-    struct
-    {
-        uint64_t enabled:1;
-        uint64_t periodic:1;
-        uint64_t lazy:1;
-        uint64_t auto_enable:1;
-        uint64_t vector:8;
-        uint64_t direct_mode:1;
-        uint64_t reserved_zero1:3;
-        uint64_t sintx:4;
-        uint64_t reserved_zero2:44;
-    };
-};
-
 struct viridian_stimer {
     struct vcpu *v;
     struct timer timer;
-    union viridian_stimer_config_msr config;
+    union hv_stimer_config config;
     uint64_t count;
     uint64_t expiration;
     bool started;
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/3] x86/viridian: drop a wrong invalid value from reference TSC implementation

[Durrant, Paul]

V2?

> -----Original Message-----
> From: Wei Liu <wei.liu.xen@gmail.com> On Behalf Of Wei Liu
> Sent: 22 December 2019 23:21
> To: Xen Development List <xen-devel@lists.xenproject.org>
> Cc: Michael Kelley <mikelley@microsoft.com>; Durrant, Paul
> <pdurrant@amazon.com>; Wei Liu <liuwe@microsoft.com>; Paul Durrant
> <paul@xen.org>; Jan Beulich <jbeulich@suse.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>
> Subject: [PATCH 1/3] x86/viridian: drop a wrong invalid value from
> reference TSC implementation
> 
> The only invalid value mentioned in Hyper-V TLFS 5.0c is 0. Michael
> Kelley confirmed that 0xFFFFFFFF was never used [0].
> 
> [0] https://lists.xen.org/archives/html/xen-devel/2019-12/msg01564.html
> 
> Signed-off-by: Wei Liu <liuwe@microsoft.com>

Reviewed-by: Paul Durrant <paul@xen.org>

> ---
>  xen/arch/x86/hvm/viridian/time.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian/time.c
> b/xen/arch/x86/hvm/viridian/time.c
> index 6b2d745f3a..0f1cd9e208 100644
> --- a/xen/arch/x86/hvm/viridian/time.c
> +++ b/xen/arch/x86/hvm/viridian/time.c
> @@ -45,14 +45,9 @@ static void update_reference_tsc(const struct domain
> *d, bool initialize)
>      if ( !host_tsc_is_safe() || d->arch.vtsc )
>      {
>          /*
> -         * The specification states that valid values of TscSequence
> range
> -         * from 0 to 0xFFFFFFFE. The value 0xFFFFFFFF is used to indicate
> -         * this mechanism is no longer a reliable source of time and that
> -         * the VM should fall back to a different source.
> -         *
> -         * Server 2012 (6.2 kernel) and 2012 R2 (6.3 kernel) actually
> -         * violate the spec. and rely on a value of 0 to indicate that
> this
> -         * enlightenment should no longer be used.
> +         * The value 0 is used to indicate this mechanism is no longer a
> +         * reliable source of time and that the VM should fall back to a
> +         * different source.
>           */
>          p->tsc_sequence = 0;
> 
> @@ -77,10 +72,7 @@ static void update_reference_tsc(const struct domain
> *d, bool initialize)
>      smp_wmb();
> 
>      seq = p->tsc_sequence + 1;
> -    if ( seq == 0xFFFFFFFF || seq == 0 ) /* Avoid both 'invalid' values
> */
> -        seq = 1;
> -
> -    p->tsc_sequence = seq;
> +    p->tsc_sequence = seq ? seq : 1; /* Avoid 'invalid' value 0 */
>  }
> 
>  static uint64_t trc_val(const struct domain *d, int64_t offset)
> --
> 2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/3] x86/viridian: drop virdian_sint_msr

[Durrant, Paul]

> -----Original Message-----
> From: Wei Liu <wei.liu.xen@gmail.com> On Behalf Of Wei Liu
> Sent: 22 December 2019 23:21
> To: Xen Development List <xen-devel@lists.xenproject.org>
> Cc: Michael Kelley <mikelley@microsoft.com>; Durrant, Paul
> <pdurrant@amazon.com>; Wei Liu <liuwe@microsoft.com>; Paul Durrant
> <paul@xen.org>; Jan Beulich <jbeulich@suse.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>
> Subject: [PATCH 2/3] x86/viridian: drop virdian_sint_msr
> 
> Use hv_synic_sint in hyperv-tlfs.h instead.
> 
> This requires adding the missing "polling" member to hv_synic_sint.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <liuwe@microsoft.com>

Reviewed-by: Paul Durrant <paul@xen.org>

> ---
>  xen/arch/x86/hvm/viridian/synic.c       | 20 ++++++++++----------
>  xen/include/asm-x86/guest/hyperv-tlfs.h |  3 ++-
>  xen/include/asm-x86/hvm/viridian.h      | 16 +---------------
>  3 files changed, 13 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian/synic.c
> b/xen/arch/x86/hvm/viridian/synic.c
> index 54c62f843f..94a2b88733 100644
> --- a/xen/arch/x86/hvm/viridian/synic.c
> +++ b/xen/arch/x86/hvm/viridian/synic.c
> @@ -143,7 +143,7 @@ int viridian_synic_wrmsr(struct vcpu *v, uint32_t idx,
> uint64_t val)
>      case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
>      {
>          unsigned int sintx = idx - HV_X64_MSR_SINT0;
> -        union viridian_sint_msr new, *vs =
> +        union hv_synic_sint new, *vs =
>              &array_access_nospec(vv->sint, sintx);
>          uint8_t vector;
> 
> @@ -151,7 +151,7 @@ int viridian_synic_wrmsr(struct vcpu *v, uint32_t idx,
> uint64_t val)
>              return X86EMUL_EXCEPTION;
> 
>          /* Vectors must be in the range 0x10-0xff inclusive */
> -        new.raw = val;
> +        new.as_uint64 = val;
>          if ( new.vector < 0x10 )
>              return X86EMUL_EXCEPTION;
> 
> @@ -256,13 +256,13 @@ int viridian_synic_rdmsr(const struct vcpu *v,
> uint32_t idx, uint64_t *val)
>      case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
>      {
>          unsigned int sintx = idx - HV_X64_MSR_SINT0;
> -        const union viridian_sint_msr *vs =
> +        const union hv_synic_sint *vs =
>              &array_access_nospec(vv->sint, sintx);
> 
>          if ( !(viridian_feature_mask(d) & HVMPV_synic) )
>              return X86EMUL_EXCEPTION;
> 
> -        *val = vs->raw;
> +        *val = vs->as_uint64;
>          break;
>      }
> 
> @@ -284,7 +284,7 @@ int viridian_synic_vcpu_init(const struct vcpu *v)
>       * initally masked.
>       */
>      for ( i = 0; i < ARRAY_SIZE(vv->sint); i++ )
> -        vv->sint[i].mask = 1;
> +        vv->sint[i].masked = 1;
> 
>      /* Initialize the mapping array with invalid values */
>      for ( i = 0; i < ARRAY_SIZE(vv->vector_to_sintx); i++ )
> @@ -321,7 +321,7 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v,
> unsigned int sintx,
>                                        uint64_t delivery)
>  {
>      struct viridian_vcpu *vv = v->arch.hvm.viridian;
> -    const union viridian_sint_msr *vs = &vv->sint[sintx];
> +    const union hv_synic_sint *vs = &vv->sint[sintx];
>      struct hv_message *msg = vv->simp.ptr;
>      struct {
>          uint32_t TimerIndex;
> @@ -360,7 +360,7 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v,
> unsigned int sintx,
>      BUILD_BUG_ON(sizeof(payload) > sizeof(msg->u.payload));
>      memcpy(msg->u.payload, &payload, sizeof(payload));
> 
> -    if ( !vs->mask )
> +    if ( !vs->masked )
>          vlapic_set_irq(vcpu_vlapic(v), vs->vector, 0);
> 
>      return true;
> @@ -371,7 +371,7 @@ bool viridian_synic_is_auto_eoi_sint(const struct vcpu
> *v,
>  {
>      const struct viridian_vcpu *vv = v->arch.hvm.viridian;
>      unsigned int sintx = vv->vector_to_sintx[vector];
> -    const union viridian_sint_msr *vs =
> +    const union hv_synic_sint *vs =
>          &array_access_nospec(vv->sint, sintx);
> 
>      if ( sintx >= ARRAY_SIZE(vv->sint) )
> @@ -401,7 +401,7 @@ void viridian_synic_save_vcpu_ctxt(const struct vcpu
> *v,
>      BUILD_BUG_ON(ARRAY_SIZE(vv->sint) != ARRAY_SIZE(ctxt->sint_msr));
> 
>      for ( i = 0; i < ARRAY_SIZE(vv->sint); i++ )
> -        ctxt->sint_msr[i] = vv->sint[i].raw;
> +        ctxt->sint_msr[i] = vv->sint[i].as_uint64;
> 
>      ctxt->simp_msr = vv->simp.msr.raw;
> 
> @@ -430,7 +430,7 @@ void viridian_synic_load_vcpu_ctxt(
>      {
>          uint8_t vector;
> 
> -        vv->sint[i].raw = ctxt->sint_msr[i];
> +        vv->sint[i].as_uint64 = ctxt->sint_msr[i];
> 
>          vector = vv->sint[i].vector;
>          if ( vector < 0x10 )
> diff --git a/xen/include/asm-x86/guest/hyperv-tlfs.h b/xen/include/asm-
> x86/guest/hyperv-tlfs.h
> index 4402854c80..fe9fb232d0 100644
> --- a/xen/include/asm-x86/guest/hyperv-tlfs.h
> +++ b/xen/include/asm-x86/guest/hyperv-tlfs.h
> @@ -819,7 +819,8 @@ union hv_synic_sint {
>  		u64 reserved1:8;
>  		u64 masked:1;
>  		u64 auto_eoi:1;
> -		u64 reserved2:46;
> +		u64 polling:1;
> +		u64 reserved2:45;
>  	} __packed;
>  };
> 
> diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-
> x86/hvm/viridian.h
> index cfbaede158..d694d83521 100644
> --- a/xen/include/asm-x86/hvm/viridian.h
> +++ b/xen/include/asm-x86/hvm/viridian.h
> @@ -28,20 +28,6 @@ struct viridian_page
>      void *ptr;
>  };
> 
> -union viridian_sint_msr
> -{
> -    uint64_t raw;
> -    struct
> -    {
> -        uint64_t vector:8;
> -        uint64_t reserved_preserved1:8;
> -        uint64_t mask:1;
> -        uint64_t auto_eoi:1;
> -        uint64_t polling:1;
> -        uint64_t reserved_preserved2:45;
> -    };
> -};
> -
>  union viridian_stimer_config_msr
>  {
>      uint64_t raw;
> @@ -77,7 +63,7 @@ struct viridian_vcpu
>      uint64_t scontrol;
>      uint64_t siefp;
>      struct viridian_page simp;
> -    union viridian_sint_msr sint[16];
> +    union hv_synic_sint sint[16];
>      uint8_t vector_to_sintx[256];
>      struct viridian_stimer stimer[4];
>      unsigned int stimer_enabled;
> --
> 2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 3/3] x86/viridian: drop viridian_stimer_config_msr

[Durrant, Paul]

> -----Original Message-----
> From: Wei Liu <wei.liu.xen@gmail.com> On Behalf Of Wei Liu
> Sent: 22 December 2019 23:21
> To: Xen Development List <xen-devel@lists.xenproject.org>
> Cc: Michael Kelley <mikelley@microsoft.com>; Durrant, Paul
> <pdurrant@amazon.com>; Wei Liu <liuwe@microsoft.com>; Paul Durrant
> <paul@xen.org>; Jan Beulich <jbeulich@suse.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>
> Subject: [PATCH 3/3] x86/viridian: drop viridian_stimer_config_msr
> 
> Use hv_stimer_config instead. No functional change.
> 
> Signed-off-by: Wei Liu <liuwe@microsoft.com>

Reviewed-by: Paul Durrant <paul@xen.org>

> ---
>  xen/arch/x86/hvm/viridian/time.c   | 28 ++++++++++++++--------------
>  xen/include/asm-x86/hvm/viridian.h | 19 +------------------
>  2 files changed, 15 insertions(+), 32 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian/time.c
> b/xen/arch/x86/hvm/viridian/time.c
> index 0f1cd9e208..3de5665c02 100644
> --- a/xen/arch/x86/hvm/viridian/time.c
> +++ b/xen/arch/x86/hvm/viridian/time.c
> @@ -220,7 +220,7 @@ static void poll_stimer(struct vcpu *v, unsigned int
> stimerx)
>       * is disabled make sure the pending bit is cleared to avoid re-
>       * polling.
>       */
> -    if ( !vs->config.enabled )
> +    if ( !vs->config.enable )
>      {
>          clear_bit(stimerx, &vv->stimer_pending);
>          return;
> @@ -239,7 +239,7 @@ static void poll_stimer(struct vcpu *v, unsigned int
> stimerx)
>      if ( vs->config.periodic )
>          start_stimer(vs);
>      else
> -        vs->config.enabled = 0;
> +        vs->config.enable = 0;
>  }
> 
>  void viridian_time_poll_timers(struct vcpu *v)
> @@ -285,7 +285,7 @@ static void time_vcpu_thaw(struct vcpu *v)
>      {
>          struct viridian_stimer *vs = &vv->stimer[i];
> 
> -        if ( vs->config.enabled )
> +        if ( vs->config.enable )
>              start_stimer(vs);
>      }
>  }
> @@ -355,12 +355,12 @@ int viridian_time_wrmsr(struct vcpu *v, uint32_t
> idx, uint64_t val)
> 
>          stop_stimer(vs);
> 
> -        vs->config.raw = val;
> +        vs->config.as_uint64 = val;
> 
>          if ( !vs->config.sintx )
> -            vs->config.enabled = 0;
> +            vs->config.enable = 0;
> 
> -        if ( vs->config.enabled )
> +        if ( vs->config.enable )
>              start_stimer(vs);
> 
>          break;
> @@ -383,11 +383,11 @@ int viridian_time_wrmsr(struct vcpu *v, uint32_t
> idx, uint64_t val)
>          vs->count = val;
> 
>          if ( !vs->count  )
> -            vs->config.enabled = 0;
> +            vs->config.enable = 0;
>          else if ( vs->config.auto_enable )
> -            vs->config.enabled = 1;
> +            vs->config.enable = 1;
> 
> -        if ( vs->config.enabled )
> +        if ( vs->config.enable )
>              start_stimer(vs);
> 
>          break;
> @@ -454,7 +454,7 @@ int viridian_time_rdmsr(const struct vcpu *v, uint32_t
> idx, uint64_t *val)
>          unsigned int stimerx = (idx - HV_X64_MSR_STIMER0_CONFIG) / 2;
>          const struct viridian_stimer *vs =
>              &array_access_nospec(vv->stimer, stimerx);
> -        union viridian_stimer_config_msr config = vs->config;
> +        union hv_stimer_config config = vs->config;
> 
>          if ( !(viridian_feature_mask(d) & HVMPV_stimer) )
>              return X86EMUL_EXCEPTION;
> @@ -464,9 +464,9 @@ int viridian_time_rdmsr(const struct vcpu *v, uint32_t
> idx, uint64_t *val)
>           * the enabled flag is clear.
>           */
>          if ( !config.periodic && test_bit(stimerx, &vv->stimer_pending) )
> -            config.enabled = 0;
> +            config.enable = 0;
> 
> -        *val = config.raw;
> +        *val = config.as_uint64;
>          break;
>      }
> 
> @@ -549,7 +549,7 @@ void viridian_time_save_vcpu_ctxt(
>      {
>          const struct viridian_stimer *vs = &vv->stimer[i];
> 
> -        ctxt->stimer_config_msr[i] = vs->config.raw;
> +        ctxt->stimer_config_msr[i] = vs->config.as_uint64;
>          ctxt->stimer_count_msr[i] = vs->count;
>      }
>  }
> @@ -564,7 +564,7 @@ void viridian_time_load_vcpu_ctxt(
>      {
>          struct viridian_stimer *vs = &vv->stimer[i];
> 
> -        vs->config.raw = ctxt->stimer_config_msr[i];
> +        vs->config.as_uint64 = ctxt->stimer_config_msr[i];
>          vs->count = ctxt->stimer_count_msr[i];
>      }
>  }
> diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-
> x86/hvm/viridian.h
> index d694d83521..d9138562e6 100644
> --- a/xen/include/asm-x86/hvm/viridian.h
> +++ b/xen/include/asm-x86/hvm/viridian.h
> @@ -28,27 +28,10 @@ struct viridian_page
>      void *ptr;
>  };
> 
> -union viridian_stimer_config_msr
> -{
> -    uint64_t raw;
> -    struct
> -    {
> -        uint64_t enabled:1;
> -        uint64_t periodic:1;
> -        uint64_t lazy:1;
> -        uint64_t auto_enable:1;
> -        uint64_t vector:8;
> -        uint64_t direct_mode:1;
> -        uint64_t reserved_zero1:3;
> -        uint64_t sintx:4;
> -        uint64_t reserved_zero2:44;
> -    };
> -};
> -
>  struct viridian_stimer {
>      struct vcpu *v;
>      struct timer timer;
> -    union viridian_stimer_config_msr config;
> +    union hv_stimer_config config;
>      uint64_t count;
>      uint64_t expiration;
>      bool started;
> --
> 2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel