All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
@ 2016-01-19 11:49 Christoffer Dall
  2016-01-19 12:37 ` Andrew Jones
  2016-01-19 15:21 ` Peter Maydell
  0 siblings, 2 replies; 14+ messages in thread
From: Christoffer Dall @ 2016-01-19 11:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Christoffer Dall

The virt board has an arch timer, which is always on.  Emit the
"always-on" property to indicate to Linux that it can switch off the
periodic timer and reduces the amount of interrupts injected into a
guest.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 hw/arm/virt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 05f9087..265fe9a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -291,6 +291,7 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
         qemu_fdt_setprop_string(vbi->fdt, "/timer", "compatible",
                                 "arm,armv7-timer");
     }
+    qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
     qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
                        GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
                        GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
-- 
2.1.2.330.g565301e.dirty

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 11:49 [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer Christoffer Dall
@ 2016-01-19 12:37 ` Andrew Jones
  2016-01-19 12:43   ` Christoffer Dall
  2016-01-19 15:21 ` Peter Maydell
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2016-01-19 12:37 UTC (permalink / raw)
  To: Christoffer Dall; +Cc: qemu-devel

On Tue, Jan 19, 2016 at 12:49:18PM +0100, Christoffer Dall wrote:
> The virt board has an arch timer, which is always on.  Emit the
> "always-on" property to indicate to Linux that it can switch off the
> periodic timer and reduces the amount of interrupts injected into a
> guest.
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 05f9087..265fe9a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -291,6 +291,7 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
>          qemu_fdt_setprop_string(vbi->fdt, "/timer", "compatible",
>                                  "arm,armv7-timer");
>      }
> +    qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
>      qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
> -- 
> 2.1.2.330.g565301e.dirty
> 
>

Hi Christoffer,

We should also patch the ACPI generation at the same time. I think
something like

 - gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
 + gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;

should do it.

Also, having the guest reduce the number of interrupts sounds good. Can
you point me to something to read about how/why a guest may choose to do
that, and what the trade-offs are?

Thanks,
drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 12:37 ` Andrew Jones
@ 2016-01-19 12:43   ` Christoffer Dall
  2016-01-19 13:32     ` Andrew Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Christoffer Dall @ 2016-01-19 12:43 UTC (permalink / raw)
  To: Andrew Jones; +Cc: qemu-devel

On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
> On Tue, Jan 19, 2016 at 12:49:18PM +0100, Christoffer Dall wrote:
> > The virt board has an arch timer, which is always on.  Emit the
> > "always-on" property to indicate to Linux that it can switch off the
> > periodic timer and reduces the amount of interrupts injected into a
> > guest.
> > 
> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > ---
> >  hw/arm/virt.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 05f9087..265fe9a 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -291,6 +291,7 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
> >          qemu_fdt_setprop_string(vbi->fdt, "/timer", "compatible",
> >                                  "arm,armv7-timer");
> >      }
> > +    qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
> >      qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
> >                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
> >                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
> > -- 
> > 2.1.2.330.g565301e.dirty
> > 
> >
> 
> Hi Christoffer,
> 
> We should also patch the ACPI generation at the same time. I think
> something like
> 
>  - gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
>  + gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;

I'm really not familiar enough with ACPI to be comfortable writing code
for this or testing this.

But if someone can pick this up and add the ACPI bits or can post a
follow-up patch, then I'm all for it :)

> 
> should do it.
> 
> Also, having the guest reduce the number of interrupts sounds good. Can
> you point me to something to read about how/why a guest may choose to do
> that, and what the trade-offs are?
> 
Not really, but you can ask Marc.

