[dpdk-dev] [PATCH v1 02/10] app/test: support resources externally linked
Jan Viktorin
viktorin at rehivetech.com
Fri May 6 18:31:48 CEST 2016
Hello Thomas,
On Fri, 06 May 2016 16:32:53 +0200
Thomas Monjalon <thomas.monjalon at 6wind.com> wrote:
> It looks a lot too much tricky to be integrated without code comments ;)
> Please make a documentation effort.
I know it's not the brightly shining code... I focused mainly on the C API of
this. Thanks a lot for comments. I will fix those and come back with something
better.
>
> 2016-05-06 12:48, Jan Viktorin:
> > --- a/app/test/Makefile
> > +++ b/app/test/Makefile
> > @@ -33,6 +33,37 @@ include $(RTE_SDK)/mk/rte.vars.mk
> >
> > ifeq ($(CONFIG_RTE_APP_TEST),y)
>
> A comment is needed here to explain the intent of the following code.
Sure.
>
> > +ifeq ($(RTE_ARCH),arm)
> > +RTE_OBJCOPY_O = elf32-littlearm
> > +RTE_OBJCOPY_B = arm
> > +else ifeq ($(RTE_ARCH),arm64)
> > +RTE_OBJCOPY_O = elf64-littleaarch64
> > +RTE_OBJCOPY_B = aarch64
> > +else ifeq ($(RTE_ARCH),i686)
> > +RTE_OBJCOPY_O = elf32-i386
> > +RTE_OBJCOPY_B = i386
> > +else ifeq ($(RTE_ARCH),x86_64)
> > +RTE_OBJCOPY_O = elf64-x86-64
> > +RTE_OBJCOPY_B = i386:x86-64
> > +else ifeq ($(RTE_ARCH),x86_x32)
> > +RTE_OBJCOPY_O = elf32-x86-64
> > +RTE_OBJCOPY_B = i386:x86-64
> > +else
> > +$(error Unrecognized RTE_ARCH: $(RTE_ARCH))
> > +endif
>
> RTE_OBJCOPY_O could be renamed RTE_OBJCOPY_TARGET.
> RTE_OBJCOPY_B could be renamed RTE_OBJCOPY_ARCH.
>
> These definitions could be done in mk/arch/
Cool, I will move those. However, I don't know all the cryptic objcopy
names for all platforms. I've tested just arm, arm64 and x86_64. I guessed
the rest (or copied from some web page).
>
> > +
> > +define resource
>
> When defining a makefile macro, the arguments must be documented on
> the previous line. Otherwise, nobody (including you) will be able
> to read it without parsing the code below.
Sure!
What about a better name (RTE_TEST_RESOURCE, TEST_RESOURCE)? Should this
macro stay in this Makefile?
>
> It is not a simple resource, so the name must avoid the confusion.
> Why not linked_resource?
>
> > +SRCS-y += $(1).res.o
> > +$(1).res.o: $(2)
> > + $(OBJCOPY) -I binary -B $(RTE_OBJCOPY_B) -O $(RTE_OBJCOPY_O) \
> > + --rename-section \
> > + .data=.rodata,alloc,load,data,contents,readonly \
> > + --redefine-sym _binary__dev_stdin_start=beg_$(1) \
> > + --redefine-sym _binary__dev_stdin_end=end_$(1) \
> > + --redefine-sym _binary__dev_stdin_size=siz_$(1) \
> > + /dev/stdin $$@ < $$<
> > +endef
>
> [...]
>
> > +#define REGISTER_LINKED_RESOURCE(_n) \
> > +extern const char beg_ ##_n; \
> > +extern const char end_ ##_n; \
> > +REGISTER_RESOURCE(_n, &beg_ ##_n, &end_ ##_n); \
>
> Please explain the begin/end trick.
The objcopy creates an object file with our data located at _binary__dev_stdin_start
until _binary__dev_stdin_end. The names are derrived from the input file name - this is
so so stupid! I used /dev/stdin here to have a deterministic name of the *_start/_end
labels. Otherwise, the code would be very messy (trying to santize the aboslute!!! path
passed by make).
In our C code, we declare
extern const char _binary__dev_stdin_start;
extern const char _binary__dev_stdin_end;
...variables placed exactly on addresses where our data were put by objcopy.
This &_binary__dev_stdin_start is address of our data.
Those names are however changed by --redefine-sym to be unique and to reflect the
name we use to refer to the data from our C code by REGISTER_LINKED_RESOURCE(<name>).
Regards
Jan
> Thanks
--
Jan Viktorin E-mail: Viktorin at RehiveTech.com
System Architect Web: www.RehiveTech.com
RehiveTech
Brno, Czech Republic
More information about the dev
mailing list