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

[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()

[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()

[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