Re: [PATCH] firmware: don't build hvmloader if it is not needed

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.02.2021 00:05, Stefano Stabellini wrote:
> On Fri, 19 Feb 2021, Jan Beulich wrote:
>> On 19.02.2021 02:42, Stefano Stabellini wrote:
>>> --- a/tools/configure.ac
>>> +++ b/tools/configure.ac
>>> @@ -307,6 +307,10 @@ AC_ARG_VAR([AWK], [Path to awk tool])
>>>  
>>>  # Checks for programs.
>>>  AC_PROG_CC
>>> +AC_LANG(C)
>>> +AC_LANG_CONFTEST([AC_LANG_SOURCE([[int main() { return 0;}]])])
>>> +AS_IF([gcc -m32 conftest.c -o - 2>/dev/null], [hvmloader=y], 
>>> [AC_MSG_WARN(hvmloader build disabled as the compiler cannot build 32bit 
>>> binaries)])
>>> +AC_SUBST(hvmloader)
>>
>> I'm puzzled: "gcc -m32" looked to work fine on its own. I suppose
>> the above fails at the linking stage, but that's not what we care
>> about (we don't link with any system libraries). Instead, as said,
>> you want to check "gcc -m32 -c" produces correct code, in
>> particular with sizeof(uint64_t) being 8. Of course all of this
>> would be easier if their headers at least caused some form of
>> error, instead of silently causing bad code to be generated.
>>
>> The way you do it, someone simply not having 32-bit C libraries
>> installed would then also have hvmloader build disabled, even if
>> their compiler and headers are fine to use.
> 
> I realize that technically this test is probing for something different:
> the ability to build and link a trivial 32-bit userspace program, rather
> than a specific check about sizeof(uint64_t). However, I thought that if
> this test failed we didn't want to continue anyway.
> 
> If you say that hvmloader doesn't link against any system libraries,
> then in theory the hvmloader build could succeed even if this test
> failed. Hence, we need to change strategy.
> 
> What do you think of something like this?
> 
> AC_LANG_CONFTEST([AC_LANG_SOURCE([[#include <assert.h>
> #include <stdint.h>
> int main() { assert(sizeof(uint64_t) == 8); return 0;}]])])
> AS_IF([gcc -m32 conftest.c -o conftest 2>/dev/null], [hvmloader=y], 
> [AC_MSG_WARN(XXX)])

The assert() would trigger at runtime only, so you'd also need to
invoke the program, wouldn't you?

> Do you have any better ideas?

An open-coded BUILD_BUG_ON()-like test would allow noticing the issue
already at compile time.

Jan