[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140428132338.GA26829@linaro.org>
Date: Mon, 28 Apr 2014 14:23:39 +0100
From: Steve Capper <steve.capper@...aro.org>
To: Jungseok Lee <jays.lee@...sung.com>
Cc: linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
Catalin.Marinas@....com, 'Marc Zyngier' <Marc.Zyngier@....com>,
'Christoffer Dall' <christoffer.dall@...aro.org>,
linux-kernel@...r.kernel.org,
'linux-samsung-soc' <linux-samsung-soc@...r.kernel.org>,
sungjinn.chung@...sung.com, 'Arnd Bergmann' <arnd@...db.de>,
kgene.kim@...sung.com, ilho215.lee@...sung.com
Subject: Re: [PATCH v3 6/7] arm64: mm: Implement 4 levels of translation
tables
On Sun, Apr 27, 2014 at 12:37:35PM +0900, Jungseok Lee wrote:
> On Thursday, April 24, 2014 1:02 AM, Steve Capper wrote:
> > On Fri, Apr 18, 2014 at 04:59:20PM +0900, Jungseok Lee wrote:
[ ... ]
> >
> > This is overly complicated. For <4 levels we set x0 to be:
> > ttbr1 + 2*PAGE_SIZE. For 4-levels, we set x0 to be ttbr1 + PAGE_SIZE, then inside the create_pgd_entry
> > macro, we check the VA for FIXADDR_TOP then add another PAGE_SIZE. This is presumably done so the same
> > PUD is used for the swapper block map and the FIXADDR map.
>
> Is it too complicated to understand the logic?
>
> 1) For <4 levels:
> PAGE_SIZE is added for FIXADDR map and x0 is passed to create pgd entry.
> 2) For =4 levels:
> PAGE_SIZE is added in the create_pgd_entry macro since FIXADDR map info
> is needed to create pud entry.
>
> However, I agree that the code should be revised if other people feel
> like it is a labyrinthine logic.
>
> > If you assume that the PUD always follows the PGD for 4-levels, then you can remove this #ifdef and
> > the conditional VA logic in set_pgd_entry. To make the logic simpler for <4 levels, you could call
> > create_pud_entry in the middle of create_pgd_entry, then put down the actual pgd after.
>
> create_pud_entry should distinguish block map from FIXADDR map although
> PUD always follows the PGD in case of 4 levels. If we would like to avoid
> unnecessary #ifdef, the conditional logic should be introduced in create_
> pgd_entry macro.
>
> I cannot find the "best" way even though I've tried to figure it out.
> I would like to find out the most clear and self-descriptive expression.
>
> Could you give an idea on how to remove both conditional VA logic and #ifdef?
Hello Jungseok,
I had the following logic in my head:
It compiles and runs on the model, but I've not tried it in anger.
=========================================================================
.macro create_pud_entry, pgd, tbl, virt, pud, tmp1, tmp2
#ifdef CONFIG_ARM64_4_LEVELS
add \tbl, \tbl, #PAGE_SIZE // bump tbl 1 page up.
// to make room for pud
add \pud, \pgd, #PAGE_SIZE // pgd points to pud which
// follows pgd
lsr \tmp1, \virt, #PUD_SHIFT
and \tmp1, \tmp1, #PTRS_PER_PUD - 1 // PUD index
orr \tmp2, \tbl, #3 // PUD entry table type
str \tmp2, [\pud, \tmp1, lsl #3]
#else
mov \pud, \tbl // pgd points to section table
// directly for < 4 levels
#endif
.endm
/*
* Macro to populate the PGD for the corresponding block entry in the next
* level (tbl) for the given virtual address.
*
* Preserves: pgd, virt
* Corrupts: tmp1, tmp2, tmp3
* Returns: tbl -> page where block mappings can be placed
* (changed to make room for pud with 4levels, preserved otherwise)
*/
.macro create_pgd_entry, pgd, tbl, virt, tmp1, tmp2, tmp3
create_pud_entry \pgd, \tbl, \virt, \tmp3, \tmp1, \tmp2
lsr \tmp1, \virt, #PGDIR_SHIFT
and \tmp1, \tmp1, #PTRS_PER_PGD - 1 // PGD index
orr \tmp2, \tmp3, #3 // PGD entry table type
str \tmp2, [\pgd, \tmp1, lsl #3]
.endm
=========================================================================
[Note I've changed the extra argument to create_pgd_entry to be at the end
to make it easier to diff callers]
So essentially, we bump up tbl if we are running with 4 levels of page
table. We put the pgd down after the pud, and this allows us to sneak a
pud in after the pgd if we need to, otherwise it points to the target
for the section mapping.
Does this work for you? (I go cross-eyed with too much assembler, so
could have easily missed something).
>
> > > + create_pgd_entry x26, x0, x1, x5, x6, x7
> > >
> >
> > So before this patch we have the following created by
> > __create_page_tables:
> >
> > +========================+ <--- TEXT_OFFSET + PHYS_OFFSET
> > | FIXADDR (pmd or pte) |
> > +------------------------+
> > | block map (pmd or pte) |
> > +------------------------+
> > | PGDs for swapper |
> > +========================+ <--- TTBR1 swapper_pg_dir
> > | block map for idmap |
> > +------------------------+
> > | PGDs for idmap |
> > +------------------------+ <--- TTBR0 idmap_pg_dir
> >
> >
> > After the patch, for 4 levels activated we have:
> > +========================+ <--- TEXT_OFFSET + PHYS_OFFSET
> > | FIXADDR (ptes) |
> > +------------------------+
> > | block map (ptes) |
> > +------------------------+
> > | PUDs for swapper |
> > +------------------------+
> > | PGDs for swapper |
> > +========================+ <--- TTBR1 swapper_pg_dir
> > | block map for idmap |
> > +------------------------+
> > | PUDs for idmap |
> > +------------------------+
> > | PGDs for idmap |
> > +------------------------+ <--- TTBR0 idmap_pg_dir
> >
> > and without 4 levels activated we have:
> > +========================+ <--- TEXT_OFFSET + PHYS_OFFSET
> > | ZERO BYTES |
> > +------------------------+
> > | FIXADDR (pmd or pte) |
> > +------------------------+
> > | block map (pmd or pte) |
> > +------------------------+
> > | PGDs for swapper |
> > +========================+ <--- TTBR1 swapper_pg_dir
> > | ZERO BYTES |
> > +------------------------+
> > | block map for idmap |
> > +------------------------+
> > | PGDs for idmap |
> > +------------------------+ <--- TTBR0 idmap_pg_dir
>
> This is definitely helpful to understand head.S.
> It would be good to add these figures to Documentation or comments.
Please feel free to grab it if you want... :-).
Otherwise, I can put a patch in for this, as I am working on some logic
to remap the physical memory a PUD blocks for 4K granule.
Cheers,
--
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists