| Message ID | 1478506083-14560-2-git-send-email-feng.wu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 07, 2016 at 04:07:58PM +0800, Feng Wu wrote: > PI hook vmx_pi_switch_to() is needed even when any previously > assigned device is detached from the domain. Since 'SN' bit is > also used to control the CPU side PI and we change the state of > SN bit in these two functions, then evaluate this bit in > vmx_deliver_posted_intr() when trying to deliver the interrupt > in posted way via software. The problem is if we deassign the > hooks while the vCPU is runnable in the runqueue with 'SN' set, > all the furture notificaton event will be suppressed. This patch > makes the hook permanently assigned. > > Signed-off-by: Feng Wu <feng.wu@intel.com> > --- > v7: > - comments changes. > > v6: > - Adjust the comments and wording. > > v5: > - Zap "pi_switch_from" hook > > v4: > - Don't zap vmx_pi_switch_from() and vmx_pi_switch_to() when > any previously assigned device is detached from the domain. > - Comments changes. > > xen/arch/x86/hvm/vmx/vmx.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 3d330b6..10546af 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -222,8 +222,13 @@ void vmx_pi_hooks_deassign(struct domain *d) > > d->arch.hvm_domain.vmx.vcpu_block = NULL; > d->arch.hvm_domain.vmx.pi_switch_from = NULL; > - d->arch.hvm_domain.vmx.pi_switch_to = NULL; > d->arch.hvm_domain.vmx.pi_do_resume = NULL; > + > + /* > + * In fact, we could remove 'vmx_pi_switch_to' inside itself if no new device I am having a hard time parsing that. What is the 'inside itself'? Otherwise, Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > + * is in the process of getting assigned and "from" hook is NULL. However, > + * it is not straightforward to find a clear solution, so just leave it here. > + */ > } > > static int vmx_domain_initialise(struct domain *d) > -- > 2.1.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
> > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > > index 3d330b6..10546af 100644 > > --- a/xen/arch/x86/hvm/vmx/vmx.c > > +++ b/xen/arch/x86/hvm/vmx/vmx.c > > @@ -222,8 +222,13 @@ void vmx_pi_hooks_deassign(struct domain *d) > > > > d->arch.hvm_domain.vmx.vcpu_block = NULL; > > d->arch.hvm_domain.vmx.pi_switch_from = NULL; > > - d->arch.hvm_domain.vmx.pi_switch_to = NULL; > > d->arch.hvm_domain.vmx.pi_do_resume = NULL; > > + > > + /* > > + * In fact, we could remove 'vmx_pi_switch_to' inside itself if no new > device > > I am having a hard time parsing that. What is the 'inside itself'? Thanks for your review. It means we could set ' d->arch.hvm_domain.vmx.pi_switch_to' to NULL in vmx_pi_switch_to(). It would be good if you have any better description! :) Thanks, Feng > > Otherwise, > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > + * is in the process of getting assigned and "from" hook is NULL. However, > > + * it is not straightforward to find a clear solution, so just leave it here. > > + */ > > } > > > > static int vmx_domain_initialise(struct domain *d) > > -- > > 2.1.0 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > https://lists.xen.org/xen-devel
On November 12, 2016 4:14:50 AM EST, "Wu, Feng" <feng.wu@intel.com> wrote: >> > diff --git a/xen/arch/x86/hvm/vmx/vmx.c >b/xen/arch/x86/hvm/vmx/vmx.c >> > index 3d330b6..10546af 100644 >> > --- a/xen/arch/x86/hvm/vmx/vmx.c >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c >> > @@ -222,8 +222,13 @@ void vmx_pi_hooks_deassign(struct domain *d) >> > >> > d->arch.hvm_domain.vmx.vcpu_block = NULL; >> > d->arch.hvm_domain.vmx.pi_switch_from = NULL; >> > - d->arch.hvm_domain.vmx.pi_switch_to = NULL; >> > d->arch.hvm_domain.vmx.pi_do_resume = NULL; >> > + >> > + /* >> > + * In fact, we could remove 'vmx_pi_switch_to' inside itself >if no new >> device >> >> I am having a hard time parsing that. What is the 'inside itself'? > >Thanks for your review. It means we could set ' >d->arch.hvm_domain.vmx.pi_switch_to' >to NULL in vmx_pi_switch_to(). It would be good if you have any better >description! :) Your above description is perfect: "We could set d-arch.hvm_domain.vmx.pi_switch_to to NULL in vmx_pi_switch_to() if no new device ..." Thanks!
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 3d330b6..10546af 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -222,8 +222,13 @@ void vmx_pi_hooks_deassign(struct domain *d) d->arch.hvm_domain.vmx.vcpu_block = NULL; d->arch.hvm_domain.vmx.pi_switch_from = NULL; - d->arch.hvm_domain.vmx.pi_switch_to = NULL; d->arch.hvm_domain.vmx.pi_do_resume = NULL; + + /* + * In fact, we could remove 'vmx_pi_switch_to' inside itself if no new device + * is in the process of getting assigned and "from" hook is NULL. However, + * it is not straightforward to find a clear solution, so just leave it here. + */ } static int vmx_domain_initialise(struct domain *d)
PI hook vmx_pi_switch_to() is needed even when any previously assigned device is detached from the domain. Since 'SN' bit is also used to control the CPU side PI and we change the state of SN bit in these two functions, then evaluate this bit in vmx_deliver_posted_intr() when trying to deliver the interrupt in posted way via software. The problem is if we deassign the hooks while the vCPU is runnable in the runqueue with 'SN' set, all the furture notificaton event will be suppressed. This patch makes the hook permanently assigned. Signed-off-by: Feng Wu <feng.wu@intel.com> --- v7: - comments changes. v6: - Adjust the comments and wording. v5: - Zap "pi_switch_from" hook v4: - Don't zap vmx_pi_switch_from() and vmx_pi_switch_to() when any previously assigned device is detached from the domain. - Comments changes. xen/arch/x86/hvm/vmx/vmx.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)