| Message ID | 20190514060205.20887-1-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | powerpc/book3s/mm: Clear MMU_FTR_HPTE_TABLE when radix isenabled. | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 1 warnings, 0 checks, 67 lines checked |
Aneesh Kumar K.V's on May 14, 2019 4:02 pm: > Avoids confusion when printing Oops message like below > > Faulting instruction address: 0xc00000000008bdb4 > Oops: Kernel access of bad area, sig: 11 [#1] > LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV > > Either ibm,pa-features or ibm,powerpc-cpu-features can be used to enable the > MMU features. We don't clear related MMU feature bits there. We use the kernel > commandline to determine what translation mode we want to use and clear the > HPTE or radix bit accordingly. On LPAR we do have to renable HASH bit if the > hypervisor can't do radix. Well we have the HPTE feature: the CPU supports hash MMU mode. It's just the the kernel is booted in radix mode. Could make a difference for KVM, if it will support an HPT guest or not. That's all highly theoretical and we have other inconsistencies already in this stuff, I'd just like to try make things a bit better in the long term. Can we just add an early_radix_enabled() in the oops printing code to select radix or hash MMU? > diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c > index b97aee03924f..0fa6cac3fe82 100644 > --- a/arch/powerpc/mm/pgtable.c > +++ b/arch/powerpc/mm/pgtable.c > @@ -77,9 +77,6 @@ static struct page *maybe_pte_to_page(pte_t pte) > > static pte_t set_pte_filter_hash(pte_t pte) > { > - if (radix_enabled()) > - return pte; > - > pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS); > if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) || > cpu_has_feature(CPU_FTR_NOEXECUTE))) { > @@ -110,6 +107,8 @@ static pte_t set_pte_filter(pte_t pte) > > if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) > return set_pte_filter_hash(pte); > + else if (radix_enabled()) > + return pte; > > /* No exec permission in the first place, move on */ > if (!pte_exec(pte) || !pte_looks_normal(pte)) > @@ -140,7 +139,7 @@ static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma, > { > struct page *pg; > > - if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) > + if (mmu_has_feature(MMU_FTR_HPTE_TABLE) || radix_enabled()) > return pte; > > /* So here, we only care about exec faults, as we use them These would still be good cleanup to make the HPTE_TABLE feature independent from radix. Thanks, Nick
On 5/16/19 10:34 AM, Nicholas Piggin wrote: > Aneesh Kumar K.V's on May 14, 2019 4:02 pm: >> Avoids confusion when printing Oops message like below >> >> Faulting instruction address: 0xc00000000008bdb4 >> Oops: Kernel access of bad area, sig: 11 [#1] >> LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV >> >> Either ibm,pa-features or ibm,powerpc-cpu-features can be used to enable the >> MMU features. We don't clear related MMU feature bits there. We use the kernel >> commandline to determine what translation mode we want to use and clear the >> HPTE or radix bit accordingly. On LPAR we do have to renable HASH bit if the >> hypervisor can't do radix. > > Well we have the HPTE feature: the CPU supports hash MMU mode. It's > just the the kernel is booted in radix mode. > We are not using mmu_features to indicate the capability of the hardware right? ie, mmu_features is an indication of current running config. We set MMU_FTR_TYPE_RADIX if the kernel is running in radix translation mode and on similar lines we should set MMU_FTR_HPTE_TABLE if the kernel is running in only hash translation mode. Whether the hardware support these translation mode is different from which mode is currently used. > Could make a difference for KVM, if it will support an HPT guest or > not. > kvm should not depend on MMU_FTR_HPTE_TABLE to identify whether the hardware supports hash page table translation. I don't think we do that. > That's all highly theoretical and we have other inconsistencies > already in this stuff, I'd just like to try make things a bit better > in the long term. > -aneesh
Aneesh Kumar K.V's on May 16, 2019 11:36 pm: > On 5/16/19 10:34 AM, Nicholas Piggin wrote: >> Aneesh Kumar K.V's on May 14, 2019 4:02 pm: >>> Avoids confusion when printing Oops message like below >>> >>> Faulting instruction address: 0xc00000000008bdb4 >>> Oops: Kernel access of bad area, sig: 11 [#1] >>> LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV >>> >>> Either ibm,pa-features or ibm,powerpc-cpu-features can be used to enable the >>> MMU features. We don't clear related MMU feature bits there. We use the kernel >>> commandline to determine what translation mode we want to use and clear the >>> HPTE or radix bit accordingly. On LPAR we do have to renable HASH bit if the >>> hypervisor can't do radix. >> >> Well we have the HPTE feature: the CPU supports hash MMU mode. It's >> just the the kernel is booted in radix mode. >> > > We are not using mmu_features to indicate the capability of the hardware > right? ie, mmu_features is an indication of current running config. It's kind of both. > We > set MMU_FTR_TYPE_RADIX if the kernel is running in radix translation > mode and on similar lines we should set MMU_FTR_HPTE_TABLE if the kernel > is running in only hash translation mode. Whether the hardware support > these translation mode is different from which mode is currently used. I don't see why that logic follows. We have MMU_FTR_TYPE_RADIX to determine if we are running in radix or HPT mode, why do we need another bit for the same thing? >> Could make a difference for KVM, if it will support an HPT guest or >> not. >> > > kvm should not depend on MMU_FTR_HPTE_TABLE to identify whether the > hardware supports hash page table translation. Why not though? > I don't think we do that. It doesn't, but the point is the bit is kind of useful now (in theory if you wanted to do something like that), but if you just make it an inverse of the current mode bit we already have, then it's useless. Point is, just use the existing radix MMU selection bit that we use everywhere else to fix the problem. If that finishes off the only 64-bit users of the bit and you want to get rid of that as well I'm fine with that too. Thanks, Nick
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c index 45b02fa11cd8..652d954444ba 100644 --- a/arch/powerpc/mm/init_64.c +++ b/arch/powerpc/mm/init_64.c @@ -355,15 +355,18 @@ static void __init early_check_vec5(void) chosen = of_get_flat_dt_subnode_by_name(root, "chosen"); if (chosen == -FDT_ERR_NOTFOUND) { cur_cpu_spec->mmu_features &= ~MMU_FTR_TYPE_RADIX; + cur_cpu_spec->mmu_features |= MMU_FTR_HPTE_TABLE; return; } vec5 = of_get_flat_dt_prop(chosen, "ibm,architecture-vec-5", &size); if (!vec5) { cur_cpu_spec->mmu_features &= ~MMU_FTR_TYPE_RADIX; + cur_cpu_spec->mmu_features |= MMU_FTR_HPTE_TABLE; return; } if (size <= OV5_INDX(OV5_MMU_SUPPORT)) { cur_cpu_spec->mmu_features &= ~MMU_FTR_TYPE_RADIX; + cur_cpu_spec->mmu_features |= MMU_FTR_HPTE_TABLE; return; } @@ -381,17 +384,23 @@ static void __init early_check_vec5(void) } /* Do radix anyway - the hypervisor said we had to */ cur_cpu_spec->mmu_features |= MMU_FTR_TYPE_RADIX; + cur_cpu_spec->mmu_features &= ~MMU_FTR_HPTE_TABLE; } else if (mmu_supported == OV5_FEAT(OV5_MMU_HASH)) { /* Hypervisor only supports hash - disable radix */ cur_cpu_spec->mmu_features &= ~MMU_FTR_TYPE_RADIX; + cur_cpu_spec->mmu_features |= MMU_FTR_HPTE_TABLE; } } void __init mmu_early_init_devtree(void) { /* Disable radix mode based on kernel command line. */ - if (disable_radix) + if (disable_radix) { cur_cpu_spec->mmu_features &= ~MMU_FTR_TYPE_RADIX; + } else { + if (early_radix_enabled()) + cur_cpu_spec->mmu_features &= ~MMU_FTR_HPTE_TABLE; + } /* * Check /chosen/ibm,architecture-vec-5 if running as a guest. diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c index b97aee03924f..0fa6cac3fe82 100644 --- a/arch/powerpc/mm/pgtable.c +++ b/arch/powerpc/mm/pgtable.c @@ -77,9 +77,6 @@ static struct page *maybe_pte_to_page(pte_t pte) static pte_t set_pte_filter_hash(pte_t pte) { - if (radix_enabled()) - return pte; - pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS); if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) || cpu_has_feature(CPU_FTR_NOEXECUTE))) { @@ -110,6 +107,8 @@ static pte_t set_pte_filter(pte_t pte) if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) return set_pte_filter_hash(pte); + else if (radix_enabled()) + return pte; /* No exec permission in the first place, move on */ if (!pte_exec(pte) || !pte_looks_normal(pte)) @@ -140,7 +139,7 @@ static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma, { struct page *pg; - if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) + if (mmu_has_feature(MMU_FTR_HPTE_TABLE) || radix_enabled()) return pte; /* So here, we only care about exec faults, as we use them
Avoids confusion when printing Oops message like below Faulting instruction address: 0xc00000000008bdb4 Oops: Kernel access of bad area, sig: 11 [#1] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV Either ibm,pa-features or ibm,powerpc-cpu-features can be used to enable the MMU features. We don't clear related MMU feature bits there. We use the kernel commandline to determine what translation mode we want to use and clear the HPTE or radix bit accordingly. On LPAR we do have to renable HASH bit if the hypervisor can't do radix. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- arch/powerpc/mm/init_64.c | 11 ++++++++++- arch/powerpc/mm/pgtable.c | 7 +++---- 2 files changed, 13 insertions(+), 5 deletions(-)