[PATCH 0/2] Follow-up viridian fixes

[PATCH 0/2] Follow-up viridian fixes

[Paul Durrant]

Further testing and code inspection revealed some problems with my recent
series [1]. This series fixes those issues.

[1] https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01078.html

Paul Durrant (2):
  viridian: remove synic poll blocking
  viridian: fix mistakes in timer expiration and re-start

 xen/arch/x86/hvm/viridian/synic.c  | 14 +-------------
 xen/arch/x86/hvm/viridian/time.c   | 12 +++++++-----
 xen/arch/x86/hvm/vlapic.c          | 13 ++++---------
 xen/include/asm-x86/hvm/viridian.h |  3 +--
 4 files changed, 13 insertions(+), 29 deletions(-)

-- 
2.20.1


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

[PATCH 1/2] viridian: remove synic poll blocking

[Paul Durrant]

This was added to try to ensure a consistent intack from repeated calls to
hvm_vcpu_has_pending_irq(). However there are other ways in which a new
IRR bit could be set between such calls. Hence the poll blocking does not
actually serve any useful purpose, so it may as well be removed to simplify
the code.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
---
 xen/arch/x86/hvm/viridian/synic.c  | 14 +-------------
 xen/arch/x86/hvm/vlapic.c          | 13 ++++---------
 xen/include/asm-x86/hvm/viridian.h |  3 +--
 3 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/synic.c b/xen/arch/x86/hvm/viridian/synic.c
index 0fca5d36e6..2791021bcc 100644
--- a/xen/arch/x86/hvm/viridian/synic.c
+++ b/xen/arch/x86/hvm/viridian/synic.c
@@ -346,21 +346,9 @@ void viridian_synic_domain_deinit(const struct domain *d)
 {
 }
 
-void viridian_synic_poll_once(struct vcpu *v)
+void viridian_synic_poll(struct vcpu *v)
 {
-    struct viridian_vcpu *vv = v->arch.hvm.viridian;
-
-    if ( vv->polled )
-        return;
-
     viridian_time_poll_timers(v);
-
-    vv->polled = true;
-}
-
-void viridian_synic_poll_unblock(const struct vcpu *v)
-{
-    v->arch.hvm.viridian->polled = false;
 }
 
 bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index aab365f80d..24e8e63c4f 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1311,15 +1311,15 @@ int vlapic_has_pending_irq(struct vcpu *v)
      * a synthetic interrupt may be asserted during the poll.
      */
     if ( has_viridian_synic(v->domain) )
-        viridian_synic_poll_once(v);
+        viridian_synic_poll(v);
 
     irr = vlapic_find_highest_irr(vlapic);
     if ( irr == -1 )
-        goto out;
+        return -1;
 
     if ( hvm_funcs.virtual_intr_delivery_enabled &&
          !nestedhvm_vcpu_in_guestmode(v) )
-        goto out;
+        return irr;
 
     /*
      * If APIC assist was set then an EOI may have been avoided.
@@ -1340,13 +1340,9 @@ int vlapic_has_pending_irq(struct vcpu *v)
          (irr & 0xf0) <= (isr & 0xf0) )
     {
         viridian_apic_assist_clear(v);
-        irr = -1;
+        return -1;
     }
 
- out:
-    if ( irr == -1 )
-        viridian_synic_poll_unblock(v);
-
     return irr;
 }
 
@@ -1381,7 +1377,6 @@ int vlapic_ack_pending_irq(struct vcpu *v, int vector, bool_t force_ack)
         vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]);
 
     vlapic_clear_irr(vector, vlapic);
-    viridian_synic_poll_unblock(v);
 
     return 1;
 }
diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-x86/hvm/viridian.h
index 5fe2039978..1bea756a26 100644
--- a/xen/include/asm-x86/hvm/viridian.h
+++ b/xen/include/asm-x86/hvm/viridian.h
@@ -143,8 +143,7 @@ void viridian_apic_assist_clear(const struct vcpu *v);
 
 bool viridian_synic_is_auto_eoi_sint(const struct vcpu *v,
                                      unsigned int vector);
-void viridian_synic_poll_once(struct vcpu *v);
-void viridian_synic_poll_unblock(const struct vcpu *v);
+void viridian_synic_poll(struct vcpu *v);
 void viridian_synic_ack_sint(const struct vcpu *v, unsigned int vector);
 
 #endif /* __ASM_X86_HVM_VIRIDIAN_H__ */
-- 
2.20.1


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

[PATCH 2/2] viridian: fix mistakes in timer expiration and re-start

[Paul Durrant]

This patch fixes a few issues:

- The specification says that a one-shot timers should be marked as
  disabled at the point of expiry, so the enabled bit should be cleared by
  stimer_expire() rather than poll_stimer(). For simplicity, call
  stimer_expire() from start_stimer() for timers that expire immediately
  rather than open-coding the expiry.

- The code omits updating the expiration value for one-shot timers and
  so the eventual message delivered via the synic will be incorrect.

- Periodic timers should only be re-started if they are still enabled so,
  now that enabled will be already clear for one-shot timers, call
  start_stimer() from poll_stimer() only if the timer is still enabled.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
---
 xen/arch/x86/hvm/viridian/time.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
