Re: [Qemu-devel] [PATCH 1/6] tests/tcg: Move i386 tests to arch specific folder

[Pranith Kumar]

Marc-André Lureau writes:

>> index 0000000..15f77af
>> --- /dev/null
>> +++ b/tests/tcg/i386/Makefile
>> @@ -0,0 +1,88 @@
>> +BUILD_DIR=../../../build/
>>
>
> Looks like you are hardcoding a custom path here.

Missed this one. Will fix it up.

>
> There are many Makefile changes that I think you should split to help
> review.

OK, I will do that. I did not think it was worth splitting it up since I was
being lazy, but yes, it will help make the review easier I guess.

>
> +SRC_PATH=../../../
>> +include $(BUILD_DIR)/config-host.mak
>> +include $(SRC_PATH)/rules.mak
>> +
>> +$(call set-vpath, $(SRC_PATH)/tests/tcg/i386)
>> +
>> +QEMU=$(BUILD_DIR)/i386-linux-user/qemu-i386
>> +QEMU_X86_64=$(BUILD_DIR)/x86_64-linux-user/qemu-x86_64
>> +CC_X86_64=$(CC_I386) -m64
>> +
>> +QEMU_INCLUDES += -I$(BUILD_DIR)
>> +CFLAGS=-Wall -O2 -g -fno-strict-aliasing
>> +#CFLAGS+=-msse2
>> +LDFLAGS=
>> +
>> +# TODO: automatically detect ARM and MIPS compilers, and run those too
>> +
>>
>
> Not relevant anymore

Noted.

>
>
>> +# runcom maps page 0, so it requires root privileges
>> +# also, pi_10.com runs indefinitely
>> +
>>
> +I386_TESTS=hello-i386 \
>> +          test-i386 \
>> +          test-i386-fprem
>> +          # runcom
>> +
>> +# native i386 compilers sometimes are not biarch.  assume cross-compilers
>> are
>> +ifneq ($(ARCH),i386)
>> +I386_TESTS+=run-test-x86_64
>>
>
> btw this is wrong, it should be test-x86_64, since run- is prepended later
>

Will fix.

>> --- a/tests/tcg/hello-i386.c
>> +++ b/tests/tcg/i386/hello-i386.c
>> @@ -1,6 +1,7 @@
>>  #include <asm/unistd.h>
>> +#include <stddef.h>
>>
>> -static inline void exit(int status)
>> +static inline void _exit(int status)
>>  {
>>    int __res;
>>    __asm__ volatile ("movl %%ecx,%%ebx\n"\
>> @@ -8,7 +9,7 @@ static inline void exit(int status)
>>                     :  "=a" (__res) : "0" (__NR_exit),"c"
>> ((long)(status)));
>>  }
>>
>> -static inline int write(int fd, const char * buf, int len)
>> +static inline size_t _write(int fd, const void * buf, size_t len)
>>  {
>>    int status;
>>    __asm__ volatile ("pushl %%ebx\n"\
>> @@ -22,6 +23,6 @@ static inline int write(int fd, const char * buf, int
>> len)
>>
>>  void _start(void)
>>  {
>> -    write(1, "Hello World\n", 12);
>> -    exit(0);
>> +    _write(1, "Hello World\n", 12);
>> +    _exit(0);
>>  }
>>
>
> This could be a seperate patch
>

OK, will do.

-- 
Pranith