[PATCH V3 0/2] powerpc: add support for ISA v2.07 compat level

[PATCH V3 0/2] powerpc: add support for ISA v2.07 compat level

[Suraj Jitindar Singh]

Version v3.00 of the ISA added a new compat level to the processor
compatibility register (PCR), an ISA v2.07 compatibility mode.

Upstream QEMU already supports this so it may as well go into the kernel
now.

Change Log:

V1 -> V2: 
- Reworked logic to set and mask the PCR, no functional change

V2 -> V3:
- Reworked logic again, no functional change

Suraj Jitindar Singh (2):
  powerpc: Define new ISA v3.00 logical PVR value and PCR register value
  powerpc/kvm: Update kvmppc_set_arch_compat() for ISA v3.00

 arch/powerpc/include/asm/reg.h |  3 +++
 arch/powerpc/kvm/book3s_hv.c   | 38 +++++++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 15 deletions(-)

-- 
2.5.5

[PATCH V3 0/2] powerpc: add support for ISA v2.07 compat level

[Suraj Jitindar Singh]

Version v3.00 of the ISA added a new compat level to the processor
compatibility register (PCR), an ISA v2.07 compatibility mode.

Upstream QEMU already supports this so it may as well go into the kernel
now.

Change Log:

V1 -> V2: 
- Reworked logic to set and mask the PCR, no functional change

V2 -> V3:
- Reworked logic again, no functional change

Suraj Jitindar Singh (2):
  powerpc: Define new ISA v3.00 logical PVR value and PCR register value
  powerpc/kvm: Update kvmppc_set_arch_compat() for ISA v3.00

 arch/powerpc/include/asm/reg.h |  3 +++
 arch/powerpc/kvm/book3s_hv.c   | 38 +++++++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 15 deletions(-)

-- 
2.5.5

[PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Suraj Jitindar Singh]

ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition for
this.

Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the processor
compatibility register (PCR). Also define a dummy ISA 3.00 compatibility
mode PCR_ARCH_300 to be used in the next patch to help with determining the
PCR value.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/include/asm/reg.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 9cd4e8c..3fb6192 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -377,6 +377,8 @@
 #define   PCR_VEC_DIS	(1ul << (63-0))	/* Vec. disable (bit NA since POWER8) */
 #define   PCR_VSX_DIS	(1ul << (63-1))	/* VSX disable (bit NA since POWER8) */
 #define   PCR_TM_DIS	(1ul << (63-2))	/* Trans. memory disable (POWER8) */
+#define   PCR_ARCH_300	0x10		/* Dummy Architecture 3.00 */
+#define   PCR_ARCH_207	0x8		/* Architecture 2.07 */
 #define   PCR_ARCH_206	0x4		/* Architecture 2.06 */
 #define   PCR_ARCH_205	0x2		/* Architecture 2.05 */
 #define	SPRN_HEIR	0x153	/* Hypervisor Emulated Instruction Register */
@@ -1218,6 +1220,7 @@
 #define PVR_ARCH_206	0x0f000003
 #define PVR_ARCH_206p	0x0f100003
 #define PVR_ARCH_207	0x0f000004
+#define PVR_ARCH_300	0x0f000005
 
 /* Macros for setting and retrieving special purpose registers */
 #ifndef __ASSEMBLY__
-- 
2.5.5

[PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Suraj Jitindar Singh]

ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition for
this.

Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the processor
compatibility register (PCR). Also define a dummy ISA 3.00 compatibility
mode PCR_ARCH_300 to be used in the next patch to help with determining the
PCR value.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/include/asm/reg.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 9cd4e8c..3fb6192 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -377,6 +377,8 @@
 #define   PCR_VEC_DIS	(1ul << (63-0))	/* Vec. disable (bit NA since POWER8) */
 #define   PCR_VSX_DIS	(1ul << (63-1))	/* VSX disable (bit NA since POWER8) */
 #define   PCR_TM_DIS	(1ul << (63-2))	/* Trans. memory disable (POWER8) */
