Re: [Qemu-devel] [RFC PATCH 3/3] Introduce libtcg infrastructure

[Alessandro Di Federico]

On Tue, 24 Jan 2017 10:08:56 +0000
Marc-André Lureau <address@hidden> wrote:

>
> > * If there's at least a *-libtcg, compile everything as position
> >   independent code.
> 
> why not limit it to libtcg code?
> 

At least we have to enable it on "common" object files (those compiled
only once, independently from the current target), therefore I see
little benefit in compiling without PIC other object files.
I also think it might be a good idea to switch to PIE executables
altogether.

> 
> > * In case we're building libtcg, install the output binary in the
> >   $PREFIX/lib directory instead of $PREFIX/bin.
> >  
> 
> If it's installed globally, I think we have to deal with versioning,
> common prefix, and list of visible symbols.
> 

Yeah, for symbol visibility I was thinking to force all the symbols to
be hidden except for those in tcg.c, which would have a `tcg_` prefix.
I didn't think about versioning yet.

> 
> What's the plan with this tcg.c? Shouldn't the functions be stubs or
> library user callbacks?
> 
> What is test() doing?
> 
> Please add a libtcg test under tests/ (even minimal, like a basic
> translate/dump).
> 

This patch was to get a first round of opinions. The test function was
only supposed to reference some functions I know I'll use to make sure
that I'm linking in all the required object files.
In the next set of patches I'll provide something that actually does
something.

> 
> >
> >  ifdef CONFIG_USER_ONLY
> > +ifdef CONFIG_LIBTCG
> > +# libtcg
> > +QEMU_PROG=libtcg-$(TARGET_NAME)$(DSOSUF)
> > +QEMU_PROG_BUILD = $(QEMU_PROG)
> > +QEMU_CFLAGS+=-fPIC
> > +
> > +# Change the install directory
> > +PROGS_INSTALL_DIR := $(libdir)
> >  
> 
> okay, we may want to rename it to something like QEMU_BUILD perhaps
> 

You mean the QEMU_PROG_BUILD? ACK.

>
> >  install: all
> > +ifdef CONFIG_LIBTINYCODE
> > +       $(INSTALL_DATA) $(SRC_PATH)/tcg/tcg-opc.h
> > $(SRC_PATH)/libtcg/tcg.h "$(DESTDIR)$(includedir)"
> > +endif
> >  
> 
> LIBTINYCODE/LIBTCG?
> 

Leftover from my internal version.

>
> > -crypto-obj-y += aes.o
> > +crypto-obj-$(call lnot,$(CONFIG_LIBTCG)) += aes.o
> >  
> 
> why that change?
> 

aes.o was being linked in, but we don't use it in LIBTCG. Maybe this
should be fixed in an higher level.

> 
> > diff --git a/libtcg/tcg.h b/libtcg/tcg.h
> > new file mode 100644
> > index 0000000000..e69de29bb2
> >  
> 
> What is this empty file for?
> 

Just a placeholder for the header file to install. This patch goal was
to show all the changes to the build system.

Thanks for the comments.

--
Alessandro Di Federico
PhD student at Politecnico di Milano