linux-kernel - Re: [07/36] AArch64: Assembly macros and definitions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 24 Jul 2012 17:40:16 +0100
From:	Catalin Marinas <catalin.marinas@....com>
To:	Christopher Covington <cov@...eaurora.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Will Deacon <Will.Deacon@....com>
Subject: Re: [07/36] AArch64: Assembly macros and definitions

On Fri, Jul 20, 2012 at 03:22:04PM +0100, Christopher Covington wrote:
> On 01/-10/-28163 02:59 PM, Catalin Marinas wrote:
> > This patch introduces several assembly macros and definitions used in
> > the .S files across arch/aarch64/ like IRQ disabling/enabling, together
> > with asm-offsets.c.
> 
> [...]
> 
> > diff --git a/arch/aarch64/include/asm/assembler.h b/arch/aarch64/include/asm/assembler.h
> > new file mode 100644
> > index 0000000..c6ac3cf
> > --- /dev/null
> > +++ b/arch/aarch64/include/asm/assembler.h
> 
> [...]
> 
> > +#define USER(x...)				\
> > +9999:	x;					\
> > +	.section __ex_table,"a";		\
> > +	.align	3;				\
> > +	.quad	9999b,9001f;			\
> > +	.previous
> 
> It appears that this macro depends on a 9001 label being defined in the
> source file somewhere after the macro is used. It might be preferable to
> incorporate the label into the macro if possible, or pass the label as
> an argument to the macro.

Good point, I changed to USER(l, x...).

> > +/*
> > + * User access macros.
> > + */
> > +	.macro	usracc, instr, reg, reg2, ptr, inc, rept, abort
> > +	.rept	\rept
> > +9999:
> > +	.if	\inc == 1
> > +	\instr\()b \reg, [\ptr], #\inc
> > +	.elseif	\inc == 4
> > +	\instr\() \reg, [\ptr], #\inc
> > +	.elseif	\inc == 8
> > +	\instr\() \reg, [\ptr], #\inc
> > +	.elseif \inc == 16
> > +	\instr\() \reg, \reg2, [\ptr], #\inc
> > +	.else
> > +	.error	"Unsupported inc macro argument"
> > +	.endif
> > +
> > +	.section __ex_table,"a"
> > +	.align	3
> > +	.quad	9999b, \abort
> > +	.previous
> 
> Could you use the USER preprocessor macro here to reduce this small
> duplication of directives?
> 
> > +	.endr
> > +	.endm
> > +
> > +	.macro	ldrusr, reg, ptr, inc, rept=1, abort=9001f
> > +	 usracc	ldr, \reg, \reg, \ptr, \inc, \rept, \abort
> > +	.endm
> > +
> > +	.macro	ldrpusr, reg, reg2, ptr, rept=1, abort=9001f
> > +	 usracc	ldp, \reg, \reg2, \ptr, 16, \rept, \abort
> > +	.endm
> 
> How about "ldpusr" to more directly match the assembly?
> 
> (Also, the 9001 label strikes again.)

For now I removed these entirely. I have additional patches for
optimised library functions that make use of these but I don't plan to
push any of them until I can benchmark on real hardware.

Thanks.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/