+#define   PCR_ARCH_300	0x10		/* Dummy Architecture 3.00 */
+#define   PCR_ARCH_207	0x8		/* Architecture 2.07 */
 #define   PCR_ARCH_206	0x4		/* Architecture 2.06 */
 #define   PCR_ARCH_205	0x2		/* Architecture 2.05 */
 #define	SPRN_HEIR	0x153	/* Hypervisor Emulated Instruction Register */
@@ -1218,6 +1220,7 @@
 #define PVR_ARCH_206	0x0f000003
 #define PVR_ARCH_206p	0x0f100003
 #define PVR_ARCH_207	0x0f000004
+#define PVR_ARCH_300	0x0f000005
 
 /* Macros for setting and retrieving special purpose registers */
 #ifndef __ASSEMBLY__
-- 
2.5.5

[PATCH V3 2/2] powerpc/kvm: Update kvmppc_set_arch_compat() for ISA v3.00

[Suraj Jitindar Singh]

The function kvmppc_set_arch_compat() is used to determine the value of the
processor compatibility register (PCR) for a guest running in a given
compatibility mode. There is currently no support for v3.00 of the ISA.

Add support for v3.00 of the ISA which adds an ISA v2.07 compatilibity mode
to the PCR.

We also add a check to ensure the processor we are running on is capable of
emulating the chosen processor (for example a POWER7 cannot emulate a
POWER8, similarly with a POWER8 and a POWER9).

