[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 10/21] aspeed/timer: Provide back-pressure in

From: Cédric Le Goater
Subject: Re: [Qemu-devel] [PATCH v2 10/21] aspeed/timer: Provide back-pressure information for short periods
Date: Wed, 3 Jul 2019 10:53:27 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2

On 01/07/2019 14:59, Peter Maydell wrote:
> On Tue, 18 Jun 2019 at 17:54, Cédric Le Goater <address@hidden> wrote:
>> From: Andrew Jeffery <address@hidden>
>> First up: This is not the way the hardware behaves.
>> However, it helps resolve real-world problems with short periods being
>> used under Linux. Commit 4451d3f59f2a ("clocksource/drivers/fttmr010:
>> Fix set_next_event handler") in Linux fixed the timer driver to
>> correctly schedule the next event for the Aspeed controller, and in
>> combination with 5daa8212c08e ("ARM: dts: aspeed: Describe random number
>> device") Linux will now set a timer with a period as low as 1us.
>> Configuring a qemu timer with such a short period results in spending
>> time handling the interrupt in the model rather than executing guest
>> code, leading to noticeable "sticky" behaviour in the guest.
>> The behaviour of Linux is correct with respect to the hardware, so we
>> need to improve our handling under emulation. The approach chosen is to
>> provide back-pressure information by calculating an acceptable minimum
>> number of ticks to be set on the model. Under Linux an additional read
>> is added in the timer configuration path to detect back-pressure, which
>> will never occur on hardware. However if back-pressure is observed, the
>> driver alerts the clock event subsystem, which then performs its own
>> next event dilation via a config option - d1748302f70b ("clockevents:
>> Make minimum delay adjustments configurable")
>> A minimum period of 5us was experimentally determined on a Lenovo
>> T480s, which I've increased to 20us for "safety".
>> Signed-off-by: Andrew Jeffery <address@hidden>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> --- a/hw/misc/aspeed_scu.c
>> +++ b/hw/misc/aspeed_scu.c
>> @@ -423,6 +423,12 @@ static void aspeed_scu_realize(DeviceState *dev, Error 
>> **errp)
>>                            TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE);
>>      sysbus_init_mmio(sbd, &s->iomem);
>> +
>> +    /*
>> +     * Reset on realize to ensure the APB clock value is calculated in time 
>> for
>> +     * use by the timer model, which is reset before the SCU.
>> +     */
>> +    aspeed_scu_reset(dev);
> This looks wrong. QEMU should always be resetting devices
> after realize and before actually running anything.

We have something better for this patch (getting rid of the reset 
ordering issue). Can we still send you the fix this week ?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]