| Message ID | 1476147473-30970-2-git-send-email-feng.wu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> From: Wu, Feng > Sent: Tuesday, October 11, 2016 8:58 AM > > PI hooks: vmx_pi_switch_from() and vmx_pi_switch_to() are > 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 these two hooks statically > assigned. Your patch has only one hook statically assigned... > > Signed-off-by: Feng Wu <feng.wu@intel.com> > --- > v5: > - Zap "pi_switch_from" hook > > xen/arch/x86/hvm/vmx/vmx.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 3d330b6..623d5bc 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -147,6 +147,11 @@ static void vmx_pi_switch_from(struct vcpu *v) > pi_set_sn(pi_desc); > } > > +/* > + * In fact, we can remove this hooks 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 void vmx_pi_switch_to(struct vcpu *v) > { > struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc; > @@ -221,9 +226,8 @@ void vmx_pi_hooks_deassign(struct domain *d) > ASSERT(d->arch.hvm_domain.vmx.vcpu_block); > > 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; > + d->arch.hvm_domain.vmx.pi_switch_from = NULL; > } > > static int vmx_domain_initialise(struct domain *d) > -- > 2.1.0
>>> On 11.10.16 at 02:57, <feng.wu@intel.com> wrote: > +/* > + * In fact, we can remove this hooks 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 void vmx_pi_switch_to(struct vcpu *v) I think this comment would better go next to where the NULL assignment is (to be). > @@ -221,9 +226,8 @@ void vmx_pi_hooks_deassign(struct domain *d) > ASSERT(d->arch.hvm_domain.vmx.vcpu_block); > > 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; > + d->arch.hvm_domain.vmx.pi_switch_from = NULL; > } Besides the discrepancy of description and implementation which Kevin has already pointed out, I'd also like to ask to replace the "statically" in title and description. "Statically" would mean the hooks get installed unconditionally at domain creation time (or even at build time). Instead I think you mean "keep two PI hooks assigned" or "permanently assign two PI hooks". Jan
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 3d330b6..623d5bc 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -147,6 +147,11 @@ static void vmx_pi_switch_from(struct vcpu *v) pi_set_sn(pi_desc); } +/* + * In fact, we can remove this hooks 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 void vmx_pi_switch_to(struct vcpu *v) { struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc; @@ -221,9 +226,8 @@ void vmx_pi_hooks_deassign(struct domain *d) ASSERT(d->arch.hvm_domain.vmx.vcpu_block); 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; + d->arch.hvm_domain.vmx.pi_switch_from = NULL; } static int vmx_domain_initialise(struct domain *d)
PI hooks: vmx_pi_switch_from() and vmx_pi_switch_to() are 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 these two hooks statically assigned. Signed-off-by: Feng Wu <feng.wu@intel.com> --- v5: - Zap "pi_switch_from" hook xen/arch/x86/hvm/vmx/vmx.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)