-Christoffer

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 12:43   ` Christoffer Dall
@ 2016-01-19 13:32     ` Andrew Jones
  2016-01-19 13:43       ` Marc Zyngier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2016-01-19 13:32 UTC (permalink / raw)
  To: Christoffer Dall; +Cc: marc.zyngier, qemu-devel

On Tue, Jan 19, 2016 at 01:43:41PM +0100, Christoffer Dall wrote:
> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
> > On Tue, Jan 19, 2016 at 12:49:18PM +0100, Christoffer Dall wrote:
> > > The virt board has an arch timer, which is always on.  Emit the
> > > "always-on" property to indicate to Linux that it can switch off the
> > > periodic timer and reduces the amount of interrupts injected into a
> > > guest.
> > > 
> > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > > ---
> > >  hw/arm/virt.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 05f9087..265fe9a 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -291,6 +291,7 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
> > >          qemu_fdt_setprop_string(vbi->fdt, "/timer", "compatible",
> > >                                  "arm,armv7-timer");
> > >      }
> > > +    qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
> > >      qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
> > >                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
> > >                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
> > > -- 
> > > 2.1.2.330.g565301e.dirty
> > > 
> > >
> > 
> > Hi Christoffer,
> > 
> > We should also patch the ACPI generation at the same time. I think
> > something like
> > 
> >  - gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
> >  + gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;
> 
> I'm really not familiar enough with ACPI to be comfortable writing code
> for this or testing this.
> 
> But if someone can pick this up and add the ACPI bits or can post a
> follow-up patch, then I'm all for it :)

I can post a follow-up patch.

> 
> > 
> > should do it.
> > 
> > Also, having the guest reduce the number of interrupts sounds good. Can
> > you point me to something to read about how/why a guest may choose to do
> > that, and what the trade-offs are?
> > 
> Not really, but you can ask Marc.

OK, CCing him. One thing I see is that without this change we're
currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
it's not true. Having that set may disable the oneshot capabilityj
necessary to switch to nohz mode? I'll just stop there with my
speculation though, so Marc won't have to correct too much...

drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 13:32     ` Andrew Jones
@ 2016-01-19 13:43       ` Marc Zyngier
  2016-01-19 14:07         ` Andrew Jones
  2016-01-19 18:48         ` Andrew Jones
  0 siblings, 2 replies; 14+ messages in thread
From: Marc Zyngier @ 2016-01-19 13:43 UTC (permalink / raw)
  To: Andrew Jones, Christoffer Dall; +Cc: qemu-devel

On 19/01/16 13:32, Andrew Jones wrote:
> On Tue, Jan 19, 2016 at 01:43:41PM +0100, Christoffer Dall wrote:
>> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
>>> On Tue, Jan 19, 2016 at 12:49:18PM +0100, Christoffer Dall wrote:
>>>> The virt board has an arch timer, which is always on.  Emit the
>>>> "always-on" property to indicate to Linux that it can switch off the
>>>> periodic timer and reduces the amount of interrupts injected into a
>>>> guest.
>>>>
>>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>>>> ---
>>>>  hw/arm/virt.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>>> index 05f9087..265fe9a 100644
>>>> --- a/hw/arm/virt.c
>>>> +++ b/hw/arm/virt.c
>>>> @@ -291,6 +291,7 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
>>>>          qemu_fdt_setprop_string(vbi->fdt, "/timer", "compatible",
>>>>                                  "arm,armv7-timer");
>>>>      }
>>>> +    qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
>>>>      qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
>>>>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
>>>>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
>>>> -- 
>>>> 2.1.2.330.g565301e.dirty
>>>>
>>>>
>>>
>>> Hi Christoffer,
>>>
>>> We should also patch the ACPI generation at the same time. I think
>>> something like
>>>
>>>  - gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
>>>  + gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;
>>
>> I'm really not familiar enough with ACPI to be comfortable writing code
>> for this or testing this.
>>
>> But if someone can pick this up and add the ACPI bits or can post a
>> follow-up patch, then I'm all for it :)
> 
> I can post a follow-up patch.
> 
>>
>>>
>>> should do it.
>>>
>>> Also, having the guest reduce the number of interrupts sounds good. Can
>>> you point me to something to read about how/why a guest may choose to do
>>> that, and what the trade-offs are?
>>>
>> Not really, but you can ask Marc.
> 
> OK, CCing him. One thing I see is that without this change we're
> currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
> it's not true. Having that set may disable the oneshot capabilityj
> necessary to switch to nohz mode? I'll just stop there with my
> speculation though, so Marc won't have to correct too much...

You're spot on. See 82a5619 in the kernel tree. When I did a similar
change in kvmtool, I saw a massive reduction in the number of timer
interrupts injected (specially when the number of vcpu is relatively high).

This also have interesting benefits when running on a model, where
you're trying to squeeze the last bits of "performance" from the monster...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 13:43       ` Marc Zyngier
@ 2016-01-19 14:07         ` Andrew Jones
  2016-01-19 18:48         ` Andrew Jones
  1 sibling, 0 replies; 14+ messages in thread
From: Andrew Jones @ 2016-01-19 14:07 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: qemu-devel, Christoffer Dall

On Tue, Jan 19, 2016 at 01:43:07PM +0000, Marc Zyngier wrote:
> On 19/01/16 13:32, Andrew Jones wrote:
> > On Tue, Jan 19, 2016 at 01:43:41PM +0100, Christoffer Dall wrote:
> >> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
> >>> On Tue, Jan 19, 2016 at 12:49:18PM +0100, Christoffer Dall wrote:
> >>>> The virt board has an arch timer, which is always on.  Emit the
> >>>> "always-on" property to indicate to Linux that it can switch off the
> >>>> periodic timer and reduces the amount of interrupts injected into a
> >>>> guest.
> >>>>
> >>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >>>> ---
> >>>>  hw/arm/virt.c | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >>>> index 05f9087..265fe9a 100644
> >>>> --- a/hw/arm/virt.c
> >>>> +++ b/hw/arm/virt.c
> >>>> @@ -291,6 +291,7 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
> >>>>          qemu_fdt_setprop_string(vbi->fdt, "/timer", "compatible",
> >>>>                                  "arm,armv7-timer");
> >>>>      }
> >>>> +    qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
> >>>>      qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
> >>>>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
> >>>>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
> >>>> -- 
> >>>> 2.1.2.330.g565301e.dirty
> >>>>
> >>>>
> >>>
> >>> Hi Christoffer,
> >>>
> >>> We should also patch the ACPI generation at the same time. I think
> >>> something like
> >>>
> >>>  - gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
> >>>  + gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;
> >>
> >> I'm really not familiar enough with ACPI to be comfortable writing code
> >> for this or testing this.
> >>
> >> But if someone can pick this up and add the ACPI bits or can post a
> >> follow-up patch, then I'm all for it :)
> > 
> > I can post a follow-up patch.
> > 
> >>
> >>>
> >>> should do it.
> >>>
> >>> Also, having the guest reduce the number of interrupts sounds good. Can
> >>> you point me to something to read about how/why a guest may choose to do
> >>> that, and what the trade-offs are?
> >>>
> >> Not really, but you can ask Marc.
> > 
> > OK, CCing him. One thing I see is that without this change we're
> > currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
> > it's not true. Having that set may disable the oneshot capabilityj
> > necessary to switch to nohz mode? I'll just stop there with my
> > speculation though, so Marc won't have to correct too much...
> 
> You're spot on. See 82a5619 in the kernel tree. When I did a similar
> change in kvmtool, I saw a massive reduction in the number of timer
> interrupts injected (specially when the number of vcpu is relatively high).
> 
> This also have interesting benefits when running on a model, where
> you're trying to squeeze the last bits of "performance" from the monster...
> 
> Thanks,

