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

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Mon, 2015-06-29 at 16:32 +0100, Andrew Cooper wrote:
> On 24/06/15 06:18, Feng Wu wrote:

> > 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 is unsafe, IMO.

However, this is called by vcpu_initialise(), within alloc_vcpu(), which
is in turn triggered by XEN_DOMCTL_max_vcpus. When we get to there, the
vcpu is RUNSTATE_offline, it's got _VPF_down in pause_flags, and the
whole domain is paused.

Honestly, I don't think there are many chances that v->processor changes
under our feet, in this case. A comment, explaining this quickly, would
be useful, though, I think.

George?

In any case, even if v->processor would be at risk of fluctuating, Feng,
what's the risk of going live with a spurious content in dest? Is it
just that the first posted interrupt will be consumed by the wrong vCPU,
but after that, things will settle properly? If yes, I think that would
be ok too (but really, I think things are safe, although by chance, in
this case).

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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