[Qemu-devel] Re: [PATCH 2/4] rom loader: make vga+rom loading target specific

[Gerd Hoffmann <kraxel@redhat.com>]

On 10/17/09 11:23, Juan Quintela wrote:
>> +ifdef TARGET_I386
>> +obj-y += loader-i386.o
>> +else
>> +obj-y += loader-dummy.o
>> +endif
>> +
>
> This is wrong (tm).
> a- TARGET_I386 on Makefiles is only defined for 32bits, not for 64bits
>     (don't blame me, I just did a direct conversion of what was there).

Oh.  Joy of inconsistency ...

> b- TARGET_* is only defined in {i386,x86_64,...}-{softmmu,linux-user,...}
>     i.e. not a chance to be read at hw/Makefile.
>
> </me does patch>
>
> More complex that I thought:
> - if you only want to compile it once, you need to compile it on
>    Makefile, not Makefile.hw: no cookie, loader.h uses target_phys_addr.

Exactly.  Due to target_phys_addr it must go to Makefile.hw, so I get it 
compiled once for 32bit targphys and once for 64bit targphys.

> - ok, move it to Makefile.hw ->  your option didn't work (it compiles)
>    because you allways compile loader-dummy.o, TARGET_I386 is not defined
>    in Makefile.hw (it is hardware independent drivers after all :)

Oops.

> - More imagination: Use only loader-i386.c, and #ifdef TARGET_I386
>    to an empty macro/real thing ->  no cookie either, TARGET_I386 is
>    poisoned in Makefile.hw (*)

Thats why I don't want #ifdef.

> - define CONFIG_LOADER_I386 for I386 and X86_64.

No.  This avoids the TARGET_I386 poisoned error, but does *not* fix the 
underlying problem.

> I preffer very much to add the "dummy" cases in one ifdef in a header
> file.

No.  Any user of rom_add_option() must be compiled per target then.

Guess easierst way is the v1 patch then:  Have target-specific code in 
loader-target.c and use #ifdefs there ...

cheers,
   Gerd