Thanks Marc! Christoffer, I'll create the ACPI patch, and do some pre-/post-
tracing to confirm the happy reduction in interrupts :-)

Also, as for this patch,

Reviewed-by: Andrew Jones <drjones@redhat.com>

drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 11:49 [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer Christoffer Dall
  2016-01-19 12:37 ` Andrew Jones
@ 2016-01-19 15:21 ` Peter Maydell
  1 sibling, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2016-01-19 15:21 UTC (permalink / raw)
  To: Christoffer Dall; +Cc: QEMU Developers

On 19 January 2016 at 11:49, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> The virt board has an arch timer, which is always on.  Emit the
> "always-on" property to indicate to Linux that it can switch off the
> periodic timer and reduces the amount of interrupts injected into a
> guest.
>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>



Applied to target-arm.next, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 13:43       ` Marc Zyngier
  2016-01-19 14:07         ` Andrew Jones
@ 2016-01-19 18:48         ` Andrew Jones
  2016-01-20 14:01           ` Andrew Jones
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2016-01-19 18:48 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: qemu-devel, Christoffer Dall

On Tue, Jan 19, 2016 at 01:43:07PM +0000, Marc Zyngier wrote:
> On 19/01/16 13:32, Andrew Jones wrote:
> > On Tue, Jan 19, 2016 at 01:43:41PM +0100, Christoffer Dall wrote:
> >> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
> >>> On Tue, Jan 19, 2016 at 12:49:18PM +0100, Christoffer Dall wrote:
> >>>> The virt board has an arch timer, which is always on.  Emit the
> >>>> "always-on" property to indicate to Linux that it can switch off the
> >>>> periodic timer and reduces the amount of interrupts injected into a
> >>>> guest.
> >>>>
> >>>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >>>> ---
> >>>>  hw/arm/virt.c | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >>>> index 05f9087..265fe9a 100644
> >>>> --- a/hw/arm/virt.c
> >>>> +++ b/hw/arm/virt.c
> >>>> @@ -291,6 +291,7 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
> >>>>          qemu_fdt_setprop_string(vbi->fdt, "/timer", "compatible",
> >>>>                                  "arm,armv7-timer");
> >>>>      }
> >>>> +    qemu_fdt_setprop(vbi->fdt, "/timer", "always-on", NULL, 0);
> >>>>      qemu_fdt_setprop_cells(vbi->fdt, "/timer", "interrupts",
> >>>>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_S_EL1_IRQ, irqflags,
> >>>>                         GIC_FDT_IRQ_TYPE_PPI, ARCH_TIMER_NS_EL1_IRQ, irqflags,
> >>>> -- 
> >>>> 2.1.2.330.g565301e.dirty
> >>>>
> >>>>
> >>>
> >>> Hi Christoffer,
> >>>
> >>> We should also patch the ACPI generation at the same time. I think
> >>> something like
> >>>
> >>>  - gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
> >>>  + gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;
> >>
> >> I'm really not familiar enough with ACPI to be comfortable writing code
> >> for this or testing this.
> >>
> >> But if someone can pick this up and add the ACPI bits or can post a
> >> follow-up patch, then I'm all for it :)
> > 
> > I can post a follow-up patch.
> > 
> >>
> >>>
> >>> should do it.
> >>>
> >>> Also, having the guest reduce the number of interrupts sounds good. Can
> >>> you point me to something to read about how/why a guest may choose to do
> >>> that, and what the trade-offs are?
> >>>
> >> Not really, but you can ask Marc.
> > 
> > OK, CCing him. One thing I see is that without this change we're
> > currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
> > it's not true. Having that set may disable the oneshot capabilityj
> > necessary to switch to nohz mode? I'll just stop there with my
> > speculation though, so Marc won't have to correct too much...
> 
> You're spot on. See 82a5619 in the kernel tree. When I did a similar
> change in kvmtool, I saw a massive reduction in the number of timer
> interrupts injected (specially when the number of vcpu is relatively high).
> 
> This also have interesting benefits when running on a model, where
> you're trying to squeeze the last bits of "performance" from the monster...
>

Hmm, I'm probably testing this wrong, but I don't see any difference in
the number of injected timer interrupts. My guest, which I boot with
UEFI, has 

CONFIG_ARM_ARCH_TIMER=y
CONFIG_ARM_ARCH_TIMER_EVTSTREAM=y
CONFIG_ARM_TIMER_SP804=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
CONFIG_NO_HZ=y
CONFIG_HZ_1000=y
CONFIG_HZ=1000

I've boot a guest using DT with and without this patch

---WITHOUT---

# ls /proc/device-tree/timer
compatible  interrupts  name
# cat /proc/interrupts                  
           CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
  3:       6958       5766       5166       5187       5576       5129 4695       4398       GIC  27 Edge      arch_timer
# sleep 120 && cat /proc/interrupts                  
           CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
  3:       7557       5986       5487       5265       6232       5868 5464       4438       GIC  27 Edge      arch_timer

---WITH---

# ls /proc/device-tree/timer
always-on  compatible  interrupts  name
# cat /proc/interrupts 
           CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
  3:       7005       6080       4996       5391       5165       5257 4930       4844       GIC  27 Edge      arch_timer
# sleep 120 && cat /proc/interrupts 
           CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
  3:       7523       6505       5264       6717       5273       5391 5526       4901       GIC  27 Edge      arch_timer



And kvm trace data has

---WITHOUT---
$ grep kvm_timer_update_irq trace.out | wc -l
94336
---WITH---
$ grep kvm_timer_update_irq trace.out | wc -l
95838


Any suggestions?

Thanks,
drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-19 18:48         ` Andrew Jones
@ 2016-01-20 14:01           ` Andrew Jones
  2016-01-20 14:28             ` Marc Zyngier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2016-01-20 14:01 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: qemu-devel, Christoffer Dall

On Tue, Jan 19, 2016 at 07:48:14PM +0100, Andrew Jones wrote:
> On Tue, Jan 19, 2016 at 01:43:07PM +0000, Marc Zyngier wrote:
> > >> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
> > > OK, CCing him. One thing I see is that without this change we're
> > > currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
> > > it's not true. Having that set may disable the oneshot capabilityj
> > > necessary to switch to nohz mode? I'll just stop there with my
> > > speculation though, so Marc won't have to correct too much...
> > 
> > You're spot on. See 82a5619 in the kernel tree. When I did a similar
> > change in kvmtool, I saw a massive reduction in the number of timer
> > interrupts injected (specially when the number of vcpu is relatively high).
> > 
> > This also have interesting benefits when running on a model, where
> > you're trying to squeeze the last bits of "performance" from the monster...
> >
> 
> Hmm, I'm probably testing this wrong, but I don't see any difference in
> the number of injected timer interrupts. My guest, which I boot with
> UEFI, has 
> 
> CONFIG_ARM_ARCH_TIMER=y
> CONFIG_ARM_ARCH_TIMER_EVTSTREAM=y
> CONFIG_ARM_TIMER_SP804=y
> CONFIG_HIGH_RES_TIMERS=y
> CONFIG_TICK_ONESHOT=y
> CONFIG_NO_HZ_COMMON=y
> # CONFIG_HZ_PERIODIC is not set
> CONFIG_NO_HZ_IDLE=y
> # CONFIG_NO_HZ_FULL is not set
> CONFIG_NO_HZ=y
> CONFIG_HZ_1000=y
> CONFIG_HZ=1000
> 
> I've boot a guest using DT with and without this patch
> 
> ---WITHOUT---
> 
> # ls /proc/device-tree/timer
> compatible  interrupts  name
> # cat /proc/interrupts                  
>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>   3:       6958       5766       5166       5187       5576       5129 4695       4398       GIC  27 Edge      arch_timer
> # sleep 120 && cat /proc/interrupts                  
>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>   3:       7557       5986       5487       5265       6232       5868 5464       4438       GIC  27 Edge      arch_timer
> 
> ---WITH---
> 
> # ls /proc/device-tree/timer
> always-on  compatible  interrupts  name
> # cat /proc/interrupts 
>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>   3:       7005       6080       4996       5391       5165       5257 4930       4844       GIC  27 Edge      arch_timer
> # sleep 120 && cat /proc/interrupts 
>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>   3:       7523       6505       5264       6717       5273       5391 5526       4901       GIC  27 Edge      arch_timer
> 
> 
> 
> And kvm trace data has
> 
> ---WITHOUT---
> $ grep kvm_timer_update_irq trace.out | wc -l
> 94336
> ---WITH---
> $ grep kvm_timer_update_irq trace.out | wc -l
> 95838
> 
>

Must be how I'm looking, because I just tried kvmtool with/without
Marc's patch that adds always-on, but don't see any reduction of
interrupts there either. I used a defconfig guest kernel. Also,
not that I think it should matter, but my host kernel is 4.4-rc4
based.

I'd like to be able to see a difference with/without this always-on
patch, not because I don't think we should take it anyway, but because
I need a test case for the ACPI counterpart.

Thanks,
drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-20 14:01           ` Andrew Jones
@ 2016-01-20 14:28             ` Marc Zyngier
  2016-01-20 15:06               ` Andrew Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2016-01-20 14:28 UTC (permalink / raw)
  To: Andrew Jones; +Cc: qemu-devel, Christoffer Dall

On 20/01/16 14:01, Andrew Jones wrote:
> On Tue, Jan 19, 2016 at 07:48:14PM +0100, Andrew Jones wrote:
>> On Tue, Jan 19, 2016 at 01:43:07PM +0000, Marc Zyngier wrote:
>>>>> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
>>>> OK, CCing him. One thing I see is that without this change we're
>>>> currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
>>>> it's not true. Having that set may disable the oneshot capabilityj
>>>> necessary to switch to nohz mode? I'll just stop there with my
>>>> speculation though, so Marc won't have to correct too much...
>>>
>>> You're spot on. See 82a5619 in the kernel tree. When I did a similar
>>> change in kvmtool, I saw a massive reduction in the number of timer
>>> interrupts injected (specially when the number of vcpu is relatively high).
>>>
>>> This also have interesting benefits when running on a model, where
>>> you're trying to squeeze the last bits of "performance" from the monster...
>>>
>>
>> Hmm, I'm probably testing this wrong, but I don't see any difference in
>> the number of injected timer interrupts. My guest, which I boot with
>> UEFI, has 
>>
>> CONFIG_ARM_ARCH_TIMER=y
>> CONFIG_ARM_ARCH_TIMER_EVTSTREAM=y
>> CONFIG_ARM_TIMER_SP804=y
>> CONFIG_HIGH_RES_TIMERS=y
>> CONFIG_TICK_ONESHOT=y
>> CONFIG_NO_HZ_COMMON=y
>> # CONFIG_HZ_PERIODIC is not set
>> CONFIG_NO_HZ_IDLE=y
>> # CONFIG_NO_HZ_FULL is not set
>> CONFIG_NO_HZ=y
>> CONFIG_HZ_1000=y
>> CONFIG_HZ=1000
>>
>> I've boot a guest using DT with and without this patch
>>
>> ---WITHOUT---
>>
>> # ls /proc/device-tree/timer
>> compatible  interrupts  name
>> # cat /proc/interrupts                  
>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>   3:       6958       5766       5166       5187       5576       5129 4695       4398       GIC  27 Edge      arch_timer
>> # sleep 120 && cat /proc/interrupts                  
>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>   3:       7557       5986       5487       5265       6232       5868 5464       4438       GIC  27 Edge      arch_timer
>>
>> ---WITH---
>>
>> # ls /proc/device-tree/timer
>> always-on  compatible  interrupts  name
>> # cat /proc/interrupts 
>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>   3:       7005       6080       4996       5391       5165       5257 4930       4844       GIC  27 Edge      arch_timer
>> # sleep 120 && cat /proc/interrupts 
>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>   3:       7523       6505       5264       6717       5273       5391 5526       4901       GIC  27 Edge      arch_timer
>>
>>
>>
>> And kvm trace data has
>>
>> ---WITHOUT---
>> $ grep kvm_timer_update_irq trace.out | wc -l
>> 94336
>> ---WITH---
>> $ grep kvm_timer_update_irq trace.out | wc -l
>> 95838
>>
>>
> 
> Must be how I'm looking, because I just tried kvmtool with/without
> Marc's patch that adds always-on, but don't see any reduction of
> interrupts there either. I used a defconfig guest kernel. Also,
> not that I think it should matter, but my host kernel is 4.4-rc4
> based.
> 
> I'd like to be able to see a difference with/without this always-on
> patch, not because I don't think we should take it anyway, but because
> I need a test case for the ACPI counterpart.

I just run a couple of quick tests, measuring interrupt rate (vmstat 1)
on the host, with one VM (2 vcpus) idling, and I'm seeing the following
thing:

Without "always-on": ~380 interrupts per second
With "always-on": ~40 interrupts per second

This is with kvmtool, 32bit host (but none of that is arch specific anyway).

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-20 14:28             ` Marc Zyngier
@ 2016-01-20 15:06               ` Andrew Jones
  2016-01-20 16:20                 ` Marc Zyngier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2016-01-20 15:06 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: qemu-devel, Christoffer Dall

On Wed, Jan 20, 2016 at 02:28:05PM +0000, Marc Zyngier wrote:
> On 20/01/16 14:01, Andrew Jones wrote:
> > On Tue, Jan 19, 2016 at 07:48:14PM +0100, Andrew Jones wrote:
> >> On Tue, Jan 19, 2016 at 01:43:07PM +0000, Marc Zyngier wrote:
> >>>>> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
> >>>> OK, CCing him. One thing I see is that without this change we're
> >>>> currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
> >>>> it's not true. Having that set may disable the oneshot capabilityj
> >>>> necessary to switch to nohz mode? I'll just stop there with my
> >>>> speculation though, so Marc won't have to correct too much...
> >>>
> >>> You're spot on. See 82a5619 in the kernel tree. When I did a similar
> >>> change in kvmtool, I saw a massive reduction in the number of timer
> >>> interrupts injected (specially when the number of vcpu is relatively high).
> >>>
> >>> This also have interesting benefits when running on a model, where
> >>> you're trying to squeeze the last bits of "performance" from the monster...
> >>>
> >>
> >> Hmm, I'm probably testing this wrong, but I don't see any difference in
> >> the number of injected timer interrupts. My guest, which I boot with
> >> UEFI, has 
> >>
> >> CONFIG_ARM_ARCH_TIMER=y
> >> CONFIG_ARM_ARCH_TIMER_EVTSTREAM=y
> >> CONFIG_ARM_TIMER_SP804=y
> >> CONFIG_HIGH_RES_TIMERS=y
> >> CONFIG_TICK_ONESHOT=y
> >> CONFIG_NO_HZ_COMMON=y
> >> # CONFIG_HZ_PERIODIC is not set
> >> CONFIG_NO_HZ_IDLE=y
> >> # CONFIG_NO_HZ_FULL is not set
> >> CONFIG_NO_HZ=y
> >> CONFIG_HZ_1000=y
> >> CONFIG_HZ=1000
> >>
> >> I've boot a guest using DT with and without this patch
> >>
> >> ---WITHOUT---
> >>
> >> # ls /proc/device-tree/timer
> >> compatible  interrupts  name
> >> # cat /proc/interrupts                  
> >>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>   3:       6958       5766       5166       5187       5576       5129 4695       4398       GIC  27 Edge      arch_timer
> >> # sleep 120 && cat /proc/interrupts                  
> >>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>   3:       7557       5986       5487       5265       6232       5868 5464       4438       GIC  27 Edge      arch_timer
> >>
> >> ---WITH---
> >>
> >> # ls /proc/device-tree/timer
> >> always-on  compatible  interrupts  name
> >> # cat /proc/interrupts 
> >>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>   3:       7005       6080       4996       5391       5165       5257 4930       4844       GIC  27 Edge      arch_timer
> >> # sleep 120 && cat /proc/interrupts 
> >>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>   3:       7523       6505       5264       6717       5273       5391 5526       4901       GIC  27 Edge      arch_timer
> >>
> >>
> >>
> >> And kvm trace data has
> >>
> >> ---WITHOUT---
> >> $ grep kvm_timer_update_irq trace.out | wc -l
> >> 94336
> >> ---WITH---
> >> $ grep kvm_timer_update_irq trace.out | wc -l
> >> 95838
> >>
> >>
> > 
> > Must be how I'm looking, because I just tried kvmtool with/without
> > Marc's patch that adds always-on, but don't see any reduction of
> > interrupts there either. I used a defconfig guest kernel. Also,
> > not that I think it should matter, but my host kernel is 4.4-rc4
> > based.
> > 
> > I'd like to be able to see a difference with/without this always-on
> > patch, not because I don't think we should take it anyway, but because
> > I need a test case for the ACPI counterpart.
> 
> I just run a couple of quick tests, measuring interrupt rate (vmstat 1)
> on the host, with one VM (2 vcpus) idling, and I'm seeing the following
> thing:
> 
> Without "always-on": ~380 interrupts per second
> With "always-on": ~40 interrupts per second
> 
> This is with kvmtool, 32bit host (but none of that is arch specific anyway).
>

For me (64bit host, one VM (8 vcpus)) of 100 'vmstat 1' samples I have the
following.

Without "always-on": mean=56.370 sd=33.404 min=1 max=244
With "always-on":    mean=51.580 sd=33.361 min=1 max=273

I'm also using kvmtool, and my guest is idle.

So a difference between 32 and 64bit hosts? Again, my guest config is
now just a defconfig. My host config is not, but I'm not sure what
options to look for other than what I wrote above, which are the same
for my host.

Thanks,
drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-20 15:06               ` Andrew Jones
@ 2016-01-20 16:20                 ` Marc Zyngier
  2016-01-20 16:47                   ` Andrew Jones
  0 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2016-01-20 16:20 UTC (permalink / raw)
  To: Andrew Jones; +Cc: Mark Rutland, qemu-devel, Christoffer Dall

On 20/01/16 15:06, Andrew Jones wrote:
> On Wed, Jan 20, 2016 at 02:28:05PM +0000, Marc Zyngier wrote:
>> On 20/01/16 14:01, Andrew Jones wrote:
>>> On Tue, Jan 19, 2016 at 07:48:14PM +0100, Andrew Jones wrote:
>>>> On Tue, Jan 19, 2016 at 01:43:07PM +0000, Marc Zyngier wrote:
>>>>>>> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
>>>>>> OK, CCing him. One thing I see is that without this change we're
>>>>>> currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
>>>>>> it's not true. Having that set may disable the oneshot capabilityj
>>>>>> necessary to switch to nohz mode? I'll just stop there with my
>>>>>> speculation though, so Marc won't have to correct too much...
>>>>>
>>>>> You're spot on. See 82a5619 in the kernel tree. When I did a similar
>>>>> change in kvmtool, I saw a massive reduction in the number of timer
>>>>> interrupts injected (specially when the number of vcpu is relatively high).
>>>>>
>>>>> This also have interesting benefits when running on a model, where
>>>>> you're trying to squeeze the last bits of "performance" from the monster...
>>>>>
>>>>
>>>> Hmm, I'm probably testing this wrong, but I don't see any difference in
>>>> the number of injected timer interrupts. My guest, which I boot with
>>>> UEFI, has 
>>>>
>>>> CONFIG_ARM_ARCH_TIMER=y
>>>> CONFIG_ARM_ARCH_TIMER_EVTSTREAM=y
>>>> CONFIG_ARM_TIMER_SP804=y
>>>> CONFIG_HIGH_RES_TIMERS=y
>>>> CONFIG_TICK_ONESHOT=y
>>>> CONFIG_NO_HZ_COMMON=y
>>>> # CONFIG_HZ_PERIODIC is not set
>>>> CONFIG_NO_HZ_IDLE=y
>>>> # CONFIG_NO_HZ_FULL is not set
>>>> CONFIG_NO_HZ=y
>>>> CONFIG_HZ_1000=y
>>>> CONFIG_HZ=1000
>>>>
>>>> I've boot a guest using DT with and without this patch
>>>>
>>>> ---WITHOUT---
>>>>
>>>> # ls /proc/device-tree/timer
>>>> compatible  interrupts  name
>>>> # cat /proc/interrupts                  
>>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>>>   3:       6958       5766       5166       5187       5576       5129 4695       4398       GIC  27 Edge      arch_timer
>>>> # sleep 120 && cat /proc/interrupts                  
>>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>>>   3:       7557       5986       5487       5265       6232       5868 5464       4438       GIC  27 Edge      arch_timer
>>>>
>>>> ---WITH---
>>>>
>>>> # ls /proc/device-tree/timer
>>>> always-on  compatible  interrupts  name
>>>> # cat /proc/interrupts 
>>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>>>   3:       7005       6080       4996       5391       5165       5257 4930       4844       GIC  27 Edge      arch_timer
>>>> # sleep 120 && cat /proc/interrupts 
>>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
>>>>   3:       7523       6505       5264       6717       5273       5391 5526       4901       GIC  27 Edge      arch_timer
>>>>
>>>>
>>>>
>>>> And kvm trace data has
>>>>
>>>> ---WITHOUT---
>>>> $ grep kvm_timer_update_irq trace.out | wc -l
>>>> 94336
>>>> ---WITH---
>>>> $ grep kvm_timer_update_irq trace.out | wc -l
>>>> 95838
>>>>
>>>>
>>>
>>> Must be how I'm looking, because I just tried kvmtool with/without
>>> Marc's patch that adds always-on, but don't see any reduction of
>>> interrupts there either. I used a defconfig guest kernel. Also,
>>> not that I think it should matter, but my host kernel is 4.4-rc4
>>> based.
>>>
>>> I'd like to be able to see a difference with/without this always-on
>>> patch, not because I don't think we should take it anyway, but because
>>> I need a test case for the ACPI counterpart.
>>
>> I just run a couple of quick tests, measuring interrupt rate (vmstat 1)
>> on the host, with one VM (2 vcpus) idling, and I'm seeing the following
>> thing:
>>
>> Without "always-on": ~380 interrupts per second
>> With "always-on": ~40 interrupts per second
>>
>> This is with kvmtool, 32bit host (but none of that is arch specific anyway).
>>
> 
> For me (64bit host, one VM (8 vcpus)) of 100 'vmstat 1' samples I have the
> following.
> 
> Without "always-on": mean=56.370 sd=33.404 min=1 max=244
> With "always-on":    mean=51.580 sd=33.361 min=1 max=273
> 
> I'm also using kvmtool, and my guest is idle.
> 
> So a difference between 32 and 64bit hosts? Again, my guest config is
> now just a defconfig. My host config is not, but I'm not sure what
> options to look for other than what I wrote above, which are the same
> for my host.

Just tried on Seattle with a 64bit guest, and there is hardly any
difference indeed. Both host and guest are "mostly" defconfig as well.
So there is a kernel configuration difference.

Running my 32bit guest on a 64bit host definitely shows a massive
difference (with 8 vcpus):

Without "always-on": ~1200 interrupts per second
With "always-on": ~50 interrupts per second

[Head scratching, poking Mark]

Right, I now know what is going on: The arm64 kernel uses
tick_setup_hrtimer_broadcast() so that it can still use the arch timer
as a broadcast timer (forcing one CPU to remain on), while the 32bit
kernel relies on the presence of a backup timer (sp804 anyone?) or the
guarantee that the timer cannot go away (always-on).

This is probably why I'm seeing such a gain with a 32bit guest, and none
with a 64bit guest (the kernel already does the right thing). As to why
there is such a difference between the two architectures, this is a
story for another day...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-20 16:20                 ` Marc Zyngier
@ 2016-01-20 16:47                   ` Andrew Jones
  2016-01-20 17:08                     ` Marc Zyngier
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2016-01-20 16:47 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Mark Rutland, qemu-devel, Christoffer Dall

On Wed, Jan 20, 2016 at 04:20:03PM +0000, Marc Zyngier wrote:
> On 20/01/16 15:06, Andrew Jones wrote:
> > On Wed, Jan 20, 2016 at 02:28:05PM +0000, Marc Zyngier wrote:
> >> On 20/01/16 14:01, Andrew Jones wrote:
> >>> On Tue, Jan 19, 2016 at 07:48:14PM +0100, Andrew Jones wrote:
> >>>> On Tue, Jan 19, 2016 at 01:43:07PM +0000, Marc Zyngier wrote:
> >>>>>>> On Tue, Jan 19, 2016 at 01:37:16PM +0100, Andrew Jones wrote:
> >>>>>> OK, CCing him. One thing I see is that without this change we're
> >>>>>> currently setting the clock feature CLOCK_EVT_FEAT_C3STOP, even though
> >>>>>> it's not true. Having that set may disable the oneshot capabilityj
> >>>>>> necessary to switch to nohz mode? I'll just stop there with my
> >>>>>> speculation though, so Marc won't have to correct too much...
> >>>>>
> >>>>> You're spot on. See 82a5619 in the kernel tree. When I did a similar
> >>>>> change in kvmtool, I saw a massive reduction in the number of timer
> >>>>> interrupts injected (specially when the number of vcpu is relatively high).
> >>>>>
> >>>>> This also have interesting benefits when running on a model, where
> >>>>> you're trying to squeeze the last bits of "performance" from the monster...
> >>>>>
> >>>>
> >>>> Hmm, I'm probably testing this wrong, but I don't see any difference in
> >>>> the number of injected timer interrupts. My guest, which I boot with
> >>>> UEFI, has 
> >>>>
> >>>> CONFIG_ARM_ARCH_TIMER=y
> >>>> CONFIG_ARM_ARCH_TIMER_EVTSTREAM=y
> >>>> CONFIG_ARM_TIMER_SP804=y
> >>>> CONFIG_HIGH_RES_TIMERS=y
> >>>> CONFIG_TICK_ONESHOT=y
> >>>> CONFIG_NO_HZ_COMMON=y
> >>>> # CONFIG_HZ_PERIODIC is not set
> >>>> CONFIG_NO_HZ_IDLE=y
> >>>> # CONFIG_NO_HZ_FULL is not set
> >>>> CONFIG_NO_HZ=y
> >>>> CONFIG_HZ_1000=y
> >>>> CONFIG_HZ=1000
> >>>>
> >>>> I've boot a guest using DT with and without this patch
> >>>>
> >>>> ---WITHOUT---
> >>>>
> >>>> # ls /proc/device-tree/timer
> >>>> compatible  interrupts  name
> >>>> # cat /proc/interrupts                  
> >>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>>>   3:       6958       5766       5166       5187       5576       5129 4695       4398       GIC  27 Edge      arch_timer
> >>>> # sleep 120 && cat /proc/interrupts                  
> >>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>>>   3:       7557       5986       5487       5265       6232       5868 5464       4438       GIC  27 Edge      arch_timer
> >>>>
> >>>> ---WITH---
> >>>>
> >>>> # ls /proc/device-tree/timer
> >>>> always-on  compatible  interrupts  name
> >>>> # cat /proc/interrupts 
> >>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>>>   3:       7005       6080       4996       5391       5165       5257 4930       4844       GIC  27 Edge      arch_timer
> >>>> # sleep 120 && cat /proc/interrupts 
> >>>>            CPU0       CPU1       CPU2       CPU3       CPU4       CPU5 CPU6       CPU7
> >>>>   3:       7523       6505       5264       6717       5273       5391 5526       4901       GIC  27 Edge      arch_timer
> >>>>
> >>>>
> >>>>
> >>>> And kvm trace data has
> >>>>
> >>>> ---WITHOUT---
> >>>> $ grep kvm_timer_update_irq trace.out | wc -l
> >>>> 94336
> >>>> ---WITH---
> >>>> $ grep kvm_timer_update_irq trace.out | wc -l
> >>>> 95838
> >>>>
> >>>>
> >>>
> >>> Must be how I'm looking, because I just tried kvmtool with/without
> >>> Marc's patch that adds always-on, but don't see any reduction of
> >>> interrupts there either. I used a defconfig guest kernel. Also,
> >>> not that I think it should matter, but my host kernel is 4.4-rc4
> >>> based.
> >>>
> >>> I'd like to be able to see a difference with/without this always-on
> >>> patch, not because I don't think we should take it anyway, but because
> >>> I need a test case for the ACPI counterpart.
> >>
> >> I just run a couple of quick tests, measuring interrupt rate (vmstat 1)
> >> on the host, with one VM (2 vcpus) idling, and I'm seeing the following
> >> thing:
> >>
> >> Without "always-on": ~380 interrupts per second
> >> With "always-on": ~40 interrupts per second
> >>
> >> This is with kvmtool, 32bit host (but none of that is arch specific anyway).
> >>
> > 
> > For me (64bit host, one VM (8 vcpus)) of 100 'vmstat 1' samples I have the
> > following.
> > 
> > Without "always-on": mean=56.370 sd=33.404 min=1 max=244
> > With "always-on":    mean=51.580 sd=33.361 min=1 max=273
> > 
> > I'm also using kvmtool, and my guest is idle.
> > 
> > So a difference between 32 and 64bit hosts? Again, my guest config is
> > now just a defconfig. My host config is not, but I'm not sure what
> > options to look for other than what I wrote above, which are the same
> > for my host.
> 
> Just tried on Seattle with a 64bit guest, and there is hardly any
> difference indeed. Both host and guest are "mostly" defconfig as well.
> So there is a kernel configuration difference.
> 
> Running my 32bit guest on a 64bit host definitely shows a massive
> difference (with 8 vcpus):
> 
> Without "always-on": ~1200 interrupts per second
> With "always-on": ~50 interrupts per second
> 
> [Head scratching, poking Mark]
> 
> Right, I now know what is going on: The arm64 kernel uses
> tick_setup_hrtimer_broadcast() so that it can still use the arch timer
> as a broadcast timer (forcing one CPU to remain on), while the 32bit
> kernel relies on the presence of a backup timer (sp804 anyone?) or the
> guarantee that the timer cannot go away (always-on).
> 
> This is probably why I'm seeing such a gain with a 32bit guest, and none
> with a 64bit guest (the kernel already does the right thing). As to why
> there is such a difference between the two architectures, this is a
> story for another day...
>

Thanks Marc! I just confirmed with an AArch32 guest using QEMU, and the
patch we've hijacked for this thread. Without the patch I get a megaton
of interrupts (~14000/s). With the patch, after letting the guest chill
for a while, I'm getting ~150/s (8 vcpus).

I guess I don't have a test case for the ACPI code though. afaik we only
have UEFI for AArch64 guests, and we don't have ACPI boot without UEFI.
Or maybe I can hack time_init to remove the tick_setup_hrtimer_broadcast
call?

drew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer
  2016-01-20 16:47                   ` Andrew Jones
@ 2016-01-20 17:08                     ` Marc Zyngier
  0 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2016-01-20 17:08 UTC (permalink / raw)
  To: Andrew Jones; +Cc: Mark Rutland, qemu-devel, Christoffer Dall

On 20/01/16 16:47, Andrew Jones wrote:
> On Wed, Jan 20, 2016 at 04:20:03PM +0000, Marc Zyngier wrote:
>> Just tried on Seattle with a 64bit guest, and there is hardly any
>> difference indeed. Both host and guest are "mostly" defconfig as well.
>> So there is a kernel configuration difference.
>>
>> Running my 32bit guest on a 64bit host definitely shows a massive
>> difference (with 8 vcpus):
>>
>> Without "always-on": ~1200 interrupts per second
>> With "always-on": ~50 interrupts per second
>>
>> [Head scratching, poking Mark]
>>
>> Right, I now know what is going on: The arm64 kernel uses
>> tick_setup_hrtimer_broadcast() so that it can still use the arch timer
>> as a broadcast timer (forcing one CPU to remain on), while the 32bit
>> kernel relies on the presence of a backup timer (sp804 anyone?) or the
>> guarantee that the timer cannot go away (always-on).
>>
>> This is probably why I'm seeing such a gain with a 32bit guest, and none
>> with a 64bit guest (the kernel already does the right thing). As to why
>> there is such a difference between the two architectures, this is a
>> story for another day...
>>
> 
> Thanks Marc! I just confirmed with an AArch32 guest using QEMU, and the
> patch we've hijacked for this thread. Without the patch I get a megaton
> of interrupts (~14000/s). With the patch, after letting the guest chill
> for a while, I'm getting ~150/s (8 vcpus).
> 
> I guess I don't have a test case for the ACPI code though. afaik we only
> have UEFI for AArch64 guests, and we don't have ACPI boot without UEFI.
> Or maybe I can hack time_init to remove the tick_setup_hrtimer_broadcast
> call?

Yeah, that should work.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-01-20 17:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-19 11:49 [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer Christoffer Dall
2016-01-19 12:37 ` Andrew Jones
2016-01-19 12:43   ` Christoffer Dall
2016-01-19 13:32     ` Andrew Jones
2016-01-19 13:43       ` Marc Zyngier
2016-01-19 14:07         ` Andrew Jones
2016-01-19 18:48         ` Andrew Jones
2016-01-20 14:01           ` Andrew Jones
2016-01-20 14:28             ` Marc Zyngier
2016-01-20 15:06               ` Andrew Jones
2016-01-20 16:20                 ` Marc Zyngier
2016-01-20 16:47                   ` Andrew Jones
2016-01-20 17:08                     ` Marc Zyngier
2016-01-19 15:21 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.