Re: [Qemu-arm] [PATCH v2 03/26] armv7m: Explicit error for bad vector table

[Michael Davidsaver]

On 12/17/2015 08:25 AM, Peter Maydell wrote:
> On 3 December 2015 at 00:18, Michael Davidsaver <address@hidden> wrote:
>> ...
>> +static
>> +uint32_t arm_v7m_load_vector(ARMCPU *cpu)
>> +
>> +{
>> +    CPUState *cs = &cpu->parent_obj;
> This isn't the right way to cast to the base class of a QOM object.
> You want:
>    CPUState *cs = CPU(cpu);

from cpu.h

> /* Since this macro is used a lot in hot code paths and in conjunction
> with
>  * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using
>  * an unchecked cast.
>  */
> #define CPU(obj) ((CPUState *)(obj))

Given the present definition of CPU() this change seems like a step
backwards in terms of safety as mis-use won't be caught at compile or
runtime.  I'll change it anyway.


>
>> +    CPUARMState *env = &cpu->env;
>> +    MemTxResult result;
>> +    hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4;
>> +    uint32_t addr;
>> +
>> +    addr = address_space_ldl(cs->as, vec,
>> +                             MEMTXATTRS_UNSPECIFIED, &result);
>> +    if (result != MEMTX_OK) {
> We could use a comment here:
>    /* Architecturally this should cause a HardFault setting HSFR.VECTTBL,
>     * which would then be immediately followed by our failing to load
>     * the entry vector for that HardFault, which is a Lockup case.
>     * Since we don't model Lockup, we just report this guest error
>     * via cpu_abort().
>     */

Added.