Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils

[Thomas Huth <thuth@redhat.com>]

On 28/08/2020 09.47, Roman Bolshakov wrote:
> On Fri, Aug 28, 2020 at 09:24:10AM +0200, Thomas Huth wrote:
>> On 28/08/2020 08.54, Roman Bolshakov wrote:
>>> On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
>>>> On 10/08/2020 15.06, Roman Bolshakov wrote:
>>>>> For compatibility with other SVR4 assemblers, '/' starts a comment on
>>>>> *-elf binutils target and thus division operator is not allowed [1][2].
>>>>> That breaks cstart64.S build:
>>>>>
>>>>>   x86/cstart64.S: Assembler messages:
>>>>>   x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1.
>>>>>
>>>>> The option is ignored on the Linux target of GNU binutils.
>>>>>
>>>>> 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html
>>>>> 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386
>>>>>
>>>>> Cc: Cameron Esfahani <dirty@apple.com>
>>>>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
>>>>> ---
>>>>>  x86/Makefile | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/x86/Makefile b/x86/Makefile
>>>>> index 8a007ab..22afbb9 100644
>>>>> --- a/x86/Makefile
>>>>> +++ b/x86/Makefile
>>>>> @@ -1 +1,3 @@
>>>>>  include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
>>>>> +
>>>>> +COMMON_CFLAGS += -Wa,--divide
>>>>
>>>> Some weeks ago, I also played with an elf cross compiler and came to the
>>>> same conclusion, that we need this option there. Unfortunately, it does
>>>> not work with clang:
>>>>
>>>>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
>>>>
>>>> You could try to wrap it with "cc-option" instead ... or use a proper
>>>> check in the configure script to detect whether it's needed or not.
>>>>
>>>
>>> Hi Thomas,
>>>
>>> Thanks for reviewing the series. I'll look into both options and will
>>> test with both gcc and clang afterwards. I can also update .travis.yml
>>> in a new patch to test the build on macOS.
>>
>> That would be great, thanks! Note that you need at least Clang v10 (the
>> one from Fedora 32 is fine) to compile the kvm-unit-tests.
>>
>> And if it's of any help, this was the stuff that I used in .travis.yml
>> for my experiments (might still be incomplete, though):
>>
>>     - os: osx
>>       osx_image: xcode12
>>       addons:
>>         homebrew:
>>           packages:
>>             - bash
>>             - coreutils
>>             - qemu
>>             - x86_64-elf-gcc
>>       env:
>>       - CONFIG="--cross-prefix=x86_64-elf-"
>>       - BUILD_DIR="build"
>>       - TESTS="umip"
>>       - ACCEL="tcg"
>>
>>     - os: osx
>>       osx_image: xcode12
>>       addons:
>>         homebrew:
>>           packages:
>>             - bash
>>             - coreutils
>>             - qemu
>>             - i386-elf-gcc
> 
> It's going to be i686-elf-gcc.

Ah, there are two flavours? Ok, good to know.

>>       env:
>>       - CONFIG="--arch=i386 --cross-prefix=x86_64-elf-"
> 
> `--cross-prefix=i686-elf-`, respectively.
> 
>>       - BUILD_DIR="build"
>>       - TESTS="umip"
>>       - ACCEL="tcg"
>>
> 
> Thanks, I'll use it as the basis. Do I need to add your Signed-off-by: here?
> or Suggested-by: is enough?

Suggested-by is fine.

> IIRC all tests pass on TCG/5.1.

Yes, please tweak the TEST="..:" lines accordingly, I just added one
test there for my initial tries.

 Thomas