From: santosh.shilimkar@ti•com (Santosh Shilimkar)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 5/6] ARM: mm: Recreate kernel mappings in early_paging_init()
Date: Wed, 9 Oct 2013 14:51:13 -0400 [thread overview]
Message-ID: <5255A5A1.60403@ti.com> (raw)
In-Reply-To: <20131009100603.GA5985@mudshark.cambridge.arm.com>
On Wednesday 09 October 2013 06:06 AM, Will Deacon wrote:
> On Tue, Oct 08, 2013 at 06:45:33PM +0100, Santosh Shilimkar wrote:
>> On Tuesday 08 October 2013 06:26 AM, Will Deacon wrote:
>>> On Mon, Oct 07, 2013 at 08:34:41PM +0100, Santosh Shilimkar wrote:
>>>> + /*
>>>> + * Cache cleaning operations for self-modifying code
>>>> + * We should clean the entries by MVA but running a
>>>> + * for loop over every pv_table entry pointer would
>>>> + * just complicate the code. isb() is added to commit
>>>> + * all the prior cp15 operations.
>>>> + */
>>>> + flush_cache_louis();
>>>> + isb();
>>>
>>> I see, you need the new __pv_tables to be visible for your page table
>>> population below, right? In which case, I'm afraid I have to go back on my
>>> original statement; you *do* need that dsb() prior to the isb() if you want
>>> to ensure that the icache maintenance is complete and synchronised.
>>>
>> Need of dsb and isb is what ARM ARM says but then I got bit biased after
>> your reply.
>
> Yeah, sorry about that. I didn't originally notice that you needed the I-cache
> flushing before the __pa stuff below.
>
No problem
>>>> + }
>>>> +
>>>> + /* remap level 1 table */
>>>> + for (i = 0; i < PTRS_PER_PGD; i++) {
>>>> + *pud0++ = __pud(__pa(pmd0) | PMD_TYPE_TABLE | L_PGD_SWAPPER);
>>>> + pmd0 += PTRS_PER_PMD;
>>>> + }
>>>> +
>>>> + __cpuc_flush_dcache_area(pud_start, sizeof(pud_start) * PTRS_PER_PGD);
>>>> + outer_clean_range(virt_to_phys(pud_start), sizeof(pud_start) * PTRS_PER_PGD);
>>>
>>> You don't need to flush these page tables if you're SMP. If you use
>>> clean_dcache_area instead, it will do the right thing. The again, why can't
>>> you use pud_populate and pmd_populate for these two loops? Is there an
>>> interaction with coherency here? (if so, why don't you need to flush the
>>> entire cache hierarchy anyway?)
>>>
>> You mean AMRMv7 SMP PT walkers can read from L1 cache and hence doesn't need
>> flushing L1. While this could be true, for some reason we don't the same
>> behavior and seeing that without flush we are seeing the issue.
>
> I would really like to know why this isn't working for you. I have a feeling
> that it's related to your interesting coherency issues on keystone. For
> example, if the physical address put in the ttbr doesn't match the physical
> address which is mapped to the kernel page tables, then we could get
> physical aliasing in the caches.
>
It might be. we will keep debugging that.
>> Initially we were doing entire cache flush but moved to the mva based
>> routines on your suggestion.
>
> If the issue is related to coherency and physical aliasing, I really think
> you should just flush the entire cache hierarchy. It's difficult to identify
> exactly what state needs to be carried over between the old and new
> mappings, but I bet it's more than just page tables.
>
You are probably right. I will go back to the full flush to avoid any
corner case till we figure out the issue.
>> Regarding the pud_populate(), since we needed L_PGD_SWAPPER, we couldn't
>> use that version but updated patch uses the set_pud() which takes the flag.
>> And pmd_populate() can't be used either because it creates pte based
>> tables which is not what we want.
>
> Ok. It certainly looks better than it did.
>
Thanks a lot. I will refresh the patch with above update.
Regards,
Santosh
next prev parent reply other threads:[~2013-10-09 18:51 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-03 21:17 [PATCH v3 0/6] ARM: mm: Extend the runtime patch stub for PAE systems Santosh Shilimkar
2013-10-03 21:17 ` [PATCH v3 1/6] ARM: mm: use phys_addr_t appropriately in p2v and v2p conversions Santosh Shilimkar
2013-10-03 21:17 ` [PATCH v3 2/6] ARM: mm: Introduce virt_to_idmap() with an arch hook Santosh Shilimkar
2013-10-03 21:17 ` [PATCH v3 3/6] ARM: mm: Move the idmap print to appropriate place in the code Santosh Shilimkar
2013-10-03 21:17 ` [PATCH v3 4/6] ARM: mm: Correct virt_to_phys patching for 64 bit physical addresses Santosh Shilimkar
2013-10-04 0:17 ` Nicolas Pitre
2013-10-04 5:37 ` Sricharan R
2013-10-04 13:02 ` Nicolas Pitre
2013-10-07 19:25 ` Santosh Shilimkar
2013-10-07 19:42 ` Nicolas Pitre
2013-10-08 11:43 ` Sricharan R
2013-10-03 21:17 ` [PATCH v3 5/6] ARM: mm: Recreate kernel mappings in early_paging_init() Santosh Shilimkar
2013-10-04 0:23 ` Nicolas Pitre
2013-10-04 15:59 ` Will Deacon
2013-10-04 16:12 ` Santosh Shilimkar
2013-10-07 19:34 ` Santosh Shilimkar
2013-10-08 10:26 ` Will Deacon
2013-10-08 17:45 ` Santosh Shilimkar
2013-10-09 10:06 ` Will Deacon
2013-10-09 18:51 ` Santosh Shilimkar [this message]
2013-10-03 21:18 ` [PATCH v3 6/6] ARM: mm: Change the order of TLB/cache maintenance operations Santosh Shilimkar
2013-10-04 0:25 ` Nicolas Pitre
2013-10-04 8:46 ` Russell King - ARM Linux
2013-10-04 13:14 ` Nicolas Pitre
2013-10-04 13:19 ` Santosh Shilimkar
2013-10-04 15:52 ` Will Deacon
2013-10-04 16:03 ` Santosh Shilimkar
2013-10-09 18:56 ` Santosh Shilimkar
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=5255A5A1.60403@ti.com \
--to=santosh.shilimkar@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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