linux-kernel - Re: [PATCH v3 10/17] ARM: LPAE: use phys_addr_t in switch

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20120921184143.GH15609@n2100.arm.linux.org.uk>
Date:	Fri, 21 Sep 2012 19:41:43 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Nicolas Pitre <nicolas.pitre@...aro.org>
Cc:	Cyril Chemparathy <cyril@...com>, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, arnd@...db.de,
	catalin.marinas@....com, grant.likely@...retlab.ca,
	will.deacon@....com, Vitaly Andrianov <vitalya@...com>
Subject: Re: [PATCH v3 10/17] ARM: LPAE: use phys_addr_t in switch_mm()

On Fri, Sep 21, 2012 at 02:33:43PM -0400, Nicolas Pitre wrote:
> On Tue, 11 Sep 2012, Cyril Chemparathy wrote:
> 
> > This patch modifies the switch_mm() processor functions to use phys_addr_t.
> > On LPAE systems, we now honor the upper 32-bits of the physical address that
> > is being passed in, and program these into TTBR as expected.
> > 
> > Signed-off-by: Cyril Chemparathy <cyril@...com>
> > Signed-off-by: Vitaly Andrianov <vitalya@...com>
> 
> Reviewed-by: Nicolas Pitre <nico@...aro.org>

Err... you may have reviewed it but did you read it?

> > diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> > index f3628fb..75b5f14 100644
> > --- a/arch/arm/include/asm/proc-fns.h
> > +++ b/arch/arm/include/asm/proc-fns.h
> > @@ -60,7 +60,7 @@ extern struct processor {
> >  	/*
> >  	 * Set the page table
> >  	 */
> > -	void (*switch_mm)(unsigned long pgd_phys, struct mm_struct *mm);
> > +	void (*switch_mm)(phys_addr_t pgd_phys, struct mm_struct *mm);
> >  	/*
> >  	 * Set a possibly extended PTE.  Non-extended PTEs should
> >  	 * ignore 'ext'.
> > @@ -82,7 +82,7 @@ extern void cpu_proc_init(void);
> >  extern void cpu_proc_fin(void);
> >  extern int cpu_do_idle(void);
> >  extern void cpu_dcache_clean_area(void *, int);
> > -extern void cpu_do_switch_mm(unsigned long pgd_phys, struct mm_struct *mm);
> > +extern void cpu_do_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);

phys_addr_t can be either 64-bit or 32-bit.  Which it ends up depends on
a configuration option.  If it's 32-bit, then mm is in r1, otherwise it
is in r2...

> >  #ifdef CONFIG_MMU
> > -	ldr	r1, [r1, #MM_CONTEXT_ID]	@ get mm->context.id
> > -	and	r3, r1, #0xff
> > -	mov	r3, r3, lsl #(48 - 32)		@ ASID
> > -	mcrr	p15, 0, r0, r3, c2		@ set TTB 0
> > +	ldr	r2, [r2, #MM_CONTEXT_ID]	@ get mm->context.id

which breaks this when phys_addr_t is 32-bit.

Doing it this way means we have to have similar conditionals in other
files which make use of the 'mm' argument.

Moving the 'mm' argument into arg0 would mean that stays as r0, but
then the pgd_phys argument is passed in either r1 when 32-bit, or
r2,r3 when 64-bit on EABI and r1,r2 on OABI.  That's hardly desirable
behaviour either, because it all too easily allows bugs to creap in.

The easiest solution would be to just change pgd_phys to be uint64_t
and be done with it.  Then you always know in assembly what registers
values are going to be passed in (except for the LE/BE issue with r0/r1).
--
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/