Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: Atish Patra <atish.patra@wdc.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	linux-efi@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Kees Cook <keescook@chromium.org>, Nick Hu <nickhu@andestech.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Anup Patel <anup.patel@wdc.com>,
	linux-kernel@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Yash Shah <yash.shah@sifive.com>, Zong Li <zong.li@sifive.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Greentime Hu <greentime.hu@sifive.com>,
	linux-riscv@lists.infradead.org, Will Deacon <will@kernel.org>,
	Ingo Molnar <mingo@kernel.org>
Subject: Re: [RFT PATCH v4 3/9] RISC-V: Implement late mapping page table allocation functions
Date: Thu, 30 Jul 2020 12:14:40 +0300
Message-ID: <20200730091440.GA534153@kernel.org> (raw)
In-Reply-To: <20200729233635.14406-4-atish.patra@wdc.com>

Hi,

On Wed, Jul 29, 2020 at 04:36:29PM -0700, Atish Patra wrote:
> Currently, page table setup is done during setup_va_final where fixmap can
> be used to create the temporary mappings. The physical frame is allocated
> from memblock_alloc_* functions. However, this won't work if page table
> mapping needs to be created for a different mm context (i.e. efi mm) at
> a later point of time.

TBH, I'm not very happy to see pte/pmd allocations, but I don't see a
way to reuse the existing routines in this case.

As a general wild idea, maybe it's worth using something like

struct pt_alloc_ops {
	pte_t *(*get_pte_virt)(phys_addr_t pa);
	phys_addr_t (*alloc_pte)(uintptr_t va);
	...
};

and add there implementations: nommu, MMU early and MMU late.

Some more comments below.
 
> Use generic kernel page allocation function & macros for any mapping
> after setup_vm_final.
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/mm/init.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 68c608a0e91f..cba03fec08c1 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -212,6 +212,7 @@ pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
>  pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
>  pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
>  static bool mmu_enabled;
> +static bool late_mapping;
>  
>  #define MAX_EARLY_MAPPING_SIZE	SZ_128M
>  
> @@ -236,7 +237,9 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot)
>  
>  static pte_t *__init get_pte_virt(phys_addr_t pa)
>  {
> -	if (mmu_enabled) {
> +	if (late_mapping)
> +		return (pte_t *) __va(pa);
> +	else if (mmu_enabled) {
>  		clear_fixmap(FIX_PTE);
>  		return (pte_t *)set_fixmap_offset(FIX_PTE, pa);
>  	} else {
> @@ -246,13 +249,19 @@ static pte_t *__init get_pte_virt(phys_addr_t pa)
>  
>  static phys_addr_t __init alloc_pte(uintptr_t va)
>  {
> +	unsigned long vaddr;
>  	/*
>  	 * We only create PMD or PGD early mappings so we
>  	 * should never reach here with MMU disabled.
>  	 */
>  	BUG_ON(!mmu_enabled);
> -
> -	return memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE);
> +	if (late_mapping) {
> +		vaddr = __get_free_page(GFP_KERNEL);
> +		if (!vaddr || !pgtable_pte_page_ctor(virt_to_page(vaddr)))
> +			BUG();

Is BUG() here really necessary? If I understand correctly, this would be
used to enable mappings for EFI runtime services, so we probably want to
propagete the error to to caller and, if really necessary, BUG() there,
don't we?

> +		return __pa(vaddr);
> +	} else
> +		return memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE);
>  }
>  
>  static void __init create_pte_mapping(pte_t *ptep,
> @@ -281,7 +290,9 @@ pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);
>  
>  static pmd_t *__init get_pmd_virt(phys_addr_t pa)
>  {
> -	if (mmu_enabled) {
> +	if (late_mapping)
> +		return (pmd_t *) __va(pa);
> +	else if (mmu_enabled) {
>  		clear_fixmap(FIX_PMD);
>  		return (pmd_t *)set_fixmap_offset(FIX_PMD, pa);
>  	} else {
> @@ -292,8 +303,13 @@ static pmd_t *__init get_pmd_virt(phys_addr_t pa)
>  static phys_addr_t __init alloc_pmd(uintptr_t va)
>  {
>  	uintptr_t pmd_num;
> +	unsigned long vaddr;
>  
> -	if (mmu_enabled)
> +	if (late_mapping) {
> +		vaddr = __get_free_page(GFP_KERNEL);
> +		BUG_ON(!vaddr);
> +		return __pa(vaddr);

Does nommu also need to allocate a page for pmd?

> +	} else if (mmu_enabled)
>  		return memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE);
>  
>  	pmd_num = (va - PAGE_OFFSET) >> PGDIR_SHIFT;
> @@ -533,6 +549,9 @@ static void __init setup_vm_final(void)
>  	/* Move to swapper page table */
>  	csr_write(CSR_SATP, PFN_DOWN(__pa_symbol(swapper_pg_dir)) | SATP_MODE);
>  	local_flush_tlb_all();
> +
> +	/* generic page allocation function must be used to setup page table */
> +	late_mapping = true;
>  }
>  #else
>  asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> -- 
> 2.24.0
> 

-- 
Sincerely yours,
Mike.

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

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 23:36 [RFT PATCH v4 0/9] Add UEFI support for RISC-V Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 1/9] RISC-V: Move DT mapping outof fixmap Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 2/9] RISC-V: Add early ioremap support Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 3/9] RISC-V: Implement late mapping page table allocation functions Atish Patra
2020-07-30  9:14   ` Mike Rapoport [this message]
2020-08-03  0:55     ` Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 4/9] include: pe.h: Add RISC-V related PE definition Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 5/9] RISC-V: Add PE/COFF header for EFI stub Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 6/9] RISC-V: Add EFI stub support Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 7/9] efi: Rename arm-init to efi-init common for all arch Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 8/9] RISC-V: Add EFI runtime services Atish Patra
2020-07-29 23:36 ` [RFT PATCH v4 9/9] RISC-V: Add page table dump support for uefi Atish Patra
2020-07-31 13:25 ` [RFT PATCH v4 0/9] Add UEFI support for RISC-V Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200730091440.GA534153@kernel.org \
    --to=rppt@kernel.org \
    --cc=anup.patel@wdc.com \
    --cc=ardb@kernel.org \
    --cc=atish.patra@wdc.com \
    --cc=greentime.hu@sifive.com \
    --cc=keescook@chromium.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=mingo@kernel.org \
    --cc=nickhu@andestech.com \
    --cc=nivedita@alum.mit.edu \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=paulmck@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    --cc=yash.shah@sifive.com \
    --cc=zong.li@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org
	public-inbox-index linux-riscv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git