Re: [Xen-devel] [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor

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

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: Monday, June 29, 2015 11:33 PM
> To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> Cc: keir@xxxxxxx; jbeulich@xxxxxxxx; Tian, Kevin; Zhang, Yang Z;
> george.dunlap@xxxxxxxxxxxxx; Dario Faggioli
> Subject: Re: [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor
> 
> On 24/06/15 06:18, Feng Wu wrote:
> > This patch initializes the VT-d Posted-interrupt Descriptor.
> >
> > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > ---
> > v3:
> > - Move pi_desc_init() to xen/arch/x86/hvm/vmx/vmcs.c
> > - Remove the 'inline' flag of pi_desc_init()
> >
> >  xen/arch/x86/hvm/vmx/vmcs.c       | 18 ++++++++++++++++++
> >  xen/include/asm-x86/hvm/vmx/vmx.h |  2 ++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> > index 3aff365..11dc1b5 100644
> > --- a/xen/arch/x86/hvm/vmx/vmcs.c
> > +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> > @@ -40,6 +40,7 @@
> >  #include <asm/flushtlb.h>
> >  #include <asm/shadow.h>
> >  #include <asm/tboot.h>
> > +#include <asm/apic.h>
> >
> >  static bool_t __read_mostly opt_vpid_enabled = 1;
> >  boolean_param("vpid", opt_vpid_enabled);
> > @@ -921,6 +922,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);
> 
> I am fairly sure that this is not a safe use of v->processor.
> Everything else in this patch looks fine, but I would like review from
> people more familiar with scheduling.

It would be very helpful if you can elaborate it a bit more about why
it is not safe using v->processor here?

Thanks,
Feng

> 
> ~Andrew
> 
> > +
> > +    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;
> > @@ -1054,6 +1069,9 @@ static int construct_vmcs(struct vcpu *v)
> >
> >      if ( cpu_has_vmx_posted_intr_processing )
> >      {
> > +        if ( iommu_intpost )
> > +            pi_desc_init(v);
> > +
> >          __vmwrite(PI_DESC_ADDR,
> virt_to_maddr(&v->arch.hvm_vmx.pi_desc));
> >          __vmwrite(POSTED_INTR_NOTIFICATION_VECTOR,
> posted_intr_vector);
> >      }
> > diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h
> b/xen/include/asm-x86/hvm/vmx/vmx.h
> > index 35f804a..5853563 100644
> > --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> > @@ -89,6 +89,8 @@ typedef enum {
> >  #define EPT_EMT_WB              6
> >  #define EPT_EMT_RSV2            7
> >
> > +#define PI_xAPIC_NDST_MASK      0xFF00
> > +
> >  void vmx_asm_vmexit_handler(struct cpu_user_regs);
> >  void vmx_asm_do_vmentry(void);
> >  void vmx_intr_assist(void);


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