* [PATCH 2/3] powerpc/mm/hash: Fix the reference bit update when handling hash fault
2016-05-31 6:26 [PATCH 1/3] powerpc/mm/radix: Update LPCR only if it is powernv Aneesh Kumar K.V
@ 2016-05-31 6:26 ` Aneesh Kumar K.V
2016-06-06 0:17 ` [2/3] " Michael Ellerman
2016-05-31 6:26 ` [PATCH 3/3] powerpc/mm/radix: Add missing tlb flush Aneesh Kumar K.V
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Aneesh Kumar K.V @ 2016-05-31 6:26 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
When we converted the asm routines to C functions, we missed updating
HPTE_R_R based on _PAGE_ACCESSED. ASM code used to copy over the lower
bits from pte via.
andi. r3,r30,0x1fe /* Get basic set of flags */
Fixes: 'commit 89ff725051d1 ("powerpc/mm: Convert __hash_page_64K to C")'
We also update the code such that we won't update the Change bit ('C'
bit) always. This was added by
'commit c5cf0e30bf3d8 ("powerpc: Fix buglet with MMU hash management")'
With hash64, we need to make sure that hardware doesn't do a pte update
directly. This is because we do end up with entries in TLB with no hash
page table entry. This happens because when we find hash bucket full,
we "evict" a more/less random entry from it. When we do that we don't
invalidate the TLB (hpte_remove) because we assume the old translation
is still technically "valid". For more info look at
'commit 0608d692463("powerpc/mm: Always invalidate tlb on hpte invalidate and
update")'. Thus it's critical that valid hash PTEs always have reference
bit set and writeable ones have change bit set. We do this by hashing a
non-dirty linux PTE as read-only and always setting _PAGE_ACCESSED
(and thus R) when hashing anything else in. Any attempt by Linux at
clearing those bits also removes the corresponding hash entry.
Commitc 5cf0e30bf3d8 did that for 'C' bit by enabling 'C' bit always.
We don't really need to do that because we never map a RW pte entry
without setting 'C' bit. On READ fault on a RW pte entry, we still map
it READ only, hence a store update in the page will still cause a hash
pte fault.
This patch reverts the part of the c5cf0e30bf3d8
("[PATCH] powerpc: Fix buglet with MMU hash management") and retain
the updatepp part.
- If we hit the updatepp path on native, the old code without that
commit, would fail to set C bcause native_hpte_updatepp()
was implemented to filter the same bits as H_PROTECT and not let C
through thus we would "upgrade" a RO HPTE to RW without setting C
thus causing the bug. So the real fix in that commit was the change
to native_hpte_updatepp
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/mm/hash_utils_64.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 59268969a0bc..b2740c67e172 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -159,6 +159,19 @@ static struct mmu_psize_def mmu_psize_defaults_gp[] = {
},
};
+/*
+ * 'R' and 'C' update notes:
+ * - Under pHyp or KVM, the updatepp path will not set C, thus it *will*
+ * create writeable HPTEs without C set, because the hcall H_PROTECT
+ * that we use in that case will not update C
+ * - The above is however not a problem, because we also don't do that
+ * fancy "no flush" variant of eviction and we use H_REMOVE which will
+ * do the right thing and thus we don't have the race I described earlier
+ *
+ * - Under bare metal, we do have the race, so we need R and C set
+ * - We make sure R is always set and never lost
+ * - C is _PAGE_DIRTY, and *should* always be set for a writeable mapping
+ */
unsigned long htab_convert_pte_flags(unsigned long pteflags)
{
unsigned long rflags = 0;
@@ -186,9 +199,14 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)
rflags |= 0x1;
}
/*
- * Always add "C" bit for perf. Memory coherence is always enabled
+ * We can't allow hardware to update hpte bits. Hence always
+ * set 'R' bit and set 'C' if it is a write fault
+ * Memory coherence is always enabled
*/
- rflags |= HPTE_R_C | HPTE_R_M;
+ rflags |= HPTE_R_R | HPTE_R_M;
+
+ if (pteflags & _PAGE_DIRTY)
+ rflags |= HPTE_R_C;
/*
* Add in WIG bits
*/
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] powerpc/mm/radix: Add missing tlb flush
2016-05-31 6:26 [PATCH 1/3] powerpc/mm/radix: Update LPCR only if it is powernv Aneesh Kumar K.V
2016-05-31 6:26 ` [PATCH 2/3] powerpc/mm/hash: Fix the reference bit update when handling hash fault Aneesh Kumar K.V
@ 2016-05-31 6:26 ` Aneesh Kumar K.V
2016-06-06 0:17 ` [3/3] " Michael Ellerman
2016-06-03 5:38 ` [PATCH 1/3] powerpc/mm/radix: Update LPCR only if it is powernv Balbir Singh
2016-06-06 0:17 ` [1/3] " Michael Ellerman
3 siblings, 1 reply; 7+ messages in thread
From: Aneesh Kumar K.V @ 2016-05-31 6:26 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
This should not have any impact on hash, because hash does tlb
invalidate with every pte update and we don't implement
flush_tlb_* functions for hash. With radix we should make an explicit
call to flush tlb outside pte update.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/mm/pgtable-book3s64.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c
index eb4451144746..670318766545 100644
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@ -33,10 +33,7 @@ int pmdp_set_access_flags(struct vm_area_struct *vma, unsigned long address,
changed = !pmd_same(*(pmdp), entry);
if (changed) {
__ptep_set_access_flags(pmdp_ptep(pmdp), pmd_pte(entry));
- /*
- * Since we are not supporting SW TLB systems, we don't
- * have any thing similar to flush_tlb_page_nohash()
- */
+ flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
}
return changed;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] powerpc/mm/radix: Update LPCR only if it is powernv
2016-05-31 6:26 [PATCH 1/3] powerpc/mm/radix: Update LPCR only if it is powernv Aneesh Kumar K.V
2016-05-31 6:26 ` [PATCH 2/3] powerpc/mm/hash: Fix the reference bit update when handling hash fault Aneesh Kumar K.V
2016-05-31 6:26 ` [PATCH 3/3] powerpc/mm/radix: Add missing tlb flush Aneesh Kumar K.V
@ 2016-06-03 5:38 ` Balbir Singh
2016-06-06 0:17 ` [1/3] " Michael Ellerman
3 siblings, 0 replies; 7+ messages in thread
From: Balbir Singh @ 2016-06-03 5:38 UTC (permalink / raw)
To: linuxppc-dev
On 31/05/16 16:26, Aneesh Kumar K.V wrote:
> LPCR cannot be updated when running in guest mode.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/pgtable-radix.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
> index 18b2c11604fa..c939e6e57a9e 100644
> --- a/arch/powerpc/mm/pgtable-radix.c
> +++ b/arch/powerpc/mm/pgtable-radix.c
> @@ -296,11 +296,6 @@ found:
> void __init radix__early_init_mmu(void)
> {
> unsigned long lpcr;
Move this to
> - /*
> - * setup LPCR UPRT based on mmu_features
> - */
> - lpcr = mfspr(SPRN_LPCR);
> - mtspr(SPRN_LPCR, lpcr | LPCR_UPRT);
>
> #ifdef CONFIG_PPC_64K_PAGES
> /* PAGE_SIZE mappings */
> @@ -343,8 +338,11 @@ void __init radix__early_init_mmu(void)
> __pte_frag_size_shift = H_PTE_FRAG_SIZE_SHIFT;
>
> radix_init_page_sizes();
> - if (!firmware_has_feature(FW_FEATURE_LPAR))
> + if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> unsigned long lpcr;
Here
> + lpcr = mfspr(SPRN_LPCR);
> + mtspr(SPRN_LPCR, lpcr | LPCR_UPRT);
> radix_init_partition_table();
> + }
>
> radix_init_pgtable();
> }
> @@ -353,16 +351,15 @@ void radix__early_init_mmu_secondary(void)
> {
> unsigned long lpcr;
Same as above
> /*
> - * setup LPCR UPRT based on mmu_features
> + * update partition table control register and UPRT
> */
> - lpcr = mfspr(SPRN_LPCR);
> - mtspr(SPRN_LPCR, lpcr | LPCR_UPRT);
> - /*
> - * update partition table control register, 64 K size.
> - */
> - if (!firmware_has_feature(FW_FEATURE_LPAR))
> + if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> + lpcr = mfspr(SPRN_LPCR);
> + mtspr(SPRN_LPCR, lpcr | LPCR_UPRT);
> +
> mtspr(SPRN_PTCR,
> __pa(partition_tb) | (PATB_SIZE_SHIFT - 12));
> + }
> }
>
> void radix__setup_initial_memory_limit(phys_addr_t first_memblock_base,
>
Balbir Singh.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [1/3] powerpc/mm/radix: Update LPCR only if it is powernv
2016-05-31 6:26 [PATCH 1/3] powerpc/mm/radix: Update LPCR only if it is powernv Aneesh Kumar K.V
` (2 preceding siblings ...)
2016-06-03 5:38 ` [PATCH 1/3] powerpc/mm/radix: Update LPCR only if it is powernv Balbir Singh
@ 2016-06-06 0:17 ` Michael Ellerman
3 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2016-06-06 0:17 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, Aneesh Kumar K.V
On Tue, 2016-31-05 at 06:26:29 UTC, "Aneesh Kumar K.V" wrote:
> LPCR cannot be updated when running in guest mode.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/d6c886006c948141f24e84aceb
cheers
^ permalink raw reply [flat|nested] 7+ messages in thread