diff mbox

[RFC,05/10] arm64: mm: avoid __pa translations in early_fixmap_init

Message ID 1456174472-30028-6-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Feb. 22, 2016, 8:54 p.m. UTC
Avoid using __pa() translations while populating the fixmap page tables,
by using __pa_symbol to take the physical addresses of bm_pud, bm_pmd and
bm_pte, and move to __pgd_populate/__pmd_populate/__pte_populate, which
takes physical addresses directly. Since the former two are now called
unconditionally, remove the BUILD_BUG()'s that prevent their use in case
their page table level is folded away.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 arch/arm64/include/asm/pgalloc.h | 2 --
 arch/arm64/mm/mmu.c              | 8 ++++----
 2 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.5.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Comments

Catalin Marinas Feb. 23, 2016, 5:12 p.m. UTC | #1
On Mon, Feb 22, 2016 at 09:54:27PM +0100, Ard Biesheuvel wrote:
> --- a/arch/arm64/mm/mmu.c

> +++ b/arch/arm64/mm/mmu.c

> @@ -679,7 +679,7 @@ void __init early_fixmap_init(void)

>  

>  	pgd = pgd_offset_k(addr);

>  	if (CONFIG_PGTABLE_LEVELS > 3 &&

> -	    !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {

> +	    !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {

>  		/*

>  		 * We only end up here if the kernel mapping and the fixmap

>  		 * share the top level pgd entry, which should only happen on


Do I miss any patches? The for-next/core branch has a pgd_none(*pgd)
check here, so this patch does not apply.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Feb. 23, 2016, 5:16 p.m. UTC | #2
On 23 February 2016 at 18:12, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Mon, Feb 22, 2016 at 09:54:27PM +0100, Ard Biesheuvel wrote:

>> --- a/arch/arm64/mm/mmu.c

>> +++ b/arch/arm64/mm/mmu.c

>> @@ -679,7 +679,7 @@ void __init early_fixmap_init(void)

>>

>>       pgd = pgd_offset_k(addr);

>>       if (CONFIG_PGTABLE_LEVELS > 3 &&

>> -         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {

>> +         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {

>>               /*

>>                * We only end up here if the kernel mapping and the fixmap

>>                * share the top level pgd entry, which should only happen on

>

> Do I miss any patches? The for-next/core branch has a pgd_none(*pgd)

> check here, so this patch does not apply.

>


This is actually based on the kaslr branch, not for-next/core

I'm happy to rebase and resend. I have added some patches for kasan,
kvm and smp as well.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Catalin Marinas Feb. 23, 2016, 5:26 p.m. UTC | #3
On Tue, Feb 23, 2016 at 06:16:50PM +0100, Ard Biesheuvel wrote:
> On 23 February 2016 at 18:12, Catalin Marinas <catalin.marinas@arm.com> wrote:

> > On Mon, Feb 22, 2016 at 09:54:27PM +0100, Ard Biesheuvel wrote:

> >> --- a/arch/arm64/mm/mmu.c

> >> +++ b/arch/arm64/mm/mmu.c

> >> @@ -679,7 +679,7 @@ void __init early_fixmap_init(void)

> >>

> >>       pgd = pgd_offset_k(addr);

> >>       if (CONFIG_PGTABLE_LEVELS > 3 &&

> >> -         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {

> >> +         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {

> >>               /*

> >>                * We only end up here if the kernel mapping and the fixmap

> >>                * share the top level pgd entry, which should only happen on

> >

> > Do I miss any patches? The for-next/core branch has a pgd_none(*pgd)

> > check here, so this patch does not apply.

> >

> 

> This is actually based on the kaslr branch, not for-next/core

> 

> I'm happy to rebase and resend.


That's fine, no need to resend. I plan to move those as well onto
for-next/core but wanted some more testing first on the initial part.

> I have added some patches for kasan, kvm and smp as well.


Patches related to the __pa clean-up? Or something else?

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Feb. 23, 2016, 5:27 p.m. UTC | #4
On 23 February 2016 at 18:26, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Feb 23, 2016 at 06:16:50PM +0100, Ard Biesheuvel wrote:

>> On 23 February 2016 at 18:12, Catalin Marinas <catalin.marinas@arm.com> wrote:

>> > On Mon, Feb 22, 2016 at 09:54:27PM +0100, Ard Biesheuvel wrote:

>> >> --- a/arch/arm64/mm/mmu.c

>> >> +++ b/arch/arm64/mm/mmu.c

>> >> @@ -679,7 +679,7 @@ void __init early_fixmap_init(void)

>> >>

>> >>       pgd = pgd_offset_k(addr);

>> >>       if (CONFIG_PGTABLE_LEVELS > 3 &&

>> >> -         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {

>> >> +         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {

>> >>               /*

>> >>                * We only end up here if the kernel mapping and the fixmap

>> >>                * share the top level pgd entry, which should only happen on

>> >

>> > Do I miss any patches? The for-next/core branch has a pgd_none(*pgd)

>> > check here, so this patch does not apply.

>> >

>>

>> This is actually based on the kaslr branch, not for-next/core

>>

>> I'm happy to rebase and resend.

>

> That's fine, no need to resend. I plan to move those as well onto

> for-next/core but wanted some more testing first on the initial part.

>

>> I have added some patches for kasan, kvm and smp as well.

>

> Patches related to the __pa clean-up? Or something else?

>


No, related to the __pa restriction

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Catalin Marinas Feb. 23, 2016, 5:32 p.m. UTC | #5
On Tue, Feb 23, 2016 at 06:27:27PM +0100, Ard Biesheuvel wrote:
> On 23 February 2016 at 18:26, Catalin Marinas <catalin.marinas@arm.com> wrote:

> > On Tue, Feb 23, 2016 at 06:16:50PM +0100, Ard Biesheuvel wrote:

> >> On 23 February 2016 at 18:12, Catalin Marinas <catalin.marinas@arm.com> wrote:

> >> > On Mon, Feb 22, 2016 at 09:54:27PM +0100, Ard Biesheuvel wrote:

> >> >> --- a/arch/arm64/mm/mmu.c

> >> >> +++ b/arch/arm64/mm/mmu.c

> >> >> @@ -679,7 +679,7 @@ void __init early_fixmap_init(void)

> >> >>

> >> >>       pgd = pgd_offset_k(addr);

> >> >>       if (CONFIG_PGTABLE_LEVELS > 3 &&

> >> >> -         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {

> >> >> +         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {

> >> >>               /*

> >> >>                * We only end up here if the kernel mapping and the fixmap

> >> >>                * share the top level pgd entry, which should only happen on

> >> >

> >> > Do I miss any patches? The for-next/core branch has a pgd_none(*pgd)

> >> > check here, so this patch does not apply.

> >> >

> >>

> >> This is actually based on the kaslr branch, not for-next/core

> >>

> >> I'm happy to rebase and resend.

> >

> > That's fine, no need to resend. I plan to move those as well onto

> > for-next/core but wanted some more testing first on the initial part.

> >

> >> I have added some patches for kasan, kvm and smp as well.

> >

> > Patches related to the __pa clean-up? Or something else?

> 

> No, related to the __pa restriction


OK. I'll wait for a while before merging this series to get more reviews
and testing. I picked the first patch though (high_memory fix).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Feb. 23, 2016, 5:43 p.m. UTC | #6
On 23 February 2016 at 18:32, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Feb 23, 2016 at 06:27:27PM +0100, Ard Biesheuvel wrote:

>> On 23 February 2016 at 18:26, Catalin Marinas <catalin.marinas@arm.com> wrote:

>> > On Tue, Feb 23, 2016 at 06:16:50PM +0100, Ard Biesheuvel wrote:

>> >> On 23 February 2016 at 18:12, Catalin Marinas <catalin.marinas@arm.com> wrote:

>> >> > On Mon, Feb 22, 2016 at 09:54:27PM +0100, Ard Biesheuvel wrote:

>> >> >> --- a/arch/arm64/mm/mmu.c

>> >> >> +++ b/arch/arm64/mm/mmu.c

>> >> >> @@ -679,7 +679,7 @@ void __init early_fixmap_init(void)

>> >> >>

>> >> >>       pgd = pgd_offset_k(addr);

>> >> >>       if (CONFIG_PGTABLE_LEVELS > 3 &&

>> >> >> -         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {

>> >> >> +         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {

>> >> >>               /*

>> >> >>                * We only end up here if the kernel mapping and the fixmap

>> >> >>                * share the top level pgd entry, which should only happen on

>> >> >

>> >> > Do I miss any patches? The for-next/core branch has a pgd_none(*pgd)

>> >> > check here, so this patch does not apply.

>> >> >

>> >>

>> >> This is actually based on the kaslr branch, not for-next/core

>> >>

>> >> I'm happy to rebase and resend.

>> >

>> > That's fine, no need to resend. I plan to move those as well onto

>> > for-next/core but wanted some more testing first on the initial part.

>> >

>> >> I have added some patches for kasan, kvm and smp as well.

>> >

>> > Patches related to the __pa clean-up? Or something else?

>>

>> No, related to the __pa restriction

>

> OK. I'll wait for a while before merging this series to get more reviews

> and testing. I picked the first patch though (high_memory fix).

>


OK, I'll hold off for now. I have enough stuff in flight as it is.
However, I suppose that you will want to address the performance
concern at some point

If anyone wants to do any benchmarking:
git://git.linaro.org/people/ard.biesheuvel/linux-arm.git arm64-pa-linear-mapping

(updated for kasan, KVM, smp etc)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Catalin Marinas Feb. 23, 2016, 6:10 p.m. UTC | #7
On Tue, Feb 23, 2016 at 06:43:37PM +0100, Ard Biesheuvel wrote:
> On 23 February 2016 at 18:32, Catalin Marinas <catalin.marinas@arm.com> wrote:

> > On Tue, Feb 23, 2016 at 06:27:27PM +0100, Ard Biesheuvel wrote:

> >> On 23 February 2016 at 18:26, Catalin Marinas <catalin.marinas@arm.com> wrote:

> >> > On Tue, Feb 23, 2016 at 06:16:50PM +0100, Ard Biesheuvel wrote:

> >> >> On 23 February 2016 at 18:12, Catalin Marinas <catalin.marinas@arm.com> wrote:

> >> >> > On Mon, Feb 22, 2016 at 09:54:27PM +0100, Ard Biesheuvel wrote:

> >> >> >> --- a/arch/arm64/mm/mmu.c

> >> >> >> +++ b/arch/arm64/mm/mmu.c

> >> >> >> @@ -679,7 +679,7 @@ void __init early_fixmap_init(void)

> >> >> >>

> >> >> >>       pgd = pgd_offset_k(addr);

> >> >> >>       if (CONFIG_PGTABLE_LEVELS > 3 &&

> >> >> >> -         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {

> >> >> >> +         !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {

> >> >> >>               /*

> >> >> >>                * We only end up here if the kernel mapping and the fixmap

> >> >> >>                * share the top level pgd entry, which should only happen on

> >> >> >

> >> >> > Do I miss any patches? The for-next/core branch has a pgd_none(*pgd)

> >> >> > check here, so this patch does not apply.

> >> >> >

> >> >>

> >> >> This is actually based on the kaslr branch, not for-next/core

> >> >>

> >> >> I'm happy to rebase and resend.

> >> >

> >> > That's fine, no need to resend. I plan to move those as well onto

> >> > for-next/core but wanted some more testing first on the initial part.

> >> >

> >> >> I have added some patches for kasan, kvm and smp as well.

> >> >

> >> > Patches related to the __pa clean-up? Or something else?

> >>

> >> No, related to the __pa restriction

> >

> > OK. I'll wait for a while before merging this series to get more reviews

> > and testing. I picked the first patch though (high_memory fix).

> 

> OK, I'll hold off for now. I have enough stuff in flight as it is.

> However, I suppose that you will want to address the performance

> concern at some point


Yes, though we first need to identify how real this concern is.
PHYS_OFFSET was a low-hanging fruit, so worth merging without additional
benchmarking.

The __pa clean-up, while nice, is relatively intrusive and would need
acks from the KVM guys as well. I wouldn't rush into merging it unless
it shows some benefits in benchmarks.

One concern I have is that we still find something in the generic code
doing a virt_to_phys() on kernel image addresses which would render all
this clean-up unnecessary (since most of the changes are not on the
critical path, we do them just to simplify virt_to_phys()).

> If anyone wants to do any benchmarking:

> git://git.linaro.org/people/ard.biesheuvel/linux-arm.git arm64-pa-linear-mapping


Thanks. We'll give this a try in the next few days.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
index ff98585d085a..bc01d2b65225 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -54,7 +54,6 @@  static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
 #else
 static inline void __pud_populate(pud_t *pud, phys_addr_t pmd, pudval_t prot)
 {
-	BUILD_BUG();
 }
 #endif	/* CONFIG_PGTABLE_LEVELS > 2 */
 
@@ -83,7 +82,6 @@  static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud)
 #else
 static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pud, pgdval_t prot)
 {
-	BUILD_BUG();
 }
 #endif	/* CONFIG_PGTABLE_LEVELS > 3 */
 
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index fbba941a6e87..e7340defa085 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -679,7 +679,7 @@  void __init early_fixmap_init(void)
 
 	pgd = pgd_offset_k(addr);
 	if (CONFIG_PGTABLE_LEVELS > 3 &&
-	    !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa(bm_pud))) {
+	    !(pgd_none(*pgd) || pgd_page_paddr(*pgd) == __pa_symbol(bm_pud))) {
 		/*
 		 * We only end up here if the kernel mapping and the fixmap
 		 * share the top level pgd entry, which should only happen on
@@ -688,12 +688,12 @@  void __init early_fixmap_init(void)
 		BUG_ON(!IS_ENABLED(CONFIG_ARM64_16K_PAGES));
 		pud = pud_offset_kimg(pgd, addr);
 	} else {
-		pgd_populate(&init_mm, pgd, bm_pud);
+		__pgd_populate(pgd, __pa_symbol(bm_pud), PUD_TYPE_TABLE);
 		pud = fixmap_pud(addr);
 	}
-	pud_populate(&init_mm, pud, bm_pmd);
+	__pud_populate(pud, __pa_symbol(bm_pmd), PUD_TYPE_TABLE);
 	pmd = fixmap_pmd(addr);
-	pmd_populate_kernel(&init_mm, pmd, bm_pte);
+	__pmd_populate(pmd, __pa_symbol(bm_pte), PMD_TYPE_TABLE);
 
 	/*
 	 * The boot-ioremap range spans multiple pmds, for which