Re: [Qemu-devel] [PATCH v2 2/4] gdbstub: Use cpu_set_pc helper

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 10:16 AM, Andreas Färber <address@hidden> wrote:
> Am 24.06.2015 um 19:04 schrieb Peter Crosthwaite:
>> On Wed, Jun 24, 2015 at 3:01 AM, Peter Maydell <address@hidden> wrote:
>>> On 24 June 2015 at 03:50, Peter Crosthwaite
>>> <address@hidden> wrote:
>>>> On Mon, Jun 22, 2015 at 10:31 AM, Andreas Färber <address@hidden> wrote:
>>>>> I believe this argument will probably go away; otherwise this should've
>>>>> been &error_abort or something instead of NULL.
>>>>>
>>>>
>>>> I'm not sure. As I don't see what is catching the case of a gdb 'c'
>>>> packet for a CPU that doesn't implement set_pc. I'd rather preserve
>>>> the existing behaviour, and have the qom wrapper do nothing if it is
>>>> not implemented.
>>>
>>> Well, this is one reason why every CPU needs to implement set_pc...
>>>
>>
>> Well. I guess it works for a common case where a continue doesn't
>> change the PC? If the debugger doesn't change the PC the "c" should
>> work even without a set_pc call so we don't want to assert on this
>> valid use case.
>
> Guys, is there any target that does not implement set_pc today? If so,
> which? I'd rather implement it than carry around the iffery and
> resulting complications.
>
> I quickly counted 17 target-* in my tree and all 17 seemed to show up in
> git-grep. Can you confirm? Didn't check the latest tilegx series.
>

There are 17 targets and 18 sets by my count:
$ git grep "set_pc.*=.*set_pc" target-*
target-alpha/cpu.c:    cc->set_pc = alpha_cpu_set_pc;
target-arm/cpu.c:    cc->set_pc = arm_cpu_set_pc;
target-arm/cpu64.c:    cc->set_pc = aarch64_cpu_set_pc;
target-cris/cpu.c:    cc->set_pc = cris_cpu_set_pc;
target-i386/cpu.c:    cc->set_pc = x86_cpu_set_pc;
target-lm32/cpu.c:    cc->set_pc = lm32_cpu_set_pc;
target-m68k/cpu.c:    cc->set_pc = m68k_cpu_set_pc;
target-microblaze/cpu.c:    cc->set_pc = mb_cpu_set_pc;
target-mips/cpu.c:    cc->set_pc = mips_cpu_set_pc;
target-moxie/cpu.c:    cc->set_pc = moxie_cpu_set_pc;
target-openrisc/cpu.c:    cc->set_pc = openrisc_cpu_set_pc;
target-ppc/translate_init.c:    cc->set_pc = ppc_cpu_set_pc;
target-s390x/cpu.c:    cc->set_pc = s390_cpu_set_pc;
target-sh4/cpu.c:    cc->set_pc = superh_cpu_set_pc;
target-sparc/cpu.c:    cc->set_pc = sparc_cpu_set_pc;
target-tricore/cpu.c:    cc->set_pc = tricore_cpu_set_pc;
target-unicore32/cpu.c:    cc->set_pc = uc32_cpu_set_pc;
target-xtensa/cpu.c:    cc->set_pc = xtensa_cpu_set_pc;

ARM is doubled up. As long as no one else is missing a double up where
there is not default set by a single base class we are OK. If there is
a missing double up with a base class implementation, then there will
be no change by your proposal anyway.

Regards,
Peter

> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>