Re: [Qemu-ppc] [Qemu-devel] [PULL 02/29] ppc/spapr: Receive and store device tree blob from SLOF

[David Gibson]

On Wed, Mar 27, 2019 at 01:36:59PM +0100, Greg Kurz wrote:
> On Wed, 27 Mar 2019 23:06:42 +1100
> David Gibson <address@hidden> wrote:
> 
> > On Wed, Mar 27, 2019 at 10:44:03AM +0100, Greg Kurz wrote:
> > > On Wed, 27 Mar 2019 19:38:27 +1100
> > > David Gibson <address@hidden> wrote:
> > >   
> > > > On Wed, Mar 27, 2019 at 08:09:26AM +0100, Greg Kurz wrote:  
> > > > > On Wed, 27 Mar 2019 11:56:10 +1100
> > > > > David Gibson <address@hidden> wrote:
> > > > >     
> > > > > > On Tue, Mar 26, 2019 at 08:09:53AM +0100, Greg Kurz wrote:    
> > > > > > > On Tue, 26 Mar 2019 10:47:15 +1100
> > > > > > > David Gibson <address@hidden> wrote:
> > > > > > >       
> > > > > > > > On Mon, Mar 25, 2019 at 05:33:21PM +0100, Greg Kurz wrote:      
> > > > > > > > > On Mon, 25 Mar 2019 11:53:47 +1100
> > > > > > > > > David Gibson <address@hidden> wrote:
> > > > > > > > >         
> > > > > > > > > > On Sun, Mar 24, 2019 at 12:03:54AM -0400, Brad Smith wrote: 
> > > > > > > > > >        
> > > > > > > > > > > Now that I am checking out 4.0.0 rc's I see this diff is 
> > > > > > > > > > > broken and
> > > > > > > > > > > depends on a function libfdt does not expose. The 
> > > > > > > > > > > breakage is
> > > > > > > > > > > hidden by the fallback check in the configure script.     
> > > > > > > > > > >      
> > > > > > > > > > 
> > > > > > > > > > Ah, bother.  That keeps happening, unfortunately.  I think 
> > > > > > > > > > it's
> > > > > > > > > > because so many people use libfdt embedded, rather than as 
> > > > > > > > > > a shared
> > > > > > > > > > library that we tend not to notice.
> > > > > > > > > >         
> > > > > > > > > 
> > > > > > > > > It's a bit more complicated. I do have latest libfdt packages 
> > > > > > > > > on my laptop:
> > > > > > > > > 
> > > > > > > > > libfdt-1.4.7-2.fc28.x86_64
> > > > > > > > > libfdt-devel-1.4.7-2.fc28.x86_64
> > > > > > > > > 
> > > > > > > > > but I still end up using the embedded one and the build 
> > > > > > > > > doesn't spot
> > > > > > > > > the missing symbols.        
> > > > > > > > 
> > > > > > > > Sorry, I wasn't clear.  I wasn't meaning in the context of 
> > > > > > > > qemu, but
> > > > > > > > for dtc generally.  A large portion of the users are things like
> > > > > > > > u-boot that have to use dtc embedded, rather than as a shared
> > > > > > > > library.  That's why we tend not to notice missing symbols from 
> > > > > > > > the
> > > > > > > > version script upstream.
> > > > > > > >       
> > > > > > > 
> > > > > > > Ok, I get it.
> > > > > > >       
> > > > > > > > > This happens because of several reasons:
> > > > > > > > > 
> > > > > > > > > 1) configure unconditionally falls back to embedded if an 
> > > > > > > > > error occurs with
> > > > > > > > >    the system packages. And, as reported by Brad, the current 
> > > > > > > > > 1.4.7 packages
> > > > > > > > >    are broken indeed:
> > > > > > > > > 
> > > > > > > > > $ objdump -T /usr/lib64/libfdt-1.4.7.so | grep fdt_check_full
> > > > > > > > > $ 
> > > > > > > > > 
> > > > > > > > > 2) when building embedded, we only build the archive, not the 
> > > > > > > > > shared lib.
> > > > > > > > >         
> > > > > > > > > > I guess we should figure out how to force the testsuite to 
> > > > > > > > > > link
> > > > > > > > > > against the shared library rather than static to test for 
> > > > > > > > > > this.  I'll
> > > > > > > > > > look into it if I have time (which is a big if).
> > > > > > > > > >         
> > > > > > > > > 
> > > > > > > > > I think we should rather build the embedded shared library 
> > > > > > > > > using
> > > > > > > > > the 'libfdt' rule of the top-level makefile of the dtc 
> > > > > > > > > sub-module
> > > > > > > > > and have QEMU to be linked against this share library instead 
> > > > > > > > > of
> > > > > > > > > the static one. AFAIK, this is what gcc does when it finds 
> > > > > > > > > both
> > > > > > > > > .a and .so.        
> > > > > > > > 
> > > > > > > > Actually, I don't think this is a good idea.  It means the 
> > > > > > > > resulting
> > > > > > > > qemu build would have to be installed with the libfdt image as 
> > > > > > > > well.
> > > > > > > > As well as complicating the install path, that means that the 
> > > > > > > > qemu
> > > > > > > > build will now actively conflict with a packaged libfdt, rather 
> > > > > > > > than
> > > > > > > > merely suboptimally failing to use it.      
> > > > > > > 
> > > > > > > Yes you're right: the resulting QEMU shouldn't be installed, 
> > > > > > > especially
> > > > > > > if it has a RPATH poiting to the build directory.
> > > > > > > 
> > > > > > > This being said, if someone wants to build AND install QEMU, 
> > > > > > > shouldn't
> > > > > > > she rely exclusively on installed libfdt packages ? In other 
> > > > > > > words,
> > > > > > > shouldn't the embedded libfdt be a QEMU developper only thing ? 
> > > > > > > What
> > > > > > > are the real life use cases for embedded libfdt ?      
> > > > > > 
> > > > > > I don't think we should insist on that, although it's certainly the
> > > > > > way distros will usually work.  If someone wants to build and 
> > > > > > install
> > > > > > qemu locally, I don't think we should insist they first work out how
> > > > > > to install a new enough libfdt for it to use.
> > > > > > 
> > > > > > Likewise a limited purpose distro for whom qemu is the only user of
> > > > > > libfdt might not want to package it separately.
> > > > > >     
> > > > > 
> > > > > Fair enough. There's still one minor fix to make in configure for
> > > > > the tarball case though:
> > > > > 
> > > > >           # Not a git build & no libfdt found, prompt for system 
> > > > > install
> > > > >           error_exit "DTC (libfdt) version >= 1.4.2 not present." \
> > > > >                      "Please install the DTC (libfdt) devel package"
> > > > > 
> > > > > Which version should that be ? Do you plan to release 1.5.0 or
> > > > > whatever anytime soon ?    
> > > > 
> > > > I released 1.5.0 about 3 weeks ago.
> > > >   
> > > 
> > > Hmm... I do see the version bumping patch in 
> > > https://github.com/dgibson/dtc/
> > > but no trace of 1.5.0 in https://github.com/dgibson/dtc/releases... so
> > > I'm not sure if 1.5.0 is actually released. If it is released indeed, then
> > > I guess we need a 1.5.1 with the version.lds fix.  
> > 
> > Looks like I forgot to push the tag to github.  github is not the
> > primary site for dtc, just a mirror.  The primary git tree is at
> >     git://git.kernel.org/pub/scm/utils/dtc/dtc.git
> > and releases are at:
> >     https://mirrors.edge.kernel.org/pub/software/utils/dtc/
> > 
> 
> Oh... thanks for the info :)
> 
> So we're up for 1.5.1 with a usable libfdt.so ?

Yeah, I guess so.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature