Re: [V2,10/68] powerpc/mm: Update _PAGE_KERNEL_RO
Hi Aneesh, On 11/23/2016 02:41 AM, Aneesh Kumar K.V wrote: Can you try this patch ? commit 43e05fa840330f0f2deae1e8cc2effd5df68079f Author: Aneesh Kumar K.VDate: Wed Nov 23 15:23:05 2016 +0530 powerpc/mm: Kernel RO fixup for cell I tested your patch with v4.7, v4.8 and v4.9-rc6, and all work OK. -Geoff
RE: [V2,10/68] powerpc/mm: Update _PAGE_KERNEL_RO
Geoff Levandwrites: > Hi Aneesh, > >> --- a/arch/powerpc/platforms/ps3/spu.c >> +++ b/arch/powerpc/platforms/ps3/spu.c >> @@ -205,7 +205,7 @@ static void spu_unmap(struct spu *spu) >> static int __init setup_areas(struct spu *spu) >> { >> struct table {char* name; unsigned long addr; unsigned long size;}; >> -static const unsigned long shadow_flags = _PAGE_NO_CACHE | 3; >> +unsigned long shadow_flags = >> pgprot_val(pgprot_noncached_wc(PAGE_KERNEL_RO)); >> >> spu_pdata(spu)->shadow = __ioremap(spu_pdata(spu)->shadow_addr, >> sizeof(struct spe_shadow), > > This shadow_flags setting doesn't work correctly for PS3. The PS3's LV1 > hypervisor wants the shadow reg pte bits N=1 and PP=11, so (rflags & 7) == 7. > It also doesn't want bit 0x8000 set. So maybe these: > > (HPTE_R_N | HPTE_R_PP & ~HPTE_R_PP0) > > For what its worth, this is the error for v4.8: > > ps3_hpte_insert:result=LV1_ILLEGAL_PARAMETER_VALUE (-17) vpn=7e4fa575c > pa=30003000 ix=bae0 v=7e4fa575c001 r=800030003126 > > I tried different shadow_flags settings to try to get htab_convert_pte_flags() > to give me the right pte bits, but couldn't find anything that would work. > > Here's the only thing I could get to work: > > --- a/arch/powerpc/mm/hash_utils_64.c > +++ b/arch/powerpc/mm/hash_utils_64.c > @@ -222,6 +222,12 @@ unsigned long htab_convert_pte_flags(unsigned long > pteflags) >*/ > rflags |= HPTE_R_M; > > + if ((pteflags & 0xc0003000UL) == 0xc0003000UL) { > + pr_info("%s: bad rflags: pteflags= %lx => rflags=%lx\n", > + __func__, pteflags, rflags); > + return 0x127; > + } > + > return rflags; > } > > And here's the output of that: > > htab_convert_pte_flags: bad rflags: pteflags= c0003000393c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000593c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000793c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000993c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000b93c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000d93c => > rflags=8126 > > Actually, the problem started with (6a119eae942c "powerpc/mm: Add a _PAGE_PTE > bit"), > but I could fix that by using 'shadow_flags = (_PAGE_PRESENT | > _PAGE_NO_CACHE | _PAGE_USER)'. > > Please let me know what I need for shadow_flags to get those pte bits set. Can you try this patch ? commit 43e05fa840330f0f2deae1e8cc2effd5df68079f Author: Aneesh Kumar K.V Date: Wed Nov 23 15:23:05 2016 +0530 powerpc/mm: Kernel RO fixup for cell Signed-off-by: Aneesh Kumar K.V diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index e88368354e49..c13242bf3098 100644 --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h @@ -27,6 +27,11 @@ /* * Individual features below. */ +/* + * kernel read only support + * We added the ppp value 0b110 in ISA 2.04 + */ +#define MMU_FTR_KERNEL_RO ASM_CONST(0x4000) /* * We need to clear top 16bits of va (from the remaining 64 bits )in @@ -103,10 +108,10 @@ #define MMU_FTRS_POWER4MMU_FTRS_DEFAULT_HPTE_ARCH_V2 #define MMU_FTRS_PPC970MMU_FTRS_POWER4 | MMU_FTR_TLBIE_CROP_VA #define MMU_FTRS_POWER5MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE -#define MMU_FTRS_POWER6MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE -#define MMU_FTRS_POWER7MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE -#define MMU_FTRS_POWER8MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE -#define MMU_FTRS_POWER9MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE +#define MMU_FTRS_POWER6MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE | MMU_FTR_KERNEL_RO +#define MMU_FTRS_POWER7MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE | MMU_FTR_KERNEL_RO +#define MMU_FTRS_POWER8MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE | MMU_FTR_KERNEL_RO +#define MMU_FTRS_POWER9MMU_FTRS_POWER4 | MMU_FTR_LOCKLESS_TLBIE | MMU_FTR_KERNEL_RO #define MMU_FTRS_CELL MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | \ MMU_FTR_CI_LARGE_PAGE #define MMU_FTRS_PA6T MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | \ diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c index 5503078090cd..78dabf065ba9 100644 --- a/arch/powerpc/mm/hash_utils_64.c +++ b/arch/powerpc/mm/hash_utils_64.c @@ -193,8 +193,12 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags) /* * Kernel read only mapped with ppp bits 0b110
Re: [V2,10/68] powerpc/mm: Update _PAGE_KERNEL_RO
Hi Aneesh, On 11/20/2016 10:03 AM, Aneesh Kumar K.V wrote: > So I am trying to understand what the PPP bit value should be. I haven't > studied PS3 enough to figure this out myself. So we use the SLB class > value of 0 for the kernel and 1 for user. What is the expected PPP bit > value for the above ioremap range ? Once we clarify that may be we can figure > out > a way to express that in linux page table and later map that correctly > in hash fault. > > Right now we map PPP bits as below: > > kernel RW = 0b000 -> class 1 /user no access > kernel RO = 0b110 -> class 1 /user no access > USER RW = 0b010 -> class 0/kernel RW > USER RO = 0b011 -> class 1/kernel RO I don't quite understand what is meant by these PPP bits and how those relate to the documented PTE bits. As for the PS3's spu shadow registers, they are read-only, and only used within the kernel, actually, only within platforms/ps3/spu.c. Here is a test I did that looped varying rflags. ps3_hpte_insert: rflags=0 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=1 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=2 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=3 result=LV1_SUCCESS (0) ps3_hpte_insert: rflags=4 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=5 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=6 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=7 result=LV1_SUCCESS (0) ps3_hpte_insert: rflags=8 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=9 result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=a result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=b result=LV1_SUCCESS (0) ps3_hpte_insert: rflags=c result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=d result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=e result=LV1_ACCESS_VIOLATION (-5) ps3_hpte_insert: rflags=f result=LV1_SUCCESS (0) Please let me know what other info you need. -Geoff
RE: [V2,10/68] powerpc/mm: Update _PAGE_KERNEL_RO
Hi, Geoff Levandwrites: > Hi Aneesh, > >> --- a/arch/powerpc/platforms/ps3/spu.c >> +++ b/arch/powerpc/platforms/ps3/spu.c >> @@ -205,7 +205,7 @@ static void spu_unmap(struct spu *spu) >> static int __init setup_areas(struct spu *spu) >> { >> struct table {char* name; unsigned long addr; unsigned long size;}; >> -static const unsigned long shadow_flags = _PAGE_NO_CACHE | 3; >> +unsigned long shadow_flags = >> pgprot_val(pgprot_noncached_wc(PAGE_KERNEL_RO)); >> >> spu_pdata(spu)->shadow = __ioremap(spu_pdata(spu)->shadow_addr, >> sizeof(struct spe_shadow), > > This shadow_flags setting doesn't work correctly for PS3. The PS3's LV1 > hypervisor wants the shadow reg pte bits N=1 and PP=11, so (rflags & 7) == 7. > It also doesn't want bit 0x8000 set. So maybe these: Ok so that shadow_flags usage was not really about read only mapping. Sorry for breaking that with the patch. > > (HPTE_R_N | HPTE_R_PP & ~HPTE_R_PP0) > > For what its worth, this is the error for v4.8: > > ps3_hpte_insert:result=LV1_ILLEGAL_PARAMETER_VALUE (-17) vpn=7e4fa575c > pa=30003000 ix=bae0 v=7e4fa575c001 r=800030003126 > > I tried different shadow_flags settings to try to get htab_convert_pte_flags() > to give me the right pte bits, but couldn't find anything that would work. > > Here's the only thing I could get to work: > > --- a/arch/powerpc/mm/hash_utils_64.c > +++ b/arch/powerpc/mm/hash_utils_64.c > @@ -222,6 +222,12 @@ unsigned long htab_convert_pte_flags(unsigned long > pteflags) >*/ > rflags |= HPTE_R_M; > > + if ((pteflags & 0xc0003000UL) == 0xc0003000UL) { > + pr_info("%s: bad rflags: pteflags= %lx => rflags=%lx\n", > + __func__, pteflags, rflags); > + return 0x127; > + } > + > return rflags; > } > > And here's the output of that: > > htab_convert_pte_flags: bad rflags: pteflags= c0003000393c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000593c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000793c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000993c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000b93c => > rflags=8126 > htab_convert_pte_flags: bad rflags: pteflags= c0003000d93c => > rflags=8126 > > Actually, the problem started with (6a119eae942c "powerpc/mm: Add a _PAGE_PTE > bit"), > but I could fix that by using 'shadow_flags = (_PAGE_PRESENT | > _PAGE_NO_CACHE | _PAGE_USER)'. > > Please let me know what I need for shadow_flags to get those pte bits set. > So I am trying to understand what the PPP bit value should be. I haven't studied PS3 enough to figure this out myself. So we use the SLB class value of 0 for the kernel and 1 for user. What is the expected PPP bit value for the above ioremap range ? Once we clarify that may be we can figure out a way to express that in linux page table and later map that correctly in hash fault. Right now we map PPP bits as below: kernel RW = 0b000 -> class 1 /user no access kernel RO = 0b110 -> class 1 /user no access USER RW = 0b010 -> class 0/kernel RW USER RO = 0b011 -> class 1/kernel RO -aneesh
RE: [V2,10/68] powerpc/mm: Update _PAGE_KERNEL_RO
Hi Aneesh, > --- a/arch/powerpc/platforms/ps3/spu.c > +++ b/arch/powerpc/platforms/ps3/spu.c > @@ -205,7 +205,7 @@ static void spu_unmap(struct spu *spu) > static int __init setup_areas(struct spu *spu) > { > struct table {char* name; unsigned long addr; unsigned long size;}; > - static const unsigned long shadow_flags = _PAGE_NO_CACHE | 3; > + unsigned long shadow_flags = > pgprot_val(pgprot_noncached_wc(PAGE_KERNEL_RO)); > > spu_pdata(spu)->shadow = __ioremap(spu_pdata(spu)->shadow_addr, > sizeof(struct spe_shadow), This shadow_flags setting doesn't work correctly for PS3. The PS3's LV1 hypervisor wants the shadow reg pte bits N=1 and PP=11, so (rflags & 7) == 7. It also doesn't want bit 0x8000 set. So maybe these: (HPTE_R_N | HPTE_R_PP & ~HPTE_R_PP0) For what its worth, this is the error for v4.8: ps3_hpte_insert:result=LV1_ILLEGAL_PARAMETER_VALUE (-17) vpn=7e4fa575c pa=30003000 ix=bae0 v=7e4fa575c001 r=800030003126 I tried different shadow_flags settings to try to get htab_convert_pte_flags() to give me the right pte bits, but couldn't find anything that would work. Here's the only thing I could get to work: --- a/arch/powerpc/mm/hash_utils_64.c +++ b/arch/powerpc/mm/hash_utils_64.c @@ -222,6 +222,12 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags) */ rflags |= HPTE_R_M; + if ((pteflags & 0xc0003000UL) == 0xc0003000UL) { + pr_info("%s: bad rflags: pteflags= %lx => rflags=%lx\n", + __func__, pteflags, rflags); + return 0x127; + } + return rflags; } And here's the output of that: htab_convert_pte_flags: bad rflags: pteflags= c0003000393c => rflags=8126 htab_convert_pte_flags: bad rflags: pteflags= c0003000593c => rflags=8126 htab_convert_pte_flags: bad rflags: pteflags= c0003000793c => rflags=8126 htab_convert_pte_flags: bad rflags: pteflags= c0003000993c => rflags=8126 htab_convert_pte_flags: bad rflags: pteflags= c0003000b93c => rflags=8126 htab_convert_pte_flags: bad rflags: pteflags= c0003000d93c => rflags=8126 Actually, the problem started with (6a119eae942c "powerpc/mm: Add a _PAGE_PTE bit"), but I could fix that by using 'shadow_flags = (_PAGE_PRESENT | _PAGE_NO_CACHE | _PAGE_USER)'. Please let me know what I need for shadow_flags to get those pte bits set. -Geoff