Commit 42f6ed91 authored by Julia Suvorova's avatar Julia Suvorova Committed by Peter Maydell
Browse files

arm: Clarify the logic of set_pc() 



Until now, the set_pc logic was unclear, which raised questions about
whether it should be used directly, applying a value to PC or adding
additional checks, for example, set the Thumb bit in Arm cpu. Let's set
the set_pc logic for “Configure the PC, as was done in the ELF file”
and implement synchronize_with_tb hook for preserving PC to cpu_tb_exec.

Signed-off-by: default avatarJulia Suvorova <jusual@mail.ru>
Acked-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190129121817.7109-1-jusual@mail.ru
Reviewed-by: default avatarPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
parent ef682cdb

hw/arm/boot.c

+0 −4
Original line number Diff line number Diff line
@@ -697,10 +697,6 @@ static void do_cpu_reset(void *opaque) 
                g_assert_not_reached();

            }



            if (!env->aarch64) {

                env->thumb = info->entry & 1;

                entry &= 0xfffffffe;

            }

            cpu_set_pc(cs, entry);

        } else {

            /* If we are booting Linux then we need to check whether we are

include/qom/cpu.h

+14 −2
Original line number Diff line number Diff line
@@ -103,9 +103,21 @@ struct TranslationBlock; 
 * @get_arch_id: Callback for getting architecture-dependent CPU ID.

 * @get_paging_enabled: Callback for inquiring whether paging is enabled.

 * @get_memory_mapping: Callback for obtaining the memory mappings.

 * @set_pc: Callback for setting the Program Counter register.

 * @set_pc: Callback for setting the Program Counter register. This

 *       should have the semantics used by the target architecture when

 *       setting the PC from a source such as an ELF file entry point;

 *       for example on Arm it will also set the Thumb mode bit based

 *       on the least significant bit of the new PC value.

 *       If the target behaviour here is anything other than "set

 *       the PC register to the value passed in" then the target must

 *       also implement the synchronize_from_tb hook.

 * @synchronize_from_tb: Callback for synchronizing state from a TCG

 * #TranslationBlock.

 *       #TranslationBlock. This is called when we abandon execution

 *       of a TB before starting it, and must set all parts of the CPU

 *       state which the previous TB in the chain may not have updated.

 *       This always includes at least the program counter; some targets

 *       will need to do more. If this hook is not implemented then the

 *       default is to call @set_pc(tb->pc).

 * @handle_mmu_fault: Callback for handling an MMU fault.

 * @get_phys_page_debug: Callback for obtaining a physical address.

 * @get_phys_page_attrs_debug: Callback for obtaining a physical address and the

target/arm/arm-powerctl.c

+0 −3
Original line number Diff line number Diff line
@@ -120,11 +120,8 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state, 


    if (info->target_aa64) {

        target_cpu->env.xregs[0] = info->context_id;

        target_cpu->env.thumb = false;

    } else {

        target_cpu->env.regs[0] = info->context_id;

        target_cpu->env.thumb = info->entry & 1;

        info->entry &= 0xfffffffe;

    }



    /* Start the new CPU at the requested address */

target/arm/cpu.c

+25 −1
Original line number Diff line number Diff line
@@ -40,8 +40,31 @@ 
static void arm_cpu_set_pc(CPUState *cs, vaddr value)

{

    ARMCPU *cpu = ARM_CPU(cs);

    CPUARMState *env = &cpu->env;



    if (is_a64(env)) {

        env->pc = value;

        env->thumb = 0;

    } else {

        env->regs[15] = value & ~1;

        env->thumb = value & 1;

    }

}



    cpu->env.regs[15] = value;

static void arm_cpu_synchronize_from_tb(CPUState *cs, TranslationBlock *tb)

{

    ARMCPU *cpu = ARM_CPU(cs);

    CPUARMState *env = &cpu->env;



    /*

     * It's OK to look at env for the current mode here, because it's

     * never possible for an AArch64 TB to chain to an AArch32 TB.

     */

    if (is_a64(env)) {

        env->pc = tb->pc;

    } else {

        env->regs[15] = tb->pc;

    }

}



static bool arm_cpu_has_work(CPUState *cs)
@@ -2099,6 +2122,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) 
    cc->cpu_exec_interrupt = arm_cpu_exec_interrupt;

    cc->dump_state = arm_cpu_dump_state;

    cc->set_pc = arm_cpu_set_pc;

    cc->synchronize_from_tb = arm_cpu_synchronize_from_tb;

    cc->gdb_read_register = arm_cpu_gdb_read_register;

    cc->gdb_write_register = arm_cpu_gdb_write_register;

#ifdef CONFIG_USER_ONLY

target/arm/cpu64.c

+0 −15
Original line number Diff line number Diff line
@@ -480,20 +480,6 @@ static void aarch64_cpu_finalizefn(Object *obj) 
{

}



static void aarch64_cpu_set_pc(CPUState *cs, vaddr value)

{

    ARMCPU *cpu = ARM_CPU(cs);

    /* It's OK to look at env for the current mode here, because it's

     * never possible for an AArch64 TB to chain to an AArch32 TB.

     * (Otherwise we would need to use synchronize_from_tb instead.)

     */

    if (is_a64(&cpu->env)) {

        cpu->env.pc = value;

    } else {

        cpu->env.regs[15] = value;

    }

}



static gchar *aarch64_gdb_arch_name(CPUState *cs)

{

    return g_strdup("aarch64");
@@ -504,7 +490,6 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data) 
    CPUClass *cc = CPU_CLASS(oc);



    cc->cpu_exec_interrupt = arm_cpu_exec_interrupt;

    cc->set_pc = aarch64_cpu_set_pc;

    cc->gdb_read_register = aarch64_cpu_gdb_read_register;

    cc->gdb_write_register = aarch64_cpu_gdb_write_register;

    cc->gdb_num_core_regs = 34;