Re: [PATCH v15 15/23] cpu: tcg_ops: move to tcg-cpu-ops.h, keep a pointer in CPUClass

[Claudio Fontana <cfontana@suse.de>]

On 2/3/21 3:48 PM, Philippe Mathieu-Daudé wrote:
> On 2/3/21 3:41 PM, Claudio Fontana wrote:
>> On 2/3/21 2:23 PM, Alex Bennée wrote:
>>> Claudio Fontana <cfontana@suse.de> writes:
> ...
>>>  /*
>>> modified   target/arm/cpu.c
>>> @@ -2248,7 +2248,7 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
>>>   * NB: cannot be const, as some elements are changed for specific
>>>   * arm cpu classes.
>>>   */
>>> -static struct TCGCPUOps arm_tcg_ops = {
>>> +static const struct TCGCPUOps arm_tcg_ops = {
>>>      .initialize = arm_translate_init,
>>>      .synchronize_from_tb = arm_cpu_synchronize_from_tb,
>>>      .cpu_exec_interrupt = arm_cpu_exec_interrupt,
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>> This does later break MIPS jazz:
>>>
>>> p/hw_mips_jazz.c.o -c ../../hw/mips/jazz.c
>>> ../../hw/mips/jazz.c: In function ‘mips_jazz_init’:
>>> ../../hw/mips/jazz.c:216:40: error: assignment of member ‘do_transaction_failed’ in read-only object
>>>      cc->tcg_ops->do_transaction_failed = mips_jazz_do_transaction_failed;
>>>
>>> which...
>>>
>>> <snip>
>>>>  
>>>> +#ifdef CONFIG_TCG
>>>> +#include "hw/core/tcg-cpu-ops.h"
>>>> +/*
>>>> + * NB: cannot be const, as some elements are changed for specific
>>>> + * mips hardware (see hw/mips/jazz.c).
>>>> + */
>>>
>>> does have a valid comment. So guess keep it as static and just don't
>>> claim ARM hacks around with it or find a more elegant solution for the
>>> Jazz hack (I'm not sure there is one).
>>
>> Yep, the ARM claim was true when I started looking at this, but now it's not anymore after the changes.
>>
>> However, I haven't found a way to remove the mips jazz hack.
>>
>> Maybe Philippe knows?
> 
> We need to test the real behavior on real hardware.
> 
> Eventually writing the test and asking on linux-mips@vger.kernel.org
> if someone can run it?
> 

I would not know how to do that tbh, ie which kind of test would capture this on mips/jazz.
Could you or anyone help there?

Or is there another option that we can see, beyond leaving things as they are (and thus not making the pointer const)?

One option I can see would be to define different tcg_ops entirely for jazz machines, just for one function pointer.
Seems a bit overkill, but I want to mention it as a possibility.

Thanks,

Claudio