linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: using page table index in setup_vm()
@ 2019-05-20  8:48 JaeJoon Jung
  2019-05-23  9:52 ` Mike Rapoport
  0 siblings, 1 reply; 7+ messages in thread
From: JaeJoon Jung @ 2019-05-20  8:48 UTC (permalink / raw)
  To: linux-riscv

From: JaeJoon Jung <rgbi3307@gmail.com>

The page table index macro are defined already in pgtable.h as below:
///arch/riscv/include/asm/pgtable.h
#define pgd_index(addr)     (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1))
#define pte_index(addr)     (((addr) >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
///arch/riscv/include/asm/pgtable-64.h
#define pmd_index(addr)     (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1))

But, In the arch/riscv/mm/init.c,
I found that it does not use above macro in setup_vm().
I wat to use this macro in setup_vm() as below:


Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
Cc: Anup Patel <anup.patel@wdc.com>
Cc: linux-riscv@lists.infradead.org


diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index bc7b77e34d09..785954b776ac 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c

 asmlinkage void __init setup_vm(void)
 {
        extern char _start;
@@ -223,13 +190,13 @@ asmlinkage void __init setup_vm(void)
        BUG_ON((pa % (PAGE_SIZE * PTRS_PER_PTE)) != 0);

 #ifndef __PAGETABLE_PMD_FOLDED
-       trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+       trampoline_pg_dir[pgd_index(PAGE_OFFSET)] =
                pfn_pgd(PFN_DOWN((uintptr_t)trampoline_pmd),
                        __pgprot(_PAGE_TABLE));
        trampoline_pmd[0] = pfn_pmd(PFN_DOWN(pa), prot);

        for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
-               size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
+               size_t o = pgd_index(PAGE_OFFSET) + i;

                swapper_pg_dir[o] =
                        pfn_pgd(PFN_DOWN((uintptr_t)swapper_pmd) + i,
@@ -238,24 +205,23 @@ asmlinkage void __init setup_vm(void)
        for (i = 0; i < ARRAY_SIZE(swapper_pmd); i++)
                swapper_pmd[i] = pfn_pmd(PFN_DOWN(pa + i * PMD_SIZE), prot);

-       swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+       swapper_pg_dir[pgd_index(FIXADDR_START)] =
                pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pmd),
                                __pgprot(_PAGE_TABLE));
-       fixmap_pmd[(FIXADDR_START >> PMD_SHIFT) % PTRS_PER_PMD] =
+       fixmap_pmd[pmd_index(FIXADDR_START)] =
                pfn_pmd(PFN_DOWN((uintptr_t)fixmap_pte),
                                __pgprot(_PAGE_TABLE));
 #else
-       trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+       trampoline_pg_dir[pgd_index(PAGE_OFFSET)] =
                pfn_pgd(PFN_DOWN(pa), prot);

        for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
-               size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
+               size_t o = pgd_index(PAGE_OFFSET) + i;

                swapper_pg_dir[o] =
                        pfn_pgd(PFN_DOWN(pa + i * PGDIR_SIZE), prot);
        }
-
-       swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
+       swapper_pg_dir[pgd_index(FIXADDR_START)] =
                pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pte),
                                __pgprot(_PAGE_TABLE));
 #endif

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: using page table index in setup_vm()
  2019-05-20  8:48 [PATCH] riscv: using page table index in setup_vm() JaeJoon Jung
@ 2019-05-23  9:52 ` Mike Rapoport
  2019-05-23 10:12   ` JaeJoon Jung
  0 siblings, 1 reply; 7+ messages in thread
From: Mike Rapoport @ 2019-05-23  9:52 UTC (permalink / raw)
  To: JaeJoon Jung; +Cc: linux-riscv

On Mon, May 20, 2019 at 05:48:24PM +0900, JaeJoon Jung wrote:
> From: JaeJoon Jung <rgbi3307@gmail.com>
> 
> The page table index macro are defined already in pgtable.h as below:
> ///arch/riscv/include/asm/pgtable.h
> #define pgd_index(addr)     (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1))
> #define pte_index(addr)     (((addr) >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
> ///arch/riscv/include/asm/pgtable-64.h
> #define pmd_index(addr)     (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1))
> 
> But, In the arch/riscv/mm/init.c,
> I found that it does not use above macro in setup_vm().
> I wat to use this macro in setup_vm() as below:

I'd suggest the following changelog:
-----------------------------------------
riscv: setup_vm: use p?d_index() instead of its open coded implementation

The page table indexing macros are defined in include/asm/pgtable.h, but
setup_vm() uses an open coded implementation.

Replace it with the appropriate macros.
-----------------------------------------


> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> Cc: Anup Patel <anup.patel@wdc.com>
> Cc: linux-riscv@lists.infradead.org
 
Otherwise

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index bc7b77e34d09..785954b776ac 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> 
>  asmlinkage void __init setup_vm(void)
>  {
>         extern char _start;
> @@ -223,13 +190,13 @@ asmlinkage void __init setup_vm(void)
>         BUG_ON((pa % (PAGE_SIZE * PTRS_PER_PTE)) != 0);
> 
>  #ifndef __PAGETABLE_PMD_FOLDED
> -       trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> +       trampoline_pg_dir[pgd_index(PAGE_OFFSET)] =
>                 pfn_pgd(PFN_DOWN((uintptr_t)trampoline_pmd),
>                         __pgprot(_PAGE_TABLE));
>         trampoline_pmd[0] = pfn_pmd(PFN_DOWN(pa), prot);
> 
>         for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
> -               size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
> +               size_t o = pgd_index(PAGE_OFFSET) + i;
> 
>                 swapper_pg_dir[o] =
>                         pfn_pgd(PFN_DOWN((uintptr_t)swapper_pmd) + i,
> @@ -238,24 +205,23 @@ asmlinkage void __init setup_vm(void)
>         for (i = 0; i < ARRAY_SIZE(swapper_pmd); i++)
>                 swapper_pmd[i] = pfn_pmd(PFN_DOWN(pa + i * PMD_SIZE), prot);
> 
> -       swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> +       swapper_pg_dir[pgd_index(FIXADDR_START)] =
>                 pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pmd),
>                                 __pgprot(_PAGE_TABLE));
> -       fixmap_pmd[(FIXADDR_START >> PMD_SHIFT) % PTRS_PER_PMD] =
> +       fixmap_pmd[pmd_index(FIXADDR_START)] =
>                 pfn_pmd(PFN_DOWN((uintptr_t)fixmap_pte),
>                                 __pgprot(_PAGE_TABLE));
>  #else
> -       trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> +       trampoline_pg_dir[pgd_index(PAGE_OFFSET)] =
>                 pfn_pgd(PFN_DOWN(pa), prot);
> 
>         for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
> -               size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
> +               size_t o = pgd_index(PAGE_OFFSET) + i;
> 
>                 swapper_pg_dir[o] =
>                         pfn_pgd(PFN_DOWN(pa + i * PGDIR_SIZE), prot);
>         }
> -
> -       swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> +       swapper_pg_dir[pgd_index(FIXADDR_START)] =
>                 pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pte),
>                                 __pgprot(_PAGE_TABLE));
>  #endif
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

-- 
Sincerely yours,
Mike.


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: using page table index in setup_vm()
  2019-05-23  9:52 ` Mike Rapoport
