[PATCH 0/2] Two patches for OVMF build system

[PATCH 0/2] Two patches for OVMF build system

[Wei Liu]

Wei Liu (2):
  tools: OVMF parallel build
  tools: provide distclean target for OVMF

 tools/firmware/ovmf-makefile | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH 1/2] tools: OVMF parallel build

[Wei Liu]

Though it doesn't work with make's "-j" option, the build system of OVMF
has an option to specify parallel threads used to run the build.

Using 4 threads to build OVMF looks like a sensible default.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/firmware/ovmf-makefile | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
index 1ad041f..2838744 100644
--- a/tools/firmware/ovmf-makefile
+++ b/tools/firmware/ovmf-makefile
@@ -1,6 +1,3 @@
-# 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
 
@@ -10,6 +7,7 @@ else
 TARGET=RELEASE
 endif
 
+# OVMF build system has its own parallel building support.
 .NOTPARALLEL:
 MAKEFLAGS  += -j1
 
@@ -18,7 +16,7 @@ all: build
 
 .PHONY: build
 build:
-	OvmfPkg/build.sh -a X64 -b $(TARGET)
+	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
 	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
 
 .PHONY: clean
-- 
1.9.1

[PATCH 2/2] tools: provide distclean target for OVMF

[Wei Liu]

