diff mbox series

[V8,1/4] tests/migration: Convert x86 boot block compilation script into Makefile

Message ID 1535778675-32170-2-git-send-email-wei@redhat.com (mailing list archive)
State New, archived
Headers show
Series tests: Add migration test for aarch64 | expand

Commit Message

Wei Huang Sept. 1, 2018, 5:11 a.m. UTC
The x86 boot block header currently is generated with a shell script.
To better support other CPUs (e.g. aarch64), we convert the script
into Makefile. This allows us to 1) support cross-compilation easily,
and 2) avoid creating a script file for every architecture.

Note that, in the new design, the cross compiler prefix can be specified by
setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
to include the C-style file correctly, it also renames the
x86-a-b-bootblock.s file extension from .s to .S.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 tests/migration/Makefile                           | 31 ++++++++++++++++++++
 tests/migration/rebuild-x86-bootblock.sh           | 33 ----------------------
 .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  0
 3 files changed, 31 insertions(+), 33 deletions(-)
 create mode 100644 tests/migration/Makefile
 delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
 rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (100%)

Comments

Juan Quintela Sept. 3, 2018, 9:32 a.m. UTC | #1
Wei Huang <wei@redhat.com> wrote:
> The x86 boot block header currently is generated with a shell script.
> To better support other CPUs (e.g. aarch64), we convert the script
> into Makefile. This allows us to 1) support cross-compilation easily,
> and 2) avoid creating a script file for every architecture.
>
> Note that, in the new design, the cross compiler prefix can be specified by
> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
> to include the C-style file correctly, it also renames the
> x86-a-b-bootblock.s file extension from .s to .S.
>
> Signed-off-by: Wei Huang <wei@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>
Andrew Jones Sept. 3, 2018, 11:08 a.m. UTC | #2
On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
> The x86 boot block header currently is generated with a shell script.
> To better support other CPUs (e.g. aarch64), we convert the script
> into Makefile. This allows us to 1) support cross-compilation easily,
> and 2) avoid creating a script file for every architecture.
> 
> Note that, in the new design, the cross compiler prefix can be specified by
> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
> to include the C-style file correctly, it also renames the
> x86-a-b-bootblock.s file extension from .s to .S.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  tests/migration/Makefile                           | 31 ++++++++++++++++++++
>  tests/migration/rebuild-x86-bootblock.sh           | 33 ----------------------
>  .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  0
>  3 files changed, 31 insertions(+), 33 deletions(-)
>  create mode 100644 tests/migration/Makefile
>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>  rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (100%)
> 
> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> new file mode 100644
> index 0000000..5d5fa07
> --- /dev/null
> +++ b/tests/migration/Makefile
> @@ -0,0 +1,31 @@
> +# To specify cross compiler prefix, use CROSS_PREFIX=
> +#   > make CROSS_PREFIX=x86_64-linux-gnu-
       ^ nit: this prompt symbol is weird (at least to me, what shell uses
              this?) To me it looks like a sh/bash redirect symbol. Can
              we change it to '$' or use nothing at all?
> +
> +override define __note
> +/* This file is automatically generated from
> + * tests/migration/x86-a-b-bootblock.s, edit that and then run
                                        ^ should be 'S'

> + * tests/migration/rebuild-x86-bootblock.sh to update,
> + * and then remember to send both in your patch submission.
> + */
> +endef
> +export __note
> +
> +.PHONY: all clean
> +all: x86-a-b-bootblock.h
> +
> +x86-a-b-bootblock.h: x86.bootsect
> +	echo "$$__note" > header.tmp
> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> +	mv header.tmp $@

The shell script this Makefile is replacing used mktemp
for a randomly named tmp dir. Shouldn't we continue to
use random names?

> +
> +x86.bootsect: x86.boot
> +	dd if=$< of=$@ bs=256 count=2 skip=124
> +
> +x86.boot: x86.o
> +	$(CROSS_PREFIX)objcopy -O binary $< $@
> +
> +x86.o: x86-a-b-bootblock.S
> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> +
> +clean:
> +	@rm -rf *.boot *.o *.bootsect

We don't want to remove the generated header file when cleaning?

> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
> deleted file mode 100755
> index 86cec5d..0000000
> --- a/tests/migration/rebuild-x86-bootblock.sh
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -#!/bin/sh
> -# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
> -# This work is licensed under the terms of the GNU GPL, version 2 or later.
> -# See the COPYING file in the top-level directory.
> -#
> -# Author: dgilbert@redhat.com
> -
> -ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
> -HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
> -
> -if [ ! -e "$ASMFILE" ]
> -then
> -  echo "Couldn't find $ASMFILE" >&2
> -  exit 1
> -fi
> -
> -ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XXXXXX)
> -cd "$ASM_WORK_DIR" &&
> -as --32 -march=i486 "$ASMFILE" -o x86.o &&
> -objcopy -O binary x86.o x86.boot &&
> -dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124 &&
> -xxd -i x86.bootsect |
> -sed -e 's/.*int.*//' > x86.hex &&
> -cat - x86.hex <<HERE > "$HEADER"
> -/* This file is automatically generated from
> - * tests/migration/x86-a-b-bootblock.s, edit that and then run
> - * tests/migration/rebuild-x86-bootblock.sh to update,
> - * and then remember to send both in your patch submission.
> - */
> -HERE
> -
> -rm x86.hex x86.bootsect x86.boot x86.o
> -cd .. && rmdir "$ASM_WORK_DIR"
> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
> similarity index 100%
> rename from tests/migration/x86-a-b-bootblock.s
> rename to tests/migration/x86-a-b-bootblock.S
> -- 
> 1.8.3.1
> 
>

Thanks,
drew
Juan Quintela Sept. 3, 2018, 11:45 a.m. UTC | #3
Andrew Jones <drjones@redhat.com> wrote:
> On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
>> The x86 boot block header currently is generated with a shell script.
>> To better support other CPUs (e.g. aarch64), we convert the script
>> into Makefile. This allows us to 1) support cross-compilation easily,
>> and 2) avoid creating a script file for every architecture.
>> 
>> Note that, in the new design, the cross compiler prefix can be specified by
>> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
>> to include the C-style file correctly, it also renames the
>> x86-a-b-bootblock.s file extension from .s to .S.
>> + * tests/migration/rebuild-x86-bootblock.sh to update,
>> + * and then remember to send both in your patch submission.
>> + */
>> +endef
>> +export __note
>> +
>> +.PHONY: all clean
>> +all: x86-a-b-bootblock.h
>> +
>> +x86-a-b-bootblock.h: x86.bootsect
>> +	echo "$$__note" > header.tmp
>> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>> +	mv header.tmp $@
>
> The shell script this Makefile is replacing used mktemp
> for a randomly named tmp dir. Shouldn't we continue to
> use random names?

We shouldn't be trying to create the file twice in parallel, no?

>
>> +
>> +x86.bootsect: x86.boot
>> +	dd if=$< of=$@ bs=256 count=2 skip=124
>> +
>> +x86.boot: x86.o
>> +	$(CROSS_PREFIX)objcopy -O binary $< $@
>> +
>> +x86.o: x86-a-b-bootblock.S
>> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
>> +
>> +clean:
>> +	@rm -rf *.boot *.o *.bootsect
>
> We don't want to remove the generated header file when cleaning?

No, because we only generated the header file when running native, have
the right crosscompilers, etc.  In general we only recompile it when we
change the test file, otherwise we store the generated ones.

Idea is:
We have a c file that does what we want.
We compile it (when we have a propper compiler) and store it on the
tree.
We compile it for each test.

This is more "like firmware" than normal "qemu" source code.

Later, Juan.
Andrew Jones Sept. 3, 2018, 12:14 p.m. UTC | #4
On Mon, Sep 03, 2018 at 01:45:33PM +0200, Juan Quintela wrote:
> Andrew Jones <drjones@redhat.com> wrote:
> > On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
> >> The x86 boot block header currently is generated with a shell script.
> >> To better support other CPUs (e.g. aarch64), we convert the script
> >> into Makefile. This allows us to 1) support cross-compilation easily,
> >> and 2) avoid creating a script file for every architecture.
> >> 
> >> Note that, in the new design, the cross compiler prefix can be specified by
> >> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
> >> to include the C-style file correctly, it also renames the
> >> x86-a-b-bootblock.s file extension from .s to .S.
> >> + * tests/migration/rebuild-x86-bootblock.sh to update,
> >> + * and then remember to send both in your patch submission.
> >> + */
> >> +endef
> >> +export __note
> >> +
> >> +.PHONY: all clean
> >> +all: x86-a-b-bootblock.h
> >> +
> >> +x86-a-b-bootblock.h: x86.bootsect
> >> +	echo "$$__note" > header.tmp
> >> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> >> +	mv header.tmp $@
> >
> > The shell script this Makefile is replacing used mktemp
> > for a randomly named tmp dir. Shouldn't we continue to
> > use random names?
> 
> We shouldn't be trying to create the file twice in parallel, no?

