* [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-06-06 12:16 [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
@ 2024-06-06 12:16 ` Aditya Gupta
2024-07-23 4:21 ` Nicholas Piggin
2024-07-23 5:22 ` Nicholas Piggin
2024-06-06 12:16 ` [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor Aditya Gupta
` (5 subsequent siblings)
6 siblings, 2 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-06-06 12:16 UTC (permalink / raw)
To: Mahesh J Salgaonkar, Madhavan Srinivasan, Nicholas Piggin,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc
From: Harsh Prateek Bora <harshpb@linux.ibm.com>
Power9/10 initialization code consists of a lot of logical OR of
various flag bits as supported by respective Power platform during its
initialization, most of which is duplicated and only selected bits are
added or removed as needed with each new platform support being added.
Remove the duplicate code and share using common macros.
Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
target/ppc/cpu_init.c | 124 +++++-------------------------------------
target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
2 files changed, 93 insertions(+), 109 deletions(-)
create mode 100644 target/ppc/cpu_init.h
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 01e358a4a5ac..3d8a112935ae 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -51,6 +51,7 @@
#include "kvm_ppc.h"
#endif
+#include "cpu_init.h"
/* #define PPC_DEBUG_SPR */
/* #define USE_APPLE_GDB */
@@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER9";
dc->desc = "POWER9";
pcc->pvr_match = ppc_pvr_match_power9;
- pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
- pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
- PCR_COMPAT_2_05;
+ pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
+ pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
pcc->init_proc = init_proc_POWER9;
pcc->check_pow = check_pow_nocheck;
pcc->check_attn = check_attn_hid0_power9;
- pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
- PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
- PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
- PPC_FLOAT_FRSQRTES |
- PPC_FLOAT_STFIWX |
- PPC_FLOAT_EXT |
- PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
- PPC_MEM_SYNC | PPC_MEM_EIEIO |
- PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
- PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
- PPC_SEGMENT_64B | PPC_SLBI |
- PPC_POPCNTB | PPC_POPCNTWD |
- PPC_CILDST;
- pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
- PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
- PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
- PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
- PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
- PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
- PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_MEM_LWSYNC |
- PPC2_BCDA_ISA206;
- pcc->msr_mask = (1ull << MSR_SF) |
- (1ull << MSR_HV) |
- (1ull << MSR_TM) |
- (1ull << MSR_VR) |
- (1ull << MSR_VSX) |
- (1ull << MSR_EE) |
- (1ull << MSR_PR) |
- (1ull << MSR_FP) |
- (1ull << MSR_ME) |
- (1ull << MSR_FE0) |
- (1ull << MSR_SE) |
- (1ull << MSR_DE) |
- (1ull << MSR_FE1) |
- (1ull << MSR_IR) |
- (1ull << MSR_DR) |
- (1ull << MSR_PMM) |
- (1ull << MSR_RI) |
- (1ull << MSR_LE);
- pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
- (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
- LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
- (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
- LPCR_DEE | LPCR_OEE))
- | LPCR_MER | LPCR_GTSE | LPCR_TC |
- LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
+ pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
+ pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
+ pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
+ pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
pcc->mmu_model = POWERPC_MMU_3_00;
#if !defined(CONFIG_USER_ONLY)
@@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
pcc->excp_model = POWERPC_EXCP_POWER9;
pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
pcc->bfd_mach = bfd_mach_ppc64;
- pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
- POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
- POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
- POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
+ pcc->flags = POWERPC_POWER9_PCC_FLAGS;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
}
@@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER10";
dc->desc = "POWER10";
pcc->pvr_match = ppc_pvr_match_power10;
- pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
- PCR_COMPAT_3_00;
- pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
- PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
+ pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
+ pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
pcc->init_proc = init_proc_POWER10;
pcc->check_pow = check_pow_nocheck;
pcc->check_attn = check_attn_hid0_power9;
- pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
- PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
- PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
- PPC_FLOAT_FRSQRTES |
- PPC_FLOAT_STFIWX |
- PPC_FLOAT_EXT |
- PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
- PPC_MEM_SYNC | PPC_MEM_EIEIO |
- PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
- PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
- PPC_SEGMENT_64B | PPC_SLBI |
- PPC_POPCNTB | PPC_POPCNTWD |
- PPC_CILDST;
- pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
- PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
- PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
- PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
- PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
- PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
- PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
- PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
- pcc->msr_mask = (1ull << MSR_SF) |
- (1ull << MSR_HV) |
- (1ull << MSR_VR) |
- (1ull << MSR_VSX) |
- (1ull << MSR_EE) |
- (1ull << MSR_PR) |
- (1ull << MSR_FP) |
- (1ull << MSR_ME) |
- (1ull << MSR_FE0) |
- (1ull << MSR_SE) |
- (1ull << MSR_DE) |
- (1ull << MSR_FE1) |
- (1ull << MSR_IR) |
- (1ull << MSR_DR) |
- (1ull << MSR_PMM) |
- (1ull << MSR_RI) |
- (1ull << MSR_LE);
- pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
- (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
- LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
- (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
- LPCR_DEE | LPCR_OEE))
- | LPCR_MER | LPCR_GTSE | LPCR_TC |
- LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
- /* DD2 adds an extra HAIL bit */
- pcc->lpcr_mask |= LPCR_HAIL;
+ pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
+ pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
+ pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
+ pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
pcc->mmu_model = POWERPC_MMU_3_00;
@@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
pcc->excp_model = POWERPC_EXCP_POWER10;
pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
pcc->bfd_mach = bfd_mach_ppc64;
- pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
- POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
- POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
- POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
- POWERPC_FLAG_BHRB;
+ pcc->flags = POWERPC_POWER10_PCC_FLAGS;
pcc->l1_dcache_size = 0x8000;
pcc->l1_icache_size = 0x8000;
}
diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
new file mode 100644
index 000000000000..e04be6a655d8
--- /dev/null
+++ b/target/ppc/cpu_init.h
@@ -0,0 +1,78 @@
+#ifndef TARGET_PPC_CPU_INIT_H
+#define TARGET_PPC_CPU_INIT_H
+
+#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
+ PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
+ PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
+ PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
+ PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
+ PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
+ PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
+ PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
+ PPC_CILDST
+
+#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
+ PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
+ PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
+ PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
+ PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
+ PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
+ PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
+
+#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
+ POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
+#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
+ POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
+
+#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
+ (1ull << MSR_SF) | \
+ (1ull << MSR_HV) | \
+ (1ull << MSR_VR) | \
+ (1ull << MSR_VSX) | \
+ (1ull << MSR_EE) | \
+ (1ull << MSR_PR) | \
+ (1ull << MSR_FP) | \
+ (1ull << MSR_ME) | \
+ (1ull << MSR_FE0) | \
+ (1ull << MSR_SE) | \
+ (1ull << MSR_DE) | \
+ (1ull << MSR_FE1) | \
+ (1ull << MSR_IR) | \
+ (1ull << MSR_DR) | \
+ (1ull << MSR_PMM) | \
+ (1ull << MSR_RI) | \
+ (1ull << MSR_LE)
+
+#define POWERPC_POWER9_PCC_MSR_MASK \
+ POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
+#define POWERPC_POWER10_PCC_MSR_MASK \
+ POWERPC_POWER9_COMMON_PCC_MSR_MASK
+#define POWERPC_POWER9_PCC_PCR_MASK \
+ PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
+#define POWERPC_POWER10_PCC_PCR_MASK \
+ POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
+#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
+ PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
+#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
+ POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
+#define POWERPC_POWER9_PCC_LPCR_MASK \
+ LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
+ (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
+ LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
+ (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
+ LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
+ LPCR_HDICE
+/* DD2 adds an extra HAIL bit */
+#define POWERPC_POWER10_PCC_LPCR_MASK \
+ POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
+#define POWERPC_POWER9_PCC_FLAGS_COMMON \
+ POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
+ POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
+ POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
+
+#define POWERPC_POWER9_PCC_FLAGS \
+ POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
+#define POWERPC_POWER10_PCC_FLAGS \
+ POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
+
+#endif /* TARGET_PPC_CPU_INIT_H */
--
2.45.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-06-06 12:16 ` [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code Aditya Gupta
@ 2024-07-23 4:21 ` Nicholas Piggin
2024-07-23 5:02 ` Aditya Gupta
2024-07-23 5:22 ` Nicholas Piggin
1 sibling, 1 reply; 24+ messages in thread
From: Nicholas Piggin @ 2024-07-23 4:21 UTC (permalink / raw)
To: Aditya Gupta, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc
On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
> From: Harsh Prateek Bora <harshpb@linux.ibm.com>
>
> Power9/10 initialization code consists of a lot of logical OR of
> various flag bits as supported by respective Power platform during its
> initialization, most of which is duplicated and only selected bits are
> added or removed as needed with each new platform support being added.
> Remove the duplicate code and share using common macros.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/cpu_init.c | 124 +++++-------------------------------------
> target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
> 2 files changed, 93 insertions(+), 109 deletions(-)
> create mode 100644 target/ppc/cpu_init.h
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 01e358a4a5ac..3d8a112935ae 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -51,6 +51,7 @@
> #include "kvm_ppc.h"
> #endif
>
> +#include "cpu_init.h"
> /* #define PPC_DEBUG_SPR */
> /* #define USE_APPLE_GDB */
>
> @@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> dc->fw_name = "PowerPC,POWER9";
> dc->desc = "POWER9";
> pcc->pvr_match = ppc_pvr_match_power9;
> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
> - pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
> - PCR_COMPAT_2_05;
> + pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
> + pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
> pcc->init_proc = init_proc_POWER9;
> pcc->check_pow = check_pow_nocheck;
> pcc->check_attn = check_attn_hid0_power9;
> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
> - PPC_FLOAT_FRSQRTES |
> - PPC_FLOAT_STFIWX |
> - PPC_FLOAT_EXT |
> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
> - PPC_SEGMENT_64B | PPC_SLBI |
> - PPC_POPCNTB | PPC_POPCNTWD |
> - PPC_CILDST;
> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
> - PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_MEM_LWSYNC |
> - PPC2_BCDA_ISA206;
> - pcc->msr_mask = (1ull << MSR_SF) |
> - (1ull << MSR_HV) |
> - (1ull << MSR_TM) |
> - (1ull << MSR_VR) |
> - (1ull << MSR_VSX) |
> - (1ull << MSR_EE) |
> - (1ull << MSR_PR) |
> - (1ull << MSR_FP) |
> - (1ull << MSR_ME) |
> - (1ull << MSR_FE0) |
> - (1ull << MSR_SE) |
> - (1ull << MSR_DE) |
> - (1ull << MSR_FE1) |
> - (1ull << MSR_IR) |
> - (1ull << MSR_DR) |
> - (1ull << MSR_PMM) |
> - (1ull << MSR_RI) |
> - (1ull << MSR_LE);
> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
> - LPCR_DEE | LPCR_OEE))
> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
> + pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
> + pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
> + pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
> pcc->mmu_model = POWERPC_MMU_3_00;
> #if !defined(CONFIG_USER_ONLY)
> @@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> pcc->excp_model = POWERPC_EXCP_POWER9;
> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
> pcc->bfd_mach = bfd_mach_ppc64;
> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
> - POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
> + pcc->flags = POWERPC_POWER9_PCC_FLAGS;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> }
> @@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> dc->fw_name = "PowerPC,POWER10";
> dc->desc = "POWER10";
> pcc->pvr_match = ppc_pvr_match_power10;
> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
> - PCR_COMPAT_3_00;
> - pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
> - PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
> pcc->init_proc = init_proc_POWER10;
> pcc->check_pow = check_pow_nocheck;
> pcc->check_attn = check_attn_hid0_power9;
> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
> - PPC_FLOAT_FRSQRTES |
> - PPC_FLOAT_STFIWX |
> - PPC_FLOAT_EXT |
> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
> - PPC_SEGMENT_64B | PPC_SLBI |
> - PPC_POPCNTB | PPC_POPCNTWD |
> - PPC_CILDST;
> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
> - PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
> - PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
> - pcc->msr_mask = (1ull << MSR_SF) |
> - (1ull << MSR_HV) |
> - (1ull << MSR_VR) |
> - (1ull << MSR_VSX) |
> - (1ull << MSR_EE) |
> - (1ull << MSR_PR) |
> - (1ull << MSR_FP) |
> - (1ull << MSR_ME) |
> - (1ull << MSR_FE0) |
> - (1ull << MSR_SE) |
> - (1ull << MSR_DE) |
> - (1ull << MSR_FE1) |
> - (1ull << MSR_IR) |
> - (1ull << MSR_DR) |
> - (1ull << MSR_PMM) |
> - (1ull << MSR_RI) |
> - (1ull << MSR_LE);
> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
> - LPCR_DEE | LPCR_OEE))
> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
> - /* DD2 adds an extra HAIL bit */
> - pcc->lpcr_mask |= LPCR_HAIL;
> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
>
> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
> pcc->mmu_model = POWERPC_MMU_3_00;
> @@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> pcc->excp_model = POWERPC_EXCP_POWER10;
> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
> pcc->bfd_mach = bfd_mach_ppc64;
> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
> - POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
> - POWERPC_FLAG_BHRB;
> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> }
> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
> new file mode 100644
> index 000000000000..e04be6a655d8
> --- /dev/null
> +++ b/target/ppc/cpu_init.h
> @@ -0,0 +1,78 @@
> +#ifndef TARGET_PPC_CPU_INIT_H
> +#define TARGET_PPC_CPU_INIT_H
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
> + PPC_CILDST
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
> +
> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
> + (1ull << MSR_SF) | \
> + (1ull << MSR_HV) | \
> + (1ull << MSR_VR) | \
> + (1ull << MSR_VSX) | \
> + (1ull << MSR_EE) | \
> + (1ull << MSR_PR) | \
> + (1ull << MSR_FP) | \
> + (1ull << MSR_ME) | \
> + (1ull << MSR_FE0) | \
> + (1ull << MSR_SE) | \
> + (1ull << MSR_DE) | \
> + (1ull << MSR_FE1) | \
> + (1ull << MSR_IR) | \
> + (1ull << MSR_DR) | \
> + (1ull << MSR_PMM) | \
> + (1ull << MSR_RI) | \
> + (1ull << MSR_LE)
> +
> +#define POWERPC_POWER9_PCC_MSR_MASK \
> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
> +#define POWERPC_POWER10_PCC_MSR_MASK \
> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
> +#define POWERPC_POWER9_PCC_PCR_MASK \
> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
> +#define POWERPC_POWER10_PCC_PCR_MASK \
> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
> +#define POWERPC_POWER9_PCC_LPCR_MASK \
> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
> + LPCR_HDICE
> +/* DD2 adds an extra HAIL bit */
> +#define POWERPC_POWER10_PCC_LPCR_MASK \
> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
> +
> +#define POWERPC_POWER9_PCC_FLAGS \
> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
> +#define POWERPC_POWER10_PCC_FLAGS \
> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
> +
> +#endif /* TARGET_PPC_CPU_INIT_H */
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-07-23 4:21 ` Nicholas Piggin
@ 2024-07-23 5:02 ` Aditya Gupta
0 siblings, 0 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-07-23 5:02 UTC (permalink / raw)
To: Nicholas Piggin, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc
On 23/07/24 09:51, Nicholas Piggin wrote:
> On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
>> From: Harsh Prateek Bora <harshpb@linux.ibm.com>
>>
>> Power9/10 initialization code consists of a lot of logical OR of
>> various flag bits as supported by respective Power platform during its
>> initialization, most of which is duplicated and only selected bits are
>> added or removed as needed with each new platform support being added.
>> Remove the duplicate code and share using common macros.
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks for the reviewed-by, Nick !
- Aditya Gupta
>
>> ---
>> target/ppc/cpu_init.c | 124 +++++-------------------------------------
>> target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
>> 2 files changed, 93 insertions(+), 109 deletions(-)
>> create mode 100644 target/ppc/cpu_init.h
>>
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 01e358a4a5ac..3d8a112935ae 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -51,6 +51,7 @@
>> #include "kvm_ppc.h"
>> #endif
>>
>> +#include "cpu_init.h"
>> /* #define PPC_DEBUG_SPR */
>> /* #define USE_APPLE_GDB */
>>
>> @@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> dc->fw_name = "PowerPC,POWER9";
>> dc->desc = "POWER9";
>> pcc->pvr_match = ppc_pvr_match_power9;
>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
>> - pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
>> - PCR_COMPAT_2_05;
>> + pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
>> + pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
>> pcc->init_proc = init_proc_POWER9;
>> pcc->check_pow = check_pow_nocheck;
>> pcc->check_attn = check_attn_hid0_power9;
>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>> - PPC_FLOAT_FRSQRTES |
>> - PPC_FLOAT_STFIWX |
>> - PPC_FLOAT_EXT |
>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>> - PPC_SEGMENT_64B | PPC_SLBI |
>> - PPC_POPCNTB | PPC_POPCNTWD |
>> - PPC_CILDST;
>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> - PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_MEM_LWSYNC |
>> - PPC2_BCDA_ISA206;
>> - pcc->msr_mask = (1ull << MSR_SF) |
>> - (1ull << MSR_HV) |
>> - (1ull << MSR_TM) |
>> - (1ull << MSR_VR) |
>> - (1ull << MSR_VSX) |
>> - (1ull << MSR_EE) |
>> - (1ull << MSR_PR) |
>> - (1ull << MSR_FP) |
>> - (1ull << MSR_ME) |
>> - (1ull << MSR_FE0) |
>> - (1ull << MSR_SE) |
>> - (1ull << MSR_DE) |
>> - (1ull << MSR_FE1) |
>> - (1ull << MSR_IR) |
>> - (1ull << MSR_DR) |
>> - (1ull << MSR_PMM) |
>> - (1ull << MSR_RI) |
>> - (1ull << MSR_LE);
>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>> - LPCR_DEE | LPCR_OEE))
>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
>> + pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
>> + pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>> pcc->mmu_model = POWERPC_MMU_3_00;
>> #if !defined(CONFIG_USER_ONLY)
>> @@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> pcc->excp_model = POWERPC_EXCP_POWER9;
>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> pcc->bfd_mach = bfd_mach_ppc64;
>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>> - POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
>> + pcc->flags = POWERPC_POWER9_PCC_FLAGS;
>> pcc->l1_dcache_size = 0x8000;
>> pcc->l1_icache_size = 0x8000;
>> }
>> @@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>> dc->fw_name = "PowerPC,POWER10";
>> dc->desc = "POWER10";
>> pcc->pvr_match = ppc_pvr_match_power10;
>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
>> - PCR_COMPAT_3_00;
>> - pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
>> - PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
>> pcc->init_proc = init_proc_POWER10;
>> pcc->check_pow = check_pow_nocheck;
>> pcc->check_attn = check_attn_hid0_power9;
>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>> - PPC_FLOAT_FRSQRTES |
>> - PPC_FLOAT_STFIWX |
>> - PPC_FLOAT_EXT |
>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>> - PPC_SEGMENT_64B | PPC_SLBI |
>> - PPC_POPCNTB | PPC_POPCNTWD |
>> - PPC_CILDST;
>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> - PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
>> - PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
>> - pcc->msr_mask = (1ull << MSR_SF) |
>> - (1ull << MSR_HV) |
>> - (1ull << MSR_VR) |
>> - (1ull << MSR_VSX) |
>> - (1ull << MSR_EE) |
>> - (1ull << MSR_PR) |
>> - (1ull << MSR_FP) |
>> - (1ull << MSR_ME) |
>> - (1ull << MSR_FE0) |
>> - (1ull << MSR_SE) |
>> - (1ull << MSR_DE) |
>> - (1ull << MSR_FE1) |
>> - (1ull << MSR_IR) |
>> - (1ull << MSR_DR) |
>> - (1ull << MSR_PMM) |
>> - (1ull << MSR_RI) |
>> - (1ull << MSR_LE);
>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>> - LPCR_DEE | LPCR_OEE))
>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>> - /* DD2 adds an extra HAIL bit */
>> - pcc->lpcr_mask |= LPCR_HAIL;
>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
>> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
>> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
>>
>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>> pcc->mmu_model = POWERPC_MMU_3_00;
>> @@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>> pcc->excp_model = POWERPC_EXCP_POWER10;
>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> pcc->bfd_mach = bfd_mach_ppc64;
>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>> - POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
>> - POWERPC_FLAG_BHRB;
>> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
>> pcc->l1_dcache_size = 0x8000;
>> pcc->l1_icache_size = 0x8000;
>> }
>> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
>> new file mode 100644
>> index 000000000000..e04be6a655d8
>> --- /dev/null
>> +++ b/target/ppc/cpu_init.h
>> @@ -0,0 +1,78 @@
>> +#ifndef TARGET_PPC_CPU_INIT_H
>> +#define TARGET_PPC_CPU_INIT_H
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
>> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
>> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
>> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
>> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
>> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
>> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
>> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
>> + PPC_CILDST
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
>> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
>> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
>> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
>> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
>> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
>> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
>> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
>> +
>> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
>> + (1ull << MSR_SF) | \
>> + (1ull << MSR_HV) | \
>> + (1ull << MSR_VR) | \
>> + (1ull << MSR_VSX) | \
>> + (1ull << MSR_EE) | \
>> + (1ull << MSR_PR) | \
>> + (1ull << MSR_FP) | \
>> + (1ull << MSR_ME) | \
>> + (1ull << MSR_FE0) | \
>> + (1ull << MSR_SE) | \
>> + (1ull << MSR_DE) | \
>> + (1ull << MSR_FE1) | \
>> + (1ull << MSR_IR) | \
>> + (1ull << MSR_DR) | \
>> + (1ull << MSR_PMM) | \
>> + (1ull << MSR_RI) | \
>> + (1ull << MSR_LE)
>> +
>> +#define POWERPC_POWER9_PCC_MSR_MASK \
>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
>> +#define POWERPC_POWER10_PCC_MSR_MASK \
>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
>> +#define POWERPC_POWER9_PCC_PCR_MASK \
>> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
>> +#define POWERPC_POWER10_PCC_PCR_MASK \
>> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
>> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
>> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
>> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
>> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
>> +#define POWERPC_POWER9_PCC_LPCR_MASK \
>> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
>> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
>> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
>> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
>> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
>> + LPCR_HDICE
>> +/* DD2 adds an extra HAIL bit */
>> +#define POWERPC_POWER10_PCC_LPCR_MASK \
>> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
>> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
>> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
>> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
>> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
>> +
>> +#define POWERPC_POWER9_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
>> +#define POWERPC_POWER10_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
>> +
>> +#endif /* TARGET_PPC_CPU_INIT_H */
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-06-06 12:16 ` [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code Aditya Gupta
2024-07-23 4:21 ` Nicholas Piggin
@ 2024-07-23 5:22 ` Nicholas Piggin
2024-07-23 15:13 ` Aditya Gupta
` (2 more replies)
1 sibling, 3 replies; 24+ messages in thread
From: Nicholas Piggin @ 2024-07-23 5:22 UTC (permalink / raw)
To: Aditya Gupta, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc
On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
> From: Harsh Prateek Bora <harshpb@linux.ibm.com>
>
> Power9/10 initialization code consists of a lot of logical OR of
> various flag bits as supported by respective Power platform during its
> initialization, most of which is duplicated and only selected bits are
> added or removed as needed with each new platform support being added.
> Remove the duplicate code and share using common macros.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> target/ppc/cpu_init.c | 124 +++++-------------------------------------
> target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
> 2 files changed, 93 insertions(+), 109 deletions(-)
> create mode 100644 target/ppc/cpu_init.h
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 01e358a4a5ac..3d8a112935ae 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -51,6 +51,7 @@
> #include "kvm_ppc.h"
> #endif
>
> +#include "cpu_init.h"
> /* #define PPC_DEBUG_SPR */
> /* #define USE_APPLE_GDB */
>
> @@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> dc->fw_name = "PowerPC,POWER9";
> dc->desc = "POWER9";
> pcc->pvr_match = ppc_pvr_match_power9;
> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
> - pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
> - PCR_COMPAT_2_05;
> + pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
> + pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
> pcc->init_proc = init_proc_POWER9;
> pcc->check_pow = check_pow_nocheck;
> pcc->check_attn = check_attn_hid0_power9;
> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
> - PPC_FLOAT_FRSQRTES |
> - PPC_FLOAT_STFIWX |
> - PPC_FLOAT_EXT |
> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
> - PPC_SEGMENT_64B | PPC_SLBI |
> - PPC_POPCNTB | PPC_POPCNTWD |
> - PPC_CILDST;
> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
> - PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_MEM_LWSYNC |
> - PPC2_BCDA_ISA206;
> - pcc->msr_mask = (1ull << MSR_SF) |
> - (1ull << MSR_HV) |
> - (1ull << MSR_TM) |
> - (1ull << MSR_VR) |
> - (1ull << MSR_VSX) |
> - (1ull << MSR_EE) |
> - (1ull << MSR_PR) |
> - (1ull << MSR_FP) |
> - (1ull << MSR_ME) |
> - (1ull << MSR_FE0) |
> - (1ull << MSR_SE) |
> - (1ull << MSR_DE) |
> - (1ull << MSR_FE1) |
> - (1ull << MSR_IR) |
> - (1ull << MSR_DR) |
> - (1ull << MSR_PMM) |
> - (1ull << MSR_RI) |
> - (1ull << MSR_LE);
> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
> - LPCR_DEE | LPCR_OEE))
> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
> + pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
> + pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
> + pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
> pcc->mmu_model = POWERPC_MMU_3_00;
> #if !defined(CONFIG_USER_ONLY)
> @@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> pcc->excp_model = POWERPC_EXCP_POWER9;
> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
> pcc->bfd_mach = bfd_mach_ppc64;
> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
> - POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
> + pcc->flags = POWERPC_POWER9_PCC_FLAGS;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> }
> @@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> dc->fw_name = "PowerPC,POWER10";
> dc->desc = "POWER10";
> pcc->pvr_match = ppc_pvr_match_power10;
> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
> - PCR_COMPAT_3_00;
> - pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
> - PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
> pcc->init_proc = init_proc_POWER10;
> pcc->check_pow = check_pow_nocheck;
> pcc->check_attn = check_attn_hid0_power9;
> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
> - PPC_FLOAT_FRSQRTES |
> - PPC_FLOAT_STFIWX |
> - PPC_FLOAT_EXT |
> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
> - PPC_SEGMENT_64B | PPC_SLBI |
> - PPC_POPCNTB | PPC_POPCNTWD |
> - PPC_CILDST;
> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
> - PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
> - PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
> - pcc->msr_mask = (1ull << MSR_SF) |
> - (1ull << MSR_HV) |
> - (1ull << MSR_VR) |
> - (1ull << MSR_VSX) |
> - (1ull << MSR_EE) |
> - (1ull << MSR_PR) |
> - (1ull << MSR_FP) |
> - (1ull << MSR_ME) |
> - (1ull << MSR_FE0) |
> - (1ull << MSR_SE) |
> - (1ull << MSR_DE) |
> - (1ull << MSR_FE1) |
> - (1ull << MSR_IR) |
> - (1ull << MSR_DR) |
> - (1ull << MSR_PMM) |
> - (1ull << MSR_RI) |
> - (1ull << MSR_LE);
> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
> - LPCR_DEE | LPCR_OEE))
> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
> - /* DD2 adds an extra HAIL bit */
> - pcc->lpcr_mask |= LPCR_HAIL;
> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
>
> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
> pcc->mmu_model = POWERPC_MMU_3_00;
> @@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> pcc->excp_model = POWERPC_EXCP_POWER10;
> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
> pcc->bfd_mach = bfd_mach_ppc64;
> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
> - POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
> - POWERPC_FLAG_BHRB;
> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
> pcc->l1_dcache_size = 0x8000;
> pcc->l1_icache_size = 0x8000;
> }
> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
> new file mode 100644
> index 000000000000..e04be6a655d8
> --- /dev/null
> +++ b/target/ppc/cpu_init.h
> @@ -0,0 +1,78 @@
> +#ifndef TARGET_PPC_CPU_INIT_H
> +#define TARGET_PPC_CPU_INIT_H
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
I would call this PPC_INSNS_FLAGS_POWER9
> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
> + PPC_CILDST
Add this here
#define PPC_INSNS_FLAGS_POWER10 PPC_INSNS_FLAGS_POWER9
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
Suggest some other name change -
PPC_INSNS_FLAGS2_POWER_COMMON
> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
> +
> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
> +
> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
PPC_MSR_MASK_POWER_COMMON
> + (1ull << MSR_SF) | \
> + (1ull << MSR_HV) | \
> + (1ull << MSR_VR) | \
> + (1ull << MSR_VSX) | \
> + (1ull << MSR_EE) | \
> + (1ull << MSR_PR) | \
> + (1ull << MSR_FP) | \
> + (1ull << MSR_ME) | \
> + (1ull << MSR_FE0) | \
> + (1ull << MSR_SE) | \
> + (1ull << MSR_DE) | \
> + (1ull << MSR_FE1) | \
> + (1ull << MSR_IR) | \
> + (1ull << MSR_DR) | \
> + (1ull << MSR_PMM) | \
> + (1ull << MSR_RI) | \
> + (1ull << MSR_LE)
> +
> +#define POWERPC_POWER9_PCC_MSR_MASK \
> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
PPC_MSR_MASK_POWER9
> +#define POWERPC_POWER10_PCC_MSR_MASK \
> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
> +#define POWERPC_POWER9_PCC_PCR_MASK \
PPC_PCR_MASK_POWER9
> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
> +#define POWERPC_POWER10_PCC_PCR_MASK \
> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
PPC_PCR_SUPPORED_POWER9
etc
> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
> +#define POWERPC_POWER9_PCC_LPCR_MASK \
> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
> + LPCR_HDICE
> +/* DD2 adds an extra HAIL bit */
> +#define POWERPC_POWER10_PCC_LPCR_MASK \
> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
POWERPC_FLAG_POWER9
> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
> +
> +#define POWERPC_POWER9_PCC_FLAGS \
> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
> +#define POWERPC_POWER10_PCC_FLAGS \
> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
> +
> +#endif /* TARGET_PPC_CPU_INIT_H */
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-07-23 5:22 ` Nicholas Piggin
@ 2024-07-23 15:13 ` Aditya Gupta
2024-07-24 4:16 ` Harsh Prateek Bora
2024-07-24 6:31 ` Aditya Gupta
2024-07-24 6:50 ` Aditya Gupta
2 siblings, 1 reply; 24+ messages in thread
From: Aditya Gupta @ 2024-07-23 15:13 UTC (permalink / raw)
To: Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Nicholas Piggin, Mahesh J Salgaonkar,
Madhavan Srinivasan, Cédric Le Goater
Hi Harsh,
Is it okay if I do, the changes in your patch ?
Thanks,
Aditya Gupta
On 23/07/24 10:52, Nicholas Piggin wrote:
> On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
>> From: Harsh Prateek Bora <harshpb@linux.ibm.com>
>>
>> Power9/10 initialization code consists of a lot of logical OR of
>> various flag bits as supported by respective Power platform during its
>> initialization, most of which is duplicated and only selected bits are
>> added or removed as needed with each new platform support being added.
>> Remove the duplicate code and share using common macros.
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>> target/ppc/cpu_init.c | 124 +++++-------------------------------------
>> target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
>> 2 files changed, 93 insertions(+), 109 deletions(-)
>> create mode 100644 target/ppc/cpu_init.h
>>
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 01e358a4a5ac..3d8a112935ae 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -51,6 +51,7 @@
>> #include "kvm_ppc.h"
>> #endif
>>
>> +#include "cpu_init.h"
>> /* #define PPC_DEBUG_SPR */
>> /* #define USE_APPLE_GDB */
>>
>> @@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> dc->fw_name = "PowerPC,POWER9";
>> dc->desc = "POWER9";
>> pcc->pvr_match = ppc_pvr_match_power9;
>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
>> - pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
>> - PCR_COMPAT_2_05;
>> + pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
>> + pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
>> pcc->init_proc = init_proc_POWER9;
>> pcc->check_pow = check_pow_nocheck;
>> pcc->check_attn = check_attn_hid0_power9;
>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>> - PPC_FLOAT_FRSQRTES |
>> - PPC_FLOAT_STFIWX |
>> - PPC_FLOAT_EXT |
>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>> - PPC_SEGMENT_64B | PPC_SLBI |
>> - PPC_POPCNTB | PPC_POPCNTWD |
>> - PPC_CILDST;
>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> - PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_MEM_LWSYNC |
>> - PPC2_BCDA_ISA206;
>> - pcc->msr_mask = (1ull << MSR_SF) |
>> - (1ull << MSR_HV) |
>> - (1ull << MSR_TM) |
>> - (1ull << MSR_VR) |
>> - (1ull << MSR_VSX) |
>> - (1ull << MSR_EE) |
>> - (1ull << MSR_PR) |
>> - (1ull << MSR_FP) |
>> - (1ull << MSR_ME) |
>> - (1ull << MSR_FE0) |
>> - (1ull << MSR_SE) |
>> - (1ull << MSR_DE) |
>> - (1ull << MSR_FE1) |
>> - (1ull << MSR_IR) |
>> - (1ull << MSR_DR) |
>> - (1ull << MSR_PMM) |
>> - (1ull << MSR_RI) |
>> - (1ull << MSR_LE);
>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>> - LPCR_DEE | LPCR_OEE))
>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
>> + pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
>> + pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>> pcc->mmu_model = POWERPC_MMU_3_00;
>> #if !defined(CONFIG_USER_ONLY)
>> @@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> pcc->excp_model = POWERPC_EXCP_POWER9;
>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> pcc->bfd_mach = bfd_mach_ppc64;
>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>> - POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
>> + pcc->flags = POWERPC_POWER9_PCC_FLAGS;
>> pcc->l1_dcache_size = 0x8000;
>> pcc->l1_icache_size = 0x8000;
>> }
>> @@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>> dc->fw_name = "PowerPC,POWER10";
>> dc->desc = "POWER10";
>> pcc->pvr_match = ppc_pvr_match_power10;
>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
>> - PCR_COMPAT_3_00;
>> - pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
>> - PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
>> pcc->init_proc = init_proc_POWER10;
>> pcc->check_pow = check_pow_nocheck;
>> pcc->check_attn = check_attn_hid0_power9;
>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>> - PPC_FLOAT_FRSQRTES |
>> - PPC_FLOAT_STFIWX |
>> - PPC_FLOAT_EXT |
>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>> - PPC_SEGMENT_64B | PPC_SLBI |
>> - PPC_POPCNTB | PPC_POPCNTWD |
>> - PPC_CILDST;
>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> - PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
>> - PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
>> - pcc->msr_mask = (1ull << MSR_SF) |
>> - (1ull << MSR_HV) |
>> - (1ull << MSR_VR) |
>> - (1ull << MSR_VSX) |
>> - (1ull << MSR_EE) |
>> - (1ull << MSR_PR) |
>> - (1ull << MSR_FP) |
>> - (1ull << MSR_ME) |
>> - (1ull << MSR_FE0) |
>> - (1ull << MSR_SE) |
>> - (1ull << MSR_DE) |
>> - (1ull << MSR_FE1) |
>> - (1ull << MSR_IR) |
>> - (1ull << MSR_DR) |
>> - (1ull << MSR_PMM) |
>> - (1ull << MSR_RI) |
>> - (1ull << MSR_LE);
>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>> - LPCR_DEE | LPCR_OEE))
>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>> - /* DD2 adds an extra HAIL bit */
>> - pcc->lpcr_mask |= LPCR_HAIL;
>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
>> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
>> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
>>
>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>> pcc->mmu_model = POWERPC_MMU_3_00;
>> @@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>> pcc->excp_model = POWERPC_EXCP_POWER10;
>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> pcc->bfd_mach = bfd_mach_ppc64;
>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>> - POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
>> - POWERPC_FLAG_BHRB;
>> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
>> pcc->l1_dcache_size = 0x8000;
>> pcc->l1_icache_size = 0x8000;
>> }
>> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
>> new file mode 100644
>> index 000000000000..e04be6a655d8
>> --- /dev/null
>> +++ b/target/ppc/cpu_init.h
>> @@ -0,0 +1,78 @@
>> +#ifndef TARGET_PPC_CPU_INIT_H
>> +#define TARGET_PPC_CPU_INIT_H
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
> I would call this PPC_INSNS_FLAGS_POWER9
>
>> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
>> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
>> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
>> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
>> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
>> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
>> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
>> + PPC_CILDST
> Add this here
>
> #define PPC_INSNS_FLAGS_POWER10 PPC_INSNS_FLAGS_POWER9
>
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
> Suggest some other name change -
>
> PPC_INSNS_FLAGS2_POWER_COMMON
>
>> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
>> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
>> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
>> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
>> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
>> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
>> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
>> +
>> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
> PPC_MSR_MASK_POWER_COMMON
>
>> + (1ull << MSR_SF) | \
>> + (1ull << MSR_HV) | \
>> + (1ull << MSR_VR) | \
>> + (1ull << MSR_VSX) | \
>> + (1ull << MSR_EE) | \
>> + (1ull << MSR_PR) | \
>> + (1ull << MSR_FP) | \
>> + (1ull << MSR_ME) | \
>> + (1ull << MSR_FE0) | \
>> + (1ull << MSR_SE) | \
>> + (1ull << MSR_DE) | \
>> + (1ull << MSR_FE1) | \
>> + (1ull << MSR_IR) | \
>> + (1ull << MSR_DR) | \
>> + (1ull << MSR_PMM) | \
>> + (1ull << MSR_RI) | \
>> + (1ull << MSR_LE)
>> +
>> +#define POWERPC_POWER9_PCC_MSR_MASK \
>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
> PPC_MSR_MASK_POWER9
>
>> +#define POWERPC_POWER10_PCC_MSR_MASK \
>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
>> +#define POWERPC_POWER9_PCC_PCR_MASK \
> PPC_PCR_MASK_POWER9
>
>> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
>> +#define POWERPC_POWER10_PCC_PCR_MASK \
>> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
>> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
> PPC_PCR_SUPPORED_POWER9
>
> etc
>
>> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
>> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
>> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
>> +#define POWERPC_POWER9_PCC_LPCR_MASK \
>> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
>> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
>> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
>> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
>> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
>> + LPCR_HDICE
>> +/* DD2 adds an extra HAIL bit */
>> +#define POWERPC_POWER10_PCC_LPCR_MASK \
>> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
>> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
> POWERPC_FLAG_POWER9
>
>> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
>> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
>> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
>> +
>> +#define POWERPC_POWER9_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
>> +#define POWERPC_POWER10_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
>> +
>> +#endif /* TARGET_PPC_CPU_INIT_H */
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-07-23 15:13 ` Aditya Gupta
@ 2024-07-24 4:16 ` Harsh Prateek Bora
0 siblings, 0 replies; 24+ messages in thread
From: Harsh Prateek Bora @ 2024-07-24 4:16 UTC (permalink / raw)
To: Aditya Gupta
Cc: qemu-devel, qemu-ppc, Nicholas Piggin, Mahesh J Salgaonkar,
Madhavan Srinivasan, Cédric Le Goater
Hi Aditya,
On 7/23/24 20:43, Aditya Gupta wrote:
> Hi Harsh,
>
>
> Is it okay if I do, the changes in your patch ?
>
Sure, feel free to update as suggested and add your sob mentioning the
summary of updates.
Thanks
Harsh
>
> Thanks,
>
> Aditya Gupta
>
>
> On 23/07/24 10:52, Nicholas Piggin wrote:
>> On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
>>> From: Harsh Prateek Bora <harshpb@linux.ibm.com>
>>>
>>> Power9/10 initialization code consists of a lot of logical OR of
>>> various flag bits as supported by respective Power platform during its
>>> initialization, most of which is duplicated and only selected bits are
>>> added or removed as needed with each new platform support being added.
>>> Remove the duplicate code and share using common macros.
>>>
>>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>>> ---
>>> target/ppc/cpu_init.c | 124 +++++-------------------------------------
>>> target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
>>> 2 files changed, 93 insertions(+), 109 deletions(-)
>>> create mode 100644 target/ppc/cpu_init.h
>>>
>>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>>> index 01e358a4a5ac..3d8a112935ae 100644
>>> --- a/target/ppc/cpu_init.c
>>> +++ b/target/ppc/cpu_init.c
>>> @@ -51,6 +51,7 @@
>>> #include "kvm_ppc.h"
>>> #endif
>>> +#include "cpu_init.h"
>>> /* #define PPC_DEBUG_SPR */
>>> /* #define USE_APPLE_GDB */
>>> @@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void
>>> *data)
>>> dc->fw_name = "PowerPC,POWER9";
>>> dc->desc = "POWER9";
>>> pcc->pvr_match = ppc_pvr_match_power9;
>>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 |
>>> PCR_COMPAT_2_07;
>>> - pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
>>> PCR_COMPAT_2_06 |
>>> - PCR_COMPAT_2_05;
>>> + pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
>>> + pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
>>> pcc->init_proc = init_proc_POWER9;
>>> pcc->check_pow = check_pow_nocheck;
>>> pcc->check_attn = check_attn_hid0_power9;
>>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING |
>>> PPC_MFTB |
>>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>>> - PPC_FLOAT_FRSQRTES |
>>> - PPC_FLOAT_STFIWX |
>>> - PPC_FLOAT_EXT |
>>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>>> - PPC_SEGMENT_64B | PPC_SLBI |
>>> - PPC_POPCNTB | PPC_POPCNTWD |
>>> - PPC_CILDST;
>>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>>> - PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL |
>>> PPC2_MEM_LWSYNC |
>>> - PPC2_BCDA_ISA206;
>>> - pcc->msr_mask = (1ull << MSR_SF) |
>>> - (1ull << MSR_HV) |
>>> - (1ull << MSR_TM) |
>>> - (1ull << MSR_VR) |
>>> - (1ull << MSR_VSX) |
>>> - (1ull << MSR_EE) |
>>> - (1ull << MSR_PR) |
>>> - (1ull << MSR_FP) |
>>> - (1ull << MSR_ME) |
>>> - (1ull << MSR_FE0) |
>>> - (1ull << MSR_SE) |
>>> - (1ull << MSR_DE) |
>>> - (1ull << MSR_FE1) |
>>> - (1ull << MSR_IR) |
>>> - (1ull << MSR_DR) |
>>> - (1ull << MSR_PMM) |
>>> - (1ull << MSR_RI) |
>>> - (1ull << MSR_LE);
>>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>>> - LPCR_DEE | LPCR_OEE))
>>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
>>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
>>> + pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
>>> + pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
>>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE |
>>> LPCR_OEE;
>>> pcc->mmu_model = POWERPC_MMU_3_00;
>>> #if !defined(CONFIG_USER_ONLY)
>>> @@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void
>>> *data)
>>> pcc->excp_model = POWERPC_EXCP_POWER9;
>>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>>> pcc->bfd_mach = bfd_mach_ppc64;
>>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>>> - POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
>>> + pcc->flags = POWERPC_POWER9_PCC_FLAGS;
>>> pcc->l1_dcache_size = 0x8000;
>>> pcc->l1_icache_size = 0x8000;
>>> }
>>> @@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void
>>> *data)
>>> dc->fw_name = "PowerPC,POWER10";
>>> dc->desc = "POWER10";
>>> pcc->pvr_match = ppc_pvr_match_power10;
>>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 |
>>> PCR_COMPAT_2_07 |
>>> - PCR_COMPAT_3_00;
>>> - pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 |
>>> PCR_COMPAT_2_07 |
>>> - PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>>> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>>> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
>>> pcc->init_proc = init_proc_POWER10;
>>> pcc->check_pow = check_pow_nocheck;
>>> pcc->check_attn = check_attn_hid0_power9;
>>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING |
>>> PPC_MFTB |
>>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>>> - PPC_FLOAT_FRSQRTES |
>>> - PPC_FLOAT_STFIWX |
>>> - PPC_FLOAT_EXT |
>>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>>> - PPC_SEGMENT_64B | PPC_SLBI |
>>> - PPC_POPCNTB | PPC_POPCNTWD |
>>> - PPC_CILDST;
>>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>>> - PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
>>> - PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
>>> - pcc->msr_mask = (1ull << MSR_SF) |
>>> - (1ull << MSR_HV) |
>>> - (1ull << MSR_VR) |
>>> - (1ull << MSR_VSX) |
>>> - (1ull << MSR_EE) |
>>> - (1ull << MSR_PR) |
>>> - (1ull << MSR_FP) |
>>> - (1ull << MSR_ME) |
>>> - (1ull << MSR_FE0) |
>>> - (1ull << MSR_SE) |
>>> - (1ull << MSR_DE) |
>>> - (1ull << MSR_FE1) |
>>> - (1ull << MSR_IR) |
>>> - (1ull << MSR_DR) |
>>> - (1ull << MSR_PMM) |
>>> - (1ull << MSR_RI) |
>>> - (1ull << MSR_LE);
>>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>>> - LPCR_DEE | LPCR_OEE))
>>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>>> - /* DD2 adds an extra HAIL bit */
>>> - pcc->lpcr_mask |= LPCR_HAIL;
>>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as
>>> P9 */
>>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
>>> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
>>> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
>>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE |
>>> LPCR_OEE;
>>> pcc->mmu_model = POWERPC_MMU_3_00;
>>> @@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void
>>> *data)
>>> pcc->excp_model = POWERPC_EXCP_POWER10;
>>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>>> pcc->bfd_mach = bfd_mach_ppc64;
>>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>>> - POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
>>> - POWERPC_FLAG_BHRB;
>>> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
>>> pcc->l1_dcache_size = 0x8000;
>>> pcc->l1_icache_size = 0x8000;
>>> }
>>> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
>>> new file mode 100644
>>> index 000000000000..e04be6a655d8
>>> --- /dev/null
>>> +++ b/target/ppc/cpu_init.h
>>> @@ -0,0 +1,78 @@
>>> +#ifndef TARGET_PPC_CPU_INIT_H
>>> +#define TARGET_PPC_CPU_INIT_H
>>> +
>>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
>> I would call this PPC_INSNS_FLAGS_POWER9
>>
>>> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
>>> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
>>> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
>>> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
>>> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
>>> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
>>> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
>>> + PPC_CILDST
>> Add this here
>>
>> #define PPC_INSNS_FLAGS_POWER10 PPC_INSNS_FLAGS_POWER9
>>
>>> +
>>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
>> Suggest some other name change -
>>
>> PPC_INSNS_FLAGS2_POWER_COMMON
>>
>>> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
>>> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
>>> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
>>> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
>>> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
>>> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
>>> +
>>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
>>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
>>> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
>>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
>>> +
>>> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
>> PPC_MSR_MASK_POWER_COMMON
>>
>>> + (1ull << MSR_SF) | \
>>> + (1ull << MSR_HV) | \
>>> + (1ull << MSR_VR) | \
>>> + (1ull << MSR_VSX) | \
>>> + (1ull << MSR_EE) | \
>>> + (1ull << MSR_PR) | \
>>> + (1ull << MSR_FP) | \
>>> + (1ull << MSR_ME) | \
>>> + (1ull << MSR_FE0) | \
>>> + (1ull << MSR_SE) | \
>>> + (1ull << MSR_DE) | \
>>> + (1ull << MSR_FE1) | \
>>> + (1ull << MSR_IR) | \
>>> + (1ull << MSR_DR) | \
>>> + (1ull << MSR_PMM) | \
>>> + (1ull << MSR_RI) | \
>>> + (1ull << MSR_LE)
>>> +
>>> +#define POWERPC_POWER9_PCC_MSR_MASK \
>>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
>> PPC_MSR_MASK_POWER9
>>
>>> +#define POWERPC_POWER10_PCC_MSR_MASK \
>>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
>>> +#define POWERPC_POWER9_PCC_PCR_MASK \
>> PPC_PCR_MASK_POWER9
>>
>>> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
>>> +#define POWERPC_POWER10_PCC_PCR_MASK \
>>> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
>>> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
>> PPC_PCR_SUPPORED_POWER9
>>
>> etc
>>
>>> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
>>> PCR_COMPAT_2_05
>>> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
>>> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
>>> +#define
>>> POWERPC_POWER9_PCC_LPCR_MASK \
>>> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD
>>> | \
>>> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL
>>> | \
>>> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD
>>> | \
>>> + (LPCR_PECE_L_MASK &
>>> (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
>>> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 |
>>> LPCR_HVICE | \
>>> + LPCR_HDICE
>>> +/* DD2 adds an extra HAIL bit */
>>> +#define POWERPC_POWER10_PCC_LPCR_MASK \
>>> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
>>> +#define
>>> POWERPC_POWER9_PCC_FLAGS_COMMON \
>> POWERPC_FLAG_POWER9
>>
>>> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE
>>> | \
>>> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR
>>> | \
>>> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
>>> +
>>> +#define POWERPC_POWER9_PCC_FLAGS \
>>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
>>> +#define POWERPC_POWER10_PCC_FLAGS \
>>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
>>> +
>>> +#endif /* TARGET_PPC_CPU_INIT_H */
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-07-23 5:22 ` Nicholas Piggin
2024-07-23 15:13 ` Aditya Gupta
@ 2024-07-24 6:31 ` Aditya Gupta
2024-07-24 6:50 ` Aditya Gupta
2 siblings, 0 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-07-24 6:31 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, qemu-ppc, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Hi Nick,
On 23/07/24 10:52, Nicholas Piggin wrote:
>> <...snip...>
>>
>> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
>> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
>> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
>> +#define POWERPC_POWER9_PCC_LPCR_MASK \
>> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
>> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
>> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
>> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
>> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
>> + LPCR_HDICE
>> +/* DD2 adds an extra HAIL bit */
>> +#define POWERPC_POWER10_PCC_LPCR_MASK \
>> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
>> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
> POWERPC_FLAG_POWER9
Can we keep PPC_FLAG_POWER9 here ?
Like if all PowerPC flags start with PPC_* ?
Thanks,
Aditya Gupta
>
>> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
>> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
>> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
>> +
>> +#define POWERPC_POWER9_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
>> +#define POWERPC_POWER10_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
>> +
>> +#endif /* TARGET_PPC_CPU_INIT_H */
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-07-23 5:22 ` Nicholas Piggin
2024-07-23 15:13 ` Aditya Gupta
2024-07-24 6:31 ` Aditya Gupta
@ 2024-07-24 6:50 ` Aditya Gupta
2024-07-24 12:04 ` BALATON Zoltan
2 siblings, 1 reply; 24+ messages in thread
From: Aditya Gupta @ 2024-07-24 6:50 UTC (permalink / raw)
To: Nicholas Piggin, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc
Hi Nick,
While doing the renaming, codes similar to this come up:
> #define PPC_INSNS_FLAGS_POWER9 \
> PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
> PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
> PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
> PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
> PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
> PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
> PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
> PPC_CILDST
Will flag names starting with POWERPC_* be better ? (eg.
POWERPC_INSNS_FLAGS_POWER9)
Idea behind that is, that POWERPC_* are the ones expected to be used in
code, and the PPC_* are the flags that can be used in header files like
this to make up some complex flags.
In above case, `PPC_INSNS_FLAGS_POWER9` and `PPC_INSNS_BASE` seem
somewhat similar to me, but are very different considering the values.
What do you think ?
- Aditya Gupta
On 23/07/24 10:52, Nicholas Piggin wrote:
> On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
>> From: Harsh Prateek Bora <harshpb@linux.ibm.com>
>>
>> Power9/10 initialization code consists of a lot of logical OR of
>> various flag bits as supported by respective Power platform during its
>> initialization, most of which is duplicated and only selected bits are
>> added or removed as needed with each new platform support being added.
>> Remove the duplicate code and share using common macros.
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>> target/ppc/cpu_init.c | 124 +++++-------------------------------------
>> target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
>> 2 files changed, 93 insertions(+), 109 deletions(-)
>> create mode 100644 target/ppc/cpu_init.h
>>
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 01e358a4a5ac..3d8a112935ae 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -51,6 +51,7 @@
>> #include "kvm_ppc.h"
>> #endif
>>
>> +#include "cpu_init.h"
>> /* #define PPC_DEBUG_SPR */
>> /* #define USE_APPLE_GDB */
>>
>> @@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> dc->fw_name = "PowerPC,POWER9";
>> dc->desc = "POWER9";
>> pcc->pvr_match = ppc_pvr_match_power9;
>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
>> - pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
>> - PCR_COMPAT_2_05;
>> + pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
>> + pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
>> pcc->init_proc = init_proc_POWER9;
>> pcc->check_pow = check_pow_nocheck;
>> pcc->check_attn = check_attn_hid0_power9;
>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>> - PPC_FLOAT_FRSQRTES |
>> - PPC_FLOAT_STFIWX |
>> - PPC_FLOAT_EXT |
>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>> - PPC_SEGMENT_64B | PPC_SLBI |
>> - PPC_POPCNTB | PPC_POPCNTWD |
>> - PPC_CILDST;
>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> - PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_MEM_LWSYNC |
>> - PPC2_BCDA_ISA206;
>> - pcc->msr_mask = (1ull << MSR_SF) |
>> - (1ull << MSR_HV) |
>> - (1ull << MSR_TM) |
>> - (1ull << MSR_VR) |
>> - (1ull << MSR_VSX) |
>> - (1ull << MSR_EE) |
>> - (1ull << MSR_PR) |
>> - (1ull << MSR_FP) |
>> - (1ull << MSR_ME) |
>> - (1ull << MSR_FE0) |
>> - (1ull << MSR_SE) |
>> - (1ull << MSR_DE) |
>> - (1ull << MSR_FE1) |
>> - (1ull << MSR_IR) |
>> - (1ull << MSR_DR) |
>> - (1ull << MSR_PMM) |
>> - (1ull << MSR_RI) |
>> - (1ull << MSR_LE);
>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>> - LPCR_DEE | LPCR_OEE))
>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
>> + pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
>> + pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>> pcc->mmu_model = POWERPC_MMU_3_00;
>> #if !defined(CONFIG_USER_ONLY)
>> @@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> pcc->excp_model = POWERPC_EXCP_POWER9;
>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> pcc->bfd_mach = bfd_mach_ppc64;
>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>> - POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
>> + pcc->flags = POWERPC_POWER9_PCC_FLAGS;
>> pcc->l1_dcache_size = 0x8000;
>> pcc->l1_icache_size = 0x8000;
>> }
>> @@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>> dc->fw_name = "PowerPC,POWER10";
>> dc->desc = "POWER10";
>> pcc->pvr_match = ppc_pvr_match_power10;
>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
>> - PCR_COMPAT_3_00;
>> - pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
>> - PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
>> pcc->init_proc = init_proc_POWER10;
>> pcc->check_pow = check_pow_nocheck;
>> pcc->check_attn = check_attn_hid0_power9;
>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>> - PPC_FLOAT_FRSQRTES |
>> - PPC_FLOAT_STFIWX |
>> - PPC_FLOAT_EXT |
>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>> - PPC_SEGMENT_64B | PPC_SLBI |
>> - PPC_POPCNTB | PPC_POPCNTWD |
>> - PPC_CILDST;
>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> - PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
>> - PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
>> - pcc->msr_mask = (1ull << MSR_SF) |
>> - (1ull << MSR_HV) |
>> - (1ull << MSR_VR) |
>> - (1ull << MSR_VSX) |
>> - (1ull << MSR_EE) |
>> - (1ull << MSR_PR) |
>> - (1ull << MSR_FP) |
>> - (1ull << MSR_ME) |
>> - (1ull << MSR_FE0) |
>> - (1ull << MSR_SE) |
>> - (1ull << MSR_DE) |
>> - (1ull << MSR_FE1) |
>> - (1ull << MSR_IR) |
>> - (1ull << MSR_DR) |
>> - (1ull << MSR_PMM) |
>> - (1ull << MSR_RI) |
>> - (1ull << MSR_LE);
>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>> - LPCR_DEE | LPCR_OEE))
>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>> - /* DD2 adds an extra HAIL bit */
>> - pcc->lpcr_mask |= LPCR_HAIL;
>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
>> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
>> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
>>
>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>> pcc->mmu_model = POWERPC_MMU_3_00;
>> @@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>> pcc->excp_model = POWERPC_EXCP_POWER10;
>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> pcc->bfd_mach = bfd_mach_ppc64;
>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>> - POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
>> - POWERPC_FLAG_BHRB;
>> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
>> pcc->l1_dcache_size = 0x8000;
>> pcc->l1_icache_size = 0x8000;
>> }
>> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
>> new file mode 100644
>> index 000000000000..e04be6a655d8
>> --- /dev/null
>> +++ b/target/ppc/cpu_init.h
>> @@ -0,0 +1,78 @@
>> +#ifndef TARGET_PPC_CPU_INIT_H
>> +#define TARGET_PPC_CPU_INIT_H
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
> I would call this PPC_INSNS_FLAGS_POWER9
>
>> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
>> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
>> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
>> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
>> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
>> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
>> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
>> + PPC_CILDST
> Add this here
>
> #define PPC_INSNS_FLAGS_POWER10 PPC_INSNS_FLAGS_POWER9
>
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
> Suggest some other name change -
>
> PPC_INSNS_FLAGS2_POWER_COMMON
>
>> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
>> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
>> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
>> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
>> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
>> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
>> +
>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
>> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
>> +
>> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
> PPC_MSR_MASK_POWER_COMMON
>
>> + (1ull << MSR_SF) | \
>> + (1ull << MSR_HV) | \
>> + (1ull << MSR_VR) | \
>> + (1ull << MSR_VSX) | \
>> + (1ull << MSR_EE) | \
>> + (1ull << MSR_PR) | \
>> + (1ull << MSR_FP) | \
>> + (1ull << MSR_ME) | \
>> + (1ull << MSR_FE0) | \
>> + (1ull << MSR_SE) | \
>> + (1ull << MSR_DE) | \
>> + (1ull << MSR_FE1) | \
>> + (1ull << MSR_IR) | \
>> + (1ull << MSR_DR) | \
>> + (1ull << MSR_PMM) | \
>> + (1ull << MSR_RI) | \
>> + (1ull << MSR_LE)
>> +
>> +#define POWERPC_POWER9_PCC_MSR_MASK \
>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
> PPC_MSR_MASK_POWER9
>
>> +#define POWERPC_POWER10_PCC_MSR_MASK \
>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
>> +#define POWERPC_POWER9_PCC_PCR_MASK \
> PPC_PCR_MASK_POWER9
>
>> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
>> +#define POWERPC_POWER10_PCC_PCR_MASK \
>> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
>> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
> PPC_PCR_SUPPORED_POWER9
>
> etc
>
>> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
>> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
>> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
>> +#define POWERPC_POWER9_PCC_LPCR_MASK \
>> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | \
>> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | \
>> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD | \
>> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE)) | \
>> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | \
>> + LPCR_HDICE
>> +/* DD2 adds an extra HAIL bit */
>> +#define POWERPC_POWER10_PCC_LPCR_MASK \
>> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
>> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
> POWERPC_FLAG_POWER9
>
>> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
>> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
>> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
>> +
>> +#define POWERPC_POWER9_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
>> +#define POWERPC_POWER10_PCC_FLAGS \
>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
>> +
>> +#endif /* TARGET_PPC_CPU_INIT_H */
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code
2024-07-24 6:50 ` Aditya Gupta
@ 2024-07-24 12:04 ` BALATON Zoltan
0 siblings, 0 replies; 24+ messages in thread
From: BALATON Zoltan @ 2024-07-24 12:04 UTC (permalink / raw)
To: Aditya Gupta
Cc: Nicholas Piggin, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora, qemu-devel, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 16132 bytes --]
On Wed, 24 Jul 2024, Aditya Gupta wrote:
> Hi Nick,
>
>
> While doing the renaming, codes similar to this come up:
>
>
>> #define PPC_INSNS_FLAGS_POWER9 \
>> PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
>> PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
>> PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
>> PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
>> PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
>> PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
>> PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
>> PPC_CILDST
>
> Will flag names starting with POWERPC_* be better ? (eg.
> POWERPC_INSNS_FLAGS_POWER9)
>
> Idea behind that is, that POWERPC_* are the ones expected to be used in code,
> and the PPC_* are the flags that can be used in header files like this to
> make up some complex flags.
>
> In above case, `PPC_INSNS_FLAGS_POWER9` and `PPC_INSNS_BASE` seem somewhat
> similar to me, but are very different considering the values.
>
> What do you think ?
These are already prefixed with PPC_INSNS_ instead of just PPC_ so they
are already distinguished enough IMO. Having mixed POWERPC_ and PPC_
prefix might be more confusing than helpful but that's just my opinion.
Regards,
BALATON Zoltan
>
>
> - Aditya Gupta
>
>
> On 23/07/24 10:52, Nicholas Piggin wrote:
>> On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
>>> From: Harsh Prateek Bora <harshpb@linux.ibm.com>
>>>
>>> Power9/10 initialization code consists of a lot of logical OR of
>>> various flag bits as supported by respective Power platform during its
>>> initialization, most of which is duplicated and only selected bits are
>>> added or removed as needed with each new platform support being added.
>>> Remove the duplicate code and share using common macros.
>>>
>>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>>> ---
>>> target/ppc/cpu_init.c | 124 +++++-------------------------------------
>>> target/ppc/cpu_init.h | 78 ++++++++++++++++++++++++++
>>> 2 files changed, 93 insertions(+), 109 deletions(-)
>>> create mode 100644 target/ppc/cpu_init.h
>>>
>>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>>> index 01e358a4a5ac..3d8a112935ae 100644
>>> --- a/target/ppc/cpu_init.c
>>> +++ b/target/ppc/cpu_init.c
>>> @@ -51,6 +51,7 @@
>>> #include "kvm_ppc.h"
>>> #endif
>>> +#include "cpu_init.h"
>>> /* #define PPC_DEBUG_SPR */
>>> /* #define USE_APPLE_GDB */
>>> @@ -6508,58 +6509,15 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void
>>> *data)
>>> dc->fw_name = "PowerPC,POWER9";
>>> dc->desc = "POWER9";
>>> pcc->pvr_match = ppc_pvr_match_power9;
>>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
>>> - pcc->pcr_supported = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 |
>>> PCR_COMPAT_2_06 |
>>> - PCR_COMPAT_2_05;
>>> + pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
>>> + pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
>>> pcc->init_proc = init_proc_POWER9;
>>> pcc->check_pow = check_pow_nocheck;
>>> pcc->check_attn = check_attn_hid0_power9;
>>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB
>>> |
>>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>>> - PPC_FLOAT_FRSQRTES |
>>> - PPC_FLOAT_STFIWX |
>>> - PPC_FLOAT_EXT |
>>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>>> - PPC_SEGMENT_64B | PPC_SLBI |
>>> - PPC_POPCNTB | PPC_POPCNTWD |
>>> - PPC_CILDST;
>>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>>> - PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL |
>>> PPC2_MEM_LWSYNC |
>>> - PPC2_BCDA_ISA206;
>>> - pcc->msr_mask = (1ull << MSR_SF) |
>>> - (1ull << MSR_HV) |
>>> - (1ull << MSR_TM) |
>>> - (1ull << MSR_VR) |
>>> - (1ull << MSR_VSX) |
>>> - (1ull << MSR_EE) |
>>> - (1ull << MSR_PR) |
>>> - (1ull << MSR_FP) |
>>> - (1ull << MSR_ME) |
>>> - (1ull << MSR_FE0) |
>>> - (1ull << MSR_SE) |
>>> - (1ull << MSR_DE) |
>>> - (1ull << MSR_FE1) |
>>> - (1ull << MSR_IR) |
>>> - (1ull << MSR_DR) |
>>> - (1ull << MSR_PMM) |
>>> - (1ull << MSR_RI) |
>>> - (1ull << MSR_LE);
>>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>>> - LPCR_DEE | LPCR_OEE))
>>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS;
>>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER9_INSNS_FLAGS2;
>>> + pcc->msr_mask = POWERPC_POWER9_PCC_MSR_MASK;
>>> + pcc->lpcr_mask = POWERPC_POWER9_PCC_LPCR_MASK;
>>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE |
>>> LPCR_OEE;
>>> pcc->mmu_model = POWERPC_MMU_3_00;
>>> #if !defined(CONFIG_USER_ONLY)
>>> @@ -6572,10 +6530,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>>> pcc->excp_model = POWERPC_EXCP_POWER9;
>>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>>> pcc->bfd_mach = bfd_mach_ppc64;
>>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>>> - POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
>>> + pcc->flags = POWERPC_POWER9_PCC_FLAGS;
>>> pcc->l1_dcache_size = 0x8000;
>>> pcc->l1_icache_size = 0x8000;
>>> }
>>> @@ -6688,60 +6643,15 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void
>>> *data)
>>> dc->fw_name = "PowerPC,POWER10";
>>> dc->desc = "POWER10";
>>> pcc->pvr_match = ppc_pvr_match_power10;
>>> - pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07 |
>>> - PCR_COMPAT_3_00;
>>> - pcc->pcr_supported = PCR_COMPAT_3_10 | PCR_COMPAT_3_00 |
>>> PCR_COMPAT_2_07 |
>>> - PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>>> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>>> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
>>> pcc->init_proc = init_proc_POWER10;
>>> pcc->check_pow = check_pow_nocheck;
>>> pcc->check_attn = check_attn_hid0_power9;
>>> - pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB
>>> |
>>> - PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
>>> - PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
>>> - PPC_FLOAT_FRSQRTES |
>>> - PPC_FLOAT_STFIWX |
>>> - PPC_FLOAT_EXT |
>>> - PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
>>> - PPC_MEM_SYNC | PPC_MEM_EIEIO |
>>> - PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>>> - PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>>> - PPC_SEGMENT_64B | PPC_SLBI |
>>> - PPC_POPCNTB | PPC_POPCNTWD |
>>> - PPC_CILDST;
>>> - pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>>> - PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>>> - PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
>>> - PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>>> - PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>>> - PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>>> - PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
>>> - PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
>>> - pcc->msr_mask = (1ull << MSR_SF) |
>>> - (1ull << MSR_HV) |
>>> - (1ull << MSR_VR) |
>>> - (1ull << MSR_VSX) |
>>> - (1ull << MSR_EE) |
>>> - (1ull << MSR_PR) |
>>> - (1ull << MSR_FP) |
>>> - (1ull << MSR_ME) |
>>> - (1ull << MSR_FE0) |
>>> - (1ull << MSR_SE) |
>>> - (1ull << MSR_DE) |
>>> - (1ull << MSR_FE1) |
>>> - (1ull << MSR_IR) |
>>> - (1ull << MSR_DR) |
>>> - (1ull << MSR_PMM) |
>>> - (1ull << MSR_RI) |
>>> - (1ull << MSR_LE);
>>> - pcc->lpcr_mask = LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>>> - (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>>> - LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>>> - (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>>> - LPCR_DEE | LPCR_OEE))
>>> - | LPCR_MER | LPCR_GTSE | LPCR_TC |
>>> - LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE;
>>> - /* DD2 adds an extra HAIL bit */
>>> - pcc->lpcr_mask |= LPCR_HAIL;
>>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9
>>> */
>>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
>>> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
>>> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
>>> pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE |
>>> LPCR_OEE;
>>> pcc->mmu_model = POWERPC_MMU_3_00;
>>> @@ -6754,11 +6664,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void
>>> *data)
>>> pcc->excp_model = POWERPC_EXCP_POWER10;
>>> pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>>> pcc->bfd_mach = bfd_mach_ppc64;
>>> - pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>>> - POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>>> - POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
>>> - POWERPC_FLAG_VSX | POWERPC_FLAG_SCV |
>>> - POWERPC_FLAG_BHRB;
>>> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
>>> pcc->l1_dcache_size = 0x8000;
>>> pcc->l1_icache_size = 0x8000;
>>> }
>>> diff --git a/target/ppc/cpu_init.h b/target/ppc/cpu_init.h
>>> new file mode 100644
>>> index 000000000000..e04be6a655d8
>>> --- /dev/null
>>> +++ b/target/ppc/cpu_init.h
>>> @@ -0,0 +1,78 @@
>>> +#ifndef TARGET_PPC_CPU_INIT_H
>>> +#define TARGET_PPC_CPU_INIT_H
>>> +
>>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS \
>> I would call this PPC_INSNS_FLAGS_POWER9
>>
>>> + PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | \
>>> + PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | \
>>> + PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES | \
>>> + PPC_FLOAT_STFIWX | PPC_FLOAT_EXT |PPC_CACHE | PPC_CACHE_ICBI | \
>>> + PPC_CACHE_DCBZ | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | \
>>> + PPC_MEM_TLBSYNC | PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC | \
>>> + PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD | \
>>> + PPC_CILDST
>> Add this here
>>
>> #define PPC_INSNS_FLAGS_POWER10 PPC_INSNS_FLAGS_POWER9
>>
>>> +
>>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON \
>> Suggest some other name change -
>>
>> PPC_INSNS_FLAGS2_POWER_COMMON
>>
>>> + PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX | \
>>> + PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
>>> + PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | \
>>> + PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | \
>>> + PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_ISA300 | PPC2_PRCNTL | \
>>> + PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206
>>> +
>>> +#define POWERPC_FAMILY_POWER9_INSNS_FLAGS2 \
>>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_TM
>>> +#define POWERPC_FAMILY_POWER10_INSNS_FLAGS2 \
>>> + POWERPC_FAMILY_POWER9_INSNS_FLAGS2_COMMON | PPC2_ISA310
>>> +
>>> +#define POWERPC_POWER9_COMMON_PCC_MSR_MASK \
>> PPC_MSR_MASK_POWER_COMMON
>>
>>> + (1ull << MSR_SF) | \
>>> + (1ull << MSR_HV) | \
>>> + (1ull << MSR_VR) | \
>>> + (1ull << MSR_VSX) | \
>>> + (1ull << MSR_EE) | \
>>> + (1ull << MSR_PR) | \
>>> + (1ull << MSR_FP) | \
>>> + (1ull << MSR_ME) | \
>>> + (1ull << MSR_FE0) | \
>>> + (1ull << MSR_SE) | \
>>> + (1ull << MSR_DE) | \
>>> + (1ull << MSR_FE1) | \
>>> + (1ull << MSR_IR) | \
>>> + (1ull << MSR_DR) | \
>>> + (1ull << MSR_PMM) | \
>>> + (1ull << MSR_RI) | \
>>> + (1ull << MSR_LE)
>>> +
>>> +#define POWERPC_POWER9_PCC_MSR_MASK \
>>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK | (1ull << MSR_TM)
>> PPC_MSR_MASK_POWER9
>>
>>> +#define POWERPC_POWER10_PCC_MSR_MASK \
>>> + POWERPC_POWER9_COMMON_PCC_MSR_MASK
>>> +#define POWERPC_POWER9_PCC_PCR_MASK \
>> PPC_PCR_MASK_POWER9
>>
>>> + PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07
>>> +#define POWERPC_POWER10_PCC_PCR_MASK \
>>> + POWERPC_POWER9_PCC_PCR_MASK | PCR_COMPAT_3_00
>>> +#define POWERPC_POWER9_PCC_PCR_SUPPORTED \
>> PPC_PCR_SUPPORED_POWER9
>>
>> etc
>>
>>> + PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
>>> +#define POWERPC_POWER10_PCC_PCR_SUPPORTED \
>>> + POWERPC_POWER9_PCC_PCR_SUPPORTED | PCR_COMPAT_3_10
>>> +#define POWERPC_POWER9_PCC_LPCR_MASK
>>> \
>>> + LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD |
>>> \
>>> + (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL |
>>> \
>>> + LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | LPCR_HR | LPCR_LD |
>>> \
>>> + (LPCR_PECE_L_MASK & (LPCR_PDEE|LPCR_HDEE|LPCR_EEE|LPCR_DEE|LPCR_OEE))
>>> | \
>>> + LPCR_MER | LPCR_GTSE | LPCR_TC | LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE
>>> | \
>>> + LPCR_HDICE
>>> +/* DD2 adds an extra HAIL bit */
>>> +#define POWERPC_POWER10_PCC_LPCR_MASK \
>>> + POWERPC_POWER9_PCC_LPCR_MASK | LPCR_HAIL
>>> +#define POWERPC_POWER9_PCC_FLAGS_COMMON \
>> POWERPC_FLAG_POWER9
>>
>>> + POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | \
>>> + POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR | \
>>> + POWERPC_FLAG_VSX | POWERPC_FLAG_SCV
>>> +
>>> +#define POWERPC_POWER9_PCC_FLAGS \
>>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_TM
>>> +#define POWERPC_POWER10_PCC_FLAGS \
>>> + POWERPC_POWER9_PCC_FLAGS_COMMON | POWERPC_FLAG_BHRB
>>> +
>>> +#endif /* TARGET_PPC_CPU_INIT_H */
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor
2024-06-06 12:16 [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
2024-06-06 12:16 ` [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code Aditya Gupta
@ 2024-06-06 12:16 ` Aditya Gupta
2024-07-23 4:30 ` Nicholas Piggin
2024-07-23 5:27 ` Nicholas Piggin
2024-06-06 12:16 ` [PATCH v5 3/5] ppc/pseries: Add Power11 cpu type Aditya Gupta
` (4 subsequent siblings)
6 siblings, 2 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-06-06 12:16 UTC (permalink / raw)
To: Mahesh J Salgaonkar, Madhavan Srinivasan, Nicholas Piggin,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza,
Frédéric Barrat
Add CPU target code to add support for new Power11 Processor.
Power11 core is same as Power10, hence reuse functions defined for
Power10.
Cc: Cédric Le Goater <clg@kaod.org>
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Frédéric Barrat <fbarrat@linux.ibm.com>
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
target/ppc/compat.c | 7 ++++++
target/ppc/cpu-models.c | 3 +++
target/ppc/cpu-models.h | 3 +++
target/ppc/cpu_init.c | 54 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 67 insertions(+)
diff --git a/target/ppc/compat.c b/target/ppc/compat.c
index ebef2cccecf3..12dd8ae290ca 100644
--- a/target/ppc/compat.c
+++ b/target/ppc/compat.c
@@ -100,6 +100,13 @@ static const CompatInfo compat_table[] = {
.pcr_level = PCR_COMPAT_3_10,
.max_vthreads = 8,
},
+ { /* POWER11, ISA3.10 */
+ .name = "power11",
+ .pvr = CPU_POWERPC_LOGICAL_3_10_PLUS,
+ .pcr = PCR_COMPAT_3_10,
+ .pcr_level = PCR_COMPAT_3_10,
+ .max_vthreads = 8,
+ },
};
static const CompatInfo *compat_by_pvr(uint32_t pvr)
diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index f2301b43f78b..ece348178188 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -734,6 +734,8 @@
"POWER9 v2.2")
POWERPC_DEF("power10_v2.0", CPU_POWERPC_POWER10_DD20, POWER10,
"POWER10 v2.0")
+ POWERPC_DEF("power11_v2.0", CPU_POWERPC_POWER11_DD20, POWER11,
+ "POWER11_v2.0")
#endif /* defined (TARGET_PPC64) */
/***************************************************************************/
@@ -909,6 +911,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
{ "power8nvl", "power8nvl_v1.0" },
{ "power9", "power9_v2.2" },
{ "power10", "power10_v2.0" },
+ { "power11", "power11_v2.0" },
#endif
/* Generic PowerPCs */
diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index 0229ef3a9a5c..ef74e387b047 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -354,6 +354,8 @@ enum {
CPU_POWERPC_POWER10_BASE = 0x00800000,
CPU_POWERPC_POWER10_DD1 = 0x00801100,
CPU_POWERPC_POWER10_DD20 = 0x00801200,
+ CPU_POWERPC_POWER11_BASE = 0x00820000,
+ CPU_POWERPC_POWER11_DD20 = 0x00821200,
CPU_POWERPC_970_v22 = 0x00390202,
CPU_POWERPC_970FX_v10 = 0x00391100,
CPU_POWERPC_970FX_v20 = 0x003C0200,
@@ -391,6 +393,7 @@ enum {
CPU_POWERPC_LOGICAL_2_07 = 0x0F000004,
CPU_POWERPC_LOGICAL_3_00 = 0x0F000005,
CPU_POWERPC_LOGICAL_3_10 = 0x0F000006,
+ CPU_POWERPC_LOGICAL_3_10_PLUS = 0x0F000007,
};
/* System version register (used on MPC 8xxx) */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 3d8a112935ae..9aa098935d05 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6669,6 +6669,60 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
pcc->l1_icache_size = 0x8000;
}
+static bool ppc_pvr_match_power11(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
+{
+ uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
+ uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
+
+ if (!best && (base == CPU_POWERPC_POWER11_BASE)) {
+ return true;
+ }
+
+ if (base != pcc_base) {
+ return false;
+ }
+
+ if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
+ return true;
+ }
+
+ return false;
+}
+
+POWERPC_FAMILY(POWER11)(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+ PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+
+ dc->fw_name = "PowerPC,POWER11";
+ dc->desc = "POWER11";
+ pcc->pvr_match = ppc_pvr_match_power11;
+ pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
+ pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
+ pcc->init_proc = init_proc_POWER10;
+ pcc->check_pow = check_pow_nocheck;
+ pcc->check_attn = check_attn_hid0_power9;
+ pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
+ pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
+ pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
+ pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
+
+ pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
+ pcc->mmu_model = POWERPC_MMU_3_00;
+#if !defined(CONFIG_USER_ONLY)
+ /* segment page size remain the same */
+ pcc->hash64_opts = &ppc_hash64_opts_POWER7;
+ pcc->radix_page_info = &POWER10_radix_page_info;
+ pcc->lrg_decr_bits = 56;
+#endif
+ pcc->excp_model = POWERPC_EXCP_POWER10;
+ pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
+ pcc->bfd_mach = bfd_mach_ppc64;
+ pcc->flags = POWERPC_POWER10_PCC_FLAGS;
+ pcc->l1_dcache_size = 0x8000;
+ pcc->l1_icache_size = 0x8000;
+}
+
#if !defined(CONFIG_USER_ONLY)
void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
{
--
2.45.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor
2024-06-06 12:16 ` [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor Aditya Gupta
@ 2024-07-23 4:30 ` Nicholas Piggin
2024-07-23 5:01 ` Aditya Gupta
2024-07-23 5:27 ` Nicholas Piggin
1 sibling, 1 reply; 24+ messages in thread
From: Nicholas Piggin @ 2024-07-23 4:30 UTC (permalink / raw)
To: Aditya Gupta, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza,
Frédéric Barrat
On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
> Add CPU target code to add support for new Power11 Processor.
>
> Power11 core is same as Power10, hence reuse functions defined for
> Power10.
>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Frédéric Barrat <fbarrat@linux.ibm.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
> target/ppc/compat.c | 7 ++++++
> target/ppc/cpu-models.c | 3 +++
> target/ppc/cpu-models.h | 3 +++
> target/ppc/cpu_init.c | 54 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 67 insertions(+)
>
> diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> index ebef2cccecf3..12dd8ae290ca 100644
> --- a/target/ppc/compat.c
> +++ b/target/ppc/compat.c
> @@ -100,6 +100,13 @@ static const CompatInfo compat_table[] = {
> .pcr_level = PCR_COMPAT_3_10,
> .max_vthreads = 8,
> },
> + { /* POWER11, ISA3.10 */
> + .name = "power11",
> + .pvr = CPU_POWERPC_LOGICAL_3_10_PLUS,
Might call that _P11 rather than _PLUS, but I can fold that in my tree.
> + .pcr = PCR_COMPAT_3_10,
> + .pcr_level = PCR_COMPAT_3_10,
> + .max_vthreads = 8,
> + },
> };
>
> static const CompatInfo *compat_by_pvr(uint32_t pvr)
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index f2301b43f78b..ece348178188 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
> @@ -734,6 +734,8 @@
> "POWER9 v2.2")
> POWERPC_DEF("power10_v2.0", CPU_POWERPC_POWER10_DD20, POWER10,
> "POWER10 v2.0")
> + POWERPC_DEF("power11_v2.0", CPU_POWERPC_POWER11_DD20, POWER11,
> + "POWER11_v2.0")
> #endif /* defined (TARGET_PPC64) */
>
> /***************************************************************************/
> @@ -909,6 +911,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
> { "power8nvl", "power8nvl_v1.0" },
> { "power9", "power9_v2.2" },
> { "power10", "power10_v2.0" },
> + { "power11", "power11_v2.0" },
> #endif
>
> /* Generic PowerPCs */
> diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
> index 0229ef3a9a5c..ef74e387b047 100644
> --- a/target/ppc/cpu-models.h
> +++ b/target/ppc/cpu-models.h
> @@ -354,6 +354,8 @@ enum {
> CPU_POWERPC_POWER10_BASE = 0x00800000,
> CPU_POWERPC_POWER10_DD1 = 0x00801100,
> CPU_POWERPC_POWER10_DD20 = 0x00801200,
> + CPU_POWERPC_POWER11_BASE = 0x00820000,
> + CPU_POWERPC_POWER11_DD20 = 0x00821200,
> CPU_POWERPC_970_v22 = 0x00390202,
> CPU_POWERPC_970FX_v10 = 0x00391100,
> CPU_POWERPC_970FX_v20 = 0x003C0200,
> @@ -391,6 +393,7 @@ enum {
> CPU_POWERPC_LOGICAL_2_07 = 0x0F000004,
> CPU_POWERPC_LOGICAL_3_00 = 0x0F000005,
> CPU_POWERPC_LOGICAL_3_10 = 0x0F000006,
> + CPU_POWERPC_LOGICAL_3_10_PLUS = 0x0F000007,
> };
>
> /* System version register (used on MPC 8xxx) */
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 3d8a112935ae..9aa098935d05 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6669,6 +6669,60 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> pcc->l1_icache_size = 0x8000;
> }
>
> +static bool ppc_pvr_match_power11(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
> +{
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (!best && (base == CPU_POWERPC_POWER11_BASE)) {
> + return true;
> + }
> +
> + if (base != pcc_base) {
> + return false;
> + }
> +
> + if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> +POWERPC_FAMILY(POWER11)(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> + PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> +
> + dc->fw_name = "PowerPC,POWER11";
> + dc->desc = "POWER11";
> + pcc->pvr_match = ppc_pvr_match_power11;
> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
> + pcc->init_proc = init_proc_POWER10;
> + pcc->check_pow = check_pow_nocheck;
> + pcc->check_attn = check_attn_hid0_power9;
> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
BTW., I still think all these new macros should be named after the exact
CPU, e.g., all these should be called POWER11 and the differences or
sameness should be handled in cpu_init.h.
I might tweak that and the names a bit locally (e.g., why is one type of
define called POWERPC_FAMILY_x and another called POWERPC_x_PCC), but
that's not a big deal and mostly an exercise in bike shed painting. The
functionality of the patch looks okay.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> +
> + pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
> + pcc->mmu_model = POWERPC_MMU_3_00;
> +#if !defined(CONFIG_USER_ONLY)
> + /* segment page size remain the same */
> + pcc->hash64_opts = &ppc_hash64_opts_POWER7;
> + pcc->radix_page_info = &POWER10_radix_page_info;
> + pcc->lrg_decr_bits = 56;
> +#endif
> + pcc->excp_model = POWERPC_EXCP_POWER10;
> + pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
> + pcc->bfd_mach = bfd_mach_ppc64;
> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
> + pcc->l1_dcache_size = 0x8000;
> + pcc->l1_icache_size = 0x8000;
> +}
> +
> #if !defined(CONFIG_USER_ONLY)
> void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
> {
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor
2024-07-23 4:30 ` Nicholas Piggin
@ 2024-07-23 5:01 ` Aditya Gupta
0 siblings, 0 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-07-23 5:01 UTC (permalink / raw)
To: Nicholas Piggin, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza,
Frédéric Barrat
Hi Nick,
On 23/07/24 10:00, Nicholas Piggin wrote:
>> <...snip...>
>>
>> + { /* POWER11, ISA3.10 */
>> + .name = "power11",
>> + .pvr = CPU_POWERPC_LOGICAL_3_10_PLUS,
> Might call that _P11 rather than _PLUS, but I can fold that in my tree.
Sure, makes sense, I can make these changes, and send a v2 soon.
>> <...snip...>
>>
>> +
>> +POWERPC_FAMILY(POWER11)(ObjectClass *oc, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(oc);
>> + PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
>> +
>> + dc->fw_name = "PowerPC,POWER11";
>> + dc->desc = "POWER11";
>> + pcc->pvr_match = ppc_pvr_match_power11;
>> + pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>> + pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
>> + pcc->init_proc = init_proc_POWER10;
>> + pcc->check_pow = check_pow_nocheck;
>> + pcc->check_attn = check_attn_hid0_power9;
>> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
>> + pcc->insns_flags2 = POWERPC_FAMILY_POWER10_INSNS_FLAGS2;
>> + pcc->msr_mask = POWERPC_POWER10_PCC_MSR_MASK;
>> + pcc->lpcr_mask = POWERPC_POWER10_PCC_LPCR_MASK;
> BTW., I still think all these new macros should be named after the exact
> CPU, e.g., all these should be called POWER11 and the differences or
> sameness should be handled in cpu_init.h.
Got it, can create macros for the Power11 things also.
Regarding this:
> + pcc->check_attn = check_attn_hid0_power9;
> + pcc->insns_flags = POWERPC_FAMILY_POWER9_INSNS_FLAGS; /* same as P9 */
Should I keep them same, or have *_POWER11_* counterparts ?
>
> I might tweak that and the names a bit locally (e.g., why is one type of
> define called POWERPC_FAMILY_x and another called POWERPC_x_PCC), but
> that's not a big deal and mostly an exercise in bike shed painting. The
> functionality of the patch looks okay.
I am okay if you want to do it, or i can do it in a separate follow up
patch.
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks for the tag Nick !
- Aditya Gupta
>
>> +
>> + pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>> + pcc->mmu_model = POWERPC_MMU_3_00;
>> +#if !defined(CONFIG_USER_ONLY)
>> + /* segment page size remain the same */
>> + pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>> + pcc->radix_page_info = &POWER10_radix_page_info;
>> + pcc->lrg_decr_bits = 56;
>> +#endif
>> + pcc->excp_model = POWERPC_EXCP_POWER10;
>> + pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>> + pcc->bfd_mach = bfd_mach_ppc64;
>> + pcc->flags = POWERPC_POWER10_PCC_FLAGS;
>> + pcc->l1_dcache_size = 0x8000;
>> + pcc->l1_icache_size = 0x8000;
>> +}
>> +
>> #if !defined(CONFIG_USER_ONLY)
>> void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
>> {
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor
2024-06-06 12:16 ` [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor Aditya Gupta
2024-07-23 4:30 ` Nicholas Piggin
@ 2024-07-23 5:27 ` Nicholas Piggin
1 sibling, 0 replies; 24+ messages in thread
From: Nicholas Piggin @ 2024-07-23 5:27 UTC (permalink / raw)
To: Aditya Gupta, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza,
Frédéric Barrat
On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
> Add CPU target code to add support for new Power11 Processor.
>
> Power11 core is same as Power10, hence reuse functions defined for
> Power10.
>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Frédéric Barrat <fbarrat@linux.ibm.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
> target/ppc/compat.c | 7 ++++++
> target/ppc/cpu-models.c | 3 +++
> target/ppc/cpu-models.h | 3 +++
> target/ppc/cpu_init.c | 54 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 67 insertions(+)
>
> diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> index ebef2cccecf3..12dd8ae290ca 100644
> --- a/target/ppc/compat.c
> +++ b/target/ppc/compat.c
> @@ -100,6 +100,13 @@ static const CompatInfo compat_table[] = {
> .pcr_level = PCR_COMPAT_3_10,
> .max_vthreads = 8,
> },
> + { /* POWER11, ISA3.10 */
> + .name = "power11",
> + .pvr = CPU_POWERPC_LOGICAL_3_10_PLUS,
> + .pcr = PCR_COMPAT_3_10,
> + .pcr_level = PCR_COMPAT_3_10,
> + .max_vthreads = 8,
> + },
> };
>
> static const CompatInfo *compat_by_pvr(uint32_t pvr)
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index f2301b43f78b..ece348178188 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
> @@ -734,6 +734,8 @@
> "POWER9 v2.2")
> POWERPC_DEF("power10_v2.0", CPU_POWERPC_POWER10_DD20, POWER10,
> "POWER10 v2.0")
> + POWERPC_DEF("power11_v2.0", CPU_POWERPC_POWER11_DD20, POWER11,
> + "POWER11_v2.0")
> #endif /* defined (TARGET_PPC64) */
>
> /***************************************************************************/
> @@ -909,6 +911,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
> { "power8nvl", "power8nvl_v1.0" },
> { "power9", "power9_v2.2" },
> { "power10", "power10_v2.0" },
> + { "power11", "power11_v2.0" },
> #endif
>
> /* Generic PowerPCs */
> diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
> index 0229ef3a9a5c..ef74e387b047 100644
> --- a/target/ppc/cpu-models.h
> +++ b/target/ppc/cpu-models.h
> @@ -354,6 +354,8 @@ enum {
> CPU_POWERPC_POWER10_BASE = 0x00800000,
> CPU_POWERPC_POWER10_DD1 = 0x00801100,
> CPU_POWERPC_POWER10_DD20 = 0x00801200,
> + CPU_POWERPC_POWER11_BASE = 0x00820000,
> + CPU_POWERPC_POWER11_DD20 = 0x00821200,
> CPU_POWERPC_970_v22 = 0x00390202,
> CPU_POWERPC_970FX_v10 = 0x00391100,
> CPU_POWERPC_970FX_v20 = 0x003C0200,
> @@ -391,6 +393,7 @@ enum {
> CPU_POWERPC_LOGICAL_2_07 = 0x0F000004,
> CPU_POWERPC_LOGICAL_3_00 = 0x0F000005,
> CPU_POWERPC_LOGICAL_3_10 = 0x0F000006,
> + CPU_POWERPC_LOGICAL_3_10_PLUS = 0x0F000007,
_PLUS (or POWER7P etc) was a model name itself and had different
PVR and maybe different features in some cases, which is a bit
different to the situation here. Could be confusing to call this
_PLUS, so I would just call it _3_10_P11, as annoying as it is.
Thanks,
Nick
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 3/5] ppc/pseries: Add Power11 cpu type
2024-06-06 12:16 [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
2024-06-06 12:16 ` [PATCH v5 1/5] target/ppc: reduce code duplication across Power9/10 init code Aditya Gupta
2024-06-06 12:16 ` [PATCH v5 2/5] target/ppc: Add Power11 DD2.0 processor Aditya Gupta
@ 2024-06-06 12:16 ` Aditya Gupta
2024-07-23 4:34 ` Nicholas Piggin
2024-06-06 12:16 ` [PATCH v5 4/5] target/ppc: Introduce 'PowerPCCPUClass::logical_pvr' Aditya Gupta
` (3 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Aditya Gupta @ 2024-06-06 12:16 UTC (permalink / raw)
To: Mahesh J Salgaonkar, Madhavan Srinivasan, Nicholas Piggin,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, David Gibson, Daniel Henrique Barboza,
Frédéric Barrat
Add sPAPR CPU Core definition for Power11
Cc: David Gibson <david@gibson.dropbear.id.au> (reviewer:sPAPR (pseries))
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com> (reviewer:sPAPR (pseries))
Cc: Cédric Le Goater <clg@kaod.org>
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Frédéric Barrat <fbarrat@linux.ibm.com>
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
docs/system/ppc/pseries.rst | 17 +++++++++++++----
hw/ppc/spapr_cpu_core.c | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/docs/system/ppc/pseries.rst b/docs/system/ppc/pseries.rst
index a876d897b6e4..bbc51aa7fcdb 100644
--- a/docs/system/ppc/pseries.rst
+++ b/docs/system/ppc/pseries.rst
@@ -14,10 +14,19 @@ virtualization capabilities.
Supported devices
=================
- * Multi processor support for many Power processors generations: POWER7,
- POWER7+, POWER8, POWER8NVL, POWER9, and Power10. Support for POWER5+ exists,
- but its state is unknown.
- * Interrupt Controller, XICS (POWER8) and XIVE (POWER9 and Power10)
+ * Multi processor support for many Power processors generations:
+ - POWER7, POWER7+
+ - POWER8, POWER8NVL
+ - POWER9
+ - Power10
+ - Power11
+ - Support for POWER5+ also exists, works with correct kernel/userspace
+ * Interrupt Controller
+ - XICS (POWER8)
+ - XIVE (Supported by below:)
+ - POWER9
+ - Power10
+ - Power11
* vPHB PCIe Host bridge.
* vscsi and vnet devices, compatible with the same devices available on a
PowerVM hypervisor with VIOS managing LPARs.
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index e7c9edd033c8..62416b7e0a7e 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -401,6 +401,7 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.2"),
DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
+ DEFINE_SPAPR_CPU_CORE_TYPE("power11_v2.0"),
#ifdef CONFIG_KVM
DEFINE_SPAPR_CPU_CORE_TYPE("host"),
#endif
--
2.45.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v5 3/5] ppc/pseries: Add Power11 cpu type
2024-06-06 12:16 ` [PATCH v5 3/5] ppc/pseries: Add Power11 cpu type Aditya Gupta
@ 2024-07-23 4:34 ` Nicholas Piggin
0 siblings, 0 replies; 24+ messages in thread
From: Nicholas Piggin @ 2024-07-23 4:34 UTC (permalink / raw)
To: Aditya Gupta, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, David Gibson, Daniel Henrique Barboza,
Frédéric Barrat
On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
> Add sPAPR CPU Core definition for Power11
>
> Cc: David Gibson <david@gibson.dropbear.id.au> (reviewer:sPAPR (pseries))
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com> (reviewer:sPAPR (pseries))
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Frédéric Barrat <fbarrat@linux.ibm.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> docs/system/ppc/pseries.rst | 17 +++++++++++++----
> hw/ppc/spapr_cpu_core.c | 1 +
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/docs/system/ppc/pseries.rst b/docs/system/ppc/pseries.rst
> index a876d897b6e4..bbc51aa7fcdb 100644
> --- a/docs/system/ppc/pseries.rst
> +++ b/docs/system/ppc/pseries.rst
> @@ -14,10 +14,19 @@ virtualization capabilities.
> Supported devices
> =================
>
> - * Multi processor support for many Power processors generations: POWER7,
> - POWER7+, POWER8, POWER8NVL, POWER9, and Power10. Support for POWER5+ exists,
> - but its state is unknown.
> - * Interrupt Controller, XICS (POWER8) and XIVE (POWER9 and Power10)
> + * Multi processor support for many Power processors generations:
> + - POWER7, POWER7+
> + - POWER8, POWER8NVL
> + - POWER9
> + - Power10
> + - Power11
> + - Support for POWER5+ also exists, works with correct kernel/userspace
> + * Interrupt Controller
> + - XICS (POWER8)
> + - XIVE (Supported by below:)
> + - POWER9
> + - Power10
> + - Power11
> * vPHB PCIe Host bridge.
> * vscsi and vnet devices, compatible with the same devices available on a
> PowerVM hypervisor with VIOS managing LPARs.
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index e7c9edd033c8..62416b7e0a7e 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -401,6 +401,7 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
> DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
> DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.2"),
> DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
> + DEFINE_SPAPR_CPU_CORE_TYPE("power11_v2.0"),
> #ifdef CONFIG_KVM
> DEFINE_SPAPR_CPU_CORE_TYPE("host"),
> #endif
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 4/5] target/ppc: Introduce 'PowerPCCPUClass::logical_pvr'
2024-06-06 12:16 [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
` (2 preceding siblings ...)
2024-06-06 12:16 ` [PATCH v5 3/5] ppc/pseries: Add Power11 cpu type Aditya Gupta
@ 2024-06-06 12:16 ` Aditya Gupta
2024-07-23 5:13 ` Nicholas Piggin
2024-06-06 12:16 ` [PATCH v5 5/5] target/ppc: Fix regression due to Power10 and Power11 having same PCR Aditya Gupta
` (2 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Aditya Gupta @ 2024-06-06 12:16 UTC (permalink / raw)
To: Mahesh J Salgaonkar, Madhavan Srinivasan, Nicholas Piggin,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
Introduce 'PnvChipClass::logical_pvr' to know corresponding logical PVR
of a PowerPC CPU.
This helps to have a one-to-one mapping between PVR and logical PVR for
a CPU, and used in a later commit to handle cases where PCR of two
generations of Power chip is same, which causes regressions with compat-mode.
Cc: Cédric Le Goater <clg@kaod.org>
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
target/ppc/cpu.h | 1 +
target/ppc/cpu_init.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2015e603d4e0..ff43e3645228 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1504,6 +1504,7 @@ struct PowerPCCPUClass {
void (*parent_parse_features)(const char *type, char *str, Error **errp);
uint32_t pvr;
+ uint32_t logical_pvr;
/*
* If @best is false, match if pcc is in the family of pvr
* Else match only if pcc is the best match for pvr in this family.
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 9aa098935d05..50f136cca7f0 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6152,6 +6152,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER7";
dc->desc = "POWER7";
+ pcc->logical_pvr = CPU_POWERPC_LOGICAL_2_06_PLUS;
pcc->pvr_match = ppc_pvr_match_power7;
pcc->pcr_mask = PCR_VEC_DIS | PCR_VSX_DIS | PCR_COMPAT_2_05;
pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
@@ -6315,6 +6316,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER8";
dc->desc = "POWER8";
+ pcc->logical_pvr = CPU_POWERPC_LOGICAL_2_07;
pcc->pvr_match = ppc_pvr_match_power8;
pcc->pcr_mask = PCR_TM_DIS | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
@@ -6508,6 +6510,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER9";
dc->desc = "POWER9";
+ pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_00;
pcc->pvr_match = ppc_pvr_match_power9;
pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
@@ -6642,6 +6645,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER10";
dc->desc = "POWER10";
+ pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_10;
pcc->pvr_match = ppc_pvr_match_power10;
pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
@@ -6696,6 +6700,7 @@ POWERPC_FAMILY(POWER11)(ObjectClass *oc, void *data)
dc->fw_name = "PowerPC,POWER11";
dc->desc = "POWER11";
+ pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_10_PLUS;
pcc->pvr_match = ppc_pvr_match_power11;
pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
--
2.45.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v5 4/5] target/ppc: Introduce 'PowerPCCPUClass::logical_pvr'
2024-06-06 12:16 ` [PATCH v5 4/5] target/ppc: Introduce 'PowerPCCPUClass::logical_pvr' Aditya Gupta
@ 2024-07-23 5:13 ` Nicholas Piggin
2024-07-23 5:42 ` Aditya Gupta
0 siblings, 1 reply; 24+ messages in thread
From: Nicholas Piggin @ 2024-07-23 5:13 UTC (permalink / raw)
To: Aditya Gupta, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
> Introduce 'PnvChipClass::logical_pvr' to know corresponding logical PVR
> of a PowerPC CPU.
> This helps to have a one-to-one mapping between PVR and logical PVR for
> a CPU, and used in a later commit to handle cases where PCR of two
> generations of Power chip is same, which causes regressions with compat-mode.
>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
> target/ppc/cpu.h | 1 +
> target/ppc/cpu_init.c | 5 +++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 2015e603d4e0..ff43e3645228 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1504,6 +1504,7 @@ struct PowerPCCPUClass {
> void (*parent_parse_features)(const char *type, char *str, Error **errp);
>
> uint32_t pvr;
> + uint32_t logical_pvr;
> /*
> * If @best is false, match if pcc is in the family of pvr
> * Else match only if pcc is the best match for pvr in this family.
I suppose so. pvr_match() is for hardware PVR, not logical. It's all
quite a maze.
I'll get you to re-post the series with paches 4-5 reordered ahead
of the power11 addition, so you can do the renaming and tweaking :)
Maybe call this 'spapr_logical_pvr' so it's clearly separate from the
other pvr matching.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 9aa098935d05..50f136cca7f0 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6152,6 +6152,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>
> dc->fw_name = "PowerPC,POWER7";
> dc->desc = "POWER7";
> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_2_06_PLUS;
> pcc->pvr_match = ppc_pvr_match_power7;
> pcc->pcr_mask = PCR_VEC_DIS | PCR_VSX_DIS | PCR_COMPAT_2_05;
> pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
> @@ -6315,6 +6316,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>
> dc->fw_name = "PowerPC,POWER8";
> dc->desc = "POWER8";
> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_2_07;
> pcc->pvr_match = ppc_pvr_match_power8;
> pcc->pcr_mask = PCR_TM_DIS | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
> pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
> @@ -6508,6 +6510,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>
> dc->fw_name = "PowerPC,POWER9";
> dc->desc = "POWER9";
> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_00;
> pcc->pvr_match = ppc_pvr_match_power9;
> pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
> pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
> @@ -6642,6 +6645,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>
> dc->fw_name = "PowerPC,POWER10";
> dc->desc = "POWER10";
> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_10;
> pcc->pvr_match = ppc_pvr_match_power10;
> pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
> pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
> @@ -6696,6 +6700,7 @@ POWERPC_FAMILY(POWER11)(ObjectClass *oc, void *data)
>
> dc->fw_name = "PowerPC,POWER11";
> dc->desc = "POWER11";
> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_10_PLUS;
> pcc->pvr_match = ppc_pvr_match_power11;
> pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
> pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 4/5] target/ppc: Introduce 'PowerPCCPUClass::logical_pvr'
2024-07-23 5:13 ` Nicholas Piggin
@ 2024-07-23 5:42 ` Aditya Gupta
0 siblings, 0 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-07-23 5:42 UTC (permalink / raw)
To: Nicholas Piggin, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
On 23/07/24 10:43, Nicholas Piggin wrote:
> On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
>> Introduce 'PnvChipClass::logical_pvr' to know corresponding logical PVR
>> of a PowerPC CPU.
>> This helps to have a one-to-one mapping between PVR and logical PVR for
>> a CPU, and used in a later commit to handle cases where PCR of two
>> generations of Power chip is same, which causes regressions with compat-mode.
>> Cc: Cédric Le Goater <clg@kaod.org>
>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
>> ---
>> target/ppc/cpu.h | 1 +
>> target/ppc/cpu_init.c | 5 +++++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index 2015e603d4e0..ff43e3645228 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -1504,6 +1504,7 @@ struct PowerPCCPUClass {
>> void (*parent_parse_features)(const char *type, char *str, Error **errp);
>>
>> uint32_t pvr;
>> + uint32_t logical_pvr;
>> /*
>> * If @best is false, match if pcc is in the family of pvr
>> * Else match only if pcc is the best match for pvr in this family.
> I suppose so. pvr_match() is for hardware PVR, not logical. It's all
> quite a maze.
>
> I'll get you to re-post the series with paches 4-5 reordered ahead
> of the power11 addition, so you can do the renaming and tweaking :)
> Maybe call this 'spapr_logical_pvr' so it's clearly separate from the
> other pvr matching.
Sure, I am reading your reviews about the renaming. Will do them, should
send next version by today.
Thanks for all reviews !
- Aditya Gupta
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 9aa098935d05..50f136cca7f0 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -6152,6 +6152,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>
>> dc->fw_name = "PowerPC,POWER7";
>> dc->desc = "POWER7";
>> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_2_06_PLUS;
>> pcc->pvr_match = ppc_pvr_match_power7;
>> pcc->pcr_mask = PCR_VEC_DIS | PCR_VSX_DIS | PCR_COMPAT_2_05;
>> pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>> @@ -6315,6 +6316,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>
>> dc->fw_name = "PowerPC,POWER8";
>> dc->desc = "POWER8";
>> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_2_07;
>> pcc->pvr_match = ppc_pvr_match_power8;
>> pcc->pcr_mask = PCR_TM_DIS | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>> pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>> @@ -6508,6 +6510,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>>
>> dc->fw_name = "PowerPC,POWER9";
>> dc->desc = "POWER9";
>> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_00;
>> pcc->pvr_match = ppc_pvr_match_power9;
>> pcc->pcr_mask = POWERPC_POWER9_PCC_PCR_MASK;
>> pcc->pcr_supported = POWERPC_POWER9_PCC_PCR_SUPPORTED;
>> @@ -6642,6 +6645,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>>
>> dc->fw_name = "PowerPC,POWER10";
>> dc->desc = "POWER10";
>> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_10;
>> pcc->pvr_match = ppc_pvr_match_power10;
>> pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>> pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
>> @@ -6696,6 +6700,7 @@ POWERPC_FAMILY(POWER11)(ObjectClass *oc, void *data)
>>
>> dc->fw_name = "PowerPC,POWER11";
>> dc->desc = "POWER11";
>> + pcc->logical_pvr = CPU_POWERPC_LOGICAL_3_10_PLUS;
>> pcc->pvr_match = ppc_pvr_match_power11;
>> pcc->pcr_mask = POWERPC_POWER10_PCC_PCR_MASK;
>> pcc->pcr_supported = POWERPC_POWER10_PCC_PCR_SUPPORTED;
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5 5/5] target/ppc: Fix regression due to Power10 and Power11 having same PCR
2024-06-06 12:16 [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
` (3 preceding siblings ...)
2024-06-06 12:16 ` [PATCH v5 4/5] target/ppc: Introduce 'PowerPCCPUClass::logical_pvr' Aditya Gupta
@ 2024-06-06 12:16 ` Aditya Gupta
2024-07-23 4:58 ` Nicholas Piggin
2024-06-06 12:22 ` [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
2024-07-22 9:12 ` Aditya Gupta
6 siblings, 1 reply; 24+ messages in thread
From: Aditya Gupta @ 2024-06-06 12:16 UTC (permalink / raw)
To: Mahesh J Salgaonkar, Madhavan Srinivasan, Nicholas Piggin,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
Power11 has the same PCR (Processor Compatibility Register) value, as
Power10.
Due to this, QEMU considers Power11 as a valid compat-mode for Power10,
ie. earlier it was possible to run QEMU with '-M pseries,max-compat-mode=power11 --cpu power10'
Same PCR also introduced a regression where `-M pseries --cpu power10`
boots as Power11 (ie. logical PVR is of Power11, even though PVR is Power10).
The regression was due to 'do_client_architecture_support' checking for
valid compat modes and finding Power11 to be a valid compat mode for
Power10 (it happens even without passing 'max-compat-mode' explicitly).
Fix compat-mode issue and regression, by ensuring a future Power
processor (with a higher logical_pvr value, eg. P11) cannot be valid
compat-mode for an older Power processor (eg. P10)
Cc: Cédric Le Goater <clg@kaod.org>
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
target/ppc/compat.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/target/ppc/compat.c b/target/ppc/compat.c
index 12dd8ae290ca..168a3c06316f 100644
--- a/target/ppc/compat.c
+++ b/target/ppc/compat.c
@@ -139,6 +139,10 @@ static bool pcc_compat(PowerPCCPUClass *pcc, uint32_t compat_pvr,
/* Outside specified range */
return false;
}
+ if (compat->pvr > pcc->logical_pvr) {
+ /* Older CPU cannot support a newer processor's compat mode */
+ return false;
+ }
if (!(pcc->pcr_supported & compat->pcr_level)) {
/* Not supported by this CPU */
return false;
--
2.45.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v5 5/5] target/ppc: Fix regression due to Power10 and Power11 having same PCR
2024-06-06 12:16 ` [PATCH v5 5/5] target/ppc: Fix regression due to Power10 and Power11 having same PCR Aditya Gupta
@ 2024-07-23 4:58 ` Nicholas Piggin
2024-07-23 5:08 ` Aditya Gupta
0 siblings, 1 reply; 24+ messages in thread
From: Nicholas Piggin @ 2024-07-23 4:58 UTC (permalink / raw)
To: Aditya Gupta, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
> Power11 has the same PCR (Processor Compatibility Register) value, as
> Power10.
>
> Due to this, QEMU considers Power11 as a valid compat-mode for Power10,
> ie. earlier it was possible to run QEMU with '-M pseries,max-compat-mode=power11 --cpu power10'
Isn't this expected to work, or no?
>
> Same PCR also introduced a regression where `-M pseries --cpu power10`
> boots as Power11 (ie. logical PVR is of Power11, even though PVR is Power10).
> The regression was due to 'do_client_architecture_support' checking for
> valid compat modes and finding Power11 to be a valid compat mode for
> Power10 (it happens even without passing 'max-compat-mode' explicitly).
>
> Fix compat-mode issue and regression, by ensuring a future Power
> processor (with a higher logical_pvr value, eg. P11) cannot be valid
> compat-mode for an older Power processor (eg. P10)
This should be done before introducing the Power11 CPU so there's no
regression inside the series.
>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
> target/ppc/compat.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> index 12dd8ae290ca..168a3c06316f 100644
> --- a/target/ppc/compat.c
> +++ b/target/ppc/compat.c
> @@ -139,6 +139,10 @@ static bool pcc_compat(PowerPCCPUClass *pcc, uint32_t compat_pvr,
> /* Outside specified range */
> return false;
> }
> + if (compat->pvr > pcc->logical_pvr) {
> + /* Older CPU cannot support a newer processor's compat mode */
> + return false;
> + }
Hmm. I suppose this is the right way to fix it.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> if (!(pcc->pcr_supported & compat->pcr_level)) {
> /* Not supported by this CPU */
> return false;
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 5/5] target/ppc: Fix regression due to Power10 and Power11 having same PCR
2024-07-23 4:58 ` Nicholas Piggin
@ 2024-07-23 5:08 ` Aditya Gupta
0 siblings, 0 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-07-23 5:08 UTC (permalink / raw)
To: Nicholas Piggin, Mahesh J Salgaonkar, Madhavan Srinivasan,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
On 23/07/24 10:28, Nicholas Piggin wrote:
> On Thu Jun 6, 2024 at 10:16 PM AEST, Aditya Gupta wrote:
>> Power11 has the same PCR (Processor Compatibility Register) value, as
>> Power10.
>>
>> Due to this, QEMU considers Power11 as a valid compat-mode for Power10,
>> ie. earlier it was possible to run QEMU with '-M pseries,max-compat-mode=power11 --cpu power10'
> Isn't this expected to work, or no?
It works, but it didn't feel logical to be able to boot an older CPU in
a compat mode of a newer CPU.
Major reason though, it caused regression where `-M pseries --cpu
power10` was booting as Power11, since Power11 was the highest PCR it
found as compatible.
The only issue I see in this patch is the assumption that a newer
processor must always have a higher PVR value, which is true as of now.
>
>> Same PCR also introduced a regression where `-M pseries --cpu power10`
>> boots as Power11 (ie. logical PVR is of Power11, even though PVR is Power10).
>> The regression was due to 'do_client_architecture_support' checking for
>> valid compat modes and finding Power11 to be a valid compat mode for
>> Power10 (it happens even without passing 'max-compat-mode' explicitly).
>>
>> Fix compat-mode issue and regression, by ensuring a future Power
>> processor (with a higher logical_pvr value, eg. P11) cannot be valid
>> compat-mode for an older Power processor (eg. P10)
> This should be done before introducing the Power11 CPU so there's no
> regression inside the series.
Sure, I will move it before the 'Add Power11 DD2.0 processor' patch.
>> Cc: Cédric Le Goater <clg@kaod.org>
>> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
>> Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> Cc: Mahesh J Salgaonkar <mahesh@linux.ibm.com>
>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
>> ---
>> target/ppc/compat.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/target/ppc/compat.c b/target/ppc/compat.c
>> index 12dd8ae290ca..168a3c06316f 100644
>> --- a/target/ppc/compat.c
>> +++ b/target/ppc/compat.c
>> @@ -139,6 +139,10 @@ static bool pcc_compat(PowerPCCPUClass *pcc, uint32_t compat_pvr,
>> /* Outside specified range */
>> return false;
>> }
>> + if (compat->pvr > pcc->logical_pvr) {
>> + /* Older CPU cannot support a newer processor's compat mode */
>> + return false;
>> + }
> Hmm. I suppose this is the right way to fix it.
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thank you for the tag, Nick !
- Aditya Gupta
>> if (!(pcc->pcr_supported & compat->pcr_level)) {
>> /* Not supported by this CPU */
>> return false;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5 0/5] Power11 support for QEMU [PSeries]
2024-06-06 12:16 [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
` (4 preceding siblings ...)
2024-06-06 12:16 ` [PATCH v5 5/5] target/ppc: Fix regression due to Power10 and Power11 having same PCR Aditya Gupta
@ 2024-06-06 12:22 ` Aditya Gupta
2024-07-22 9:12 ` Aditya Gupta
6 siblings, 0 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-06-06 12:22 UTC (permalink / raw)
To: Mahesh J Salgaonkar, Madhavan Srinivasan, Nicholas Piggin,
Cédric Le Goater, Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc
Hello Nick & Cedric,
Based on your comments on considering the pseries for 9.1, and having to
wait for skiboot parts for powernv, I have split the patch series into
pseries and powernv.
There might be little delay in posting the powernv part, where I am
still looking into having power11's instance_init.
Also, I have applied Harsh's patch that should simplify the rest of the
patches.
Thanks,
Aditya Gupta
On 06/06/24 17:46, Aditya Gupta wrote:
> Overview
> ============
>
> Split "Power11 support for QEMU" into 2 patch series: pseries & powernv.
>
> This patch series is for pseries support for Power11.
>
> As Power11 core is same as Power10, hence much of the code has been reused from
> Power10.
>
> Power11 was added in Linux in:
> commit c2ed087ed35c ("powerpc: Add Power11 architected and raw mode")
>
> Git Tree for Testing
> ====================
>
> QEMU: https://github.com/adi-g15-ibm/qemu/tree/p11-v5-pseries
>
> Has been tested with following cases:
> * '-M pseries' / '-M pseries -cpu Power11'
> * '-smp' option tested
> * with compat mode: 'max-cpu-compat=power10' and 'max-cpu-compat=power9'
> * with/without device 'virtio-scsi-pci'
> * with/without -kernel and -drive with qcow_file
>
> Linux with Power11 support: https://github.com/torvalds/linux, since v6.9-rc1
>
> Changelog
> =========
> v5:
> + split patch series into pseries+powernv
> + patch #1: apply harsh's patch to reduce duplication
> + patch #2: simplified, by removing duplication
> + patch #3: update docs, according to harsh's suggestion
> + patch #4: no functional change, #define used for P9 & P10 pcr_supported
> + patch #5: no change
>
> v4:
> + patch #5: fix memory leak in pnv_chip_power10_quad_realize
> - no change in other patches
>
> v3:
> + patch #1: version power11 as power11_v2.0
> + patch #2: split target hw/pseries code into patch #2
> + patch #3,#4: fix regression due to Power10 and Power11 having same PCR
> + patch #5: create pnv_chip_power11_dt_populate and split pnv_chip_power10_common_realize as per review
> + patch #6-#11: no change
> - remove commit to make Power11 as default
>
> v2:
> + split powernv patch into homer,lpc,occ,psi,sbe
> + reduce code duplication by reusing power10 code
> + make power11 as default
> + rebase on qemu upstream/master
> + add more information in commit descriptions
> + update docs
> + update skiboot.lid
>
>
> Aditya Gupta (4):
> target/ppc: Add Power11 DD2.0 processor
> ppc/pseries: Add Power11 cpu type
> target/ppc: Introduce 'PowerPCCPUClass::logical_pvr'
> target/ppc: Fix regression due to Power10 and Power11 having same PCR
>
> Harsh Prateek Bora (1):
> target/ppc: reduce code duplication across Power9/10 init code
>
> docs/system/ppc/pseries.rst | 17 +++-
> hw/ppc/spapr_cpu_core.c | 1 +
> target/ppc/compat.c | 11 +++
> target/ppc/cpu-models.c | 3 +
> target/ppc/cpu-models.h | 3 +
> target/ppc/cpu.h | 1 +
> target/ppc/cpu_init.c | 183 +++++++++++++++---------------------
> target/ppc/cpu_init.h | 78 +++++++++++++++
> 8 files changed, 184 insertions(+), 113 deletions(-)
> create mode 100644 target/ppc/cpu_init.h
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v5 0/5] Power11 support for QEMU [PSeries]
2024-06-06 12:16 [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
` (5 preceding siblings ...)
2024-06-06 12:22 ` [PATCH v5 0/5] Power11 support for QEMU [PSeries] Aditya Gupta
@ 2024-07-22 9:12 ` Aditya Gupta
6 siblings, 0 replies; 24+ messages in thread
From: Aditya Gupta @ 2024-07-22 9:12 UTC (permalink / raw)
To: Nicholas Piggin, Cédric Le Goater
Cc: qemu-devel, qemu-ppc, Harsh Prateek Bora, Madhavan Srinivasan,
Mahesh J Salgaonkar
Any comments on this ?
This series is containing only the pseries support for Power11, hence
independent of skiboot patches. powernv is on hold till skiboot changes
are released.
Thanks,
Aditya Gupta
On 06/06/24 17:46, Aditya Gupta wrote:
> Overview
> ============
>
> Split "Power11 support for QEMU" into 2 patch series: pseries & powernv.
>
> This patch series is for pseries support for Power11.
>
> As Power11 core is same as Power10, hence much of the code has been reused from
> Power10.
>
> Power11 was added in Linux in:
> commit c2ed087ed35c ("powerpc: Add Power11 architected and raw mode")
>
> Git Tree for Testing
> ====================
>
> QEMU: https://github.com/adi-g15-ibm/qemu/tree/p11-v5-pseries
>
> Has been tested with following cases:
> * '-M pseries' / '-M pseries -cpu Power11'
> * '-smp' option tested
> * with compat mode: 'max-cpu-compat=power10' and 'max-cpu-compat=power9'
> * with/without device 'virtio-scsi-pci'
> * with/without -kernel and -drive with qcow_file
>
> Linux with Power11 support: https://github.com/torvalds/linux, since v6.9-rc1
>
> Changelog
> =========
> v5:
> + split patch series into pseries+powernv
> + patch #1: apply harsh's patch to reduce duplication
> + patch #2: simplified, by removing duplication
> + patch #3: update docs, according to harsh's suggestion
> + patch #4: no functional change, #define used for P9 & P10 pcr_supported
> + patch #5: no change
>
> v4:
> + patch #5: fix memory leak in pnv_chip_power10_quad_realize
> - no change in other patches
>
> v3:
> + patch #1: version power11 as power11_v2.0
> + patch #2: split target hw/pseries code into patch #2
> + patch #3,#4: fix regression due to Power10 and Power11 having same PCR
> + patch #5: create pnv_chip_power11_dt_populate and split pnv_chip_power10_common_realize as per review
> + patch #6-#11: no change
> - remove commit to make Power11 as default
>
> v2:
> + split powernv patch into homer,lpc,occ,psi,sbe
> + reduce code duplication by reusing power10 code
> + make power11 as default
> + rebase on qemu upstream/master
> + add more information in commit descriptions
> + update docs
> + update skiboot.lid
>
>
> Aditya Gupta (4):
> target/ppc: Add Power11 DD2.0 processor
> ppc/pseries: Add Power11 cpu type
> target/ppc: Introduce 'PowerPCCPUClass::logical_pvr'
> target/ppc: Fix regression due to Power10 and Power11 having same PCR
>
> Harsh Prateek Bora (1):
> target/ppc: reduce code duplication across Power9/10 init code
>
> docs/system/ppc/pseries.rst | 17 +++-
> hw/ppc/spapr_cpu_core.c | 1 +
> target/ppc/compat.c | 11 +++
> target/ppc/cpu-models.c | 3 +
> target/ppc/cpu-models.h | 3 +
> target/ppc/cpu.h | 1 +
> target/ppc/cpu_init.c | 183 +++++++++++++++---------------------
> target/ppc/cpu_init.h | 78 +++++++++++++++
> 8 files changed, 184 insertions(+), 113 deletions(-)
> create mode 100644 target/ppc/cpu_init.h
>
^ permalink raw reply [flat|nested] 24+ messages in thread