qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [Qemu-arm] [PATCH 09/11] armv7m: Split systick out from


From: Peter Maydell
Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH 09/11] armv7m: Split systick out from NVIC
Date: Mon, 20 Feb 2017 18:51:39 +0000

On 20 February 2017 at 17:43, Philippe Mathieu-Daudé <address@hidden> wrote:
> On 02/20/2017 12:36 PM, Peter Maydell wrote:
>> +/* Conversion factor from qemu timer to SysTick frequencies.  */
>> +static inline int64_t systick_scale(SysTickState *s)
>> +{
>> +    if (s->control & SYSTICK_CLKSOURCE) {
>> +        return system_clock_scale;
>> +    } else {
>> +        return 1000;
>
>
> this magic seems to be SYSTICK_SCALE
> (Paul Brook's commit 9ee6e8bb85)

This patch is just copying this function from one file to the other...
at some point we might want to try to improve the handling of
the clock scale factors but that leads into complication, because
the external-clocksource scale factor should really be board
level configurable. (Slightly confusingly the leg of this if/else
which is board-configurable is the one for the internal clock,
because we're modelling the ability of the stellaris board to
configure the PLLs so as to downclock the whole CPU, which
incidentally affects the timing of the internal systick clock.
Modelling this properly needs to wait on Fred Konrad's clocktree
patches getting in, I think.) For the moment I chose to just
leave the existing source alone, mostly.

>> +static void systick_instance_init(Object *obj)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> +    SysTickState *s = SYSTICK(obj);
>> +
>> +    memory_region_init_io(&s->iomem, obj, &systick_ops, s, "systick",
>> 0xe0);
>
>
> It seems to me the correct size is 0x10. The manual describes the range
> 0x20-0xFF as _reserved_. This _reserved_ description is at the end of the
> 'SysTick' chapter which seems weird to me... I rather think this range
> belong to the 'Interrupt Controller'@'System Control Space'.

The table of the System Control Space (B3-3 in the v7M ARM ARM
rev E.b) defines the Systick space as 0xE000E010 - 0xE000E0FF,
which makes it 0xE0 bytes in size. The NVIC doesn't start
until 0xE000E100 in this table.

The table of the SysTick registers (B3-7) agrees, in that
it defines the registers as covering 0xE000E010 to 0xE000E0FF.
In the current architecture everything from 0x20..0xFF is
reserved, which just means there's nothing there, but if
there was ever a need for systick to get another register
for some reason then it would go into that space (and if
we then implemented the new register in QEMU we wouldn't need
to change the NVIC code, only the systick code).

I think the way this patch has modelled things matches the
architecture -- it's a guest error to access the gap between
0XE000E020 and 0xE000E0FF, but we should report it as an
attempt to access an invalid systick register rather than
an access to an invalid NVIC register.

thanks
-- PMM



reply via email to

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