> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Thursday, September 1, 2016 5:23 PM > To: Wu, Feng <feng...@intel.com> > Cc: andrew.coop...@citrix.com; dario.faggi...@citrix.com; > george.dun...@eu.citrix.com; Tian, Kevin <kevin.t...@intel.com>; xen- > de...@lists.xen.org > Subject: RE: [PATCH v3 1/6] VMX: Statically assign two PI hooks > > >>> On 01.09.16 at 11:13, <feng...@intel.com> wrote: > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: Thursday, September 1, 2016 4:17 PM > >> >>> On 31.08.16 at 05:56, <feng...@intel.com> wrote: > >> > --- a/xen/arch/x86/hvm/vmx/vmx.c > >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c > >> > @@ -207,8 +207,6 @@ void vmx_pi_hooks_assign(struct domain *d) > >> > ASSERT(!d->arch.hvm_domain.vmx.vcpu_block); > >> > > >> > d->arch.hvm_domain.vmx.vcpu_block = vmx_vcpu_block; > >> > - d->arch.hvm_domain.vmx.pi_switch_from = vmx_pi_switch_from; > >> > - d->arch.hvm_domain.vmx.pi_switch_to = vmx_pi_switch_to; > >> > d->arch.hvm_domain.vmx.pi_do_resume = vmx_pi_do_resume; > >> > } > >> > >> I'm sure I've said so before: While I can see why you want the > >> adjustment to vmx_pi_hooks_deassign(), I don't see why leaving > >> out this and ... > > > > As mentioned in the comments of this patch, these two hooks are also > > needed for CPU side PI, so we cannot install here and ... > > I've just read it again, and to me it doesn't say so. I.e. ... > > >> > @@ -236,6 +232,12 @@ static int vmx_domain_initialise(struct domain *d) > >> > if ( (rc = vmx_alloc_vlapic_mapping(d)) != 0 ) > >> > return rc; > >> > > >> > + if ( iommu_intpost ) > >> > + { > >> > + d->arch.hvm_domain.vmx.pi_switch_from = vmx_pi_switch_from; > >> > + d->arch.hvm_domain.vmx.pi_switch_to = vmx_pi_switch_to; > >> > + } > >> > + > >> > return 0; > >> > } > >> > >> ... this would conflict with the goal of the patch. > > > > ... Yes this is incorrect, I should install these two hooks when CPU side > > PI is > > supported. > > ... you'd then additionally need to explain what bug there is without > these hooks and with only CPU side PI present (iirc that's functionality > which has been there before the VT-d part).
Oh, my bad, I mixed things up. Then I should install these two hooks when the first device assignment happens and don't zap it after that. Thanks for the comments! Thanks, Feng > > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
