x86/Viridian: don't mark IRQ vectors as pending when vLAPIC is disabled

[PATCH] x86/Viridian: don't mark IRQ vectors as pending when vLAPIC is disabled

Posted by Jan Beulich 1 year, 5 months ago

In software-disabled state an LAPIC does not accept any interrupt
requests and hence no IRR bit would newly become set while in this
state. As a result it is also wrong for us to mark Viridian IPI or timer
vectors as having a pending request when the vLAPIC is in this state.
Such interrupts are simply lost.

Introduce a local variable in send_ipi() to help readability.

Fixes: fda96b7382ea ("viridian: add implementation of the HvSendSyntheticClusterIpi hypercall")
Fixes: 26fba3c85571 ("viridian: add implementation of synthetic timers")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/viridian/synic.c
+++ b/xen/arch/x86/hvm/viridian/synic.c
@@ -359,7 +359,7 @@ bool viridian_synic_deliver_timer_msg(st
     BUILD_BUG_ON(sizeof(payload) > sizeof(msg->u.payload));
     memcpy(msg->u.payload, &payload, sizeof(payload));
 
-    if ( !vs->masked )
+    if ( !vs->masked && vlapic_enabled(vcpu_vlapic(v)) )
         vlapic_set_irq(vcpu_vlapic(v), vs->vector, 0);
 
     return true;
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -811,7 +811,12 @@ static void send_ipi(struct hypercall_vp
         cpu_raise_softirq_batch_begin();
 
     for_each_vp ( vpmask, vp )
-        vlapic_set_irq(vcpu_vlapic(currd->vcpu[vp]), vector, 0);
+    {
+        struct vlapic *vlapic = vcpu_vlapic(currd->vcpu[vp]);
+
+        if ( vlapic_enabled(vlapic) )
+            vlapic_set_irq(vlapic, vector, 0);
+    }
 
     if ( nr > 1 )
         cpu_raise_softirq_batch_finish();

Re: [PATCH] x86/Viridian: don't mark IRQ vectors as pending when vLAPIC is disabled

Posted by Paul Durrant 1 year, 5 months ago

On 21/11/2022 12:13, Jan Beulich wrote:
> In software-disabled state an LAPIC does not accept any interrupt
> requests and hence no IRR bit would newly become set while in this
> state. As a result it is also wrong for us to mark Viridian IPI or timer
> vectors as having a pending request when the vLAPIC is in this state.
> Such interrupts are simply lost.
> 
> Introduce a local variable in send_ipi() to help readability.
> 
> Fixes: fda96b7382ea ("viridian: add implementation of the HvSendSyntheticClusterIpi hypercall")
> Fixes: 26fba3c85571 ("viridian: add implementation of synthetic timers")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

Re: [PATCH] x86/Viridian: don't mark IRQ vectors as pending when vLAPIC is disabled

Posted by Andrew Cooper 1 year, 5 months ago

On 21/11/2022 12:13, Jan Beulich wrote:
> In software-disabled state an LAPIC does not accept any interrupt
> requests and hence no IRR bit would newly become set while in this
> state. As a result it is also wrong for us to mark Viridian IPI or timer
> vectors as having a pending request when the vLAPIC is in this state.
> Such interrupts are simply lost.
>
> Introduce a local variable in send_ipi() to help readability.
>
> Fixes: fda96b7382ea ("viridian: add implementation of the HvSendSyntheticClusterIpi hypercall")
> Fixes: 26fba3c85571 ("viridian: add implementation of synthetic timers")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>