Based on work by: Paul Mackerras <paulus@samba.org>

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/kvm/book3s_hv.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3686471..5d83ecb 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -301,39 +301,47 @@ static void kvmppc_set_pvr_hv(struct kvm_vcpu *vcpu, u32 pvr)
 
 static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
 {
-	unsigned long pcr = 0;
+	unsigned long host_pcr_bit = 0, guest_pcr_bit = 0;
 	struct kvmppc_vcore *vc = vcpu->arch.vcore;
 
+	/* We can (emulate) our own architecture version and anything older */
+	if (cpu_has_feature(CPU_FTR_ARCH_300))
+		host_pcr_bit = PCR_ARCH_300;
+	else if (cpu_has_feature(CPU_FTR_ARCH_207S))
+		host_pcr_bit = PCR_ARCH_207;
+	else if (cpu_has_feature(CPU_FTR_ARCH_206))
+		host_pcr_bit = PCR_ARCH_206;
+	else
+		host_pcr_bit = PCR_ARCH_205;
+
+	/* Determine lowest PCR bit needed to run guest in given PVR level */
 	if (arch_compat) {
 		switch (arch_compat) {
 		case PVR_ARCH_205:
-			/*
-			 * If an arch bit is set in PCR, all the defined
-			 * higher-order arch bits also have to be set.
-			 */
-			pcr = PCR_ARCH_206 | PCR_ARCH_205;
+			guest_pcr_bit = PCR_ARCH_205;
 			break;
 		case PVR_ARCH_206:
 		case PVR_ARCH_206p:
-			pcr = PCR_ARCH_206;
+			guest_pcr_bit = PCR_ARCH_206;
 			break;
 		case PVR_ARCH_207:
+			guest_pcr_bit = PCR_ARCH_207;
+			break;
+		case PVR_ARCH_300:
+			guest_pcr_bit = PCR_ARCH_300;
 			break;
 		default:
 			return -EINVAL;
 		}
-
-		if (!cpu_has_feature(CPU_FTR_ARCH_207S)) {
-			/* POWER7 can't emulate POWER8 */
-			if (!(pcr & PCR_ARCH_206))
-				return -EINVAL;
-			pcr &= ~PCR_ARCH_206;
-		}
 	}
 
+	/* Check requested PCR bits don't exceed our capabilities */
+	if (guest_pcr_bit > host_pcr_bit)
+		return -EINVAL;
+
 	spin_lock(&vc->lock);
 	vc->arch_compat = arch_compat;
-	vc->pcr = pcr;
+	vc->pcr = host_pcr_bit - guest_pcr_bit;
 	spin_unlock(&vc->lock);
 
 	return 0;
-- 
2.5.5

[PATCH V3 2/2] powerpc/kvm: Update kvmppc_set_arch_compat() for ISA v3.00

[Suraj Jitindar Singh]

The function kvmppc_set_arch_compat() is used to determine the value of the
processor compatibility register (PCR) for a guest running in a given
compatibility mode. There is currently no support for v3.00 of the ISA.

Add support for v3.00 of the ISA which adds an ISA v2.07 compatilibity mode
to the PCR.

We also add a check to ensure the processor we are running on is capable of
emulating the chosen processor (for example a POWER7 cannot emulate a
POWER8, similarly with a POWER8 and a POWER9).

Based on work by: Paul Mackerras <paulus@samba.org>

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/kvm/book3s_hv.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3686471..5d83ecb 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -301,39 +301,47 @@ static void kvmppc_set_pvr_hv(struct kvm_vcpu *vcpu, u32 pvr)
 
 static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
 {
-	unsigned long pcr = 0;
+	unsigned long host_pcr_bit = 0, guest_pcr_bit = 0;
 	struct kvmppc_vcore *vc = vcpu->arch.vcore;
 
+	/* We can (emulate) our own architecture version and anything older */
+	if (cpu_has_feature(CPU_FTR_ARCH_300))
+		host_pcr_bit = PCR_ARCH_300;
+	else if (cpu_has_feature(CPU_FTR_ARCH_207S))
+		host_pcr_bit = PCR_ARCH_207;
+	else if (cpu_has_feature(CPU_FTR_ARCH_206))
+		host_pcr_bit = PCR_ARCH_206;
+	else
+		host_pcr_bit = PCR_ARCH_205;
+
+	/* Determine lowest PCR bit needed to run guest in given PVR level */
 	if (arch_compat) {
 		switch (arch_compat) {
 		case PVR_ARCH_205:
-			/*
-			 * If an arch bit is set in PCR, all the defined
-			 * higher-order arch bits also have to be set.
-			 */
-			pcr = PCR_ARCH_206 | PCR_ARCH_205;
+			guest_pcr_bit = PCR_ARCH_205;
 			break;
 		case PVR_ARCH_206:
 		case PVR_ARCH_206p:
-			pcr = PCR_ARCH_206;
+			guest_pcr_bit = PCR_ARCH_206;
 			break;
 		case PVR_ARCH_207:
+			guest_pcr_bit = PCR_ARCH_207;
+			break;
+		case PVR_ARCH_300:
+			guest_pcr_bit = PCR_ARCH_300;
 			break;
 		default:
 			return -EINVAL;
 		}
-
-		if (!cpu_has_feature(CPU_FTR_ARCH_207S)) {
-			/* POWER7 can't emulate POWER8 */
-			if (!(pcr & PCR_ARCH_206))
-				return -EINVAL;
-			pcr &= ~PCR_ARCH_206;
-		}
 	}
 
+	/* Check requested PCR bits don't exceed our capabilities */
+	if (guest_pcr_bit > host_pcr_bit)
+		return -EINVAL;
+
 	spin_lock(&vc->lock);
 	vc->arch_compat = arch_compat;
-	vc->pcr = pcr;
+	vc->pcr = host_pcr_bit - guest_pcr_bit;
 	spin_unlock(&vc->lock);
 
 	return 0;
-- 
2.5.5

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Michael Ellerman]

Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:

> ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition for
> this.
>
> Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the processor
> compatibility register (PCR). Also define a dummy ISA 3.00 compatibility
> mode PCR_ARCH_300 to be used in the next patch to help with determining the
> PCR value.

What's "dummy" about the PCR value?

AFAICS that value is reserved in the ISA.

Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0 ?

cheers

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Michael Ellerman]

Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:

> ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition for
> this.
>
> Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the processor
> compatibility register (PCR). Also define a dummy ISA 3.00 compatibility
> mode PCR_ARCH_300 to be used in the next patch to help with determining the
> PCR value.

What's "dummy" about the PCR value?

AFAICS that value is reserved in the ISA.

Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0 ?

cheers

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Suraj Jitindar Singh]

On Tue, 2016-11-08 at 19:21 +1100, Michael Ellerman wrote:
> Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
> 
> > 
> > ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition
> > for
> > this.
> > 
> > Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the
> > processor
> > compatibility register (PCR). Also define a dummy ISA 3.00
> > compatibility
> > mode PCR_ARCH_300 to be used in the next patch to help with
> > determining the
> > PCR value.
> What's "dummy" about the PCR value?
Then next patch needs some PCR bit to specify that we want to emulate
v3.00 and/or that the host can emulate v3.00 to follow the pattern used
to determine that the host is capable of emulating the given compat
level and for determining which PCR bits to set. But no such bit is
defined for V3.00 compat mode yet so a "dummy" one is used to represent
this even though it's never defined in the ISA.
> 
> AFAICS that value is reserved in the ISA.
Yes it is a reserved bit in the PCR register but it will never actually
be set, it will always be cleared by "host_pcr_bit - guest_pcr_bit;"
> 
> Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0 ?
Basically yes, and although I know nothing's given, it would follow the
current pattern for whatever the next ISA version is to use 0x10 to
mean V3.00 compat mode. Otherwise this will need to be updated at some
point when that's released... In fact if the compat bits are no longer
sequential this will need rewriting.
> 
> cheers

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Suraj Jitindar Singh]

On Tue, 2016-11-08 at 19:21 +1100, Michael Ellerman wrote:
> Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
> 
> > 
> > ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition
> > for
> > this.
> > 
> > Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the
> > processor
> > compatibility register (PCR). Also define a dummy ISA 3.00
> > compatibility
> > mode PCR_ARCH_300 to be used in the next patch to help with
> > determining the
> > PCR value.
> What's "dummy" about the PCR value?
Then next patch needs some PCR bit to specify that we want to emulate
v3.00 and/or that the host can emulate v3.00 to follow the pattern used
to determine that the host is capable of emulating the given compat
level and for determining which PCR bits to set. But no such bit is
defined for V3.00 compat mode yet so a "dummy" one is used to represent
this even though it's never defined in the ISA.
> 
> AFAICS that value is reserved in the ISA.
Yes it is a reserved bit in the PCR register but it will never actually
be set, it will always be cleared by "host_pcr_bit - guest_pcr_bit;"
> 
> Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0 ?
Basically yes, and although I know nothing's given, it would follow the
current pattern for whatever the next ISA version is to use 0x10 to
mean V3.00 compat mode. Otherwise this will need to be updated at some
point when that's released... In fact if the compat bits are no longer
sequential this will need rewriting.
> 
> cheers

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Michael Ellerman]

Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:

> On Tue, 2016-11-08 at 19:21 +1100, Michael Ellerman wrote:
>> Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
>> 
>> > 
>> > ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition
>> > for
>> > this.
>> > 
>> > Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the
>> > processor
>> > compatibility register (PCR). Also define a dummy ISA 3.00
>> > compatibility
>> > mode PCR_ARCH_300 to be used in the next patch to help with
>> > determining the
>> > PCR value.
>> What's "dummy" about the PCR value?

> Then next patch needs some PCR bit to specify that we want to emulate
> v3.00 and/or that the host can emulate v3.00 to follow the pattern used
> to determine that the host is capable of emulating the given compat
> level and for determining which PCR bits to set. But no such bit is
> defined for V3.00 compat mode yet so a "dummy" one is used to represent
> this even though it's never defined in the ISA.
>> 
>> AFAICS that value is reserved in the ISA.