@ 2019-05-23 10:12   ` JaeJoon Jung
  2019-06-28  6:04     ` Paul Walmsley
  0 siblings, 1 reply; 7+ messages in thread
From: JaeJoon Jung @ 2019-05-23 10:12 UTC (permalink / raw)
  To: Mike Rapoport; +Cc: linux-riscv

Thanks for your professional advice.
I'd like to take your appropriate suggestion.

Sincerely yours,
JaeJoon Jung.

On Thu, 23 May 2019 at 18:52, Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Mon, May 20, 2019 at 05:48:24PM +0900, JaeJoon Jung wrote:
> > From: JaeJoon Jung <rgbi3307@gmail.com>
> >
> > The page table index macro are defined already in pgtable.h as below:
> > ///arch/riscv/include/asm/pgtable.h
> > #define pgd_index(addr)     (((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1))
> > #define pte_index(addr)     (((addr) >> PAGE_SHIFT) & (PTRS_PER_PTE - 1))
> > ///arch/riscv/include/asm/pgtable-64.h
> > #define pmd_index(addr)     (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1))
> >
> > But, In the arch/riscv/mm/init.c,
> > I found that it does not use above macro in setup_vm().
> > I wat to use this macro in setup_vm() as below:
>
> I'd suggest the following changelog:
> -----------------------------------------
> riscv: setup_vm: use p?d_index() instead of its open coded implementation
>
> The page table indexing macros are defined in include/asm/pgtable.h, but
> setup_vm() uses an open coded implementation.
>
> Replace it with the appropriate macros.
> -----------------------------------------
>
>
> > Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> > Cc: Anup Patel <anup.patel@wdc.com>
> > Cc: linux-riscv@lists.infradead.org
>
> Otherwise
>
> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
>
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index bc7b77e34d09..785954b776ac 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> >
> >  asmlinkage void __init setup_vm(void)
> >  {
> >         extern char _start;
> > @@ -223,13 +190,13 @@ asmlinkage void __init setup_vm(void)
> >         BUG_ON((pa % (PAGE_SIZE * PTRS_PER_PTE)) != 0);
> >
> >  #ifndef __PAGETABLE_PMD_FOLDED
> > -       trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> > +       trampoline_pg_dir[pgd_index(PAGE_OFFSET)] =
> >                 pfn_pgd(PFN_DOWN((uintptr_t)trampoline_pmd),
> >                         __pgprot(_PAGE_TABLE));
> >         trampoline_pmd[0] = pfn_pmd(PFN_DOWN(pa), prot);
> >
> >         for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
> > -               size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
> > +               size_t o = pgd_index(PAGE_OFFSET) + i;
> >
> >                 swapper_pg_dir[o] =
> >                         pfn_pgd(PFN_DOWN((uintptr_t)swapper_pmd) + i,
> > @@ -238,24 +205,23 @@ asmlinkage void __init setup_vm(void)
> >         for (i = 0; i < ARRAY_SIZE(swapper_pmd); i++)
> >                 swapper_pmd[i] = pfn_pmd(PFN_DOWN(pa + i * PMD_SIZE), prot);
> >
> > -       swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> > +       swapper_pg_dir[pgd_index(FIXADDR_START)] =
> >                 pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pmd),
> >                                 __pgprot(_PAGE_TABLE));
> > -       fixmap_pmd[(FIXADDR_START >> PMD_SHIFT) % PTRS_PER_PMD] =
> > +       fixmap_pmd[pmd_index(FIXADDR_START)] =
> >                 pfn_pmd(PFN_DOWN((uintptr_t)fixmap_pte),
> >                                 __pgprot(_PAGE_TABLE));
> >  #else
> > -       trampoline_pg_dir[(PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> > +       trampoline_pg_dir[pgd_index(PAGE_OFFSET)] =
> >                 pfn_pgd(PFN_DOWN(pa), prot);
> >
> >         for (i = 0; i < (-PAGE_OFFSET)/PGDIR_SIZE; ++i) {
> > -               size_t o = (PAGE_OFFSET >> PGDIR_SHIFT) % PTRS_PER_PGD + i;
> > +               size_t o = pgd_index(PAGE_OFFSET) + i;
> >
> >                 swapper_pg_dir[o] =
> >                         pfn_pgd(PFN_DOWN(pa + i * PGDIR_SIZE), prot);
> >         }
> > -
> > -       swapper_pg_dir[(FIXADDR_START >> PGDIR_SHIFT) % PTRS_PER_PGD] =
> > +       swapper_pg_dir[pgd_index(FIXADDR_START)] =
> >                 pfn_pgd(PFN_DOWN((uintptr_t)fixmap_pte),
> >                                 __pgprot(_PAGE_TABLE));
> >  #endif
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
>
> --
> Sincerely yours,
> Mike.
>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: using page table index in setup_vm()
  2019-05-23 10:12   ` JaeJoon Jung
@ 2019-06-28  6:04     ` Paul Walmsley
  2019-06-28  6:11       ` Christoph Hellwig
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Walmsley @ 2019-06-28  6:04 UTC (permalink / raw)
  To: JaeJoon Jung; +Cc: linux-riscv, Mike Rapoport

Hi

On Thu, 23 May 2019, JaeJoon Jung wrote:

> Thanks for your professional advice.
> I'd like to take your appropriate suggestion.

Are you planning to repost this patch with the new patch description?


- Paul

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: using page table index in setup_vm()
  2019-06-28  6:04     ` Paul Walmsley
@ 2019-06-28  6:11       ` Christoph Hellwig
  2019-06-28  8:11         ` Anup Patel
  0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-06-28  6:11 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: linux-riscv, JaeJoon Jung, Mike Rapoport

On Thu, Jun 27, 2019 at 11:04:56PM -0700, Paul Walmsley wrote:
> Hi
> 
> On Thu, 23 May 2019, JaeJoon Jung wrote:
> 
> > Thanks for your professional advice.
> > I'd like to take your appropriate suggestion.
> 
> Are you planning to repost this patch with the new patch description?

Note that this is going to confict with Anups bigger rework of the
area.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: using page table index in setup_vm()
  2019-06-28  6:11       ` Christoph Hellwig
@ 2019-06-28  8:11         ` Anup Patel
  2019-06-28 10:38           ` JaeJoon Jung
  0 siblings, 1 reply; 7+ messages in thread
From: Anup Patel @ 2019-06-28  8:11 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-riscv, Mike Rapoport, JaeJoon Jung, Paul Walmsley

On Fri, Jun 28, 2019 at 11:41 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Jun 27, 2019 at 11:04:56PM -0700, Paul Walmsley wrote:
> > Hi
> >
> > On Thu, 23 May 2019, JaeJoon Jung wrote:
> >
> > > Thanks for your professional advice.
> > > I'd like to take your appropriate suggestion.
> >
> > Are you planning to repost this patch with the new patch description?
>
> Note that this is going to confict with Anups bigger rework of the
> area.

Yes, it does conflict. Thanks for catching.

Refer,
https://patchwork.kernel.org/patch/10980791/

Regards,
Anup

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: using page table index in setup_vm()
  2019-06-28  8:11         ` Anup Patel
@ 2019-06-28 10:38           ` JaeJoon Jung
  0 siblings, 0 replies; 7+ messages in thread
From: JaeJoon Jung @ 2019-06-28 10:38 UTC (permalink / raw)
  To: Anup Patel; +Cc: Christoph Hellwig, linux-riscv, Mike Rapoport, Paul Walmsley

Yes, it does conflict with Anups bigger rework of the area.
I'd like that my patch belong to Anups.
Thanks Christoph and Anup for checking.
Regards,
JaeJoon Jung

On Fri, 28 Jun 2019 at 17:11, Anup Patel <anup@brainfault.org> wrote:
>
> On Fri, Jun 28, 2019 at 11:41 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Thu, Jun 27, 2019 at 11:04:56PM -0700, Paul Walmsley wrote:
> > > Hi
> > >
> > > On Thu, 23 May 2019, JaeJoon Jung wrote:
> > >
> > > > Thanks for your professional advice.
> > > > I'd like to take your appropriate suggestion.
> > >
> > > Are you planning to repost this patch with the new patch description?
> >
> > Note that this is going to confict with Anups bigger rework of the
> > area.
>
> Yes, it does conflict. Thanks for catching.
>
> Refer,
> https://patchwork.kernel.org/patch/10980791/
>
> Regards,
> Anup

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-06-28 10:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20  8:48 [PATCH] riscv: using page table index in setup_vm() JaeJoon Jung
2019-05-23  9:52 ` Mike Rapoport
2019-05-23 10:12   ` JaeJoon Jung
2019-06-28  6:04     ` Paul Walmsley
2019-06-28  6:11       ` Christoph Hellwig
2019-06-28  8:11         ` Anup Patel
2019-06-28 10:38           ` JaeJoon Jung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).