From owner-freebsd-arm@FreeBSD.ORG Wed Nov 21 16:00:03 2012 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 4A3CA7B; Wed, 21 Nov 2012 16:00:03 +0000 (UTC) (envelope-from giovanni.trematerra@gmail.com) Received: from mail-qc0-f182.google.com (mail-qc0-f182.google.com [209.85.216.182]) by mx1.freebsd.org (Postfix) with ESMTP id E16D78FC14; Wed, 21 Nov 2012 16:00:02 +0000 (UTC) Received: by mail-qc0-f182.google.com with SMTP id k19so6270280qcs.13 for ; Wed, 21 Nov 2012 08:00:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=pKTvd6kgakN3WuEDuztAtrd1YZzqM9ERqrzRVZDymh0=; b=TBvQe+VhUCQUlTUrPNX+mzJZcA7eipNXR0frkN5e9kqher0xD5uhozAiH6fZp114nT tvF50NKxC/rvxvXqi7GIiO71yuzSJ+Ezl+M5zRKuhHGs3J2qDzCnalpIZRwGTy6xQsQQ ySZLDyZBfWIeVB4/73KPDZIPSiRkUPn0wttcIYYwBa7eUvEq36owe2k1VrFZCKCeS5bQ ksG4dXFTk72yaadD+efrncody7TpAENem3KOT0zQDOgQImu/oyrQJN067PJLyUVtsjV+ v9TfvDvUtBZWMXacsyYGF2dABvhLw3Uflvi7hv7w0UNQngk1GhHilvndWr4CcJ6pzbQo oVGQ== MIME-Version: 1.0 Received: by 10.224.185.79 with SMTP id cn15mr18621534qab.14.1353513602024; Wed, 21 Nov 2012 08:00:02 -0800 (PST) Sender: giovanni.trematerra@gmail.com Received: by 10.229.117.1 with HTTP; Wed, 21 Nov 2012 08:00:01 -0800 (PST) In-Reply-To: <50ACE2B4.8010904@semihalf.com> References: <50AA4E87.3000505@semihalf.com> <50ACE2B4.8010904@semihalf.com> Date: Wed, 21 Nov 2012 17:00:01 +0100 X-Google-Sender-Auth: AmnGVRxrMDbLkMK_PtAPFMPNj40 Message-ID: Subject: Re: ARM/SMP, Some patches for review. From: Giovanni Trematerra To: =?UTF-8?B?xYF1a2FzeiBQxYJhY2hubw==?= Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: freebsd-arm@freebsd.org, cognet@freebsd.org X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Nov 2012 16:00:03 -0000 On Wed, Nov 21, 2012 at 3:18 PM, =C5=81ukasz P=C5=82achno wrote: > On 20.11.2012 00:08, Giovanni Trematerra wrote: >> >> On Mon, Nov 19, 2012 at 4:21 PM, =C5=81ukasz P=C5=82achno wrote: >>> >>> Hi, >>> >>> I would like to propose few changes for ARM specific code. >>> Three attached patches for freebsd-current allows building SMP-safe wor= ld >>> for ARM targets and turns on TEX remap for ARMv6 and ARMv7 targets. >>> >>> More details inside patch files. >>> >>> Change introduced by "commit-2" removes armv7 targets (armv7 and pj4b) >>> from >>> kernel.tramp. >>> AFAIK this feature is not working properly for armv7 targets and is >>> causing >>> problem during compilation: >>> - LOCORE is defined during kernel compilation but not defined during >>> kernel.tramp compilation, so #include pmap.h causes build errors. >>> >>> I do not think adding hack like this: >>> #ifndef LOCORE >>> #define LOCORE >>> #endif >>> >>> to allow building something that is already broken is a good idea, so I >>> removed cpufunc_asm_pj4b.S and cpufunc_asm_armv7.S from Makefile.arm >> >> >> In commit-2.txt >> you should include style changes in sys/arm/arm/cpufunc_asm_armv7.S >> into a different patch. > > > fixed > > >> >> @@ -63,7 +64,6 @@ FILES_CPU_FUNC =3D $S/$M/$M/cpufunc_asm_arm7tdmi.= S \ >> $S/$M/$M/cpufunc_asm_xscale.S $S/$M/$M/cpufunc_asm.S \ >> $S/$M/$M/cpufunc_asm_xscale_c3.S $S/$M/$M/cpufunc_asm_armv5_ec.= S >> \ >> $S/$M/$M/cpufunc_asm_fa526.S $S/$M/$M/cpufunc_asm_sheeva.S \ >> - $S/$M/$M/cpufunc_asm_pj4b.S $S/$M/$M/cpufunc_asm_armv7.S >> >> You left a trailing back slash but beside that you should clean up >> sys/arm/arm/elf_trampoline.c >> and not make kernel.tramp to build at all for armv7 cpus or you'll end >> up with a linker error >> during generation of the kernel.tramp. >> > > Fixed, updated set of patches is attached. > > - TEX remap is supported only for armv6 (changed to avoid breaking armv6 > targets) > - Fixed issues with build for pre-armv6 targets (tested with make tinder= box > TARGETS=3Darm > 1_SMP_fixes.diff You'll endup to get a panic for PandaBoard systems. The arm11 functions don't handle the SMP case. So I propose to merge the changes below or commit them first. Index: sys/arm/arm/cpufunc.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/arm/arm/cpufunc.c (revision 243182) +++ sys/arm/arm/cpufunc.c (working copy) @@ -1079,18 +1079,18 @@ struct cpu_functions cortexa_cpufuncs =3D { /* Other functions */ cpufunc_nullop, /* flush_prefetchbuf */ - arm11_drain_writebuf, /* drain_writebuf */ + armv7_drain_writebuf, /* drain_writebuf */ cpufunc_nullop, /* flush_brnchtgt_C */ (void *)cpufunc_nullop, /* flush_brnchtgt_E */ - arm11_sleep, /* sleep */ + armv7_cpu_sleep, /* sleep */ /* Soft functions */ cpufunc_null_fixup, /* dataabt_fixup */ cpufunc_null_fixup, /* prefetchabt_fixup */ - arm11_context_switch, /* context_switch */ + armv7_context_switch, /* context_switch */ cortexa_setup /* cpu setup */ }; 2_ARM_cleanup.diff Changes to sys/arm/arm/machdep.c don't seem style changes and they should live in a separate patch with a different motivation. I'm not sure changes in sys/arm/arm/locore.S are style ones. I think that things like this aren't so readable. #if (ARM_ARCH_6 + ARM_ARCH_7A) !=3D 0 Instead of things like that wouldn't be better to define different macros when the sum is zero or non zero and stick with the #if defined/!defined thing? I mean in sys/arm/arm/cpuconf.h we could make something like this #if (ARM_ARCH_6 + ARM_ARCH_7A) !=3D 0 #define ARM_ARCH_6_7A #endif 3_kernel_trampoline.diff I think we should not make kernel_trampoline at all for the unsupported CPU= s. I propose this change to Makefile.arm Index: sys/conf/Makefile.arm =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/conf/Makefile.arm (revision 243182) +++ sys/conf/Makefile.arm (working copy) @@ -51,6 +51,7 @@ SYSTEM_LD_TAIL +=3D;sed s/" + SIZEOF_HEADERS"// ldsc ${SYSTEM_LD_}; \ ${OBJCOPY} -S -O binary ${FULLKERNEL}.noheader \ ${KERNEL_KO}.bin; \ + ${NM} ${FULLKERNEL}.noheader | sort > ${FULLKERNEL}.map; \ rm ${FULLKERNEL}.noheader .if defined(MFS_IMAGE) @@ -62,9 +63,11 @@ FILES_CPU_FUNC =3D $S/$M/$M/cpufunc_asm_arm7tdmi.S \ $S/$M/$M/cpufunc_asm_sa1.S $S/$M/$M/cpufunc_asm_arm10.S \ $S/$M/$M/cpufunc_asm_xscale.S $S/$M/$M/cpufunc_asm.S \ $S/$M/$M/cpufunc_asm_xscale_c3.S $S/$M/$M/cpufunc_asm_armv5_ec.S \ $S/$M/$M/cpufunc_asm_fa526.S $S/$M/$M/cpufunc_asm_sheeva.S \ - $S/$M/$M/cpufunc_asm_pj4b.S $S/$M/$M/cpufunc_asm_armv7.S + $S/$M/$M/cpufunc_asm_armv6.S +NO_TRAMP!=3D grep 'CPU_CORTEXA\|CPU_MV_PJ4B' opt_global.h || true ; echo + +.if ${NO_TRAMP} =3D=3D "" KERNEL_EXTRA=3Dtrampoline KERNEL_EXTRA_INSTALL=3Dkernel.gz.tramp trampoline: ${KERNEL_KO}.tramp @@ -110,6 +113,7 @@ ${KERNEL_KO}.tramp: ${KERNEL_KO} $S/$M/$M/inckern. ${KERNEL_KO}.gz.tramp.bin rm ${KERNEL_KO}.tmp.gz ${KERNEL_KO}.tramp.noheader opt_kernname.h \ inflate-tramp.o tmphack.S +.endif MKMODULESENV+=3D MACHINE=3D${MACHINE} 4_tex-remap.diff Some style(9) consideration. #include(s) should be grouped together in alphabetical order. So you should fix the pmap.h includes that you made. I'll try to test the pachset ASAP. -- Gianni