[PATCH] Fix "ibm,processor-radix-AP-encodings"

The top 3 bits of the lower order byte should contain the
AP encoding, we assume the top 3 bits of the MSB.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---

 - Detected while reviewing Chris Smart's patch to add radix-AP-encoding
   to skiboot
 - Also fixed typo (sift/shift)

 arch/powerpc/mm/pgtable-radix.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index af897d9..d525b0b 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c

@@ -245,10 +245,10 @@ static int __init radix_dt_scan_page_sizes(unsigned long node,
 
 		struct mmu_psize_def *def;
 
-		/* top 3 bit is AP encoding */
-		shift = be32_to_cpu(prop[0]) & ~(0xe << 28);
-		ap = be32_to_cpu(prop[0]) >> 29;
-		pr_info("Page size sift = %d AP=0x%x\n", shift, ap);
+		/* top 3 bits of the lower order byte is AP encoding */
+		shift = be32_to_cpu(prop[0]) & 0x1f;
+		ap = (be32_to_cpu(prop[0]) >> 5) & 0x7;
+		pr_info("Page size shift = %d AP=0x%x\n", shift, ap);
 
 		idx = get_idx_from_shift(shift);
 		if (idx < 0)

-- 
2.5.5

Balbir Singh [off-list ref] writes:

The top 3 bits of the lower order byte should contain the
AP encoding, we assume the top 3 bits of the MSB.

Are you sure, Power architecture documents always confuse about MSB vs
lowe order bytes. ?

-aneesh

On Wed, Sep 28, 2016 at 12:43 PM, Aneesh Kumar K.V
[off-list ref] wrote:

Balbir Singh [off-list ref] writes:

quoted

The top 3 bits of the lower order byte should contain the
AP encoding, we assume the top 3 bits of the MSB.

Balbir, could you reword this so it says "Currently we wrongly assume
<blah>" or similar. The current commit message made me think you were
changing it to look at the top 3 bits of the MSB rather than changing
it look at the LSB.

Are you sure, Power architecture documents always confuse about MSB vs
lowe order bytes. ?

PAPR seems to be pretty consistent about "low order" meaning "least
significant." Additionally the PAPR that describes
ibm,processor-radix-AP-encodings says that it is formatted this way so
it can be used when constructing the register argument to tlbie. The
modes of tlbie that use the AP field place it in bits 56:59 so I think
Balbir's fix is correct.

Reviewed-By: Oliver O'Halloran <oohall@gmail.com>

Oliver O'Halloran [off-list ref] writes:

On Wed, Sep 28, 2016 at 12:43 PM, Aneesh Kumar K.V
[off-list ref] wrote:

quoted

Balbir Singh [off-list ref] writes:

quoted

The top 3 bits of the lower order byte should contain the
AP encoding, we assume the top 3 bits of the MSB.

Balbir, could you reword this so it says "Currently we wrongly assume
<blah>" or similar. The current commit message made me think you were
changing it to look at the top 3 bits of the MSB rather than changing
it look at the LSB.

Yeah please reword it to describe what the current code does, why it's
incorrect, and how we are fixing it.

cheers

Michael Ellerman [off-list ref] writes:

Oliver O'Halloran [off-list ref] writes:

quoted

On Wed, Sep 28, 2016 at 12:43 PM, Aneesh Kumar K.V
[off-list ref] wrote:

quoted

Balbir Singh [off-list ref] writes:

quoted

The top 3 bits of the lower order byte should contain the
AP encoding, we assume the top 3 bits of the MSB.

Balbir, could you reword this so it says "Currently we wrongly assume
<blah>" or similar. The current commit message made me think you were
changing it to look at the top 3 bits of the MSB rather than changing
it look at the LSB.

Yeah please reword it to describe what the current code does, why it's
incorrect, and how we are fixing it.

I have an updated patch for this, which redo the AP-encoding parsing
based on spec update. Wil send it to the list once we finalize the
details

-aneesh