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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <alpine.LFD.2.02.1209211449150.6667@xanadu.home>
Date:	Fri, 21 Sep 2012 14:53:22 -0400 (EDT)
From:	Nicolas Pitre <nicolas.pitre@...aro.org>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
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, 21 Sep 2012, Russell King - ARM Linux wrote:

> 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?

Sure I did.

> > > 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...

Right.  And that configuration option is CONFIG_ARM_LPAE.

> > >  #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.

... which can't happen in this case because this code is only compiled 
when CONFIG_ARM_LPAE=y.

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

No because none of the other files may ever be used when 
CONFIG_ARM_LPAE=y.


Nicolas
--
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/