Re: [V2,10/68] powerpc/mm: Update _PAGE_KERNEL_RO

2016-11-23 Thread Geoff Levand 

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.V 
Date:   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

2016-11-23 Thread Aneesh Kumar K.V 
Geoff Levand  writes:

> 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

2016-11-20 Thread Geoff Levand 
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

2016-11-20 Thread Aneesh Kumar K.V 

Hi,

Geoff Levand  writes:

> 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

2016-11-19 Thread Geoff Levand 
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