Re: [PATCH 6/7] Nested VMX: Update APIC-v(RVI/SVI) when vmexit to L1

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 09/08/13 09:49, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
>
> If enabling APIC-v, all interrupts to L1 is delivered through APIC-v.
> But when L2 is running, external interrupt will casue L1 vmexit with
> reason external interrupt. Then L1 will pick up the interrupt through
> vmcs. So we need to update APIC-v related structure accordingly;
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  xen/arch/x86/hvm/vmx/vvmx.c |   37 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 5dfbc54..9ba169d 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1283,6 +1283,41 @@ static void sync_exception_state(struct vcpu *v)
>      }
>  }
>  
> +static void nvmx_update_apicv(struct vcpu *v)
> +{
> +    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> +    struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> +    unsigned long reason = __get_vvmcs(nvcpu->nv_vvmcx, VM_EXIT_REASON);
> +    uint32_t intr_info = __get_vvmcs(nvcpu->nv_vvmcx, VM_EXIT_INTR_INFO);
> +
> +    if ( !cpu_has_vmx_virtual_intr_delivery )
> +        return;

Is it worth hoisting this check outside of the function, to avoid
needless __get_vvmcs() calls on some hardware?

> +
> +    if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
> +            nvmx->intr.source == hvm_intsrc_lapic &&
> +            (intr_info & INTR_INFO_VALID_MASK) )
> +    {
> +        unsigned long status;
> +        uint32_t rvi, ppr;
> +        uint32_t vector = intr_info & 0xff;
> +        struct vlapic *vlapic = vcpu_vlapic(v);
> +
> +        WARN_ON(vector <= 0);
> +
> +        vlapic_ack_pending_irq(v, vector, 1);
> +
> +        ppr = vlapic_set_ppr(vlapic);
> +        BUG_ON( (ppr & 0xf0) != (vector & 0xf0) );
> +
> +        status = (vector << 8) & 0xff00;

Given "vector = intr_info & 0xff;" above, this "& 0xff00" looks useless.

~Andrew

> +        rvi = vlapic_has_pending_irq(v);
> +        if ( rvi != -1 )
> +            status |= rvi & 0xff;
> +
> +        __vmwrite(GUEST_INTR_STATUS, status);
> +    }
> +}
> +
>  static void virtual_vmexit(struct cpu_user_regs *regs)
>  {
>      struct vcpu *v = current;
> @@ -1328,6 +1363,8 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
>      /* updating host cr0 to sync TS bit */
>      __vmwrite(HOST_CR0, v->arch.hvm_vmx.host_cr0);
>  
> +    nvmx_update_apicv(v);
> +
>      vmreturn(regs, VMSUCCEED);
>  }
>