Re: [Qemu-devel] [PATCH] hw/arm/virt: Add always-on property to the virt board timer

[Marc Zyngier <marc.zyngier@arm.com>]

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...