From: Alexandre Ghiti <alexghiti@rivosinc.com> To: Andrew Jones <ajones@ventanamicro.com> Cc: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Guo Ren <guoren@kernel.org>, Rob Herring <robh+dt@kernel.org>, Frank Rowand <frowand.list@gmail.com>, Conor Dooley <conor@kernel.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arch@vger.kernel.org Subject: Re: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable Date: Fri, 27 Jan 2023 09:48:01 +0100 [thread overview] Message-ID: <CAHVXubgCK23digBakrPjC7_J-OVD9Bu2=hcvDYvvtnLnyj7Ajw@mail.gmail.com> (raw) In-Reply-To: <20230125114044.qcr2canalvljevcu@orel> On Wed, Jan 25, 2023 at 12:40 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Wed, Jan 25, 2023 at 09:12:13AM +0100, Alexandre Ghiti wrote: > > Use directly phys_ram_base instead, riscv_pfn_base is just the pfn of > > the address contained in phys_ram_base. > > > > Even if there is no functional change intended in this patch, actually > > setting phys_ram_base that early changes the behaviour of > > kernel_mapping_pa_to_va during the early boot: phys_ram_base used to be > > zero before this patch and now it is set to the physical start address of > > the kernel. But it does not break the conversion of a kernel physical > > address into a virtual address since kernel_mapping_pa_to_va should only > > be used on kernel physical addresses, i.e. addresses greater than the > > physical start address of the kernel. > > afaict, only CONFIG_XIP_KERNEL kernels use phys_ram_base prior to > setup_bootmem() and, for them, this change only redundantly sets > phys_ram_base to the same thing, so I believe this is a no functional > change patch. > Good, thanks for checking again > > > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > --- > > arch/riscv/include/asm/page.h | 3 +-- > > arch/riscv/mm/init.c | 6 +----- > > 2 files changed, 2 insertions(+), 7 deletions(-) > > > > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h > > index 9f432c1b5289..728eee53152a 100644 > > --- a/arch/riscv/include/asm/page.h > > +++ b/arch/riscv/include/asm/page.h > > @@ -91,8 +91,7 @@ typedef struct page *pgtable_t; > > #endif > > > > #ifdef CONFIG_MMU > > -extern unsigned long riscv_pfn_base; > > -#define ARCH_PFN_OFFSET (riscv_pfn_base) > > +#define ARCH_PFN_OFFSET (PFN_DOWN(phys_ram_base)) > > #else > > #define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT) > > #endif /* CONFIG_MMU */ > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > > index 478d6763a01a..225a7d2b65cc 100644 > > --- a/arch/riscv/mm/init.c > > +++ b/arch/riscv/mm/init.c > > @@ -271,9 +271,6 @@ static void __init setup_bootmem(void) > > #ifdef CONFIG_MMU > > struct pt_alloc_ops pt_ops __initdata; > > > > -unsigned long riscv_pfn_base __ro_after_init; > > -EXPORT_SYMBOL(riscv_pfn_base); > > - > > pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss; > > pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss; > > static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss; > > @@ -285,7 +282,6 @@ static pmd_t __maybe_unused early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAG > > > > #ifdef CONFIG_XIP_KERNEL > > #define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops)) > > -#define riscv_pfn_base (*(unsigned long *)XIP_FIXUP(&riscv_pfn_base)) > > #define trampoline_pg_dir ((pgd_t *)XIP_FIXUP(trampoline_pg_dir)) > > #define fixmap_pte ((pte_t *)XIP_FIXUP(fixmap_pte)) > > #define early_pg_dir ((pgd_t *)XIP_FIXUP(early_pg_dir)) > > @@ -985,7 +981,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > > kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr; > > kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr; > > > > - riscv_pfn_base = PFN_DOWN(kernel_map.phys_addr); > > + phys_ram_base = kernel_map.phys_addr; > > nit: I'd put this in the #else part of the #ifdef CONFIG_XIP_KERNEL above > to have some consistency with that #ifdef arm and also avoid the redundant > assignment of phys_ram_base for CONFIG_XIP_KERNEL. True, but as this is removed in the next patch, I guess we can live with that. > > > > > /* > > * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit > > -- > > 2.37.2 > > > > Otherwise, > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks! Alex > > Thanks, > drew
WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Ghiti <alexghiti@rivosinc.com> To: Andrew Jones <ajones@ventanamicro.com> Cc: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Guo Ren <guoren@kernel.org>, Rob Herring <robh+dt@kernel.org>, Frank Rowand <frowand.list@gmail.com>, Conor Dooley <conor@kernel.org>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arch@vger.kernel.org Subject: Re: [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable Date: Fri, 27 Jan 2023 09:48:01 +0100 [thread overview] Message-ID: <CAHVXubgCK23digBakrPjC7_J-OVD9Bu2=hcvDYvvtnLnyj7Ajw@mail.gmail.com> (raw) In-Reply-To: <20230125114044.qcr2canalvljevcu@orel> On Wed, Jan 25, 2023 at 12:40 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Wed, Jan 25, 2023 at 09:12:13AM +0100, Alexandre Ghiti wrote: > > Use directly phys_ram_base instead, riscv_pfn_base is just the pfn of > > the address contained in phys_ram_base. > > > > Even if there is no functional change intended in this patch, actually > > setting phys_ram_base that early changes the behaviour of > > kernel_mapping_pa_to_va during the early boot: phys_ram_base used to be > > zero before this patch and now it is set to the physical start address of > > the kernel. But it does not break the conversion of a kernel physical > > address into a virtual address since kernel_mapping_pa_to_va should only > > be used on kernel physical addresses, i.e. addresses greater than the > > physical start address of the kernel. > > afaict, only CONFIG_XIP_KERNEL kernels use phys_ram_base prior to > setup_bootmem() and, for them, this change only redundantly sets > phys_ram_base to the same thing, so I believe this is a no functional > change patch. > Good, thanks for checking again > > > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > --- > > arch/riscv/include/asm/page.h | 3 +-- > > arch/riscv/mm/init.c | 6 +----- > > 2 files changed, 2 insertions(+), 7 deletions(-) > > > > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h > > index 9f432c1b5289..728eee53152a 100644 > > --- a/arch/riscv/include/asm/page.h > > +++ b/arch/riscv/include/asm/page.h > > @@ -91,8 +91,7 @@ typedef struct page *pgtable_t; > > #endif > > > > #ifdef CONFIG_MMU > > -extern unsigned long riscv_pfn_base; > > -#define ARCH_PFN_OFFSET (riscv_pfn_base) > > +#define ARCH_PFN_OFFSET (PFN_DOWN(phys_ram_base)) > > #else > > #define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT) > > #endif /* CONFIG_MMU */ > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > > index 478d6763a01a..225a7d2b65cc 100644 > > --- a/arch/riscv/mm/init.c > > +++ b/arch/riscv/mm/init.c > > @@ -271,9 +271,6 @@ static void __init setup_bootmem(void) > > #ifdef CONFIG_MMU > > struct pt_alloc_ops pt_ops __initdata; > > > > -unsigned long riscv_pfn_base __ro_after_init; > > -EXPORT_SYMBOL(riscv_pfn_base); > > - > > pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss; > > pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss; > > static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss; > > @@ -285,7 +282,6 @@ static pmd_t __maybe_unused early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAG > > > > #ifdef CONFIG_XIP_KERNEL > > #define pt_ops (*(struct pt_alloc_ops *)XIP_FIXUP(&pt_ops)) > > -#define riscv_pfn_base (*(unsigned long *)XIP_FIXUP(&riscv_pfn_base)) > > #define trampoline_pg_dir ((pgd_t *)XIP_FIXUP(trampoline_pg_dir)) > > #define fixmap_pte ((pte_t *)XIP_FIXUP(fixmap_pte)) > > #define early_pg_dir ((pgd_t *)XIP_FIXUP(early_pg_dir)) > > @@ -985,7 +981,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa) > > kernel_map.va_pa_offset = PAGE_OFFSET - kernel_map.phys_addr; > > kernel_map.va_kernel_pa_offset = kernel_map.virt_addr - kernel_map.phys_addr; > > > > - riscv_pfn_base = PFN_DOWN(kernel_map.phys_addr); > > + phys_ram_base = kernel_map.phys_addr; > > nit: I'd put this in the #else part of the #ifdef CONFIG_XIP_KERNEL above > to have some consistency with that #ifdef arm and also avoid the redundant > assignment of phys_ram_base for CONFIG_XIP_KERNEL. True, but as this is removed in the next patch, I guess we can live with that. > > > > > /* > > * The default maximal physical memory size is KERN_VIRT_SIZE for 32-bit > > -- > > 2.37.2 > > > > Otherwise, > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks! Alex > > Thanks, > drew _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-01-27 8:48 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-25 8:12 [PATCH v5 0/2] riscv: Use PUD/P4D/PGD pages for the linear mapping Alexandre Ghiti 2023-01-25 8:12 ` Alexandre Ghiti 2023-01-25 8:12 ` [PATCH v5 1/2] riscv: Get rid of riscv_pfn_base variable Alexandre Ghiti 2023-01-25 8:12 ` Alexandre Ghiti 2023-01-25 11:40 ` Andrew Jones 2023-01-25 11:40 ` Andrew Jones 2023-01-27 8:48 ` Alexandre Ghiti [this message] 2023-01-27 8:48 ` Alexandre Ghiti 2023-01-28 14:07 ` kernel test robot 2023-01-28 14:07 ` kernel test robot 2023-01-28 14:58 ` kernel test robot 2023-01-28 14:58 ` kernel test robot 2023-03-01 7:56 ` Alexandre Ghiti 2023-03-01 7:56 ` Alexandre Ghiti 2023-01-25 8:12 ` [PATCH v5 2/2] riscv: Use PUD/P4D/PGD pages for the linear mapping Alexandre Ghiti 2023-01-25 8:12 ` Alexandre Ghiti 2023-01-25 11:42 ` Andrew Jones 2023-01-25 11:42 ` Andrew Jones
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='CAHVXubgCK23digBakrPjC7_J-OVD9Bu2=hcvDYvvtnLnyj7Ajw@mail.gmail.com' \ --to=alexghiti@rivosinc.com \ --cc=ajones@ventanamicro.com \ --cc=aou@eecs.berkeley.edu \ --cc=conor@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=frowand.list@gmail.com \ --cc=guoren@kernel.org \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=robh+dt@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.