index 7a759e70b3..692f014fc4 100644
--- a/xen/arch/x86/hvm/viridian/time.c
+++ b/xen/arch/x86/hvm/viridian/time.c
@@ -187,7 +187,7 @@ static void stop_stimer(struct viridian_stimer *vs)
 
 static void stimer_expire(void *data)
 {
-    const struct viridian_stimer *vs = data;
+    struct viridian_stimer *vs = data;
     struct vcpu *v = vs->v;
     struct viridian_vcpu *vv = v->arch.hvm.viridian;
     unsigned int stimerx = vs - &vv->stimer[0];
@@ -197,6 +197,9 @@ static void stimer_expire(void *data)
 
     set_bit(stimerx, &vv->stimer_pending);
     vcpu_kick(v);
+
+    if ( !vs->config.periodic )
+        vs->config.enabled = 0;
 }
 
 static void start_stimer(struct viridian_stimer *vs)
@@ -256,7 +259,8 @@ static void start_stimer(struct viridian_stimer *vs)
         expiration = vs->count;
         if ( expiration - now <= 0 )
         {
-            set_bit(stimerx, &vv->stimer_pending);
+            vs->expiration = expiration;
+            stimer_expire(vs);
             return;
         }
     }
@@ -285,10 +289,8 @@ static void poll_stimer(struct vcpu *v, unsigned int stimerx)
 
     clear_bit(stimerx, &vv->stimer_pending);
 
-    if ( vs->config.periodic )
+    if ( vs->config.enabled )
         start_stimer(vs);
-    else
-        vs->config.enabled = 0;
 }
 
 void viridian_time_poll_timers(struct vcpu *v)
-- 
2.20.1


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

Re: [PATCH 0/2] Follow-up viridian fixes

[Jan Beulich]

>>> On 18.03.19 at 10:06, <paul.durrant@citrix.com> wrote:
> Further testing and code inspection revealed some problems with my recent
> series [1]. This series fixes those issues.
> 
> [1] 
> https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01078.html 
> 
> Paul Durrant (2):
>   viridian: remove synic poll blocking
>   viridian: fix mistakes in timer expiration and re-start

Is there a particular reason you don't want to fold them into
that series?

Jan



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

Re: [PATCH 0/2] Follow-up viridian fixes

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 18 March 2019 10:21
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel <xen-devel@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH 0/2] Follow-up viridian fixes
> 
> >>> On 18.03.19 at 10:06, <paul.durrant@citrix.com> wrote:
> > Further testing and code inspection revealed some problems with my recent
> > series [1]. This series fixes those issues.
> >
> > [1]
> > https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01078.html
> >
> > Paul Durrant (2):
> >   viridian: remove synic poll blocking
> >   viridian: fix mistakes in timer expiration and re-start
> 
> Is there a particular reason you don't want to fold them into
> that series?
> 

Only that I believe it's fully acked and so I didn't want to disturb it at this stage.

  Paul

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

Re: [PATCH 0/2] Follow-up viridian fixes

[Jan Beulich]

>>> On 18.03.19 at 11:22, <Paul.Durrant@citrix.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 18 March 2019 10:21
>> To: Paul Durrant <Paul.Durrant@citrix.com>
>> Cc: xen-devel <xen-devel@lists.xenproject.org>
>> Subject: Re: [Xen-devel] [PATCH 0/2] Follow-up viridian fixes
>> 
>> >>> On 18.03.19 at 10:06, <paul.durrant@citrix.com> wrote:
>> > Further testing and code inspection revealed some problems with my recent
>> > series [1]. This series fixes those issues.
>> >
>> > [1]
>> > https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01078.html 
>> >
>> > Paul Durrant (2):
>> >   viridian: remove synic poll blocking
>> >   viridian: fix mistakes in timer expiration and re-start
>> 
>> Is there a particular reason you don't want to fold them into
>> that series?
>> 
> 
> Only that I believe it's fully acked and so I didn't want to disturb it at 
> this stage.

But the tree isn't fully open yet, so it won't go in immediately anyway.
For the folding in of the first of the two patches here you may freely
retain my acks. I'd have to look at the second patch here in more
detail, so I can't say the same there, but afaict it would affect the
ack on a single patch only.

Jan



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

Re: [PATCH 0/2] Follow-up viridian fixes

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 18 March 2019 10:35
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel <xen-devel@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH 0/2] Follow-up viridian fixes
> 
> >>> On 18.03.19 at 11:22, <Paul.Durrant@citrix.com> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: 18 March 2019 10:21
> >> To: Paul Durrant <Paul.Durrant@citrix.com>
> >> Cc: xen-devel <xen-devel@lists.xenproject.org>
> >> Subject: Re: [Xen-devel] [PATCH 0/2] Follow-up viridian fixes
> >>
> >> >>> On 18.03.19 at 10:06, <paul.durrant@citrix.com> wrote:
> >> > Further testing and code inspection revealed some problems with my recent
> >> > series [1]. This series fixes those issues.
> >> >
> >> > [1]
> >> > https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01078.html
> >> >
> >> > Paul Durrant (2):
> >> >   viridian: remove synic poll blocking
> >> >   viridian: fix mistakes in timer expiration and re-start
> >>
> >> Is there a particular reason you don't want to fold them into
> >> that series?
> >>
> >
> > Only that I believe it's fully acked and so I didn't want to disturb it at
> > this stage.
> 
> But the tree isn't fully open yet, so it won't go in immediately anyway.
> For the folding in of the first of the two patches here you may freely
> retain my acks. I'd have to look at the second patch here in more
> detail, so I can't say the same there, but afaict it would affect the
> ack on a single patch only.

Ok, I'll send a v8 of that series. Patch #1 of this series would amend patch #9 of that series and patch #2 of this series will need to fold into patch #10.

  Paul

> 
> Jan
> 


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