Re: [PATCH] powerpc: Add macros for the ibm_architecture_vec[] lengths

2014-09-14 Thread Michael Ellerman
On Tue, 2014-09-09 at 11:03 +1000, Stewart Smith wrote:
 Michael Ellerman m...@ellerman.id.au writes:
  The encoding of the lengths in the ibm_architecture_vec array is
  interesting to say the least. It's non-obvious how the number of bytes
  we provide relates to the length value.
 
  In fact we already got it wrong once, see 11e9ed43ca8a Fix up
  ibm_architecture_vec definition.
 
  So add some macros to make it (hopefully) clearer. These at least have
  the property that the integer present in the code is equal to the number
  of bytes that follows it.
 
  Signed-off-by: Michael Ellerman m...@ellerman.id.au
 
 Seems at least as correct as the code was before... so,
 
 Reviewed-by: Stewart Smith stew...@linux.vnet.ibm.com
 
 (not actually compiled or tested or anything, but my internal C
 preprocesser says it looks okay :)

Thanks for the review, it's one of those patches it would be easy to get wrong
because it's so simple in theory.

As a test I deliberately broke the NUM_VECTORS value, and .. my system booted
just fine! So I think I'll give this a bit more scrutiny and work out what's
going on there first :)

cheers



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Add macros for the ibm_architecture_vec[] lengths

2014-09-08 Thread Stewart Smith
Michael Ellerman m...@ellerman.id.au writes:
 The encoding of the lengths in the ibm_architecture_vec array is
 interesting to say the least. It's non-obvious how the number of bytes
 we provide relates to the length value.

 In fact we already got it wrong once, see 11e9ed43ca8a Fix up
 ibm_architecture_vec definition.

 So add some macros to make it (hopefully) clearer. These at least have
 the property that the integer present in the code is equal to the number
 of bytes that follows it.

 Signed-off-by: Michael Ellerman m...@ellerman.id.au

Seems at least as correct as the code was before... so,

Reviewed-by: Stewart Smith stew...@linux.vnet.ibm.com

(not actually compiled or tested or anything, but my internal C
preprocesser says it looks okay :)

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: Add macros for the ibm_architecture_vec[] lengths

2014-08-29 Thread Michael Ellerman
The encoding of the lengths in the ibm_architecture_vec array is
interesting to say the least. It's non-obvious how the number of bytes
we provide relates to the length value.

In fact we already got it wrong once, see 11e9ed43ca8a Fix up
ibm_architecture_vec definition.

So add some macros to make it (hopefully) clearer. These at least have
the property that the integer present in the code is equal to the number
of bytes that follows it.

Signed-off-by: Michael Ellerman m...@ellerman.id.au
---
 arch/powerpc/kernel/prom_init.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 1a85d8f96739..962ac852ce0b 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -642,6 +642,15 @@ static void __init early_cmdline_parse(void)
 #define W(x)   ((x)  24)  0xff, ((x)  16)  0xff, \
((x)  8)  0xff, (x)  0xff
 
+/* Firmware expects the value to be n - 1, where n is the # of vectors */
+#define NUM_VECTORS(n) ((n) - 1)
+
+/*
+ * Firmware expects 1 + n - 2, where n is the length of the option vector in
+ * bytes. The 1 accounts for the length byte itself, the - 2 .. ?
+ */
+#define VECTOR_LENGTH(n)   (1 + (n) - 2)
+
 unsigned char ibm_architecture_vec[] = {
W(0xfffe), W(0x003a),   /* POWER5/POWER5+ */
W(0x), W(0x003e),   /* POWER6 */
@@ -652,16 +661,16 @@ unsigned char ibm_architecture_vec[] = {
W(0x), W(0x0f03),   /* all 2.06-compliant */
W(0x), W(0x0f02),   /* all 2.05-compliant */
W(0xfffe), W(0x0f01),   /* all 2.04-compliant and earlier */
-   6 - 1,  /* 6 option vectors */
+   NUM_VECTORS(6), /* 6 option vectors */
 
/* option vector 1: processor architectures supported */
-   3 - 2,  /* length */
+   VECTOR_LENGTH(2),   /* length */
0,  /* don't ignore, don't halt */
OV1_PPC_2_00 | OV1_PPC_2_01 | OV1_PPC_2_02 | OV1_PPC_2_03 |
OV1_PPC_2_04 | OV1_PPC_2_05 | OV1_PPC_2_06 | OV1_PPC_2_07,
 
/* option vector 2: Open Firmware options supported */
-   34 - 2, /* length */
+   VECTOR_LENGTH(33),  /* length */
OV2_REAL_MODE,
0, 0,
W(0x),  /* real_base */
@@ -675,17 +684,17 @@ unsigned char ibm_architecture_vec[] = {
48, /* max log_2(hash table size) */
 
/* option vector 3: processor options supported */
-   3 - 2,  /* length */
+   VECTOR_LENGTH(2),   /* length */
0,  /* don't ignore, don't halt */
OV3_FP | OV3_VMX | OV3_DFP,
 
/* option vector 4: IBM PAPR implementation */
-   3 - 2,  /* length */
+   VECTOR_LENGTH(2),   /* length */
0,  /* don't halt */
OV4_MIN_ENT_CAP,/* minimum VP entitled capacity */
 
/* option vector 5: PAPR/OF options */
-   19 - 2, /* length */
+   VECTOR_LENGTH(18),  /* length */
0,  /* don't ignore, don't halt */
OV5_FEAT(OV5_LPAR) | OV5_FEAT(OV5_SPLPAR) | OV5_FEAT(OV5_LARGE_PAGES) |
OV5_FEAT(OV5_DRCONF_MEMORY) | OV5_FEAT(OV5_DONATE_DEDICATE_CPU) |
@@ -718,12 +727,12 @@ unsigned char ibm_architecture_vec[] = {
OV5_FEAT(OV5_PFO_HW_RNG) | OV5_FEAT(OV5_PFO_HW_ENCR) |
OV5_FEAT(OV5_PFO_HW_842),
OV5_FEAT(OV5_SUB_PROCESSORS),
+
/* option vector 6: IBM PAPR hints */
-   4 - 2,  /* length */
+   VECTOR_LENGTH(3),   /* length */
0,
0,
OV6_LINUX,
-
 };
 
 /* Old method - ELF header with PT_NOTE sections only works on BE */
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev