Re: [Xen-devel] [PATCH v5 4/7] VMX: Make sure PI is in proper state before install the hooks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.10.16 at 02:57, <feng.wu@xxxxxxxxx> wrote:
>  static void pi_desc_init(struct vcpu *v)
>  {
> -    uint32_t dest;
> -
>      v->arch.hvm_vmx.pi_desc.nv = posted_intr_vector;
>  
> -    dest = cpu_physical_id(v->processor);
> -
> -    if ( x2apic_enabled )
> -        v->arch.hvm_vmx.pi_desc.ndst = dest;
> -    else
> -        v->arch.hvm_vmx.pi_desc.ndst = MASK_INSR(dest, PI_xAPIC_NDST_MASK);
> +    /*
> +     * Mark NDST as invalid, then we can use this invalid value as a
> +     * marker to whether update NDST or not in vmx_pi_hooks_assign().
> +     */
> +    v->arch.hvm_vmx.pi_desc.ndst = 0xffffffff;

I think I had at the same time asked to make this a #define, so the
two (currently) instance can be connected together.

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -211,14 +211,39 @@ static void vmx_pi_do_resume(struct vcpu *v)
>  /* This function is called when pcidevs_lock is held */
>  void vmx_pi_hooks_assign(struct domain *d)
>  {
> +    struct vcpu *v;
> +
>      if ( !iommu_intpost || !has_hvm_container_domain(d) )
>          return;
>  
>      ASSERT(!d->arch.hvm_domain.vmx.vcpu_block);
>  
> -    d->arch.hvm_domain.vmx.vcpu_block = vmx_vcpu_block;
> +    /*
> +     * We carefully handle the timing here:
> +     * - Install the context switch first
> +     * - Then set the NDST field
> +     * - Install the block and resume hooks in the end
> +     *
> +     * This can make sure the PI (especially the NDST feild) is
> +     * in proper state when we call vmx_vcpu_block().
> +     */
>      d->arch.hvm_domain.vmx.pi_switch_from = vmx_pi_switch_from;
>      d->arch.hvm_domain.vmx.pi_switch_to = vmx_pi_switch_to;
> +
> +    for_each_vcpu ( d, v )
> +    {
> +        unsigned int dest = cpu_physical_id(v->processor);
> +        struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;
> +
> +        /*
> +         * We don't need to update NDST if 'arch.hvm_domain.vmx.pi_switch_to'
> +         * already gets called

I think you mean "got" or "has got". Also I think you'd better refer to
the actual VMX function instead of the hook pointer field.

Jan


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