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

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Sun, 20 Nov 2016 23:33:46 +0530

Hi,

Geoff Levand <[email-masked]> 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 0x8000000000000000 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=7e4fa575c0000 pa=300000003000 ix=bae0 v=7e4fa575c001 r=8000300000003126
>
> 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 & 0xc000300000000000UL) == 0xc000300000000000UL) {
> +		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= c00030000000393c => rflags=8000000000000126
>  htab_convert_pte_flags: bad rflags: pteflags= c00030000000593c => rflags=8000000000000126
>  htab_convert_pte_flags: bad rflags: pteflags= c00030000000793c => rflags=8000000000000126
>  htab_convert_pte_flags: bad rflags: pteflags= c00030000000993c => rflags=8000000000000126
>  htab_convert_pte_flags: bad rflags: pteflags= c00030000000b93c => rflags=8000000000000126
>  htab_convert_pte_flags: bad rflags: pteflags= c00030000000d93c => rflags=8000000000000126
>
> 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