Re: [PATCH 1/2] xen/arm: remove workaround to inject evtchn_irq on irq enable

[Julien Grall <julien.grall@linaro.org>]

Hi Stefano,

On 06/24/2014 07:11 PM, Stefano Stabellini wrote:
> evtchn_upcall_pending is already set by common code at vcpu creation,
> therefore on ARM we also need to call vgic_vcpu_inject_irq for it.
> Currently we do that from vgic_enable_irqs as a workaround.
> 
> Do this properly by calling vgic_vcpu_inject_irq in the appropriate
> places at vcpu creation time, making sure to call it after the vcpu is
> up (_VPF_down has been cleared).

While it's works perfectly on common case, as the toolstack is always
setting VGCF_online.

It would be possible to call the hypercall DOMCTL_vcpusetcontext without
this flags enable. If so, the new VCPU will never receive event channel
interrupt.

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/domain.c       |    4 +++-
>  xen/arch/arm/domain_build.c |    2 ++
>  xen/arch/arm/vgic.c         |   18 ++++--------------
>  3 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index e20ba0b..c29b063 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -651,8 +651,10 @@ int arch_set_info_guest(
>      v->is_initialised = 1;
>  
>      if ( ctxt->flags & VGCF_online )
> +    {
>          clear_bit(_VPF_down, &v->pause_flags);
> -    else
> +        vgic_vcpu_inject_irq(v, v->domain->arch.evtchn_irq);

I'd like a comment above each vgic_vcpu_inject(v, evtchn_irq) to explain
why we need them.

So in the future we won't need to spend hours to search in log because
someone has moved the line.

> +    } else

Coding style:

else
{

Regards,

-- 
Julien Grall