Re: [Qemu-devel] [RFC v2 10/34] include/exec: Split target_long def to new header

[Peter Crosthwaite]

On Mon, Jun 1, 2015 at 1:32 PM, Richard Henderson <address@hidden> wrote:
> On 06/01/2015 12:51 PM, Paolo Bonzini wrote:
>>
>>
>>
>> On 01/06/2015 21:24, Richard Henderson wrote:
>>>
>>> On 05/30/2015 11:11 PM, Peter Crosthwaite wrote:
>>>>
>>>> This is currently provided by cpu-defs and is a target specific
>>>> definition. However, to prepare for multi-arch only the bare minimum
>>>> content from cpu-defs.h should be exported to core code. And this is
>>>> all we need. So split it to a new header that the target_multi cpu.h
>>>> can include to save on having to include the ill-defined cpu-defs.h.
>>>>
>>>> Allow multiple inclusion for multi-arch where multiple cpu.h's need
>>>> to be included and target_long will vary for each.
>>>>
>>>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>>> ---
>>>>   include/exec/cpu-defs.h    | 23 +-------------------
>>>>   include/exec/target-long.h | 52
>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>>   2 files changed, 53 insertions(+), 22 deletions(-)
>>>>   create mode 100644 include/exec/target-long.h
>>>
>>>
>>> Multiple inclusion with a typedef?  How's that supposed to work?
>>
>>
>> He later #defines target_{,u}long to e.g. arm_target_{,u}long.
>
>
> Ok, here's where I'm not liking things.  It shouldn't be a typedef in some
> places and a define others.  From this description, it sounds like it ought
> to always be a define.
>

The #define-always change does make for a cleaner end result but I
stayed away from it purely because I was thinking typedefs are better
for type-definitions. But if we are open to the change of the #define
based implementation I am all for it as the target-foo/cpu.h change
pattern in minimised.

We still have a similar problems with cpu-defs.h/CPUTLBEntry though. I
have to think harder about how that can be done, but one solution is
to conditionally change the tlb_table defs in CPU_COMMON to be just a
dummy uint8_t[] in MULTI_ARCH case. This is ok, as the struct fields
are only accessible by arch-obj-y which will get the full-service
definition via non TARGET_MULTI_ARCH arch-obj-y compile. The work is
half done for us, as CPUTLBTable already has a uint8_t padding system
in place.

CPUIOTLBEntry can be moved to another header as it has no arch specific deps.

All in all, we can do this with 0 #define foo arm_foo in arch cpu.h's,
with these edits.

Regards,
Peter

>
> r~
>
>