linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Geoff Levand <geoffrey.levand@am.sony.com>
Cc: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>,
	Linux/PPC Development <linuxppc-dev@ozlabs.org>
Subject: Re: PS3 early lock-up
Date: Tue, 05 Aug 2008 11:40:30 +1000	[thread overview]
Message-ID: <1217900430.24157.142.camel@pasglop> (raw)
In-Reply-To: <48979F6F.1050507@am.sony.com>


> > Which should be 0x194.
> 
> That is 0x190.
> 
> 0x194 = _PAGE_EXEC | _PAGE_ACCESSED | _PAGE_DIRTY | _PAGE_COHERENT | PP_RWXX

Right, _PAGE_EXEC should only be set for the part covering the kernel
text. In any case, it shouldn't be what you showed.
 
> > Can you find out where that stupid value comes from ?
> 
> I didn't have time to look at in detail, but it fails from the
> ioremap call in ps3_map_htab (arch/powerpc/platfroms/ps3/htab.c):
> 
>  htab = (__force struct hash_pte *)ioremap_flags(htab_addr, htab_size,
>      pgprot_val(PAGE_READONLY_X));
> 
> IIRC, lv1 doesn't allow a read/write mapping of the htab, and that is
> why I used pgprot_val(PAGE_READONLY_X) here.

Why are you mapping it in the first place btw ? Do you actually use that
mapping ?

> I guess the value returned from pgprot_val(PAGE_READONLY_X)
> changed in recent kernels, and that is what is causing the failure.

Ok, there's definitely something fishy about passing the PP bits down
to ioremap. The reason it fails is that I no longer let _PAGE_USER
go down to the mapping. However, ioremap passes those bits as-is
to the hash insert code, it should instead perform the same munging
as the asm hashing code does, to turn that into a supervisor only
mapping.

However, there is a deeper issue with what you are doing. With using
only 2 PP bits, as is the case with linux, you -cannot- have a supervisor
read-only mapping that isn't also readable by userspace. It's possible
the newer 3 PPP bits encoding, but I don't know if Cell supports it and
linux currently doesn't use it.

That means that currently, your hash table is user readable... oops :-)

(This is a bug with other early IO mappings too btw, I'll have
to fix that).

However, it appears to me that you don't use the mapping of the hash
table anyway. So just remove the ioremap :-) I'll look at fixing
the attribute parsing for ioremap_prot() in the long run though.

Ben.

> Just FYI, I put these in:
> 
>   printk("%s:%d: flags = %x\n", __func__, __LINE__, (_PAGE_ACCESSED | _PAGE_DIRTY | _PAGE_COHERENT | PP_RWXX));
>   printk("%s:%d: flags = %x\n", __func__, __LINE__, pgprot_val(PAGE_READONLY_X));
> 
> and got this (and lv1_write_htab_entry failed):
> 
>   ps3_map_htab:288: flags = 190
>   ps3_map_htab:289: flags = 117
> 
> -Geoff
> 
> 

  reply	other threads:[~2008-08-05  1:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-04 15:48 PS3 early lock-up Geert Uytterhoeven
2008-08-04 21:44 ` Benjamin Herrenschmidt
2008-08-04 21:52   ` Benjamin Herrenschmidt
2008-08-05  0:31     ` Geoff Levand
2008-08-05  1:40       ` Benjamin Herrenschmidt [this message]
2008-08-05  9:43         ` Geert Uytterhoeven
2008-08-05 10:28           ` Benjamin Herrenschmidt
2008-08-05 11:31             ` Benjamin Herrenschmidt
2008-08-05 23:24             ` Geoff Levand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1217900430.24157.142.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=Geert.Uytterhoeven@sonycom.com \
    --cc=geoffrey.levand@am.sony.com \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).