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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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@xxxxxxxxxx>
> > > > 
> > > > ---
> > > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > > > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > 
> > > >      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@xxxxxxxxxx>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel