Re: [Xen-devel] [PATCH for-4.13] x86/vmx: always sync PIR to IRR before vmentry

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.11.2019 11:16, Roger Pau Monne wrote:
> When using posted interrupts on Intel hardware it's possible that the
> vCPU resumes execution with a stale local APIC IRR register because
> depending on the interrupts to be injected vlapic_has_pending_irq
> might not be called, and thus PIR won't be synced into IRR.
> 
> Fix this by making sure PIR is always synced to IRR in vmx_intr_assist
> regardless of what interrupts are pending.

For this part, did you consider pulling ahead to the beginning
of hvm_vcpu_has_pending_irq() its call to vlapic_has_pending_irq()?
I ask because ...

> --- a/xen/arch/x86/hvm/vmx/intr.c
> +++ b/xen/arch/x86/hvm/vmx/intr.c
> @@ -232,6 +232,14 @@ void vmx_intr_assist(void)
>      enum hvm_intblk intblk;
>      int pt_vector;
>  
> +    if ( cpu_has_vmx_posted_intr_processing )
> +        /*
> +         * Always force PIR to be synced to IRR before vmentry, this is also
> +         * done by vlapic_has_pending_irq but it's possible other pending
> +         * interrupts prevent the execution of that function.
> +         */
> +        vmx_sync_pir_to_irr(v);

... this addition looks more like papering over some issue than
actually taking care of it.

Then again I wonder whether the PIR->IRR sync is actually
legitimate to perform when v != current. If it's not, then there
might be a wider set of problems (see e.g.
hvm_local_events_need_delivery()). But of course the adjustment
to hvm_vcpu_has_pending_irq() could also be to make the call
early only when v == current.

A last question is that on the consequences of overly aggressive
sync-ing - that'll harm performance, but shouldn't affect
correctness if I'm not mistaken.

Jan

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