From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so1.wedos.net (wes1-so1.wedos.net [46.28.106.15]) by dpdk.org (Postfix) with ESMTP id 1EA5637AF for ; Fri, 6 May 2016 18:33:41 +0200 (CEST) Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz [147.229.13.147]) by wes1-so1.wedos.net (Postfix) with ESMTPSA id 3r1cm46DlzzBpY; Fri, 6 May 2016 18:33:40 +0200 (CEST) Date: Fri, 6 May 2016 18:31:48 +0200 From: Jan Viktorin To: Thomas Monjalon Cc: dev@dpdk.org, Bruce Richardson , David Marchand Message-ID: <20160506183148.42408fb5@pcviktorin.fit.vutbr.cz> In-Reply-To: <2406220.vlQ29PEzMg@xps13> References: <1462531720-26217-1-git-send-email-viktorin@rehivetech.com> <1462531720-26217-3-git-send-email-viktorin@rehivetech.com> <2406220.vlQ29PEzMg@xps13> Organization: RehiveTech MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 02/10] app/test: support resources externally linked X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 May 2016 16:33:41 -0000 Hello Thomas, On Fri, 06 May 2016 16:32:53 +0200 Thomas Monjalon 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(). Regards Jan > Thanks -- Jan Viktorin E-mail: Viktorin@RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic