| Message ID | 1414435207-30240-4-git-send-email-punit.agrawal@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote: > arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ > - sys_compat.o > + sys_compat.o \ > + $(addprefix ../../arm/kernel/, \ > + opcodes.o) This is not particularly nice. While it means that this file gets built on both arm64 and arm, it means that it routinely won't get tested on arm64 when changes to it happen. The second reason it's not nice is that the whole $(addprefix thing really isn't needed. You're only adding the prefix to one name. So, a simpler way to write it is "../../arm/kernel/opcodes.o".
Hi Russell, Thanks for having a look. Russell King - ARM Linux <linux@arm.linux.org.uk> writes: > On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote: >> arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ >> - sys_compat.o >> + sys_compat.o \ >> + $(addprefix ../../arm/kernel/, \ >> + opcodes.o) > > This is not particularly nice. While it means that this file gets built > on both arm64 and arm, it means that it routinely won't get tested on > arm64 when changes to it happen. I agree. An earlier version of this patch copied opcodes.c to arm64 but then it wouldn't benefit from any fixes that would go in the original. The current approach misses out on the testing. Although better than the previous approach, it is not optimal. > > The second reason it's not nice is that the whole $(addprefix thing > really isn't needed. You're only adding the prefix to one name. So, > a simpler way to write it is "../../arm/kernel/opcodes.o". This one is easy to make nice. Fixed in my local copy. Cheers, Punit
On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote: > Port support for AArch32 instruction condition code checking from arm > to arm64. > > Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> > --- > arch/arm64/include/asm/opcodes.h | 1 + > arch/arm64/kernel/Makefile | 4 +++- > 2 files changed, 4 insertions(+), 1 deletion(-) > create mode 100644 arch/arm64/include/asm/opcodes.h > > diff --git a/arch/arm64/include/asm/opcodes.h b/arch/arm64/include/asm/opcodes.h > new file mode 100644 > index 0000000..4e603ea > --- /dev/null > +++ b/arch/arm64/include/asm/opcodes.h > @@ -0,0 +1 @@ > +#include <../../arm/include/asm/opcodes.h> > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index 5bd029b..798f7d7 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -18,7 +18,9 @@ arm64-obj-y := cputable.o debug-monitors.o entry.o irq.o fpsimd.o \ > cpuinfo.o > > arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ > - sys_compat.o > + sys_compat.o \ > + $(addprefix ../../arm/kernel/, \ > + opcodes.o) Apart from the addprefix that Russell mentioned: Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Wed, Oct 29, 2014 at 03:21:48PM +0000, Russell King - ARM Linux wrote: > On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote: > > arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ > > - sys_compat.o > > + sys_compat.o \ > > + $(addprefix ../../arm/kernel/, \ > > + opcodes.o) > > This is not particularly nice. Not that bad, you can look at it as a decoding library. > While it means that this file gets built on both arm64 and arm, it > means that it routinely won't get tested on arm64 when changes to it > happen. Low risk, it's a small file unlikely to change in the future. Anyway, since you mention testing, I wonder how regularly SWP emulation gets tested on arm32 (and in future arm64). If Punit has some tests already, it may be good to add them somewhere like tools/testing/selftests/arm/ (probably separately from this series).
On Wed, Nov 05, 2014 at 04:14:54PM +0000, Catalin Marinas wrote: > Anyway, since you mention testing, I wonder how regularly SWP emulation > gets tested on arm32 (and in future arm64). If Punit has some tests > already, it may be good to add them somewhere like > tools/testing/selftests/arm/ (probably separately from this series). Sounds like you want atomic.h implemented using SWP and CP15 barriers ;) There *is* lib/atomic64_test.c but, perversely, it's single-threaded and unlikely to spot issues even if we got it running in userspace. Will
diff --git a/arch/arm64/include/asm/opcodes.h b/arch/arm64/include/asm/opcodes.h new file mode 100644 index 0000000..4e603ea --- /dev/null +++ b/arch/arm64/include/asm/opcodes.h @@ -0,0 +1 @@ +#include <../../arm/include/asm/opcodes.h> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 5bd029b..798f7d7 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -18,7 +18,9 @@ arm64-obj-y := cputable.o debug-monitors.o entry.o irq.o fpsimd.o \ cpuinfo.o arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ - sys_compat.o + sys_compat.o \ + $(addprefix ../../arm/kernel/, \ + opcodes.o) arm64-obj-$(CONFIG_FUNCTION_TRACER) += ftrace.o entry-ftrace.o arm64-obj-$(CONFIG_MODULES) += arm64ksyms.o module.o arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o topology.o
Port support for AArch32 instruction condition code checking from arm to arm64. Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> --- arch/arm64/include/asm/opcodes.h | 1 + arch/arm64/kernel/Makefile | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/include/asm/opcodes.h