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

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, September 04, 2015 10:47 PM
> To: Wu, Feng
> Cc: Andrew Cooper; Tian, Kevin; xen-devel@xxxxxxxxxxxxx; Keir Fraser
> Subject: Re: [PATCH v6 07/18] vmx: Initialize VT-d Posted-Interrupts 
> Descriptor
> 
> >>> 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.

It is for ' x2apic_enabled '.

> > @@ -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.

I will add some comments to this function.

Thanks,
Feng

> 
> Jan


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