Richard Henderson <richard.hender...@linaro.org> writes:
> On 9/11/19 5:26 AM, Alex Bennée wrote: >> >> Aleksandar Markovic <aleksandar.m.m...@gmail.com> writes: >> >>> 10.09.2019. 21.34, "Alex Bennée" <alex.ben...@linaro.org> је написао/ла: >>>> >>>> This is preparatory for plugins which will want to report the >>>> architecture to plugins. Move the ELF_ARCH definition out of the >>>> loader and into its own header. >>>> >>>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org> >>>> -- >>> >>> Hi, Alex. >>> >>> I advice some caution here >>> . For example, EM_NANOMIPS, and some other EM_xxx constants are not >>> included in the new header. >> >> EM_ARCH was never set to EM_NANOMIPS. In fact I can see e_machine is >> checked against it so I guess it is only ever used to checking the >> loading elf directly. >> >> In fact EM_ARCH is only really the default fallback case for checking >> the elf type if there is not a more "forgiving" check done by arch >> specific code (elf_check_arch). The only other cace is the fallback for >> EM_MACHINE unless PPC does something with it. >> >>> I am not sure what exactly you want to achieve >>> with this refactoring. But you may miss your goal, since some EM_xxx will >>> be missing from your new header. Is this the right way to achieve what you >>> want, and could you unintentionally introduce bugs? Can you outline in more >>> details your intentions around the new header? Is this refactoring the only >>> way? >> >> As mentioned in the other reply maybe exposing a value from configure >> into config-target.[mak|h] would be a better approach? > > I think using EM_FOO to tell a plugin about the architecture is just going to > be confusing. As seen here wrt EM_NANOMIPS, but arguably elsewhere with > EM_SPARC vs EM_SPARC64. > > You need to decide what it is that you want the plugin to know. It is just be > gross architecture? In which case QEMU_ARCH_FOO is probably enough. Is it > the > instruction set currently executing? In which case neither EM_FOO nor > QEMU_ARCH_FOO is sufficient -- e.g. arm thumb -- and you'll have to invent > something new, because no two architectures handle this in the same way. The > closest you might be able to get without invention from whole cloth is the > capstone cap_arch+cap_mode values from cc->disas_set_info(). But that only > handles the most popular of targets. I think I'm going to stick with the gross TARGET for now. It mostly seems a way of avoiding building per-arch plugins. I assume most out of tree plugins will be targeted at a specific machine anyway. Anyway I think that means I'll drop this series unless 1-3 are worth keeping as simple clean-ups leaving out 4? > > In any case, a single header that every target must touch is the wrong > approach. If you want to do some cleanup, move these defines into e.g. > linux-user/$TARGET/target_elf.h. (And I wouldn't bother touching bsd-user in > its current condition.) > > > r~ -- Alex Bennée