| Message ID | 1347385155-11643-11-git-send-email-cyril@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@ti.com> > Signed-off-by: Vitaly Andrianov <vitalya@ti.com> Reviewed-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/include/asm/proc-fns.h | 4 ++-- > arch/arm/mm/proc-v7-3level.S | 17 +++++++++++++---- > 2 files changed, 15 insertions(+), 6 deletions(-) > > 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); > #ifdef CONFIG_ARM_LPAE > extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte); > #else > diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S > index 8de0f1d..c4f4251 100644 > --- a/arch/arm/mm/proc-v7-3level.S > +++ b/arch/arm/mm/proc-v7-3level.S > @@ -39,6 +39,14 @@ > #define TTB_FLAGS_SMP (TTB_IRGN_WBWA|TTB_S|TTB_RGN_OC_WBWA) > #define PMD_FLAGS_SMP (PMD_SECT_WBWA|PMD_SECT_S) > > +#ifndef __ARMEB__ > +# define rpgdl r0 > +# define rpgdh r1 > +#else > +# define rpgdl r1 > +# define rpgdh r0 > +#endif > + > /* > * cpu_v7_switch_mm(pgd_phys, tsk) > * > @@ -47,10 +55,11 @@ > */ > ENTRY(cpu_v7_switch_mm) > #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 > + and r2, r2, #0xff > + mov r2, r2, lsl #(48 - 32) @ ASID > + orr rpgdh, rpgdh, r2 @ upper 32-bits of pgd phys > + mcrr p15, 0, rpgdl, rpgdh, c2 @ set TTB 0 > isb > #endif > mov pc, lr > -- > 1.7.9.5 >
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@ti.com> > > Signed-off-by: Vitaly Andrianov <vitalya@ti.com> > > Reviewed-by: Nicolas Pitre <nico@linaro.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).
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@ti.com> > > > Signed-off-by: Vitaly Andrianov <vitalya@ti.com> > > > > Reviewed-by: Nicolas Pitre <nico@linaro.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
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); #ifdef CONFIG_ARM_LPAE extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte); #else diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S index 8de0f1d..c4f4251 100644 --- a/arch/arm/mm/proc-v7-3level.S +++ b/arch/arm/mm/proc-v7-3level.S @@ -39,6 +39,14 @@ #define TTB_FLAGS_SMP (TTB_IRGN_WBWA|TTB_S|TTB_RGN_OC_WBWA) #define PMD_FLAGS_SMP (PMD_SECT_WBWA|PMD_SECT_S) +#ifndef __ARMEB__ +# define rpgdl r0 +# define rpgdh r1 +#else +# define rpgdl r1 +# define rpgdh r0 +#endif + /* * cpu_v7_switch_mm(pgd_phys, tsk) * @@ -47,10 +55,11 @@ */ ENTRY(cpu_v7_switch_mm) #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 + and r2, r2, #0xff + mov r2, r2, lsl #(48 - 32) @ ASID + orr rpgdh, rpgdh, r2 @ upper 32-bits of pgd phys + mcrr p15, 0, rpgdl, rpgdh, c2 @ set TTB 0 isb #endif mov pc, lr