Re: [Qemu-devel] [PATCH] [ARM] Fix sp804 dual-timer

[Peter Maydell]

On 29 September 2011 03:34, Peter Chubb <address@hidden> wrote:
> -    /* ??? Don't know the PrimeCell ID for this device.  */
>     if (offset < 0x20) {
>         return arm_timer_read(s->timer[0], offset);
> -    } else {
> +    }
> +    if (offset < 0x40) {
>         return arm_timer_read(s->timer[1], offset - 0x20);
>   }

if (offset < 0x20) {..} else if (offset < 0x40) {..}
would be consistent with what you did in the write function.

> +    /* Timer Peripheral ID Registers, one byte per word:
> +     * [11:0]  - Part number    = 0x804
> +     * [19:12] - Designer Id    = 0x41   ('A')
> +     * [23:20] - Revision       = 1
> +     * [31:24] - Configurations = 0
> +     */
> +    case 0xfe0: /* TimerPeriphID0 */
> +        return 0x04;
> +    case 0xfe4: /* TimerPeriphID1 */
> +        return 0x18;
> +    case 0xfe8: /* TimerPeriphID2 */
> +        return 0x14;
> +    case 0xfec: /* TimerPeriphID3 */
> +        return 0x00;

[etc]

It's neater to do the 8 ID registers with an array, as the other
pl* devices do -- look at hw/pl061.c:pl061_read() for an example.

> +    cpu_abort (cpu_single_env, "sp804_read: Bad offset %x\n",
> +               (int)offset);

Don't cpu_abort() for something a malicious guest can trigger.
(Yes, we do this in some other devices at the moment, but we
shouldn't be introducing new instances of the problem.)

Maybe we should update the comment that says "docs for
this device don't seem to be available"...

-- PMM