[PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[Julien Grall]

The firmware directory is not built at all on ARM. Attempting to update
it using the target subtree-force-update will fail when try to update
seabios.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

    I've noticed it while trying to update the QEMU tree used by Xen on
    a platform where iasl is not present (required by seabios in order
    to update it).

    I think this should go in Xen 4.6 and possibly backport to Xen 4.5
---
 tools/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/Makefile b/tools/Makefile
index 45cb4b2..2618559 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -305,7 +305,9 @@ endif
 ifeq ($(CONFIG_QEMU_TRAD),y)
 	$(MAKE) qemu-xen-traditional-dir-force-update
 endif
+ifeq ($(CONFIG_X86),y)
 	$(MAKE) -C firmware subtree-force-update
+endif
 
 subtree-force-update-all:
 	$(MAKE) qemu-xen-dir-force-update
-- 
2.1.4

Re: [PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[Wei Liu]

On Fri, Aug 07, 2015 at 06:27:18PM +0100, Julien Grall wrote:
> The firmware directory is not built at all on ARM. Attempting to update
> it using the target subtree-force-update will fail when try to update
> seabios.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> 
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> 
>     I've noticed it while trying to update the QEMU tree used by Xen on
>     a platform where iasl is not present (required by seabios in order
>     to update it).
> 
>     I think this should go in Xen 4.6 and possibly backport to Xen 4.5
> ---
>  tools/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 45cb4b2..2618559 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -305,7 +305,9 @@ endif
>  ifeq ($(CONFIG_QEMU_TRAD),y)
>  	$(MAKE) qemu-xen-traditional-dir-force-update
>  endif
> +ifeq ($(CONFIG_X86),y)
>  	$(MAKE) -C firmware subtree-force-update
> +endif
>  

This is not optimal. What if you want to build OVMF on arm in the
future? You also can't preclude you don't have any other firmwares that
need to be built on ARM in the future.

I think a proper way of doing this is to make CONFIG_SEABIOS=n when
you're building on ARM. See tools/configure.ac.

Wei.

>  subtree-force-update-all:
>  	$(MAKE) qemu-xen-dir-force-update
> -- 
> 2.1.4

Re: [PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[Julien Grall]

Hi Wei,

On 08/08/2015 16:16, Wei Liu wrote:
> On Fri, Aug 07, 2015 at 06:27:18PM +0100, Julien Grall wrote:
>> The firmware directory is not built at all on ARM. Attempting to update
>> it using the target subtree-force-update will fail when try to update
>> seabios.
>>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>>
>> ---
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>>
>>      I've noticed it while trying to update the QEMU tree used by Xen on
>>      a platform where iasl is not present (required by seabios in order
>>      to update it).
>>
>>      I think this should go in Xen 4.6 and possibly backport to Xen 4.5
>> ---
>>   tools/Makefile | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 45cb4b2..2618559 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -305,7 +305,9 @@ endif
>>   ifeq ($(CONFIG_QEMU_TRAD),y)
>>   	$(MAKE) qemu-xen-traditional-dir-force-update
>>   endif
>> +ifeq ($(CONFIG_X86),y)
>>   	$(MAKE) -C firmware subtree-force-update
>> +endif
>>
>
> This is not optimal. What if you want to build OVMF on arm in the
> future? You also can't preclude you don't have any other firmwares that
> need to be built on ARM in the future.
> I think a proper way of doing this is to make CONFIG_SEABIOS=n when
> you're building on ARM. See tools/configure.ac.

tools/Makefile only build the firmware directory for x86 see:

SUBDIRS-$(CONFIG_X86) += firmware

Hence why I wrote the patch in the current way.

Building the firmware directory for would require more work than replace 
SUBDIRS-$(CONFIG_X86) to SUBDIRS-y.
In general, I do agree that we enable this with configure.ac but, IHMO 
this is not Xen 4.6 material...

Although I would be happy to fix it for Xen 4.7.

Regards,

-- 
Julien Grall

Re: [PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[Ian Campbell]

On Sun, 2015-08-09 at 14:49 +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 08/08/2015 16:16, Wei Liu wrote:
> > On Fri, Aug 07, 2015 at 06:27:18PM +0100, Julien Grall wrote:
> > > The firmware directory is not built at all on ARM. Attempting to 
> > > update
> > > it using the target subtree-force-update will fail when try to update
> > > seabios.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@citrix.com>
> > > 
> > > ---
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > 
> > >      I've noticed it while trying to update the QEMU tree used by Xen 
> > > on
> > >      a platform where iasl is not present (required by seabios in 
> > > order
> > >      to update it).
> > > 
> > >      I think this should go in Xen 4.6 and possibly backport to Xen 
> > > 4.5
> > > ---
> > >   tools/Makefile | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/tools/Makefile b/tools/Makefile
> > > index 45cb4b2..2618559 100644
> > > --- a/tools/Makefile
> > > +++ b/tools/Makefile
> > > @@ -305,7 +305,9 @@ endif
> > >   ifeq ($(CONFIG_QEMU_TRAD),y)
> > >   	$(MAKE) qemu-xen-traditional-dir-force-update
> > >   endif
> > > +ifeq ($(CONFIG_X86),y)
> > >   	$(MAKE) -C firmware subtree-force-update
> > > +endif
> > > 
> > 
> > This is not optimal. What if you want to build OVMF on arm in the
> > future?

Slight aside, but I already looked at doing this but concluded that the
right answer was to add this to raisin not xen.git. As it happens on ARM we
would boot the UEFI binary directly, so we don't need to compile it into
hvmloader or just through other hoops, so it is a bit easier than on x86.

> >  You also can't preclude you don't have any other firmwares that
> > need to be built on ARM in the future.
> > I think a proper way of doing this is to make CONFIG_SEABIOS=n when
> > you're building on ARM. See tools/configure.ac.
> 
> tools/Makefile only build the firmware directory for x86 see:
> 
> SUBDIRS-$(CONFIG_X86) += firmware
> 
> Hence why I wrote the patch in the current way.

I think having the update rule match (in spirit at least) the SUBDIRS rules
make sense as a patch for now, so I'm in favour of taking this patch as it
is.

> Building the firmware directory for would require more work than replace 
> SUBDIRS-$(CONFIG_X86) to SUBDIRS-y.
> In general, I do agree that we enable this with configure.ac but, IHMO 
> this is not Xen 4.6 material...
> 
> Although I would be happy to fix it for Xen 4.7.
> 
> Regards,
> 

Re: [PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[Wei Liu]

On Tue, Aug 11, 2015 at 01:22:24PM +0100, Ian Campbell wrote:
> On Sun, 2015-08-09 at 14:49 +0100, Julien Grall wrote:
> > Hi Wei,
> > 
> > On 08/08/2015 16:16, Wei Liu wrote:
> > > On Fri, Aug 07, 2015 at 06:27:18PM +0100, Julien Grall wrote:
> > > > The firmware directory is not built at all on ARM. Attempting to 
> > > > update
> > > > it using the target subtree-force-update will fail when try to update
> > > > seabios.
> > > > 
> > > > Signed-off-by: Julien Grall <julien.grall@citrix.com>
> > > > 
> > > > ---
> > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > 
> > > >      I've noticed it while trying to update the QEMU tree used by Xen 
> > > > on
> > > >      a platform where iasl is not present (required by seabios in 
> > > > order
> > > >      to update it).
> > > > 
> > > >      I think this should go in Xen 4.6 and possibly backport to Xen 
> > > > 4.5
> > > > ---
> > > >   tools/Makefile | 2 ++
> > > >   1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/tools/Makefile b/tools/Makefile
> > > > index 45cb4b2..2618559 100644
> > > > --- a/tools/Makefile
> > > > +++ b/tools/Makefile
> > > > @@ -305,7 +305,9 @@ endif
> > > >   ifeq ($(CONFIG_QEMU_TRAD),y)
> > > >   	$(MAKE) qemu-xen-traditional-dir-force-update
> > > >   endif
> > > > +ifeq ($(CONFIG_X86),y)
> > > >   	$(MAKE) -C firmware subtree-force-update
> > > > +endif
> > > > 
> > > 
> > > This is not optimal. What if you want to build OVMF on arm in the
> > > future?
> 
> Slight aside, but I already looked at doing this but concluded that the
> right answer was to add this to raisin not xen.git. As it happens on ARM we
> would boot the UEFI binary directly, so we don't need to compile it into
> hvmloader or just through other hoops, so it is a bit easier than on x86.
> 

Right. Makes sense.

> > >  You also can't preclude you don't have any other firmwares that
> > > need to be built on ARM in the future.
> > > I think a proper way of doing this is to make CONFIG_SEABIOS=n when
> > > you're building on ARM. See tools/configure.ac.
> > 
> > tools/Makefile only build the firmware directory for x86 see:
> > 
> > SUBDIRS-$(CONFIG_X86) += firmware
> > 
> > Hence why I wrote the patch in the current way.
> 
> I think having the update rule match (in spirit at least) the SUBDIRS rules
> make sense as a patch for now, so I'm in favour of taking this patch as it
> is.
> 

Fine by me then.

Acked-by: Wei Liu <wei.liu2@citrix.com>

> > Building the firmware directory for would require more work than replace 
> > SUBDIRS-$(CONFIG_X86) to SUBDIRS-y.
> > In general, I do agree that we enable this with configure.ac but, IHMO 
> > this is not Xen 4.6 material...
> > 
> > Although I would be happy to fix it for Xen 4.7.
> > 
> > Regards,
> > 

Re: [PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[Wei Liu]

On Fri, Aug 07, 2015 at 06:27:18PM +0100, Julien Grall wrote:
> The firmware directory is not built at all on ARM. Attempting to update
> it using the target subtree-force-update will fail when try to update
> seabios.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

> 
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> 
>     I've noticed it while trying to update the QEMU tree used by Xen on
>     a platform where iasl is not present (required by seabios in order
>     to update it).
> 
>     I think this should go in Xen 4.6 and possibly backport to Xen 4.5
> ---
>  tools/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 45cb4b2..2618559 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -305,7 +305,9 @@ endif
>  ifeq ($(CONFIG_QEMU_TRAD),y)
>  	$(MAKE) qemu-xen-traditional-dir-force-update
>  endif
> +ifeq ($(CONFIG_X86),y)
>  	$(MAKE) -C firmware subtree-force-update
> +endif
>  
>  subtree-force-update-all:
>  	$(MAKE) qemu-xen-dir-force-update
> -- 
> 2.1.4

Re: [PATCH for-4.6] tools: Don't try to update the firmware directory on ARM

[Ian Campbell]

On Thu, 2015-08-13 at 10:44 +0100, Wei Liu wrote:
> On Fri, Aug 07, 2015 at 06:27:18PM +0100, Julien Grall wrote:
> > The firmware directory is not built at all on ARM. Attempting to update
> > it using the target subtree-force-update will fail when try to update
> > seabios.
> > 
> > Signed-off-by: Julien Grall <julien.grall@citrix.com>
> 
> Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Acked + applied, thanks.