OVMF's distclean target was missed in f9c9b1277 (tools: fix "make
distclean").

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/firmware/ovmf-makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
index 2838744..c922d4d 100644
--- a/tools/firmware/ovmf-makefile
+++ b/tools/firmware/ovmf-makefile
@@ -22,3 +22,6 @@ build:
 .PHONY: clean
 clean:
 	rm -rf ovmf.bin Build/*
+
+.PHONY: distclean
+distclean: clean
-- 
1.9.1

Re: [PATCH 1/2] tools: OVMF parallel build

[Ian Campbell]

On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> Though it doesn't work with make's "-j" option, the build system of OVMF
> has an option to specify parallel threads used to run the build.
> 
> Using 4 threads to build OVMF looks like a sensible default.

It's a bit arbitrary, but it'll do, I suppose.

It's worth noting that this won't interact with gnu makes token handling
which is used to make sure there are only N processes across the
recursive build. So this will consume 1 token but launch 4 threads. So
if -j4 is used you'll actually get 7 things happening.

I think we can live with that.

So I'm happy with the patch, but I'd like to give others a chance to
disagree before applying.

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  tools/firmware/ovmf-makefile | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> index 1ad041f..2838744 100644
> --- a/tools/firmware/ovmf-makefile
> +++ b/tools/firmware/ovmf-makefile
> @@ -1,6 +1,3 @@
> -# 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
>  
> @@ -10,6 +7,7 @@ else
>  TARGET=RELEASE
>  endif
>  
> +# OVMF build system has its own parallel building support.
>  .NOTPARALLEL:
>  MAKEFLAGS  += -j1

TBH I'm not sure what if anything these actually do...

>  
> @@ -18,7 +16,7 @@ all: build
>  
>  .PHONY: build
>  build:
> -	OvmfPkg/build.sh -a X64 -b $(TARGET)
> +	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
>  	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
>  
>  .PHONY: clean

Re: [PATCH 2/2] tools: provide distclean target for OVMF

[Ian Campbell]

On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> OVMF's distclean target was missed in f9c9b1277 (tools: fix "make
> distclean").

tools/firmware/Makefile has:
subdir-distclean-ovmf: .phony
        rm -rf ovmf-dir ovmf-dir-remote

Shouldn't this be avoiding recursing into ovmf-dir in the first place?

Or is there some other reason to want this?

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  tools/firmware/ovmf-makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> index 2838744..c922d4d 100644
> --- a/tools/firmware/ovmf-makefile
> +++ b/tools/firmware/ovmf-makefile
> @@ -22,3 +22,6 @@ build:
>  .PHONY: clean
>  clean:
>  	rm -rf ovmf.bin Build/*
> +
> +.PHONY: distclean
> +distclean: clean

Re: [PATCH 2/2] tools: provide distclean target for OVMF

[Wei Liu]

On Mon, Mar 02, 2015 at 02:20:57PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> > OVMF's distclean target was missed in f9c9b1277 (tools: fix "make
> > distclean").
> 
> tools/firmware/Makefile has:
> subdir-distclean-ovmf: .phony
>         rm -rf ovmf-dir ovmf-dir-remote
> 
> Shouldn't this be avoiding recursing into ovmf-dir in the first place?
> 
> Or is there some other reason to want this?
> 

If you're in firmware directory and do "make distclean", make will
recurse into ovmf to do "make distclean".

Wei.

> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > ---
> >  tools/firmware/ovmf-makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > index 2838744..c922d4d 100644
> > --- a/tools/firmware/ovmf-makefile
> > +++ b/tools/firmware/ovmf-makefile
> > @@ -22,3 +22,6 @@ build:
> >  .PHONY: clean
> >  clean:
> >  	rm -rf ovmf.bin Build/*
> > +
> > +.PHONY: distclean
> > +distclean: clean
> 

Re: [PATCH 2/2] tools: provide distclean target for OVMF

[Ian Campbell]

On Mon, 2015-03-02 at 14:31 +0000, Wei Liu wrote:
> On Mon, Mar 02, 2015 at 02:20:57PM +0000, Ian Campbell wrote:
> > On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> > > OVMF's distclean target was missed in f9c9b1277 (tools: fix "make
> > > distclean").
> > 
> > tools/firmware/Makefile has:
> > subdir-distclean-ovmf: .phony
> >         rm -rf ovmf-dir ovmf-dir-remote
> > 
> > Shouldn't this be avoiding recursing into ovmf-dir in the first place?
> > 
> > Or is there some other reason to want this?
> > 
> 
> If you're in firmware directory and do "make distclean", make will
> recurse into ovmf to do "make distclean".

Really? Despite the rule I quoted above? How strange, I wonder why that
is.

I'd have expected "make distclean" to behave the same whether run in the
directory or recursed into frmo a higher level.

> 
> Wei.
> 
> > > 
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > ---
> > >  tools/firmware/ovmf-makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > > index 2838744..c922d4d 100644
> > > --- a/tools/firmware/ovmf-makefile
> > > +++ b/tools/firmware/ovmf-makefile
> > > @@ -22,3 +22,6 @@ build:
> > >  .PHONY: clean
> > >  clean:
> > >  	rm -rf ovmf.bin Build/*
> > > +
> > > +.PHONY: distclean
> > > +distclean: clean
> > 

Re: [PATCH 2/2] tools: provide distclean target for OVMF

[Wei Liu]

On Mon, Mar 02, 2015 at 02:35:31PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-02 at 14:31 +0000, Wei Liu wrote:
> > On Mon, Mar 02, 2015 at 02:20:57PM +0000, Ian Campbell wrote:
> > > On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> > > > OVMF's distclean target was missed in f9c9b1277 (tools: fix "make
> > > > distclean").
> > > 
> > > tools/firmware/Makefile has:
> > > subdir-distclean-ovmf: .phony
> > >         rm -rf ovmf-dir ovmf-dir-remote
> > > 
> > > Shouldn't this be avoiding recursing into ovmf-dir in the first place?
> > > 
> > > Or is there some other reason to want this?
> > > 
> > 
> > If you're in firmware directory and do "make distclean", make will
> > recurse into ovmf to do "make distclean".
> 
> Really? Despite the rule I quoted above? How strange, I wonder why that
> is.
> 
> I'd have expected "make distclean" to behave the same whether run in the
> directory or recursed into frmo a higher level.
> 

The problem is that subdir-distclean-ovmf should have been
subdir-distclean-ovmf-dir. I think you prefer I fix it this way?

subidr-clean-ovmf should be -ovmf-dir following the same rule. I shall
fix them all in one go.

Wei.

> > 
> > Wei.
> > 
> > > > 
> > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > ---
> > > >  tools/firmware/ovmf-makefile | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > > > index 2838744..c922d4d 100644
> > > > --- a/tools/firmware/ovmf-makefile
> > > > +++ b/tools/firmware/ovmf-makefile
> > > > @@ -22,3 +22,6 @@ build:
> > > >  .PHONY: clean
> > > >  clean:
> > > >  	rm -rf ovmf.bin Build/*
> > > > +
> > > > +.PHONY: distclean
> > > > +distclean: clean
> > > 
> 

Re: [PATCH 2/2] tools: provide distclean target for OVMF

[Ian Campbell]

On Mon, 2015-03-02 at 14:45 +0000, Wei Liu wrote:
> On Mon, Mar 02, 2015 at 02:35:31PM +0000, Ian Campbell wrote:
> > On Mon, 2015-03-02 at 14:31 +0000, Wei Liu wrote:
> > > On Mon, Mar 02, 2015 at 02:20:57PM +0000, Ian Campbell wrote:
> > > > On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> > > > > OVMF's distclean target was missed in f9c9b1277 (tools: fix "make
> > > > > distclean").
> > > > 
> > > > tools/firmware/Makefile has:
> > > > subdir-distclean-ovmf: .phony
> > > >         rm -rf ovmf-dir ovmf-dir-remote
> > > > 
> > > > Shouldn't this be avoiding recursing into ovmf-dir in the first place?
> > > > 
> > > > Or is there some other reason to want this?
> > > > 
> > > 
> > > If you're in firmware directory and do "make distclean", make will
> > > recurse into ovmf to do "make distclean".
> > 
> > Really? Despite the rule I quoted above? How strange, I wonder why that
> > is.
> > 
> > I'd have expected "make distclean" to behave the same whether run in the
> > directory or recursed into frmo a higher level.
> > 
> 
> The problem is that subdir-distclean-ovmf should have been
> subdir-distclean-ovmf-dir. I think you prefer I fix it this way?

You guessed right!

> subidr-clean-ovmf should be -ovmf-dir following the same rule. I shall
> fix them all in one go.

Yes please.

> 
> Wei.
> 
> > > 
> > > Wei.
> > > 
> > > > > 
> > > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > > > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > > ---
> > > > >  tools/firmware/ovmf-makefile | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > > > > index 2838744..c922d4d 100644
> > > > > --- a/tools/firmware/ovmf-makefile
> > > > > +++ b/tools/firmware/ovmf-makefile
> > > > > @@ -22,3 +22,6 @@ build:
> > > > >  .PHONY: clean
> > > > >  clean:
> > > > >  	rm -rf ovmf.bin Build/*
> > > > > +
> > > > > +.PHONY: distclean
> > > > > +distclean: clean
> > > > 
> > 

Re: [PATCH 1/2] tools: OVMF parallel build

[Wei Liu]

On Mon, Mar 02, 2015 at 02:18:58PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> > Though it doesn't work with make's "-j" option, the build system of OVMF
> > has an option to specify parallel threads used to run the build.
> > 
> > Using 4 threads to build OVMF looks like a sensible default.
> 
> It's a bit arbitrary, but it'll do, I suppose.
> 
> It's worth noting that this won't interact with gnu makes token handling
> which is used to make sure there are only N processes across the
> recursive build. So this will consume 1 token but launch 4 threads. So
> if -j4 is used you'll actually get 7 things happening.
> 
> I think we can live with that.
> 
> So I'm happy with the patch, but I'd like to give others a chance to
> disagree before applying.
> 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > ---
> >  tools/firmware/ovmf-makefile | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > index 1ad041f..2838744 100644
> > --- a/tools/firmware/ovmf-makefile
> > +++ b/tools/firmware/ovmf-makefile
> > @@ -1,6 +1,3 @@
> > -# 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
> >  
> > @@ -10,6 +7,7 @@ else
> >  TARGET=RELEASE
> >  endif
> >  
> > +# OVMF build system has its own parallel building support.
> >  .NOTPARALLEL:
> >  MAKEFLAGS  += -j1
> 
> TBH I'm not sure what if anything these actually do...
> 

Without this OVMF won't build. It plays with MAKEFLAGS to enforce there
is only one thread used. I haven't really looked into whether it's the
correct rune but it seems to work fine.

Wei.


> >  
> > @@ -18,7 +16,7 @@ all: build
> >  
> >  .PHONY: build
> >  build:
> > -	OvmfPkg/build.sh -a X64 -b $(TARGET)
> > +	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
> >  	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
> >  
> >  .PHONY: clean
> 

Re: [PATCH 1/2] tools: OVMF parallel build

[Ian Campbell]

On Mon, 2015-03-02 at 14:18 +0000, Ian Campbell wrote:
> On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> > Though it doesn't work with make's "-j" option, the build system of OVMF
> > has an option to specify parallel threads used to run the build.
> > 
> > Using 4 threads to build OVMF looks like a sensible default.
> 
> It's a bit arbitrary, but it'll do, I suppose.
> 
> It's worth noting that this won't interact with gnu makes token handling
> which is used to make sure there are only N processes across the
> recursive build. So this will consume 1 token but launch 4 threads. So
> if -j4 is used you'll actually get 7 things happening.
> 
> I think we can live with that.
> 
> So I'm happy with the patch, but I'd like to give others a chance to
> disagree before applying.

They had their chance...
> 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

... applied.

Did the distclean issue which the second patch was addressing get fixed
another way or is there a patch outstanding somewhere?

Re: [PATCH 1/2] tools: OVMF parallel build

[Wei Liu]

On Wed, Mar 11, 2015 at 04:41:32PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-02 at 14:18 +0000, Ian Campbell wrote:
> > On Mon, 2015-03-02 at 10:52 +0000, Wei Liu wrote:
> > > Though it doesn't work with make's "-j" option, the build system of OVMF
> > > has an option to specify parallel threads used to run the build.
> > > 
> > > Using 4 threads to build OVMF looks like a sensible default.
> > 
> > It's a bit arbitrary, but it'll do, I suppose.
> > 
> > It's worth noting that this won't interact with gnu makes token handling
> > which is used to make sure there are only N processes across the
> > recursive build. So this will consume 1 token but launch 4 threads. So
> > if -j4 is used you'll actually get 7 things happening.
> > 
> > I think we can live with that.
> > 
> > So I'm happy with the patch, but I'd like to give others a chance to
> > disagree before applying.
> 
> They had their chance...
> > 
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> ... applied.
> 
> Did the distclean issue which the second patch was addressing get fixed
> another way or is there a patch outstanding somewhere?

Urgh. I thought I posted it somewhere, but apparently I was wrong.

I will send it now.

Wei.