Re: [Xen-devel] [RFC 14/16] hack: arm/domain: simplify context restore from idle vcpu

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrii,

On 28/11/2018 21:32, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@xxxxxxxx>
>
> Simplify context restore from idle vcpu to the one ran before it.
> This improves low cpu load but high irq rate use-cases.
While I agree that the context switch today is pretty inefficient, I would be 
interest to know how much you improve it.
Also, it would be nice if you can explain why you improve it that way. Why do 
you only need to restore the timer and gic?
Lastly, there are code coming up that will require p2m_save_store and 
p2m_restore_state to work in pair (see Cortex A76 erratum [1]). So that solution 
may not work very well in all the cases.
Overall, I think you can save much more in the context switch than what you 
currently do. For instance, if you are switch from vCPU A to idle vCPU you don't 
need to save the context.
You would only do it if after the idle vCPU you are scheduling a different vCPU 
or the vCPU is been migrated.
Another place of optimization when switching between 2 non-idle vCPU is 
save/restore of the VFP. You can avoid restoring them and instead trap it when 
the guest is using it.
A last place is the number of isb scattered over the context switch code. A lot 
of them are unnecessary at all because the system registers asssociated to 
EL1/EL0 are out-of-context.
I know that Stefano has been working on some patches. Stefano would it be 
possible to share your work?
Cheers,

[1] https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg03241.html

> Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
> ---
>   xen/arch/arm/domain.c   | 21 +++++++++++----------
>   xen/include/xen/sched.h |  1 +
>   2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 1d926dc..8e886b7 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -187,9 +187,6 @@ static void ctxt_switch_to(struct vcpu *n)
>       WRITE_SYSREG32(vpidr, VPIDR_EL2);
>       WRITE_SYSREG(n->arch.vmpidr, VMPIDR_EL2);
>   
> -    /* VGIC */
> -    gic_restore_state(n);
> -
>       /* VFP */
>       vfp_restore_state(n);
>   
> @@ -263,11 +260,6 @@ static void ctxt_switch_to(struct vcpu *n)
>       WRITE_SYSREG(n->arch.csselr, CSSELR_EL1);
>   
>       isb();
> -
> -    /* This is could trigger an hardware interrupt from the virtual
> -     * timer. The interrupt needs to be injected into the guest. */
> -    WRITE_SYSREG32(n->arch.cntkctl, CNTKCTL_EL1);
> -    virt_timer_restore(n);
>   }
>   
>   /* Update per-VCPU guest runstate shared memory area (if registered). */
> @@ -302,8 +294,17 @@ static void update_runstate_area(struct vcpu *v)
>   static void schedule_tail(struct vcpu *prev)
>   {
>       ctxt_switch_from(prev);
> -
> -    ctxt_switch_to(current);
> +    if ( !(is_idle_vcpu(prev) && (prev->prev == current)) )
> +        ctxt_switch_to(current);
> +    /* VGIC */
> +    if ( !is_idle_vcpu(current) )
> +    {
> +        gic_restore_state(current);
> +        WRITE_SYSREG32(current->arch.cntkctl, CNTKCTL_EL1);
> +        virt_timer_restore(current);
> +    }
> +    else
> +        current->prev = prev;
>   
>       local_irq_enable();
>   
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 0309c1f..e85108d 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -272,6 +272,7 @@ struct vcpu
>       struct vpci_vcpu vpci;
>   
>       struct arch_vcpu arch;
> +    struct vcpu *prev;
>   };
>   
>   /* Per-domain lock can be recursively acquired in fault handlers. */
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel