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)
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel