Re: [RFC PATCH 8/9] x86/SVM: Add interrupt management code via AVIC

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Sep 19, 2016 at 12:52:47AM -0500, Suravee Suthikulpanit wrote:
> Enabling AVIC implicitly disables the V_IRQ, V_INTR_PRIO, V_IGN_TPR,
> and V_INTR_VECTOR fields in the VMCB Control Word. Therefore, this patch
> introduces new interrupt injection code via AVIC backing page.
> 
> Also, the AVIC hardware automatically synchronizes TPR and CR8/vTPR, when
> values are updated. Therefore, xen does not need to handle this when enable
> AVIC.

s/this when enable AVIC/when AVIC is enabled/

> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  xen/arch/x86/hvm/svm/avic.c        | 31 +++++++++++++++++++++++++++++++
>  xen/arch/x86/hvm/svm/intr.c        |  4 ++++
>  xen/arch/x86/hvm/svm/svm.c         | 12 ++++++++++--
>  xen/arch/x86/hvm/svm/vmcb.c        |  6 +++++-
>  xen/include/asm-x86/hvm/svm/avic.h |  1 +
>  5 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> index cd8a9d4..4144223 100644
> --- a/xen/arch/x86/hvm/svm/avic.c
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -576,3 +576,34 @@ void svm_avic_vmexit_do_noaccel(struct cpu_user_regs *regs)
>  
>      return;
>  }
> +
> +/***************************************************************

Twinkle twinkle little stars, there are too many of you..

> + * AVIC INTR INJECTION

Also the comment could be deleted as it explains pretty well the flow.
> + */
> +void svm_avic_deliver_posted_intr(struct vcpu *v, u8 vec)
> +{
> +    struct vlapic *vlapic = vcpu_vlapic(v);
> +
> +    /* Fallback to use non-AVIC if vcpu is not enabled with AVIC */

Please add an period at the end.

> +    if ( !svm_avic_vcpu_enabled(v) )
> +    {
> +        if ( !vlapic_test_and_set_vector(vec, &vlapic->regs->data[APIC_IRR]) )
> +            vcpu_kick(v);
> +        return;
> +    }
> +
> +    if ( !(guest_cpu_user_regs()->eflags & X86_EFLAGS_IF) )
> +        return;
> +
> +    if ( vlapic_test_and_set_vector(vec, &vlapic->regs->data[APIC_IRR]) )
> +        return;
> +
> +    /*
> +     * If vcpu is running on another cpu, hit the doorbell to signal
> +     * it to process interrupt. Otherwise, kick it.
> +     */
> +    if ( v->is_running && (v != current) )
> +        wrmsrl(AVIC_DOORBELL, cpu_data[v->processor].apicid);

What is the consequence if say the destination CPU ends up switching to
a different guest - and the doorball hits at that point?

If the different guest is not AVIC enabled .. what then? The CPU ignores
it? Say the CPU is running at that point without VMCB (it is running
dom0), or with an HVM guest without AVIC? Will we get AVIC IPI delievery
not completed #VMEXIT on the destination CPU? (or on this one?)

And what if this new guest is AVIC enabled and there are no IRR in the
backing page? [I presume nothing will happen]

> +    else
> +        vcpu_kick(v);
> +}

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