diff mbox

[v2] xen/arm: introduce vwfi parameter

Message ID 1487784587-16380-1-git-send-email-sstabellini@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stefano Stabellini Feb. 22, 2017, 5:29 p.m. UTC
Introduce new Xen command line parameter called "vwfi", which stands for
virtual wfi. The default is "trap": Xen traps the guest wfi instruction
and calls vcpu_block on the guest vcpu. The behavior can be changed
setting vwfi to "native", in that case Xen doesn't trap the instruction
at all, running it in guest context.

The result is strong reduction in irq latency (from 5000ns to 2000ns,
measured using https://github.com/edgarigl/tbm, the physical timer, and
1 pcpu dedicated to 1 vcpu). The downside is that the scheduler thinks
that the guest is busy when actually is sleeping, leading to suboptimal
scheduling decisions.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
CC: dario.faggioli@citrix.com
CC: george.dunlap@citrix.com
CC: edgar.iglesias@xilinx.com
---
 docs/misc/xen-command-line.markdown | 12 ++++++++++++
 xen/arch/arm/traps.c                | 17 +++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

Comments

Edgar E. Iglesias Feb. 23, 2017, 9:25 a.m. UTC | #1
On Wed, Feb 22, 2017 at 09:29:47AM -0800, Stefano Stabellini wrote:
> Introduce new Xen command line parameter called "vwfi", which stands for
> virtual wfi. The default is "trap": Xen traps the guest wfi instruction
> and calls vcpu_block on the guest vcpu. The behavior can be changed
> setting vwfi to "native", in that case Xen doesn't trap the instruction
> at all, running it in guest context.
> 
> The result is strong reduction in irq latency (from 5000ns to 2000ns,
> measured using https://github.com/edgarigl/tbm, the physical timer, and
> 1 pcpu dedicated to 1 vcpu). The downside is that the scheduler thinks
> that the guest is busy when actually is sleeping, leading to suboptimal
> scheduling decisions.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: dario.faggioli@citrix.com
> CC: george.dunlap@citrix.com
> CC: edgar.iglesias@xilinx.com


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  docs/misc/xen-command-line.markdown | 12 ++++++++++++
>  xen/arch/arm/traps.c                | 17 +++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index c0106fb..6dea172 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1638,6 +1638,18 @@ Note that if **watchdog** option is also specified vpmu will be turned off.
>  As the virtualisation is not 100% safe, don't use the vpmu flag on
>  production systems (see http://xenbits.xen.org/xsa/advisory-163.html)!
>  
> +### vwfi
> +> `= trap | native
> +
> +> Default: `trap`
> +
> +WFI is the ARM instruction to "wait for interrupt". This option, which
> +is ARM specific, changes the way guest WFI is implemented in Xen. By
> +default, Xen traps the instruction, then blocks the guest vcpu. When setting
> +vwfi to `native`, Xen doesn't trap the instruction, running it in guest
> +context. Setting vwfi to `native` reduces irq latency, but leads to
> +suboptimal scheduling decisions.
> +
>  ### watchdog
>  > `= force | <boolean>`
>  
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 4f96a2b..3a40717 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -102,6 +102,19 @@ static int debug_stack_lines = 40;
>  
>  integer_param("debug_stack_lines", debug_stack_lines);
>  
> +static enum {
> +	TRAP,
> +	NATIVE,
> +} vwfi;
> +
> +static void __init parse_vwfi(const char *s)
> +{
> +	if ( !strcmp(s, "native") )
> +		vwfi = NATIVE;
> +	else
> +		vwfi = TRAP;
> +}
> +custom_param("vwfi", parse_vwfi);
>  
>  void init_traps(void)
>  {
> @@ -128,8 +141,8 @@ void init_traps(void)
>  
>      /* Setup hypervisor traps */
>      WRITE_SYSREG(HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
> -                 HCR_TWE|HCR_TWI|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB,
> -                 HCR_EL2);
> +                 (vwfi != NATIVE ? HCR_TWI : 0) |HCR_TWE|
> +                 HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB,HCR_EL2);
>      isb();
>  }
>  
> -- 
> 1.9.1
>
Julien Grall Feb. 28, 2017, 3:44 p.m. UTC | #2
Hi Stefano,

On 22/02/17 17:29, Stefano Stabellini wrote:
> Introduce new Xen command line parameter called "vwfi", which stands for
> virtual wfi. The default is "trap": Xen traps the guest wfi instruction
> and calls vcpu_block on the guest vcpu. The behavior can be changed
> setting vwfi to "native", in that case Xen doesn't trap the instruction
> at all, running it in guest context.
>
> The result is strong reduction in irq latency (from 5000ns to 2000ns,
> measured using https://github.com/edgarigl/tbm, the physical timer, and
> 1 pcpu dedicated to 1 vcpu). The downside is that the scheduler thinks
> that the guest is busy when actually is sleeping, leading to suboptimal
> scheduling decisions.

This patch is reducing WFI latency and I think we should have similar 
option for WFE.

WFE will be used for instance when a processor is trying to acquire a 
lock already taken and wait until an event is received. So you want to a 
good latency here too.

We should probably have a single option handling the both. I am not sure 
whether it is worth to allow native for one but not the other.

Cheers,
Stefano Stabellini Feb. 28, 2017, 8:58 p.m. UTC | #3
On Tue, 28 Feb 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 22/02/17 17:29, Stefano Stabellini wrote:
> > Introduce new Xen command line parameter called "vwfi", which stands for
> > virtual wfi. The default is "trap": Xen traps the guest wfi instruction
> > and calls vcpu_block on the guest vcpu. The behavior can be changed
> > setting vwfi to "native", in that case Xen doesn't trap the instruction
> > at all, running it in guest context.
> > 
> > The result is strong reduction in irq latency (from 5000ns to 2000ns,
> > measured using https://github.com/edgarigl/tbm, the physical timer, and
> > 1 pcpu dedicated to 1 vcpu). The downside is that the scheduler thinks
> > that the guest is busy when actually is sleeping, leading to suboptimal
> > scheduling decisions.
> 
> This patch is reducing WFI latency and I think we should have similar option
> for WFE.
> 
> WFE will be used for instance when a processor is trying to acquire a lock
> already taken and wait until an event is received. So you want to a good
> latency here too.
> 
> We should probably have a single option handling the both. I am not sure
> whether it is worth to allow native for one but not the other.

The reason why I didn't do the same for WFE is that WFE is already
implemented with vcpu_yield which has good performance, and doesn't
cause the idle vcpu to be scheduled.

That said, it is true that if the user wants WFI not to be trapped, she
is probably going to also want WFE not to be trapped, because the
use-case is the same. There is not much point in trapping WFE in Xen,
then go back to the same vcpu.

I'll keep the same command line option name "vwfi".
diff mbox

Patch

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index c0106fb..6dea172 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1638,6 +1638,18 @@  Note that if **watchdog** option is also specified vpmu will be turned off.
 As the virtualisation is not 100% safe, don't use the vpmu flag on
 production systems (see http://xenbits.xen.org/xsa/advisory-163.html)!
 
+### vwfi
+> `= trap | native
+
+> Default: `trap`
+
+WFI is the ARM instruction to "wait for interrupt". This option, which
+is ARM specific, changes the way guest WFI is implemented in Xen. By
+default, Xen traps the instruction, then blocks the guest vcpu. When setting
+vwfi to `native`, Xen doesn't trap the instruction, running it in guest
+context. Setting vwfi to `native` reduces irq latency, but leads to
+suboptimal scheduling decisions.
+
 ### watchdog
 > `= force | <boolean>`
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 4f96a2b..3a40717 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -102,6 +102,19 @@  static int debug_stack_lines = 40;
 
 integer_param("debug_stack_lines", debug_stack_lines);
 
+static enum {
+	TRAP,
+	NATIVE,
+} vwfi;
+
+static void __init parse_vwfi(const char *s)
+{
+	if ( !strcmp(s, "native") )
+		vwfi = NATIVE;
+	else
+		vwfi = TRAP;
+}
+custom_param("vwfi", parse_vwfi);
 
 void init_traps(void)
 {
@@ -128,8 +141,8 @@  void init_traps(void)
 
     /* Setup hypervisor traps */
     WRITE_SYSREG(HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
-                 HCR_TWE|HCR_TWI|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB,
-                 HCR_EL2);
+                 (vwfi != NATIVE ? HCR_TWI : 0) |HCR_TWE|
+                 HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB,HCR_EL2);
     isb();
 }