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

[Ian Campbell <ian.campbell@citrix.com>]

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,
>