Xen devel - Oct 2013 - [PATCH] tools: ovmf debug build only if tools debug is enabled

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
 tools/firmware/ovmf-makefile |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
index 073ed44..efb4fb1 100644
--- a/tools/firmware/ovmf-makefile
+++ b/tools/firmware/ovmf-makefile
@@ -1,3 +1,6 @@
+XEN_ROOT = $(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
 # OVMF building system is not ready yet to run in parallel.
 # Force it to be serial in order to exploit parallelism for neighbors.
 
@@ -9,8 +12,14 @@ all: ovmf.bin
 
 .PHONY: ovmf.bin
 ovmf.bin:
-	OvmfPkg/build.sh -a X64
-	cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
+	if [ "$(debug)" == y ]; then \
+		OvmfPkg/build.sh -a X64; \
+		cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin; \
+	else \
+		OvmfPkg/build.sh -a X64 -b RELEASE; \
+		cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd ovmf.bin; \
+	fi
+
 
 .PHONY: clean
 clean:
-- 
1.7.9.5

Does it make a big difference in terms of speed when running DEBUG build
and RELEASE build?

On Thu, Oct 31, 2013 at 03:02:50PM +0100, Fabio Fantoni
wrote:
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> ---
>  tools/firmware/ovmf-makefile |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> index 073ed44..efb4fb1 100644
> --- a/tools/firmware/ovmf-makefile
> +++ b/tools/firmware/ovmf-makefile
> @@ -1,3 +1,6 @@
> +XEN_ROOT = $(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
>  # OVMF building system is not ready yet to run in parallel.
>  # Force it to be serial in order to exploit parallelism for neighbors.
>  
> @@ -9,8 +12,14 @@ all: ovmf.bin
>  
>  .PHONY: ovmf.bin
>  ovmf.bin:
> -	OvmfPkg/build.sh -a X64
> -	cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
> +	if [ "$(debug)" == y ]; then \
> +		OvmfPkg/build.sh -a X64; \
> +		cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin; \
> +	else \
> +		OvmfPkg/build.sh -a X64 -b RELEASE; \
> +		cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd ovmf.bin; \
> +	fi
> +
>  
>  .PHONY: clean
>  clean:
> -- 
> 1.7.9.5

On 31/10/13 14:02, Fabio Fantoni wrote:
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> ---
>  tools/firmware/ovmf-makefile |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> index 073ed44..efb4fb1 100644
> --- a/tools/firmware/ovmf-makefile
> +++ b/tools/firmware/ovmf-makefile
> @@ -1,3 +1,6 @@
> +XEN_ROOT = $(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
>  # OVMF building system is not ready yet to run in parallel.
>  # Force it to be serial in order to exploit parallelism for neighbors.
>  
> @@ -9,8 +12,14 @@ all: ovmf.bin
>  
>  .PHONY: ovmf.bin
I know this is not your bad code, but this rule creates a file called
ovmf.bin, so is very explicitly not a .PHONY rule
>  ovmf.bin:
> -	OvmfPkg/build.sh -a X64
> -	cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
> +	if [ "$(debug)" == y ]; then \
> +		OvmfPkg/build.sh -a X64; \
> +		cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin; \
> +	else \
> +		OvmfPkg/build.sh -a X64 -b RELEASE; \
> +		cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd ovmf.bin; \
> +	fi
> +
This is rather ugly, and will add yet more spew into the logs (which is
in need of some pruning anyway).  In this case the entire if statement
will be printed, including both branches, before being executed.

The more "make" way of doing it would be something like:

ovmf.bin:
ifeq ($(debug),y)
        OvmfPkg/build.sh -a X64
        cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd $@
else
        OvmfPkg/build.sh -a X64 -b RELEASE
        cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd $@
endif

~Andrew

Il 31/10/2013 15:34, Andrew Cooper ha scritto:
> On 31/10/13 14:02, Fabio Fantoni wrote:
>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>> ---
>>   tools/firmware/ovmf-makefile |   13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/firmware/ovmf-makefile
b/tools/firmware/ovmf-makefile
>> index 073ed44..efb4fb1 100644
>> --- a/tools/firmware/ovmf-makefile
>> +++ b/tools/firmware/ovmf-makefile
>> @@ -1,3 +1,6 @@
>> +XEN_ROOT = $(CURDIR)/../../..
>> +include $(XEN_ROOT)/tools/Rules.mk
>> +
>>   # OVMF building system is not ready yet to run in parallel.
>>   # Force it to be serial in order to exploit parallelism for
neighbors.
>>   
>> @@ -9,8 +12,14 @@ all: ovmf.bin
>>   
>>   .PHONY: ovmf.bin
> I know this is not your bad code, but this rule creates a file called
> ovmf.bin, so is very explicitly not a .PHONY rule
>
>>   ovmf.bin:
>> -	OvmfPkg/build.sh -a X64
>> -	cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
>> +	if [ "$(debug)" == y ]; then \
>> +		OvmfPkg/build.sh -a X64; \
>> +		cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin; \
>> +	else \
>> +		OvmfPkg/build.sh -a X64 -b RELEASE; \
>> +		cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd ovmf.bin; \
>> +	fi
>> +
> This is rather ugly, and will add yet more spew into the logs (which is
> in need of some pruning anyway).  In this case the entire if statement
> will be printed, including both branches, before being executed.
>
> The more "make" way of doing it would be something like:
>
> ovmf.bin:
> ifeq ($(debug),y)
>          OvmfPkg/build.sh -a X64
>          cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd $@
> else
>          OvmfPkg/build.sh -a X64 -b RELEASE
>          cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd $@
> endif
>
> ~Andrew
I already tried with this method before, but it seems that the 
conditional part of the makefile doesn''t work inside a target.
Later I changed it with the current patch (following "tools/Makefile")
tested and working.
Do I need to set +x before the "if" to not print that?

Il 31/10/2013 15:23, Wei Liu ha scritto:
> Does it make a big difference in terms of speed when running DEBUG build
> and RELEASE build?
In the working test (few months ago) the difference was noticeble - if I 
remember correctly - while now, without a uefi domu full working I
don''t
know.
I should do this patch after that one with the gcc!=44 but in the 
meantime a was working on other tasks so i forgot it, sorry.
>
> On Thu, Oct 31, 2013 at 03:02:50PM +0100, Fabio Fantoni wrote:
>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>> ---
>>   tools/firmware/ovmf-makefile |   13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/firmware/ovmf-makefile
b/tools/firmware/ovmf-makefile
>> index 073ed44..efb4fb1 100644
>> --- a/tools/firmware/ovmf-makefile
>> +++ b/tools/firmware/ovmf-makefile
>> @@ -1,3 +1,6 @@
>> +XEN_ROOT = $(CURDIR)/../../..
>> +include $(XEN_ROOT)/tools/Rules.mk
>> +
>>   # OVMF building system is not ready yet to run in parallel.
>>   # Force it to be serial in order to exploit parallelism for
neighbors.
>>   
>> @@ -9,8 +12,14 @@ all: ovmf.bin
>>   
>>   .PHONY: ovmf.bin
>>   ovmf.bin:
>> -	OvmfPkg/build.sh -a X64
>> -	cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
>> +	if [ "$(debug)" == y ]; then \
>> +		OvmfPkg/build.sh -a X64; \
>> +		cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin; \
>> +	else \
>> +		OvmfPkg/build.sh -a X64 -b RELEASE; \
>> +		cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd ovmf.bin; \
>> +	fi
>> +
>>   
>>   .PHONY: clean
>>   clean:
>> -- 
>> 1.7.9.5

Il 31/10/2013 15:52, Fabio Fantoni ha scritto:
> Il 31/10/2013 15:34, Andrew Cooper ha scritto:
>> On 31/10/13 14:02, Fabio Fantoni wrote:
>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>> ---
>>>   tools/firmware/ovmf-makefile |   13 +++++++++++--
>>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/firmware/ovmf-makefile 
>>> b/tools/firmware/ovmf-makefile
>>> index 073ed44..efb4fb1 100644
>>> --- a/tools/firmware/ovmf-makefile
>>> +++ b/tools/firmware/ovmf-makefile
>>> @@ -1,3 +1,6 @@
>>> +XEN_ROOT = $(CURDIR)/../../..
>>> +include $(XEN_ROOT)/tools/Rules.mk
>>> +
>>>   # OVMF building system is not ready yet to run in parallel.
>>>   # Force it to be serial in order to exploit parallelism for 
>>> neighbors.
>>>   @@ -9,8 +12,14 @@ all: ovmf.bin
>>>     .PHONY: ovmf.bin
>> I know this is not your bad code, but this rule creates a file called
>> ovmf.bin, so is very explicitly not a .PHONY rule
>>
>>>   ovmf.bin:
>>> -    OvmfPkg/build.sh -a X64
>>> -    cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
>>> +    if [ "$(debug)" == y ]; then \
>>> +        OvmfPkg/build.sh -a X64; \
>>> +        cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin; \
>>> +    else \
>>> +        OvmfPkg/build.sh -a X64 -b RELEASE; \
>>> +        cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd ovmf.bin; \
>>> +    fi
>>> +
>> This is rather ugly, and will add yet more spew into the logs (which is
>> in need of some pruning anyway).  In this case the entire if statement
>> will be printed, including both branches, before being executed.
>>
>> The more "make" way of doing it would be something like:
>>
>> ovmf.bin:
>> ifeq ($(debug),y)
>>          OvmfPkg/build.sh -a X64
>>          cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd $@
>> else
>>          OvmfPkg/build.sh -a X64 -b RELEASE
>>          cp Build/OvmfX64/RELEASE_GCC*/FV/OVMF.fd $@
>> endif
>>
>> ~Andrew
>
> I already tried with this method before, but it seems that the 
> conditional part of the makefile doesn''t work inside a target.
> Later I changed it with the current patch (following
"tools/Makefile")
> tested and working.
> Do I need to set +x before the "if" to not print that?
I not found a working parameter to disable the print of instructions to 
execute, is it really needed?
If yes, how should I do it?

Thanks for any reply.

On Wed, Dec 04, 2013 at 03:47:02PM +0100, Fabio Fantoni wrote:
[...]
> I not found a working parameter to disable the print of instructions
> to execute, is it really needed?
> If yes, how should I do it?
> 
> Thanks for any reply.
I have a patch in queue for this:


From 646417d80779c592a578a20962aced402d94e580 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Wed, 4 Dec 2013 16:21:23 +0000
Subject: [PATCH] tools/ovmf-makefile: only build debug target when specified

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/ovmf-makefile |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
index 073ed44..1ad041f 100644
--- a/tools/firmware/ovmf-makefile
+++ b/tools/firmware/ovmf-makefile
@@ -1,16 +1,25 @@
 # OVMF building system is not ready yet to run in parallel.
 # Force it to be serial in order to exploit parallelism for neighbors.
 
+XEN_ROOT=$(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+ifeq ($(debug),y)
+TARGET=DEBUG
+else
+TARGET=RELEASE
+endif
+
 .NOTPARALLEL:
 MAKEFLAGS  += -j1
 
 .PHONY: all
-all: ovmf.bin
+all: build
 
-.PHONY: ovmf.bin
-ovmf.bin:
-	OvmfPkg/build.sh -a X64
-	cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
+.PHONY: build
+build:
+	OvmfPkg/build.sh -a X64 -b $(TARGET)
+	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
 
 .PHONY: clean
 clean:
-- 
1.7.10.4

Il 04/12/2013 17:23, Wei Liu ha scritto:
> On Wed, Dec 04, 2013 at 03:47:02PM +0100, Fabio Fantoni wrote:
> [...]
>> I not found a working parameter to disable the print of instructions
>> to execute, is it really needed?
>> If yes, how should I do it?
>>
>> Thanks for any reply.
> I have a patch in queue for this:
>
>
>  From 646417d80779c592a578a20962aced402d94e580 Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Wed, 4 Dec 2013 16:21:23 +0000
> Subject: [PATCH] tools/ovmf-makefile: only build debug target when
specified
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Thispatch is better then no longer considered my patch.
> ---
>   tools/firmware/ovmf-makefile |   19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> index 073ed44..1ad041f 100644
> --- a/tools/firmware/ovmf-makefile
> +++ b/tools/firmware/ovmf-makefile
> @@ -1,16 +1,25 @@
>   # OVMF building system is not ready yet to run in parallel.
>   # Force it to be serial in order to exploit parallelism for neighbors.
>   
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +ifeq ($(debug),y)
> +TARGET=DEBUG
> +else
> +TARGET=RELEASE
> +endif
> +
>   .NOTPARALLEL:
>   MAKEFLAGS  += -j1
>   
>   .PHONY: all
> -all: ovmf.bin
> +all: build
>   
> -.PHONY: ovmf.bin
> -ovmf.bin:
> -	OvmfPkg/build.sh -a X64
> -	cp Build/OvmfX64/DEBUG_GCC*/FV/OVMF.fd ovmf.bin
> +.PHONY: build
> +build:
> +	OvmfPkg/build.sh -a X64 -b $(TARGET)
> +	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
>   
>   .PHONY: clean
>   clean: