Re: [RFC PATCH 18/19] target/mips: Restrict some TCG specific CPUClass handlers

[Claudio Fontana]

On 12/7/20 9:53 AM, Claudio Fontana wrote:
> On 12/7/20 8:59 AM, Claudio Fontana wrote:
>> On 12/7/20 12:39 AM, Philippe Mathieu-Daudé wrote:
>>> Restrict the following CPUClass handlers to TCG:
>>> - do_interrupt
>>
>> In this patch it seems do_interrupt is changed to be CONFIG_USER_ONLY, it is 
>> not restricted to TCG.
> 
> Of course it _is_ as only TCG can do -user- , but would it be better to wrap 
> everything around CONFIG_TCG for what is tcg-only, and then add a subsection 
> in there for CONFIG_USER_ONLY?


Need coffee, sorry. I think you can see the issue there, sorry for the 
confusion.

Thanks,

Claudio


> 
>> Was this desired, should be commented as such?
>>
>> Also, should the whole function then be #ifdefed out in helpers.c?
>> I guess I am vouching for moving the ifndef CONFIG_USER_ONLY one line up in 
>> helpers.c.
>>
>> But you wanted this CONFIG_TCG-only?
>>
>>
>>> - do_transaction_failed
>>> - do_unaligned_access
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> Cc: Claudio Fontana <cfontana@suse.de>
>>>
>>>  target/mips/cpu.c | 8 +++++---
>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
>>> index 8a4486e3ea1..03bd35b7903 100644
>>> --- a/target/mips/cpu.c
>>> +++ b/target/mips/cpu.c
>>> @@ -483,7 +483,6 @@ static void mips_cpu_class_init(ObjectClass *c, void 
>>> *data)
>>>  
>>>      cc->class_by_name = mips_cpu_class_by_name;
>>>      cc->has_work = mips_cpu_has_work;
>>> -    cc->do_interrupt = mips_cpu_do_interrupt;
>>>      cc->cpu_exec_interrupt = mips_cpu_exec_interrupt;
>>>      cc->dump_state = mips_cpu_dump_state;
>>>      cc->set_pc = mips_cpu_set_pc;
>>> @@ -491,8 +490,7 @@ static void mips_cpu_class_init(ObjectClass *c, void 
>>> *data)
>>>      cc->gdb_read_register = mips_cpu_gdb_read_register;
>>>      cc->gdb_write_register = mips_cpu_gdb_write_register;
>>>  #ifndef CONFIG_USER_ONLY
>>> -    cc->do_transaction_failed = mips_cpu_do_transaction_failed;
>>> -    cc->do_unaligned_access = mips_cpu_do_unaligned_access;
>>> +    cc->do_interrupt = mips_cpu_do_interrupt;
>>>      cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
>>>      cc->vmsd = &vmstate_mips_cpu;
>>>  #endif
>>> @@ -500,6 +498,10 @@ static void mips_cpu_class_init(ObjectClass *c, void 
>>> *data)
>>>  #ifdef CONFIG_TCG
>>>      cc->tcg_initialize = mips_tcg_init;
>>>      cc->tlb_fill = mips_cpu_tlb_fill;
>>> +#if !defined(CONFIG_USER_ONLY)
>>> +    cc->do_unaligned_access = mips_cpu_do_unaligned_access;
>>> +    cc->do_transaction_failed = mips_cpu_do_transaction_failed;
>>> +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */
>>>  #endif
>>>  
>>>      cc->gdb_num_core_regs = 73;
>>>
>>
>