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

[Andrew Jones <drjones@redhat.com>]

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