> Yes it is a reserved bit in the PCR register but it will never actually
> be set, it will always be cleared by "host_pcr_bit - guest_pcr_bit;"

>> 
>> Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0 ?

> Basically yes, and although I know nothing's given, it would follow the
> current pattern for whatever the next ISA version is to use 0x10 to
> mean V3.00 compat mode. Otherwise this will need to be updated at some
> point when that's released... In fact if the compat bits are no longer
> sequential this will need rewriting.


OK thanks.

Please send a v4 with that detail in a comment and a better explanation
in the change log.

I think a block comment before the #define would be best, ie. something
like:

#define   PCR_ARCH_207	0x8		/* Architecture 2.07 */

/*
 * All that helpful detail from above ...
 */
#define   PCR_ARCH_300	0x10


We should also ask if we can get 0x10 reserved in the ISA to mean 3.00.

cheers

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Michael Ellerman]

Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:

> On Tue, 2016-11-08 at 19:21 +1100, Michael Ellerman wrote:
>> Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
>> 
>> > 
>> > ISA 3.00 adds the logical PVR value 0x0f000005, so add a definition
>> > for
>> > this.
>> > 
>> > Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in the
>> > processor
>> > compatibility register (PCR). Also define a dummy ISA 3.00
>> > compatibility
>> > mode PCR_ARCH_300 to be used in the next patch to help with
>> > determining the
>> > PCR value.
>> What's "dummy" about the PCR value?

> Then next patch needs some PCR bit to specify that we want to emulate
> v3.00 and/or that the host can emulate v3.00 to follow the pattern used
> to determine that the host is capable of emulating the given compat
> level and for determining which PCR bits to set. But no such bit is
> defined for V3.00 compat mode yet so a "dummy" one is used to represent
> this even though it's never defined in the ISA.
>> 
>> AFAICS that value is reserved in the ISA.

> Yes it is a reserved bit in the PCR register but it will never actually
> be set, it will always be cleared by "host_pcr_bit - guest_pcr_bit;"

>> 
>> Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0 ?

> Basically yes, and although I know nothing's given, it would follow the
> current pattern for whatever the next ISA version is to use 0x10 to
> mean V3.00 compat mode. Otherwise this will need to be updated at some
> point when that's released... In fact if the compat bits are no longer
> sequential this will need rewriting.


OK thanks.

Please send a v4 with that detail in a comment and a better explanation
in the change log.

I think a block comment before the #define would be best, ie. something
like:

#define   PCR_ARCH_207	0x8		/* Architecture 2.07 */

/*
 * All that helpful detail from above ...
 */
#define   PCR_ARCH_300	0x10


We should also ask if we can get 0x10 reserved in the ISA to mean 3.00.

cheers

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Suraj Jitindar Singh]