Yeah, you're right. This should be safe.

> 
> >
> >> +
> >> +x86.bootsect: x86.boot
> >> +	dd if=$< of=$@ bs=256 count=2 skip=124
> >> +
> >> +x86.boot: x86.o
> >> +	$(CROSS_PREFIX)objcopy -O binary $< $@
> >> +
> >> +x86.o: x86-a-b-bootblock.S
> >> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
> >> +
> >> +clean:
> >> +	@rm -rf *.boot *.o *.bootsect
> >
> > We don't want to remove the generated header file when cleaning?
> 
> No, because we only generated the header file when running native, have
> the right crosscompilers, etc.  In general we only recompile it when we
> change the test file, otherwise we store the generated ones.
> 
> Idea is:
> We have a c file that does what we want.
> We compile it (when we have a propper compiler) and store it on the
> tree.
> We compile it for each test.
> 
> This is more "like firmware" than normal "qemu" source code.
>

Right. I momentarily forget that when replying to this patch.

Thanks,
drew
Wei Huang Sept. 4, 2018, 3:51 p.m. UTC | #5
On 09/03/2018 06:08 AM, Andrew Jones wrote:
> On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
>> The x86 boot block header currently is generated with a shell script.
>> To better support other CPUs (e.g. aarch64), we convert the script
>> into Makefile. This allows us to 1) support cross-compilation easily,
>> and 2) avoid creating a script file for every architecture.
>>
>> Note that, in the new design, the cross compiler prefix can be specified by
>> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
>> to include the C-style file correctly, it also renames the
>> x86-a-b-bootblock.s file extension from .s to .S.
>>
>> Signed-off-by: Wei Huang <wei@redhat.com>
>> ---
>>  tests/migration/Makefile                           | 31 ++++++++++++++++++++
>>  tests/migration/rebuild-x86-bootblock.sh           | 33 ----------------------
>>  .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  0
>>  3 files changed, 31 insertions(+), 33 deletions(-)
>>  create mode 100644 tests/migration/Makefile
>>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>>  rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (100%)
>>
>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
>> new file mode 100644
>> index 0000000..5d5fa07
>> --- /dev/null
>> +++ b/tests/migration/Makefile
>> @@ -0,0 +1,31 @@
>> +# To specify cross compiler prefix, use CROSS_PREFIX=
>> +#   > make CROSS_PREFIX=x86_64-linux-gnu-
>        ^ nit: this prompt symbol is weird (at least to me, what shell uses
>               this?) To me it looks like a sh/bash redirect symbol. Can
>               we change it to '$' or use nothing at all?

I will replace '>' with '$', which is indeed the commonly-used one.

