Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Fix PC overwriting

[Peter Maydell]

On Thu, 17 Jan 2019 at 19:27, Peter Maydell <address@hidden> wrote:
>
> On Thu, 17 Jan 2019 at 10:58, Julia Suvorova <address@hidden> wrote:
> >
> > On 17.01.2019 13:13, Stefan Hajnoczi wrote:
> > > generic_loader_reset() calls cpu_reset(s->cpu) followed by
> > > CPUClass->set_pc(s->cpu, s->addr).
> > >
> > > ARM's arm_cpu_set_pc() doesn't special-case the Thumb bit (that's only
> > > done in arm_cpu_reset()) so we end up with an invalid PC for Thumb mode
> > > addresses.
> > >
> > > Maybe the following arm_cpu_reset() code should be moved to
> > > arm_cpu_set_pc():
> > >
> > >    env->regs[15] = initial_pc & ~1;
> > >    env->thumb = initial_pc & 1;
> > >
> > > Then arm_cpu_reset() can call arm_cpu_set_pc() instead of duplicating
> > > this code.
> >
> > No, set_pc() is called in cpu_tb_exec() to restore the PC value and
> > therefore should not be changed.
>
> The set_pc hook is also called for the gdbstub 'c' and 's' packets
> if they supply an address. I am not sure what the correct behaviour
> there is (it might be tricky to find out or test, because the
> 'c' and 's' packets are deprecated in favour of vCont which doesn't
> allow the address argument at all, and recent gdb neither emits
> 'c addr' nor supports it in its gdbserver implementation).

I asked Linaro's gdb developer, and they thought that the gdb
'c addr' behaviour ought to be "look at bit 0 and switch to
Thumb or Arm mode accordingly".

thanks
-- PMM