Re: [Xen-devel] [PATCH v6 07/18] vmx: Initialize VT-d Posted-Interrupts Descriptor

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.08.15 at 03:57, <feng.wu@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -39,6 +39,7 @@
>  #include <asm/flushtlb.h>
>  #include <asm/shadow.h>
>  #include <asm/tboot.h>
> +#include <asm/apic.h>

I can't seem to spot what this is needed for.

> @@ -951,6 +952,20 @@ void virtual_vmcs_vmwrite(void *vvmcs, u32 
> vmcs_encoding, u64 val)
>      virtual_vmcs_exit(vvmcs);
>  }
>  
> +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);
> +}
> +
>  static int construct_vmcs(struct vcpu *v)
>  {
>      struct domain *d = v->domain;
> @@ -1089,6 +1104,9 @@ static int construct_vmcs(struct vcpu *v)
>  
>      if ( cpu_has_vmx_posted_intr_processing )
>      {
> +        if ( iommu_intpost )
> +            pi_desc_init(v);

If this is going to remain the only call site of the function, I'd suggest
expanding it here. This is because calling the function from elsewhere
is, at least at the first glance, unsafe: It doesn't update pi_desc
atomically, which is in (only apparent?) conflict with the atomic
modifications helpers added in an earlier patch.

If further call sites will get added by later patches, clarifying in a
comment why the non-atomic updates are okay would seem
necessary; alternatively change them to become atomic.

Jan


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