On Thu, 2016-11-10 at 21:36 +1100, Michael Ellerman wrote:
> Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
> 
> > 
> > On Tue, 2016-11-08 at 19:21 +1100, Michael Ellerman wrote:
> > > 
> > > Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
> > > 
> > > > 
> > > > 
> > > > ISA 3.00 adds the logical PVR value 0x0f000005, so add a
> > > > definition
> > > > for
> > > > this.
> > > > 
> > > > Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in
> > > > the
> > > > processor
> > > > compatibility register (PCR). Also define a dummy ISA 3.00
> > > > compatibility
> > > > mode PCR_ARCH_300 to be used in the next patch to help with
> > > > determining the
> > > > PCR value.
> > > What's "dummy" about the PCR value?
> > 
> > Then next patch needs some PCR bit to specify that we want to
> > emulate
> > v3.00 and/or that the host can emulate v3.00 to follow the pattern
> > used
> > to determine that the host is capable of emulating the given compat
> > level and for determining which PCR bits to set. But no such bit is
> > defined for V3.00 compat mode yet so a "dummy" one is used to
> > represent
> > this even though it's never defined in the ISA.
> > > 
> > > 
> > > AFAICS that value is reserved in the ISA.
> > 
> > Yes it is a reserved bit in the PCR register but it will never
> > actually
> > be set, it will always be cleared by "host_pcr_bit -
> > guest_pcr_bit;"
> > 
> > > 
> > > 
> > > Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0
> > > ?
> > 
> > Basically yes, and although I know nothing's given, it would follow
> > the
> > current pattern for whatever the next ISA version is to use 0x10 to
> > mean V3.00 compat mode. Otherwise this will need to be updated at
> > some
> > point when that's released... In fact if the compat bits are no
> > longer
> > sequential this will need rewriting.
> 
> OK thanks.
> 
> Please send a v4 with that detail in a comment and a better
> explanation
> in the change log.
> 
> I think a block comment before the #define would be best, ie.
> something
Will do and send a V4
> like:
> 
> #define   PCR_ARCH_207	0x8		/* Architecture 2.07
> */
> 
> /*
>  * All that helpful detail from above ...
>  */
> #define   PCR_ARCH_300	0x10
> 
> 
> We should also ask if we can get 0x10 reserved in the ISA to mean
> 3.00.
Probably a good idea, might ask you about the process for this on
Monday...
> 
> cheers

Re: [PATCH V3 1/2] powerpc: Define new ISA v3.00 logical PVR value and PCR register value

[Suraj Jitindar Singh]

On Thu, 2016-11-10 at 21:36 +1100, Michael Ellerman wrote:
> Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
> 
> > 
> > On Tue, 2016-11-08 at 19:21 +1100, Michael Ellerman wrote:
> > > 
> > > Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
> > > 
> > > > 
> > > > 
> > > > ISA 3.00 adds the logical PVR value 0x0f000005, so add a
> > > > definition
> > > > for
> > > > this.
> > > > 
> > > > Define PCR_ARCH_207 to reflect ISA 2.07 compatibility mode in
> > > > the
> > > > processor
> > > > compatibility register (PCR). Also define a dummy ISA 3.00
> > > > compatibility
> > > > mode PCR_ARCH_300 to be used in the next patch to help with
> > > > determining the
> > > > PCR value.
> > > What's "dummy" about the PCR value?
> > 
> > Then next patch needs some PCR bit to specify that we want to
> > emulate
> > v3.00 and/or that the host can emulate v3.00 to follow the pattern
> > used
> > to determine that the host is capable of emulating the given compat
> > level and for determining which PCR bits to set. But no such bit is
> > defined for V3.00 compat mode yet so a "dummy" one is used to
> > represent
> > this even though it's never defined in the ISA.
> > > 
> > > 
> > > AFAICS that value is reserved in the ISA.
> > 
> > Yes it is a reserved bit in the PCR register but it will never
> > actually
> > be set, it will always be cleared by "host_pcr_bit -
> > guest_pcr_bit;"
> > 
> > > 
> > > 
> > > Are we assuming/hoping that ISA 4.0 will use 0x10 to mean ISA 3.0
> > > ?
> > 
> > Basically yes, and although I know nothing's given, it would follow
> > the
> > current pattern for whatever the next ISA version is to use 0x10 to
> > mean V3.00 compat mode. Otherwise this will need to be updated at
> > some
> > point when that's released... In fact if the compat bits are no
> > longer
> > sequential this will need rewriting.
> 
> OK thanks.
> 
> Please send a v4 with that detail in a comment and a better
> explanation
> in the change log.
> 
> I think a block comment before the #define would be best, ie.
> something
Will do and send a V4
> like:
> 
> #define   PCR_ARCH_207	0x8		/* Architecture 2.07
> */
> 
> /*
>  * All that helpful detail from above ...
>  */
> #define   PCR_ARCH_300	0x10
> 
> 
> We should also ask if we can get 0x10 reserved in the ISA to mean
> 3.00.
Probably a good idea, might ask you about the process for this on
Monday...
> 
> cheers