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

[Roman Bolshakov <r.bolshakov@yadro.com>]

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.

>       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?

IIRC all tests pass on TCG/5.1.

Regards,
Roman