>> +
>> +override define __note
>> +/* This file is automatically generated from
>> + * tests/migration/x86-a-b-bootblock.s, edit that and then run
>                                         ^ should be 'S'

OK, will do.

> 
>> + * tests/migration/rebuild-x86-bootblock.sh to update,
>> + * and then remember to send both in your patch submission.
>> + */
>> +endef
>> +export __note
>> +
>> +.PHONY: all clean
>> +all: x86-a-b-bootblock.h
>> +
>> +x86-a-b-bootblock.h: x86.bootsect
>> +	echo "$$__note" > header.tmp
>> +	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>> +	mv header.tmp $@
> 
> The shell script this Makefile is replacing used mktemp
> for a randomly named tmp dir. Shouldn't we continue to
> use random names?>
>> +
>> +x86.bootsect: x86.boot
>> +	dd if=$< of=$@ bs=256 count=2 skip=124
>> +
>> +x86.boot: x86.o
>> +	$(CROSS_PREFIX)objcopy -O binary $< $@
>> +
>> +x86.o: x86-a-b-bootblock.S
>> +	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
>> +
>> +clean:
>> +	@rm -rf *.boot *.o *.bootsect
> 
> We don't want to remove the generated header file when cleaning?
> 
>> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
>> deleted file mode 100755
>> index 86cec5d..0000000
>> --- a/tests/migration/rebuild-x86-bootblock.sh
>> +++ /dev/null
>> @@ -1,33 +0,0 @@
>> -#!/bin/sh
>> -# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
>> -# This work is licensed under the terms of the GNU GPL, version 2 or later.
>> -# See the COPYING file in the top-level directory.
>> -#
>> -# Author: dgilbert@redhat.com
>> -
>> -ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
>> -HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
>> -
>> -if [ ! -e "$ASMFILE" ]
>> -then
>> -  echo "Couldn't find $ASMFILE" >&2
>> -  exit 1
>> -fi
>> -
>> -ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XXXXXX)
>> -cd "$ASM_WORK_DIR" &&
>> -as --32 -march=i486 "$ASMFILE" -o x86.o &&
>> -objcopy -O binary x86.o x86.boot &&
>> -dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124 &&
>> -xxd -i x86.bootsect |
>> -sed -e 's/.*int.*//' > x86.hex &&
>> -cat - x86.hex <<HERE > "$HEADER"
>> -/* This file is automatically generated from
>> - * tests/migration/x86-a-b-bootblock.s, edit that and then run
>> - * tests/migration/rebuild-x86-bootblock.sh to update,
>> - * and then remember to send both in your patch submission.
>> - */
>> -HERE
>> -
>> -rm x86.hex x86.bootsect x86.boot x86.o
>> -cd .. && rmdir "$ASM_WORK_DIR"
>> diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
>> similarity index 100%
>> rename from tests/migration/x86-a-b-bootblock.s
>> rename to tests/migration/x86-a-b-bootblock.S
>> -- 
>> 1.8.3.1
>>
>>
> 
> Thanks,
> drew 
>
diff mbox series

Patch

diff --git a/tests/migration/Makefile b/tests/migration/Makefile
new file mode 100644
index 0000000..5d5fa07
--- /dev/null
+++ b/tests/migration/Makefile
@@ -0,0 +1,31 @@ 
+# To specify cross compiler prefix, use CROSS_PREFIX=
+#   > make CROSS_PREFIX=x86_64-linux-gnu-
+
+override define __note
+/* This file is automatically generated from
+ * tests/migration/x86-a-b-bootblock.s, edit that and then run
+ * tests/migration/rebuild-x86-bootblock.sh to update,
+ * and then remember to send both in your patch submission.
+ */
+endef
+export __note
+
+.PHONY: all clean
+all: x86-a-b-bootblock.h
+
+x86-a-b-bootblock.h: x86.bootsect
+	echo "$$__note" > header.tmp
+	xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
+	mv header.tmp $@
+
+x86.bootsect: x86.boot
+	dd if=$< of=$@ bs=256 count=2 skip=124
+
+x86.boot: x86.o
+	$(CROSS_PREFIX)objcopy -O binary $< $@
+
+x86.o: x86-a-b-bootblock.S
+	$(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
+
+clean:
+	@rm -rf *.boot *.o *.bootsect
diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
deleted file mode 100755
index 86cec5d..0000000
--- a/tests/migration/rebuild-x86-bootblock.sh
+++ /dev/null
@@ -1,33 +0,0 @@ 
-#!/bin/sh
-# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
-# This work is licensed under the terms of the GNU GPL, version 2 or later.
-# See the COPYING file in the top-level directory.
-#
-# Author: dgilbert@redhat.com
-
-ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
-HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
-
-if [ ! -e "$ASMFILE" ]
-then
-  echo "Couldn't find $ASMFILE" >&2
-  exit 1
-fi
-
-ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XXXXXX)
-cd "$ASM_WORK_DIR" &&
-as --32 -march=i486 "$ASMFILE" -o x86.o &&
-objcopy -O binary x86.o x86.boot &&
-dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124 &&
-xxd -i x86.bootsect |
-sed -e 's/.*int.*//' > x86.hex &&
-cat - x86.hex <<HERE > "$HEADER"
-/* This file is automatically generated from
- * tests/migration/x86-a-b-bootblock.s, edit that and then run
- * tests/migration/rebuild-x86-bootblock.sh to update,
- * and then remember to send both in your patch submission.
- */
-HERE
-
-rm x86.hex x86.bootsect x86.boot x86.o
-cd .. && rmdir "$ASM_WORK_DIR"
diff --git a/tests/migration/x86-a-b-bootblock.s b/tests/migration/x86-a-b-bootblock.S
similarity index 100%
rename from tests/migration/x86-a-b-bootblock.s
rename to tests/migration/x86-a-b-bootblock.S