Re: [Xen-devel] [PATCH v7 1/6] VMX: Permanently assign PI hook vmx_pi_switch_to()

2016-11-12 Thread Konrad Rzeszutek Wilk
On November 12, 2016 4:14:50 AM EST, "Wu, Feng"  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!


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v7 1/6] VMX: Permanently assign PI hook vmx_pi_switch_to()

2016-11-12 Thread Wu, Feng
> > 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 
> > + * 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

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v7 1/6] VMX: Permanently assign PI hook vmx_pi_switch_to()

2016-11-11 Thread Konrad Rzeszutek Wilk
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 
> ---
> 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 
> + * 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

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


[Xen-devel] [PATCH v7 1/6] VMX: Permanently assign PI hook vmx_pi_switch_to()

2016-11-07 Thread Feng Wu
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 
---
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
+ * 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