| Message ID | 1479434244-10223-2-git-send-email-feng.wu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> From: Wu, Feng > Sent: Friday, November 18, 2016 9:57 AM > > PI hook vmx_pi_switch_to() is needed even when any previously even when -> even after > 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 which two functions? > 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> > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > v8: > - Comments changes > > 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 | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 3d330b6..f3911f2 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -222,8 +222,14 @@ 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 set 'd->arch.hvm_domain.vmx.pi_switch_to' to NULL > + * in vmx_pi_switch_to() 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. > + */ better to move this comment ahead of earlier zaps? Also seems the comment is different from what commit msg says. Please make them correlated. > } > > static int vmx_domain_initialise(struct domain *d) > -- > 2.1.0
> -----Original Message----- > From: Tian, Kevin > Sent: Friday, November 18, 2016 11:14 AM > To: Wu, Feng <feng.wu@intel.com>; xen-devel@lists.xen.org > Cc: jbeulich@suse.com; andrew.cooper3@citrix.com; > george.dunlap@eu.citrix.com; dario.faggioli@citrix.com > Subject: RE: [PATCH v8 1/7] VMX: Permanently assign PI hook > vmx_pi_switch_to() > > > From: Wu, Feng > > Sent: Friday, November 18, 2016 9:57 AM > > > > PI hook vmx_pi_switch_to() is needed even when any previously > > even when -> even after > > > 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 > > which two functions? vmx_pi_switch_to() and vmx_pi_switch_from(), I will explicitly describe it in the next version. Thanks, Feng > > > 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> > > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > --- > > v8: > > - Comments changes > > > > 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 | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > > index 3d330b6..f3911f2 100644 > > --- a/xen/arch/x86/hvm/vmx/vmx.c > > +++ b/xen/arch/x86/hvm/vmx/vmx.c > > @@ -222,8 +222,14 @@ 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 set 'd->arch.hvm_domain.vmx.pi_switch_to' to NULL > > + * in vmx_pi_switch_to() 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. > > + */ > > better to move this comment ahead of earlier zaps? Also seems the > comment is different from what commit msg says. Please make > them correlated. Sure. Thanks, Feng > > > } > > > > static int vmx_domain_initialise(struct domain *d) > > -- > > 2.1.0
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 3d330b6..f3911f2 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -222,8 +222,14 @@ 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 set 'd->arch.hvm_domain.vmx.pi_switch_to' to NULL + * in vmx_pi_switch_to() 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)