On 01/07/2019 14:59, Peter Maydell wrote:
> On Tue, 18 Jun 2019 at 17:54, Cédric Le Goater <c...@kaod.org> wrote:
>>
>> From: Andrew Jeffery <and...@aj.id.au>
>>
>> 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 <and...@aj.id.au>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>
>> --- 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.
Ah yes ... We need to rework SCU with class data to provide a cleaner
model for all SoCs. AST2600 will require it anyhow.
Thanks,
C.
>
>> }
>
> thanks
> -- PMM
>
