linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	paulus@samba.org
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v3 7/8] powerpc/mm: Consolidate radix and hash address map details
Date: Thu, 18 Apr 2019 11:20:35 +1000	[thread overview]
Message-ID: <1555549336.anl2oa7tyc.astroid@bobo.none> (raw)
In-Reply-To: <878sw8vmha.fsf@concordia.ellerman.id.au>

Michael Ellerman's on April 17, 2019 10:34 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Aneesh Kumar K.V's on April 16, 2019 8:07 pm:
>>> We now have
>>> 
>>> 4K page size config
>>> 
>>>  kernel_region_map_size = 16TB
>>>  kernel vmalloc start   = 0xc000100000000000
>>>  kernel IO start        = 0xc000200000000000
>>>  kernel vmemmap start   = 0xc000300000000000
>>> 
>>> with 64K page size config:
>>> 
>>>  kernel_region_map_size = 512TB
>>>  kernel vmalloc start   = 0xc008000000000000
>>>  kernel IO start        = 0xc00a000000000000
>>>  kernel vmemmap start   = 0xc00c000000000000
>>
>> Hey Aneesh,
>>
>> I like the series, I like consolidating the address spaces into 0xc,
>> and making the layouts match or similar isn't a bad thing. I don't
>> see any real reason to force limitations on one layout or another --
>> you could make the argument that 4k radix should match 64k radix
>> as much as matching 4k hash IMO.
> 
> I don't think I agree. The 4K and 64K layouts must be different because
> the page size is different and therefore the span of the page tables is
> different. (Unless you shrunk the 64K layouts to match 4K but that would
> be silly)

Both 4K and 64K radix map 52 bits of EA.

> On the other hand there's no good reason why hash & radix need to
> differ, it's just that radix has a more strictly defined layout and it
> didn't match what hash used when we added radix. We really should have
> done this realignment of the hash address ranges before we merged radix.

Well radix is limited by hardware, hash is more limited by software.

There is no real reason to define the addressing limit for radix any
less than what the hardware supports, for example. Hash is free to
match those limits if that's what we implement but it also does not
need to.

>> I wouldn't like to tie them too strongly to the same base defines
>> that force them to stay in sync.
>>
>> Can we drop this patch? Or at least keep the users of the H_ and R_
>> defines and set them to the same thing in map.h?
> 
> I don't understand why that would be a good thing?

Hash could be expanded in future, radix may be expanded or get
different page table layouts. I don't see why you'd tie them
together artificially.

> We have all this indirection through variables at the moment, for what
> appear to be constants. It makes the code harder to follow and it's less
> efficient as well.

What part is less efficient that matters?

Thanks,
Nick


  reply	other threads:[~2019-04-18  1:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-16 10:07 [PATCH v3 0/8] Update hash MMU kernel mapping to be in sync with radix Aneesh Kumar K.V
2019-04-16 10:07 ` [PATCH v3 1/8] powerpc/mm/hash64: Add a variable to track the end of IO mapping Aneesh Kumar K.V
2019-04-16 10:07 ` [PATCH v3 2/8] powerpc/mm/hash64: Map all the kernel regions in the same 0xc range Aneesh Kumar K.V
2019-04-16 10:07 ` [PATCH v3 3/8] powerpc/mm: Validate address values against different region limits Aneesh Kumar K.V
2019-04-16 10:07 ` [PATCH v3 4/8] powerpc/mm: Drop the unnecessary region check Aneesh Kumar K.V
2019-04-16 10:07 ` [PATCH v3 5/8] powerpc/mm/hash: Simplify the region id calculation Aneesh Kumar K.V
2019-04-16 10:07 ` [PATCH v3 6/8] powerpc/mm: Print kernel map details to dmesg Aneesh Kumar K.V
2019-04-16 10:07 ` [PATCH v3 7/8] powerpc/mm: Consolidate radix and hash address map details Aneesh Kumar K.V
2019-04-16 14:03   ` Nicholas Piggin
2019-04-17  3:00     ` Aneesh Kumar K.V
2019-04-17 12:34     ` Michael Ellerman
2019-04-18  1:20       ` Nicholas Piggin [this message]
2019-04-16 10:07 ` [PATCH v3 8/8] powerpc/mm/hash: Rename KERNEL_REGION_ID to LINEAR_MAP_REGION_ID Aneesh Kumar K.V

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=1555549336.anl2oa7tyc.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).