[PATCH 00/13] misc ppc patches

[PATCH 00/13] misc ppc patches

[Nicholas Piggin]

Here's a bunch of patches that have been on the list in various
forms, but haven't made it upstream yet. I would like to get
them merged for 9.0 if possible. The checkstop and attn ones are
bigger but they're not so important so I could defer them.

Main change is merging Ben's DEXCR patch from last year with my
POWER10 pa-features patch -- his was posted first and added a
POWER10 entry so I took his but updated title and changelog.

And changing the SAO and copy-paste patches to always cease
advertising them, rather than depending on the host, as per
discussion with David.

Thanks,
Nick

Benjamin Gray (1):
  ppc/spapr: Add pa-features for POWER10 machines

Nicholas Piggin (12):
  ppc: Drop support for POWER9 and POWER10 DD1 chips
  target/ppc: POWER10 does not have transactional memory
  ppc/spapr|pnv: Remove SAO from pa-features
  ppc/spapr: Remove copy-paste from pa-features
  ppc/spapr: Adjust ibm,pa-features for POWER9
  ppc/pnv: Permit ibm,pa-features set per machine variant
  ppc/pnv: Set POWER9, POWER10 ibm,pa-features bits
  target/ppc: Prevent supervisor from modifying MSR[ME]
  spapr: set MSR[ME] and MSR[FP] on client entry
  target/ppc: Make checkstop actually stop the system
  target/ppc: improve checkstop logging
  target/ppc: Implement attn instruction on BookS 64-bit processors

 target/ppc/cpu.h         |  16 +++++-
 target/ppc/helper.h      |   4 ++
 hw/ppc/pnv.c             | 104 ++++++++++++++++++++++++++++++++++-----
 hw/ppc/spapr.c           |  63 ++++++++++++++++++++----
 hw/ppc/spapr_cpu_core.c  |   4 +-
 target/ppc/cpu-models.c  |   4 --
 target/ppc/cpu_init.c    |  93 +++++++++++++++++++++++++++++-----
 target/ppc/excp_helper.c |  52 +++++++++++++++-----
 target/ppc/helper_regs.c |   5 ++
 target/ppc/kvm.c         |  11 -----
 target/ppc/translate.c   |  11 +++++
 11 files changed, 304 insertions(+), 63 deletions(-)

-- 
2.42.0

[PATCH 01/13] ppc: Drop support for POWER9 and POWER10 DD1 chips

[Nicholas Piggin]

The POWER9 DD1 and POWER10 DD1 chips are not public and are no longer of
any use in QEMU. Remove them.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr_cpu_core.c |  2 --
 target/ppc/cpu-models.c |  4 ----
 target/ppc/cpu_init.c   |  7 ++-----
 target/ppc/kvm.c        | 11 -----------
 4 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 40b7c52f7f..50523ead25 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -394,10 +394,8 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
     DEFINE_SPAPR_CPU_CORE_TYPE("power8_v2.0"),
     DEFINE_SPAPR_CPU_CORE_TYPE("power8e_v2.1"),
     DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"),
-    DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
     DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
     DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.2"),
-    DEFINE_SPAPR_CPU_CORE_TYPE("power10_v1.0"),
     DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
 #ifdef CONFIG_KVM
     DEFINE_SPAPR_CPU_CORE_TYPE("host"),
diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index 36e465b390..f2301b43f7 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -728,14 +728,10 @@
                 "POWER8 v2.0")
     POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10,         POWER8,
                 "POWER8NVL v1.0")
-    POWERPC_DEF("power9_v1.0",   CPU_POWERPC_POWER9_DD1,             POWER9,
-                "POWER9 v1.0")
     POWERPC_DEF("power9_v2.0",   CPU_POWERPC_POWER9_DD20,            POWER9,
                 "POWER9 v2.0")
     POWERPC_DEF("power9_v2.2",   CPU_POWERPC_POWER9_DD22,            POWER9,
                 "POWER9 v2.2")
-    POWERPC_DEF("power10_v1.0",  CPU_POWERPC_POWER10_DD1,            POWER10,
-                "POWER10 v1.0")
     POWERPC_DEF("power10_v2.0",  CPU_POWERPC_POWER10_DD20,           POWER10,
                 "POWER10 v2.0")
 #endif /* defined (TARGET_PPC64) */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 1d3d1db7c3..572cbdf25f 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6350,10 +6350,7 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
         return false;
     }
 
-    if ((pvr & 0x0f00) == 0x100) {
-        /* DD1.x always matches power9_v1.0 */
-        return true;
-    } else if ((pvr & 0x0f00) == 0x200) {
+    if ((pvr & 0x0f00) == 0x200) {
         if ((pvr & 0xf) < 2) {
             /* DD2.0, DD2.1 match power9_v2.0 */
             if ((pcc->pvr & 0xf) == 0) {
@@ -6536,7 +6533,7 @@ static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
     }
 
     if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
-        /* Major DD version matches to power10_v1.0 and power10_v2.0 */
+        /* Major DD version matches power10_v2.0 */
         return true;
     }
 
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index bcf30a5400..525fbe3892 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2369,17 +2369,6 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
 
 #if defined(TARGET_PPC64)
     pcc->radix_page_info = kvmppc_get_radix_page_info();
-
-    if ((pcc->pvr & 0xffffff00) == CPU_POWERPC_POWER9_DD1) {
-        /*
-         * POWER9 DD1 has some bugs which make it not really ISA 3.00
-         * compliant.  More importantly, advertising ISA 3.00
-         * architected mode may prevent guests from activating
-         * necessary DD1 workarounds.
-         */
-        pcc->pcr_supported &= ~(PCR_COMPAT_3_00 | PCR_COMPAT_2_07
-                                | PCR_COMPAT_2_06 | PCR_COMPAT_2_05);
-    }
 #endif /* defined(TARGET_PPC64) */
 }
 
-- 
2.42.0

Re: [PATCH 01/13] ppc: Drop support for POWER9 and POWER10 DD1 chips

[Harsh Prateek Bora]

On 3/12/24 00:21, Nicholas Piggin wrote:
> The POWER9 DD1 and POWER10 DD1 chips are not public and are no longer of
> any use in QEMU. Remove them.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/spapr_cpu_core.c |  2 --
>   target/ppc/cpu-models.c |  4 ----
>   target/ppc/cpu_init.c   |  7 ++-----
>   target/ppc/kvm.c        | 11 -----------
>   4 files changed, 2 insertions(+), 22 deletions(-)

Do we want to squash in removal of the macro as well?

diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index 0229ef3a9a..a5167873ae 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -348,7 +348,6 @@ enum {
      CPU_POWERPC_POWER8NVL_BASE     = 0x004C0000,
      CPU_POWERPC_POWER8NVL_v10      = 0x004C0100,
      CPU_POWERPC_POWER9_BASE        = 0x004E0000,
-    CPU_POWERPC_POWER9_DD1         = 0x004E1100,
      CPU_POWERPC_POWER9_DD20        = 0x004E1200,
      CPU_POWERPC_POWER9_DD22        = 0x004E1202,
      CPU_POWERPC_POWER10_BASE       = 0x00800000,

With that,

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

regards,
Harsh

> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 40b7c52f7f..50523ead25 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -394,10 +394,8 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>       DEFINE_SPAPR_CPU_CORE_TYPE("power8_v2.0"),
>       DEFINE_SPAPR_CPU_CORE_TYPE("power8e_v2.1"),
>       DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"),
> -    DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
>       DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
>       DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.2"),
> -    DEFINE_SPAPR_CPU_CORE_TYPE("power10_v1.0"),
>       DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
>   #ifdef CONFIG_KVM
>       DEFINE_SPAPR_CPU_CORE_TYPE("host"),
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index 36e465b390..f2301b43f7 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
> @@ -728,14 +728,10 @@
>                   "POWER8 v2.0")
>       POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10,         POWER8,
>                   "POWER8NVL v1.0")
> -    POWERPC_DEF("power9_v1.0",   CPU_POWERPC_POWER9_DD1,             POWER9,
> -                "POWER9 v1.0")
>       POWERPC_DEF("power9_v2.0",   CPU_POWERPC_POWER9_DD20,            POWER9,
>                   "POWER9 v2.0")
>       POWERPC_DEF("power9_v2.2",   CPU_POWERPC_POWER9_DD22,            POWER9,
>                   "POWER9 v2.2")
> -    POWERPC_DEF("power10_v1.0",  CPU_POWERPC_POWER10_DD1,            POWER10,
> -                "POWER10 v1.0")
>       POWERPC_DEF("power10_v2.0",  CPU_POWERPC_POWER10_DD20,           POWER10,
>                   "POWER10 v2.0")
>   #endif /* defined (TARGET_PPC64) */
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 1d3d1db7c3..572cbdf25f 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6350,10 +6350,7 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
>           return false;
>       }
>   
> -    if ((pvr & 0x0f00) == 0x100) {
> -        /* DD1.x always matches power9_v1.0 */
> -        return true;
> -    } else if ((pvr & 0x0f00) == 0x200) {
> +    if ((pvr & 0x0f00) == 0x200) {
>           if ((pvr & 0xf) < 2) {
>               /* DD2.0, DD2.1 match power9_v2.0 */
>               if ((pcc->pvr & 0xf) == 0) {
> @@ -6536,7 +6533,7 @@ static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
>       }
>   
>       if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
> -        /* Major DD version matches to power10_v1.0 and power10_v2.0 */
> +        /* Major DD version matches power10_v2.0 */
>           return true;
>       }
>   
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index bcf30a5400..525fbe3892 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2369,17 +2369,6 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
>   
>   #if defined(TARGET_PPC64)
>       pcc->radix_page_info = kvmppc_get_radix_page_info();
> -
> -    if ((pcc->pvr & 0xffffff00) == CPU_POWERPC_POWER9_DD1) {
> -        /*
> -         * POWER9 DD1 has some bugs which make it not really ISA 3.00
> -         * compliant.  More importantly, advertising ISA 3.00
> -         * architected mode may prevent guests from activating
> -         * necessary DD1 workarounds.
> -         */
> -        pcc->pcr_supported &= ~(PCR_COMPAT_3_00 | PCR_COMPAT_2_07
> -                                | PCR_COMPAT_2_06 | PCR_COMPAT_2_05);
> -    }
>   #endif /* defined(TARGET_PPC64) */
>   }
>   

Re: [PATCH 01/13] ppc: Drop support for POWER9 and POWER10 DD1 chips

[Harsh Prateek Bora]

On 3/12/24 10:20, Harsh Prateek Bora wrote:
> 
> 
> On 3/12/24 00:21, Nicholas Piggin wrote:
>> The POWER9 DD1 and POWER10 DD1 chips are not public and are no longer of
>> any use in QEMU. Remove them.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   hw/ppc/spapr_cpu_core.c |  2 --
>>   target/ppc/cpu-models.c |  4 ----
>>   target/ppc/cpu_init.c   |  7 ++-----
>>   target/ppc/kvm.c        | 11 -----------
>>   4 files changed, 2 insertions(+), 22 deletions(-)
> 
> Do we want to squash in removal of the macro as well?
>

<snip>
Actually both, correcting diff:

diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index 0229ef3a9a..7d89b41214 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -348,11 +348,9 @@ enum {
      CPU_POWERPC_POWER8NVL_BASE     = 0x004C0000,
      CPU_POWERPC_POWER8NVL_v10      = 0x004C0100,
      CPU_POWERPC_POWER9_BASE        = 0x004E0000,
-    CPU_POWERPC_POWER9_DD1         = 0x004E1100,
      CPU_POWERPC_POWER9_DD20        = 0x004E1200,
      CPU_POWERPC_POWER9_DD22        = 0x004E1202,
      CPU_POWERPC_POWER10_BASE       = 0x00800000,
-    CPU_POWERPC_POWER10_DD1        = 0x00801100,
      CPU_POWERPC_POWER10_DD20       = 0x00801200,
      CPU_POWERPC_970_v22            = 0x00390202,
      CPU_POWERPC_970FX_v10          = 0x00391100,

> 
> With that,
> 
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> 
> regards,
> Harsh
> 
>>
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index 40b7c52f7f..50523ead25 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -394,10 +394,8 @@ static const TypeInfo spapr_cpu_core_type_infos[] 
>> = {
>>       DEFINE_SPAPR_CPU_CORE_TYPE("power8_v2.0"),
>>       DEFINE_SPAPR_CPU_CORE_TYPE("power8e_v2.1"),
>>       DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"),
>> -    DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
>>       DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
>>       DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.2"),
>> -    DEFINE_SPAPR_CPU_CORE_TYPE("power10_v1.0"),
>>       DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
>>   #ifdef CONFIG_KVM
>>       DEFINE_SPAPR_CPU_CORE_TYPE("host"),
>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
>> index 36e465b390..f2301b43f7 100644
>> --- a/target/ppc/cpu-models.c
>> +++ b/target/ppc/cpu-models.c
>> @@ -728,14 +728,10 @@
>>                   "POWER8 v2.0")
>>       POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10,         
>> POWER8,
>>                   "POWER8NVL v1.0")
>> -    POWERPC_DEF("power9_v1.0",   CPU_POWERPC_POWER9_DD1,             
>> POWER9,
>> -                "POWER9 v1.0")
>>       POWERPC_DEF("power9_v2.0",   CPU_POWERPC_POWER9_DD20,            
>> POWER9,
>>                   "POWER9 v2.0")
>>       POWERPC_DEF("power9_v2.2",   CPU_POWERPC_POWER9_DD22,            
>> POWER9,
>>                   "POWER9 v2.2")
>> -    POWERPC_DEF("power10_v1.0",  CPU_POWERPC_POWER10_DD1,            
>> POWER10,
>> -                "POWER10 v1.0")
>>       POWERPC_DEF("power10_v2.0",  CPU_POWERPC_POWER10_DD20,           
>> POWER10,
>>                   "POWER10 v2.0")
>>   #endif /* defined (TARGET_PPC64) */
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 1d3d1db7c3..572cbdf25f 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -6350,10 +6350,7 @@ static bool 
>> ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
>>           return false;
>>       }
>> -    if ((pvr & 0x0f00) == 0x100) {
>> -        /* DD1.x always matches power9_v1.0 */
>> -        return true;
>> -    } else if ((pvr & 0x0f00) == 0x200) {
>> +    if ((pvr & 0x0f00) == 0x200) {
>>           if ((pvr & 0xf) < 2) {
>>               /* DD2.0, DD2.1 match power9_v2.0 */
>>               if ((pcc->pvr & 0xf) == 0) {
>> @@ -6536,7 +6533,7 @@ static bool 
>> ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr, bool best)
>>       }
>>       if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
>> -        /* Major DD version matches to power10_v1.0 and power10_v2.0 */
>> +        /* Major DD version matches power10_v2.0 */
>>           return true;
>>       }
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index bcf30a5400..525fbe3892 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -2369,17 +2369,6 @@ static void 
>> kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
>>   #if defined(TARGET_PPC64)
>>       pcc->radix_page_info = kvmppc_get_radix_page_info();
>> -
>> -    if ((pcc->pvr & 0xffffff00) == CPU_POWERPC_POWER9_DD1) {
>> -        /*
>> -         * POWER9 DD1 has some bugs which make it not really ISA 3.00
>> -         * compliant.  More importantly, advertising ISA 3.00
>> -         * architected mode may prevent guests from activating
>> -         * necessary DD1 workarounds.
>> -         */
>> -        pcc->pcr_supported &= ~(PCR_COMPAT_3_00 | PCR_COMPAT_2_07
>> -                                | PCR_COMPAT_2_06 | PCR_COMPAT_2_05);
>> -    }
>>   #endif /* defined(TARGET_PPC64) */
>>   }
> 

Re: [PATCH 01/13] ppc: Drop support for POWER9 and POWER10 DD1 chips

[Nicholas Piggin]

On Tue Mar 12, 2024 at 2:55 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 3/12/24 10:20, Harsh Prateek Bora wrote:
> > 
> > 
> > On 3/12/24 00:21, Nicholas Piggin wrote:
> >> The POWER9 DD1 and POWER10 DD1 chips are not public and are no longer of
> >> any use in QEMU. Remove them.
> >>
> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> ---
> >>   hw/ppc/spapr_cpu_core.c |  2 --
> >>   target/ppc/cpu-models.c |  4 ----
> >>   target/ppc/cpu_init.c   |  7 ++-----
> >>   target/ppc/kvm.c        | 11 -----------
> >>   4 files changed, 2 insertions(+), 22 deletions(-)
> > 
> > Do we want to squash in removal of the macro as well?
> >
>
> <snip>
> Actually both, correcting diff:
>
> diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
> index 0229ef3a9a..7d89b41214 100644
> --- a/target/ppc/cpu-models.h
> +++ b/target/ppc/cpu-models.h
> @@ -348,11 +348,9 @@ enum {
>       CPU_POWERPC_POWER8NVL_BASE     = 0x004C0000,
>       CPU_POWERPC_POWER8NVL_v10      = 0x004C0100,
>       CPU_POWERPC_POWER9_BASE        = 0x004E0000,
> -    CPU_POWERPC_POWER9_DD1         = 0x004E1100,
>       CPU_POWERPC_POWER9_DD20        = 0x004E1200,
>       CPU_POWERPC_POWER9_DD22        = 0x004E1202,
>       CPU_POWERPC_POWER10_BASE       = 0x00800000,
> -    CPU_POWERPC_POWER10_DD1        = 0x00801100,
>       CPU_POWERPC_POWER10_DD20       = 0x00801200,
>       CPU_POWERPC_970_v22            = 0x00390202,
>       CPU_POWERPC_970FX_v10          = 0x00391100,

That would make sense, but we do seem to use this list as somewhat of a
reference or at least historic graveyard too (note all the other CPUs we
no longer support). So I was going to just leave them there.

Thanks,
Nick

Re: [PATCH 01/13] ppc: Drop support for POWER9 and POWER10 DD1 chips

[Harsh Prateek Bora]

On 3/12/24 14:29, Nicholas Piggin wrote:
> On Tue Mar 12, 2024 at 2:55 PM AEST, Harsh Prateek Bora wrote:
>>
>>
>> On 3/12/24 10:20, Harsh Prateek Bora wrote:
>>>
>>>
>>> On 3/12/24 00:21, Nicholas Piggin wrote:
>>>> The POWER9 DD1 and POWER10 DD1 chips are not public and are no longer of
>>>> any use in QEMU. Remove them.
>>>>
>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>>> ---
>>>>    hw/ppc/spapr_cpu_core.c |  2 --
>>>>    target/ppc/cpu-models.c |  4 ----
>>>>    target/ppc/cpu_init.c   |  7 ++-----
>>>>    target/ppc/kvm.c        | 11 -----------
>>>>    4 files changed, 2 insertions(+), 22 deletions(-)
>>>
>>> Do we want to squash in removal of the macro as well?
>>>
>>
>> <snip>
>> Actually both, correcting diff:
>>
>> diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
>> index 0229ef3a9a..7d89b41214 100644
>> --- a/target/ppc/cpu-models.h
>> +++ b/target/ppc/cpu-models.h
>> @@ -348,11 +348,9 @@ enum {
>>        CPU_POWERPC_POWER8NVL_BASE     = 0x004C0000,
>>        CPU_POWERPC_POWER8NVL_v10      = 0x004C0100,
>>        CPU_POWERPC_POWER9_BASE        = 0x004E0000,
>> -    CPU_POWERPC_POWER9_DD1         = 0x004E1100,
>>        CPU_POWERPC_POWER9_DD20        = 0x004E1200,
>>        CPU_POWERPC_POWER9_DD22        = 0x004E1202,
>>        CPU_POWERPC_POWER10_BASE       = 0x00800000,
>> -    CPU_POWERPC_POWER10_DD1        = 0x00801100,
>>        CPU_POWERPC_POWER10_DD20       = 0x00801200,
>>        CPU_POWERPC_970_v22            = 0x00390202,
>>        CPU_POWERPC_970FX_v10          = 0x00391100,
> 
> That would make sense, but we do seem to use this list as somewhat of a
> reference or at least historic graveyard too (note all the other CPUs we
> no longer support). So I was going to just leave them there.

Oh ok, in that case, it's fine.

regards,
Harsh
> 
> Thanks,
> Nick

[PATCH 02/13] target/ppc: POWER10 does not have transactional memory

[Nicholas Piggin]

POWER10 hardware implements a degenerate transactional memory facility
in POWER8/9 PCR compatibility modes to permit migration from older
CPUs, but POWER10 / ISA v3.1 mode does not support it so the CPU model
should not support it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 572cbdf25f..d7e84a2f40 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6573,7 +6573,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
                         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_ISA310 |
+                        PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
                         PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_HV) |
@@ -6617,7 +6617,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
     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;
+                 POWERPC_FLAG_VSX | POWERPC_FLAG_SCV;
     pcc->l1_dcache_size = 0x8000;
     pcc->l1_icache_size = 0x8000;
 }
-- 
2.42.0

Re: [PATCH 02/13] target/ppc: POWER10 does not have transactional memory

[Harsh Prateek Bora]

Hi Nick,

One query/comment below:

On 3/12/24 00:21, Nicholas Piggin wrote:
> POWER10 hardware implements a degenerate transactional memory facility
> in POWER8/9 PCR compatibility modes to permit migration from older
> CPUs, but POWER10 / ISA v3.1 mode does not support it so the CPU model
> should not support it.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   target/ppc/cpu_init.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 572cbdf25f..d7e84a2f40 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6573,7 +6573,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>                           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_ISA310 |
> +                        PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
>                           PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
>       pcc->msr_mask = (1ull << MSR_SF) |
>                       (1ull << MSR_HV) |
> @@ -6617,7 +6617,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>       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;
> +                 POWERPC_FLAG_VSX | POWERPC_FLAG_SCV;
>       pcc->l1_dcache_size = 0x8000;
>       pcc->l1_icache_size = 0x8000;
>   }

Shouldn't we also have below change included with this:

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index aac095e5fd..faefc0420e 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6641,7 +6641,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
                          PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206 | PPC2_ATTN;
      pcc->msr_mask = (1ull << MSR_SF) |
                      (1ull << MSR_HV) |
-                    (1ull << MSR_TM) |
                      (1ull << MSR_VR) |
                      (1ull << MSR_VSX) |
                      (1ull << MSR_EE) |

Otherwise,
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

Re: [PATCH 02/13] target/ppc: POWER10 does not have transactional memory

[Nicholas Piggin]

On Tue Mar 12, 2024 at 6:10 PM AEST, Harsh Prateek Bora wrote:
> Hi Nick,
>
> One query/comment below:
>
> On 3/12/24 00:21, Nicholas Piggin wrote:
> > POWER10 hardware implements a degenerate transactional memory facility
> > in POWER8/9 PCR compatibility modes to permit migration from older
> > CPUs, but POWER10 / ISA v3.1 mode does not support it so the CPU model
> > should not support it.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   target/ppc/cpu_init.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> > index 572cbdf25f..d7e84a2f40 100644
> > --- a/target/ppc/cpu_init.c
> > +++ b/target/ppc/cpu_init.c
> > @@ -6573,7 +6573,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> >                           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_ISA310 |
> > +                        PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
> >                           PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
> >       pcc->msr_mask = (1ull << MSR_SF) |
> >                       (1ull << MSR_HV) |
> > @@ -6617,7 +6617,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> >       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;
> > +                 POWERPC_FLAG_VSX | POWERPC_FLAG_SCV;
> >       pcc->l1_dcache_size = 0x8000;
> >       pcc->l1_icache_size = 0x8000;
> >   }
>
> Shouldn't we also have below change included with this:
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index aac095e5fd..faefc0420e 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6641,7 +6641,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>                           PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206 | PPC2_ATTN;
>       pcc->msr_mask = (1ull << MSR_SF) |
>                       (1ull << MSR_HV) |
> -                    (1ull << MSR_TM) |
>                       (1ull << MSR_VR) |
>                       (1ull << MSR_VSX) |
>                       (1ull << MSR_EE) |

I think you're probably right. I'll do some testing...

Thanks,
Nick

>
> Otherwise,
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

[PATCH 03/13] ppc/spapr|pnv: Remove SAO from pa-features

[Nicholas Piggin]

SAO is a page table attribute that strengthens the memory ordering of
accesses. QEMU with MTTCG does not implement this, so clear it in
ibm,pa-features. This is an obscure feature that has been removed from
POWER10 ISA v3.1, there isn't much concern with removing it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv.c   |  2 +-
 hw/ppc/spapr.c | 14 ++++++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0b47b92baa..aa9786e970 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -150,7 +150,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
     uint32_t page_sizes_prop[64];
     size_t page_sizes_prop_size;
     const uint8_t pa_features[] = { 24, 0,
-                                    0xf6, 0x3f, 0xc7, 0xc0, 0x80, 0xf0,
+                                    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
                                     0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
                                     0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
                                     0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 55263f0815..5099f12cc6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -234,16 +234,16 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
                                  void *fdt, int offset)
 {
     uint8_t pa_features_206[] = { 6, 0,
-        0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
+        0xf6, 0x1f, 0xc7, 0x00, 0x00, 0xc0 };
     uint8_t pa_features_207[] = { 24, 0,
-        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
+        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0,
         0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
         0x80, 0x00, 0x80, 0x00, 0x00, 0x00 };
     uint8_t pa_features_300[] = { 66, 0,
         /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
-        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, SSO, 5: LE|CFAR|EB|LSQ */
-        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0, /* 0 - 5 */
+        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
+        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
         /* 6: DS207 */
         0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
         /* 16: Vector */
@@ -284,6 +284,12 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         return;
     }
 
+    /*
+     * SSO (SAO) ordering is supported on KVM and thread=single hosts,
+     * but not MTTCG, so disable it. To advertise it, a cap would have
+     * to be added, or support implemented for MTTCG.
+     */
+
     if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
         /*
          * Note: we keep CI large pages off by default because a 64K capable
-- 
2.42.0

Re: [PATCH 03/13] ppc/spapr|pnv: Remove SAO from pa-features

[Harsh Prateek Bora]

Hi Nick,

One cosmetic comment, in case you are doing a re-spin:

On 3/12/24 00:21, Nicholas Piggin wrote:
> SAO is a page table attribute that strengthens the memory ordering of
> accesses. QEMU with MTTCG does not implement this, so clear it in
> ibm,pa-features. This is an obscure feature that has been removed from
> POWER10 ISA v3.1, there isn't much concern with removing it.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/pnv.c   |  2 +-
>   hw/ppc/spapr.c | 14 ++++++++++----
>   2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 0b47b92baa..aa9786e970 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -150,7 +150,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>       uint32_t page_sizes_prop[64];
>       size_t page_sizes_prop_size;
>       const uint8_t pa_features[] = { 24, 0,
> -                                    0xf6, 0x3f, 0xc7, 0xc0, 0x80, 0xf0,
> +                                    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
>                                       0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
>                                       0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
>                                       0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 55263f0815..5099f12cc6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -234,16 +234,16 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>                                    void *fdt, int offset)
>   {
>       uint8_t pa_features_206[] = { 6, 0,
> -        0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> +        0xf6, 0x1f, 0xc7, 0x00, 0x00, 0xc0 };
>       uint8_t pa_features_207[] = { 24, 0,
> -        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
> +        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0,
>           0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
>           0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00 };
>       uint8_t pa_features_300[] = { 66, 0,
>           /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> -        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, SSO, 5: LE|CFAR|EB|LSQ */
> -        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0, /* 0 - 5 */
> +        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */

Do we want to mention in comments SSO (disabled), also ..

> +        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
>           /* 6: DS207 */
>           0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
>           /* 16: Vector */
> @@ -284,6 +284,12 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           return;
>       }
>   
> +    /*
> +     * SSO (SAO) ordering is supported on KVM and thread=single hosts,
> +     * but not MTTCG, so disable it. To advertise it, a cap would have
> +     * to be added, or support implemented for MTTCG.
> +     */
> +

This comment could go in the beginning where we are actually disabling it.

Otherwise,

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>


>       if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
>           /*
>            * Note: we keep CI large pages off by default because a 64K capable

[PATCH 04/13] ppc/spapr: Remove copy-paste from pa-features

[Nicholas Piggin]

TCG does not support copy/paste instructions. Remove it from
ibm,pa-features. This has never been implemented under TCG or
practically usable under KVM, so it won't be missed.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5099f12cc6..7d7da30f60 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -254,8 +254,8 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
         /* 30: MMR, 32: LE atomic, 34: EBB + ext EBB */
         0x80, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
-        /* 36: SPR SO, 38: Copy/Paste, 40: Radix MMU */
-        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 36 - 41 */
+        /* 36: SPR SO, 40: Radix MMU */
+        0x80, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
         /* 42: PM, 44: PC RA, 46: SC vec'd */
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
         /* 48: SIMD, 50: QP BFP, 52: String */
@@ -288,6 +288,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
      * SSO (SAO) ordering is supported on KVM and thread=single hosts,
      * but not MTTCG, so disable it. To advertise it, a cap would have
      * to be added, or support implemented for MTTCG.
+     *
+     * Copy/paste is not supported by TCG, so it is not advertised. KVM
+     * can execute them but it has no accelerator drivers which are usable,
+     * so there isn't much need for it anyway.
      */
 
     if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
-- 
2.42.0

Re: [PATCH 04/13] ppc/spapr: Remove copy-paste from pa-features

[Harsh Prateek Bora]

Hi Nick,

On 3/12/24 00:21, Nicholas Piggin wrote:
> TCG does not support copy/paste instructions. Remove it from
> ibm,pa-features. This has never been implemented under TCG or

s/or/nor ?

> practically usable under KVM, so it won't be missed.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/spapr.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5099f12cc6..7d7da30f60 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -254,8 +254,8 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
>           /* 30: MMR, 32: LE atomic, 34: EBB + ext EBB */
>           0x80, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> -        /* 36: SPR SO, 38: Copy/Paste, 40: Radix MMU */
> -        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 36 - 41 */
> +        /* 36: SPR SO, 40: Radix MMU */
> +        0x80, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
>           /* 42: PM, 44: PC RA, 46: SC vec'd */
>           0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
>           /* 48: SIMD, 50: QP BFP, 52: String */
> @@ -288,6 +288,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>        * SSO (SAO) ordering is supported on KVM and thread=single hosts,
>        * but not MTTCG, so disable it. To advertise it, a cap would have
>        * to be added, or support implemented for MTTCG.
> +     *
> +     * Copy/paste is not supported by TCG, so it is not advertised. KVM
> +     * can execute them but it has no accelerator drivers which are usable,
> +     * so there isn't much need for it anyway.
>        */

If doing a re-spin, you may consider comments on prev patch applicable
above as well. Either ways, with prev typo fixed:

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

>   
>       if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {

[PATCH 05/13] ppc/spapr: Adjust ibm,pa-features for POWER9

[Nicholas Piggin]

"MMR" and "SPR SO" are not implemented in POWER9, so clear those bits.
HTM is not set by default, and only later if the cap is set, so remove
the comment that suggests otherwise.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7d7da30f60..247f920f07 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -248,14 +248,14 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
         /* 16: Vector */
         0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
-        /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
+        /* 18: Vec. Scalar, 20: Vec. XOR */
         0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
         /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
-        /* 30: MMR, 32: LE atomic, 34: EBB + ext EBB */
-        0x80, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
-        /* 36: SPR SO, 40: Radix MMU */
-        0x80, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
+        /* 32: LE atomic, 34: EBB + ext EBB */
+        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+        /* 40: Radix MMU */
+        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
         /* 42: PM, 44: PC RA, 46: SC vec'd */
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
         /* 48: SIMD, 50: QP BFP, 52: String */
-- 
2.42.0

Re: [PATCH 05/13] ppc/spapr: Adjust ibm,pa-features for POWER9

[Harsh Prateek Bora]

On 3/12/24 00:21, Nicholas Piggin wrote:
> "MMR" and "SPR SO" are not implemented in POWER9, so clear those bits.
> HTM is not set by default, and only later if the cap is set, so remove
> the comment that suggests otherwise.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> ---
>   hw/ppc/spapr.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7d7da30f60..247f920f07 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -248,14 +248,14 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
>           /* 16: Vector */
>           0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> -        /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
> +        /* 18: Vec. Scalar, 20: Vec. XOR */
>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
>           /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
>           0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> -        /* 30: MMR, 32: LE atomic, 34: EBB + ext EBB */
> -        0x80, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> -        /* 36: SPR SO, 40: Radix MMU */
> -        0x80, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> +        /* 32: LE atomic, 34: EBB + ext EBB */
> +        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> +        /* 40: Radix MMU */
> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
>           /* 42: PM, 44: PC RA, 46: SC vec'd */
>           0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
>           /* 48: SIMD, 50: QP BFP, 52: String */

[PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[Nicholas Piggin]

From: Benjamin Gray <bgray@linux.ibm.com>

Add POWER10 pa-features entry.

Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
advertised. Each DEXCR aspect is allocated a bit in the device tree,
using the 68--71 byte range (inclusive). The functionality of the
[P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
bit 0 (BE).

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
[npiggin: reword title and changelog, adjust a few bits]
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 247f920f07..128bfe11a8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         /* 60: NM atomic, 62: RNG */
         0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
     };
+    /* 3.1 removes SAO, HTM support */
+    uint8_t pa_features_31[] = { 74, 0,
+        /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
+        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
+        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
+        /* 6: DS207 */
+        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
+        /* 16: Vector */
+        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
+        /* 18: Vec. Scalar, 20: Vec. XOR */
+        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
+        /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
+        /* 32: LE atomic, 34: EBB + ext EBB */
+        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+        /* 40: Radix MMU */
+        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
+        /* 42: PM, 44: PC RA, 46: SC vec'd */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
+        /* 48: SIMD, 50: QP BFP, 52: String */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
+        /* 54: DecFP, 56: DecI, 58: SHA */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
+        /* 60: NM atomic, 62: RNG */
+        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
+        /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
+        0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
+        /* 72: [P]HASHCHK */
+        0x80, 0x00,                         /* 72 - 73 */
+    };
     uint8_t *pa_features = NULL;
     size_t pa_size;
 
@@ -280,6 +310,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         pa_features = pa_features_300;
         pa_size = sizeof(pa_features_300);
     }
+    if (ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_10, 0, cpu->compat_pvr)) {
+        pa_features = pa_features_31;
+        pa_size = sizeof(pa_features_31);
+    }
     if (!pa_features) {
         return;
     }
-- 
2.42.0

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[Philippe Mathieu-Daudé]

On 11/3/24 19:51, Nicholas Piggin wrote:
> From: Benjamin Gray <bgray@linux.ibm.com>
> 
> Add POWER10 pa-features entry.
> 
> Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
> advertised. Each DEXCR aspect is allocated a bit in the device tree,
> using the 68--71 byte range (inclusive). The functionality of the
> [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
> bit 0 (BE).
> 
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> [npiggin: reword title and changelog, adjust a few bits]
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 247f920f07..128bfe11a8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           /* 60: NM atomic, 62: RNG */
>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>       };
> +    /* 3.1 removes SAO, HTM support */
> +    uint8_t pa_features_31[] = { 74, 0,

Nitpicking because pre-existing, all these arrays could be static const.

> +        /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> +        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> +        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> +        /* 6: DS207 */
> +        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> +        /* 16: Vector */
> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> +        /* 18: Vec. Scalar, 20: Vec. XOR */
> +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
> +        /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> +        /* 32: LE atomic, 34: EBB + ext EBB */
> +        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> +        /* 40: Radix MMU */
> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> +        /* 42: PM, 44: PC RA, 46: SC vec'd */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> +        /* 48: SIMD, 50: QP BFP, 52: String */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> +        /* 54: DecFP, 56: DecI, 58: SHA */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> +        /* 60: NM atomic, 62: RNG */
> +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> +        /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
> +        0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
> +        /* 72: [P]HASHCHK */
> +        0x80, 0x00,                         /* 72 - 73 */
> +    };
>       uint8_t *pa_features = NULL;
>       size_t pa_size;
>   
> @@ -280,6 +310,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           pa_features = pa_features_300;
>           pa_size = sizeof(pa_features_300);
>       }
> +    if (ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_10, 0, cpu->compat_pvr)) {
> +        pa_features = pa_features_31;
> +        pa_size = sizeof(pa_features_31);
> +    }
>       if (!pa_features) {
>           return;
>       }

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 3417 bytes --]

On Mon, 11 Mar 2024, Philippe Mathieu-Daudé wrote:
> On 11/3/24 19:51, Nicholas Piggin wrote:
>> From: Benjamin Gray <bgray@linux.ibm.com>
>> 
>> Add POWER10 pa-features entry.
>> 
>> Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
>> advertised. Each DEXCR aspect is allocated a bit in the device tree,
>> using the 68--71 byte range (inclusive). The functionality of the
>> [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
>> bit 0 (BE).
>> 
>> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
>> [npiggin: reword title and changelog, adjust a few bits]
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>> 
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 247f920f07..128bfe11a8 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState 
>> *spapr,
>>           /* 60: NM atomic, 62: RNG */
>>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>>       };
>> +    /* 3.1 removes SAO, HTM support */
>> +    uint8_t pa_features_31[] = { 74, 0,
>
> Nitpicking because pre-existing, all these arrays could be static const.

If we are at it then maybe also s/0x00/   0/ because having a stream of 
0x80 and 0x00 is not the most readable.

Regards,
BALATON Zoltan

>> +        /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 
>> */
>> +        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
>> +        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
>> +        /* 6: DS207 */
>> +        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
>> +        /* 16: Vector */
>> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
>> +        /* 18: Vec. Scalar, 20: Vec. XOR */
>> +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
>> +        /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
>> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
>> +        /* 32: LE atomic, 34: EBB + ext EBB */
>> +        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
>> +        /* 40: Radix MMU */
>> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
>> +        /* 42: PM, 44: PC RA, 46: SC vec'd */
>> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
>> +        /* 48: SIMD, 50: QP BFP, 52: String */
>> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
>> +        /* 54: DecFP, 56: DecI, 58: SHA */
>> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
>> +        /* 60: NM atomic, 62: RNG */
>> +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>> +        /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
>> +        0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
>> +        /* 72: [P]HASHCHK */
>> +        0x80, 0x00,                         /* 72 - 73 */
>> +    };
>>       uint8_t *pa_features = NULL;
>>       size_t pa_size;
>>   @@ -280,6 +310,10 @@ static void spapr_dt_pa_features(SpaprMachineState 
>> *spapr,
>>           pa_features = pa_features_300;
>>           pa_size = sizeof(pa_features_300);
>>       }
>> +    if (ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_10, 0, 
>> cpu->compat_pvr)) {
>> +        pa_features = pa_features_31;
>> +        pa_size = sizeof(pa_features_31);
>> +    }
>>       if (!pa_features) {
>>           return;
>>       }
>
>
>

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[Nicholas Piggin]

On Tue Mar 12, 2024 at 7:07 AM AEST, BALATON Zoltan wrote:
> On Mon, 11 Mar 2024, Philippe Mathieu-Daudé wrote:
> > On 11/3/24 19:51, Nicholas Piggin wrote:
> >> From: Benjamin Gray <bgray@linux.ibm.com>
> >> 
> >> Add POWER10 pa-features entry.
> >> 
> >> Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
> >> advertised. Each DEXCR aspect is allocated a bit in the device tree,
> >> using the 68--71 byte range (inclusive). The functionality of the
> >> [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
> >> bit 0 (BE).
> >> 
> >> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> >> [npiggin: reword title and changelog, adjust a few bits]
> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> ---
> >>   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
> >>   1 file changed, 34 insertions(+)
> >> 
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 247f920f07..128bfe11a8 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState 
> >> *spapr,
> >>           /* 60: NM atomic, 62: RNG */
> >>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> >>       };
> >> +    /* 3.1 removes SAO, HTM support */
> >> +    uint8_t pa_features_31[] = { 74, 0,
> >
> > Nitpicking because pre-existing, all these arrays could be static const.
>
> If we are at it then maybe also s/0x00/   0/ because having a stream of 
> 0x80 and 0x00 is not the most readable.

Eh, it's more readable because it aligns colums. But probably better
more readable and  less error prone would be like -

    PA_FEATURE_SET(pa_features_31,  6, 0); /* DS207 */
    PA_FEATURE_SET(pa_features_31, 18, 0); /* Vector scalar */

I just didn't quite find something I like yet. I won't change style
before adding the missing bits either way, but certainly would be
good to clean it up after.

Thanks,
Nick

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]

On Tue, 12 Mar 2024, Nicholas Piggin wrote:
> On Tue Mar 12, 2024 at 7:07 AM AEST, BALATON Zoltan wrote:
>> On Mon, 11 Mar 2024, Philippe Mathieu-Daudé wrote:
>>> On 11/3/24 19:51, Nicholas Piggin wrote:
>>>> From: Benjamin Gray <bgray@linux.ibm.com>
>>>>
>>>> Add POWER10 pa-features entry.
>>>>
>>>> Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
>>>> advertised. Each DEXCR aspect is allocated a bit in the device tree,
>>>> using the 68--71 byte range (inclusive). The functionality of the
>>>> [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
>>>> bit 0 (BE).
>>>>
>>>> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
>>>> [npiggin: reword title and changelog, adjust a few bits]
>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>>> ---
>>>>   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index 247f920f07..128bfe11a8 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState
>>>> *spapr,
>>>>           /* 60: NM atomic, 62: RNG */
>>>>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>>>>       };
>>>> +    /* 3.1 removes SAO, HTM support */
>>>> +    uint8_t pa_features_31[] = { 74, 0,
>>>
>>> Nitpicking because pre-existing, all these arrays could be static const.
>>
>> If we are at it then maybe also s/0x00/   0/ because having a stream of
>> 0x80 and 0x00 is not the most readable.
>
> Eh, it's more readable because it aligns colums.

Not sure it you've noticed the 3 spaces before the 0 replacing 0x0 that 
would keep alignment. But it's not something that needs to be changed just 
commented on it as it came up but I don't expect it to be done now on the 
day of the freeze. It's more important to get the already reviewed and 
queued patches in a pull request to not miss the release. So this comment 
is just for the fuuture.

Regards,
BALATON Zoltan

> But probably better
> more readable and  less error prone would be like -
>
>    PA_FEATURE_SET(pa_features_31,  6, 0); /* DS207 */
>    PA_FEATURE_SET(pa_features_31, 18, 0); /* Vector scalar */
>
> I just didn't quite find something I like yet. I won't change style
> before adding the missing bits either way, but certainly would be
> good to clean it up after.
>
> Thanks,
> Nick
>
>

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[Nicholas Piggin]

On Tue Mar 12, 2024 at 7:59 PM AEST, BALATON Zoltan wrote:
> On Tue, 12 Mar 2024, Nicholas Piggin wrote:
> > On Tue Mar 12, 2024 at 7:07 AM AEST, BALATON Zoltan wrote:
> >> On Mon, 11 Mar 2024, Philippe Mathieu-Daudé wrote:
> >>> On 11/3/24 19:51, Nicholas Piggin wrote:
> >>>> From: Benjamin Gray <bgray@linux.ibm.com>
> >>>>
> >>>> Add POWER10 pa-features entry.
> >>>>
> >>>> Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
> >>>> advertised. Each DEXCR aspect is allocated a bit in the device tree,
> >>>> using the 68--71 byte range (inclusive). The functionality of the
> >>>> [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
> >>>> bit 0 (BE).
> >>>>
> >>>> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> >>>> [npiggin: reword title and changelog, adjust a few bits]
> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >>>> ---
> >>>>   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
> >>>>   1 file changed, 34 insertions(+)
> >>>>
> >>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>>> index 247f920f07..128bfe11a8 100644
> >>>> --- a/hw/ppc/spapr.c
> >>>> +++ b/hw/ppc/spapr.c
> >>>> @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState
> >>>> *spapr,
> >>>>           /* 60: NM atomic, 62: RNG */
> >>>>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> >>>>       };
> >>>> +    /* 3.1 removes SAO, HTM support */
> >>>> +    uint8_t pa_features_31[] = { 74, 0,
> >>>
> >>> Nitpicking because pre-existing, all these arrays could be static const.
> >>
> >> If we are at it then maybe also s/0x00/   0/ because having a stream of
> >> 0x80 and 0x00 is not the most readable.
> >
> > Eh, it's more readable because it aligns colums.
>
> Not sure it you've noticed the 3 spaces before the 0 replacing 0x0 that 
> would keep alignment.

Oh, yeah I guess that would be a more obvious zero rather than hunting
for 8s. You're right.

> But it's not something that needs to be changed just 
> commented on it as it came up but I don't expect it to be done now on the 
> day of the freeze. It's more important to get the already reviewed and 
> queued patches in a pull request to not miss the release. So this comment 
> is just for the fuuture.

Yeah we should rework it completely. Anyway thanks for taking a look.

Thanks,
Nick

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[Nicholas Piggin]

On Tue Mar 12, 2024 at 6:05 AM AEST, Philippe Mathieu-Daudé wrote:
> On 11/3/24 19:51, Nicholas Piggin wrote:
> > From: Benjamin Gray <bgray@linux.ibm.com>
> > 
> > Add POWER10 pa-features entry.
> > 
> > Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
> > advertised. Each DEXCR aspect is allocated a bit in the device tree,
> > using the 68--71 byte range (inclusive). The functionality of the
> > [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
> > bit 0 (BE).
> > 
> > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> > [npiggin: reword title and changelog, adjust a few bits]
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 34 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 247f920f07..128bfe11a8 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> >           /* 60: NM atomic, 62: RNG */
> >           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> >       };
> > +    /* 3.1 removes SAO, HTM support */
> > +    uint8_t pa_features_31[] = { 74, 0,
>
> Nitpicking because pre-existing, all these arrays could be static const.

That's true. I was looking for a nicer way to do it, probably generate
the bits with macros and share between spapr and pnv. This is just a
quick dumb approach to getting the missing bits in for now.

Thanks,
Nick

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[Harsh Prateek Bora]

On 3/12/24 00:21, Nicholas Piggin wrote:
> From: Benjamin Gray <bgray@linux.ibm.com>
> 
> Add POWER10 pa-features entry.
> 
> Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is

s/and and/and

> advertised. Each DEXCR aspect is allocated a bit in the device tree,
> using the 68--71 byte range (inclusive). The functionality of the
> [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
> bit 0 (BE).
> 
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> [npiggin: reword title and changelog, adjust a few bits]
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 247f920f07..128bfe11a8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           /* 60: NM atomic, 62: RNG */
>           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>       };
> +    /* 3.1 removes SAO, HTM support */
> +    uint8_t pa_features_31[] = { 74, 0,
> +        /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> +        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> +        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> +        /* 6: DS207 */
> +        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> +        /* 16: Vector */
> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> +        /* 18: Vec. Scalar, 20: Vec. XOR */
> +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
> +        /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> +        /* 32: LE atomic, 34: EBB + ext EBB */
> +        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> +        /* 40: Radix MMU */
> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> +        /* 42: PM, 44: PC RA, 46: SC vec'd */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> +        /* 48: SIMD, 50: QP BFP, 52: String */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> +        /* 54: DecFP, 56: DecI, 58: SHA */
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> +        /* 60: NM atomic, 62: RNG */
> +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> +        /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
> +        0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
> +        /* 72: [P]HASHCHK */

Do we want to mention [P]HASHST as well in comment above ?

> +        0x80, 0x00,                         /* 72 - 73 */
> +    };
>       uint8_t *pa_features = NULL;
>       size_t pa_size;
>   

In future, we may want to have helpers returning pointer to the
pa_features array and corresponding size conditionally based on the
required ISA support needed, instead of having local arrays bloat this
routine.

For now, with cosmetic fixes,

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> @@ -280,6 +310,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           pa_features = pa_features_300;
>           pa_size = sizeof(pa_features_300);
>       }
> +    if (ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_10, 0, cpu->compat_pvr)) {
> +        pa_features = pa_features_31;
> +        pa_size = sizeof(pa_features_31);
> +    }
>       if (!pa_features) {
>           return;
>       }

Re: [PATCH 06/13] ppc/spapr: Add pa-features for POWER10 machines

[Nicholas Piggin]

On Tue Mar 12, 2024 at 7:34 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 3/12/24 00:21, Nicholas Piggin wrote:
> > From: Benjamin Gray <bgray@linux.ibm.com>
> > 
> > Add POWER10 pa-features entry.
> > 
> > Notably DEXCR and and [P]HASHST/[P]HASHCHK instruction support is
>
> s/and and/and
>
> > advertised. Each DEXCR aspect is allocated a bit in the device tree,
> > using the 68--71 byte range (inclusive). The functionality of the
> > [P]HASHST/[P]HASHCHK instructions is separately declared in byte 72,
> > bit 0 (BE).
> > 
> > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> > [npiggin: reword title and changelog, adjust a few bits]
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 34 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 247f920f07..128bfe11a8 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> >           /* 60: NM atomic, 62: RNG */
> >           0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> >       };
> > +    /* 3.1 removes SAO, HTM support */
> > +    uint8_t pa_features_31[] = { 74, 0,
> > +        /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> > +        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> > +        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> > +        /* 6: DS207 */
> > +        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> > +        /* 16: Vector */
> > +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> > +        /* 18: Vec. Scalar, 20: Vec. XOR */
> > +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
> > +        /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> > +        /* 32: LE atomic, 34: EBB + ext EBB */
> > +        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> > +        /* 40: Radix MMU */
> > +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> > +        /* 42: PM, 44: PC RA, 46: SC vec'd */
> > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> > +        /* 48: SIMD, 50: QP BFP, 52: String */
> > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> > +        /* 54: DecFP, 56: DecI, 58: SHA */
> > +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> > +        /* 60: NM atomic, 62: RNG */
> > +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> > +        /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
> > +        0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
> > +        /* 72: [P]HASHCHK */
>
> Do we want to mention [P]HASHST as well in comment above ?

Sure. I'll do a quick respin.

Thanks,
Nick

>
> > +        0x80, 0x00,                         /* 72 - 73 */
> > +    };
> >       uint8_t *pa_features = NULL;
> >       size_t pa_size;
> >   
>
> In future, we may want to have helpers returning pointer to the
> pa_features array and corresponding size conditionally based on the
> required ISA support needed, instead of having local arrays bloat this
> routine.
>
> For now, with cosmetic fixes,
>
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>
> > @@ -280,6 +310,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> >           pa_features = pa_features_300;
> >           pa_size = sizeof(pa_features_300);
> >       }
> > +    if (ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_10, 0, cpu->compat_pvr)) {
> > +        pa_features = pa_features_31;
> > +        pa_size = sizeof(pa_features_31);
> > +    }
> >       if (!pa_features) {
> >           return;
> >       }

[PATCH 07/13] ppc/pnv: Permit ibm, pa-features set per machine variant

[Nicholas Piggin]

This allows different pa-features for powernv8/9/10.

Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: "Frédéric Barrat" <fbarrat@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index aa9786e970..52d964f77a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -133,7 +133,7 @@ static int get_cpus_node(void *fdt)
  * device tree, used in XSCOM to address cores and in interrupt
  * servers.
  */
-static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
+static int pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
 {
     PowerPCCPU *cpu = pc->threads[0];
     CPUState *cs = CPU(cpu);
@@ -149,11 +149,6 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
     uint32_t cpufreq = 1000000000;
     uint32_t page_sizes_prop[64];
     size_t page_sizes_prop_size;
-    const uint8_t pa_features[] = { 24, 0,
-                                    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
-                                    0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
-                                    0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
-                                    0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
     int offset;
     char *nodename;
     int cpus_offset = get_cpus_node(fdt);
@@ -236,15 +231,14 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
                            page_sizes_prop, page_sizes_prop_size)));
     }
 
-    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
-                       pa_features, sizeof(pa_features))));
-
     /* Build interrupt servers properties */
     for (i = 0; i < smt_threads; i++) {
         servers_prop[i] = cpu_to_be32(pc->pir + i);
     }
     _FDT((fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
                        servers_prop, sizeof(*servers_prop) * smt_threads)));
+
+    return offset;
 }
 
 static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
@@ -299,6 +293,17 @@ PnvChip *pnv_chip_add_phb(PnvChip *chip, PnvPHB *phb)
     return chip;
 }
 
+/*
+ * Same as spapr pa_features_207 except pnv always enables CI largepages bit.
+ * HTM is always enabled because TCG does implement HTM, it's just a
+ * degenerate implementation.
+ */
+static const uint8_t pa_features_207[] = { 24, 0,
+                 0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
+                 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
+                 0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
+                 0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
+
 static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
 {
     static const char compat[] = "ibm,power8-xscom\0ibm,xscom";
@@ -311,8 +316,12 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
 
     for (i = 0; i < chip->nr_cores; i++) {
         PnvCore *pnv_core = chip->cores[i];
+        int offset;
+
+        offset = pnv_dt_core(chip, pnv_core, fdt);
 
-        pnv_dt_core(chip, pnv_core, fdt);
+        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
+                           pa_features_207, sizeof(pa_features_207))));
 
         /* Interrupt Control Presenters (ICP). One per core. */
         pnv_dt_icp(chip, fdt, pnv_core->pir, CPU_CORE(pnv_core)->nr_threads);
@@ -335,8 +344,12 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
 
     for (i = 0; i < chip->nr_cores; i++) {
         PnvCore *pnv_core = chip->cores[i];
+        int offset;
 
-        pnv_dt_core(chip, pnv_core, fdt);
+        offset = pnv_dt_core(chip, pnv_core, fdt);
+
+        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
+                           pa_features_207, sizeof(pa_features_207))));
     }
 
     if (chip->ram_size) {
@@ -358,8 +371,12 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
 
     for (i = 0; i < chip->nr_cores; i++) {
         PnvCore *pnv_core = chip->cores[i];
+        int offset;
+
+        offset = pnv_dt_core(chip, pnv_core, fdt);
 
-        pnv_dt_core(chip, pnv_core, fdt);
+        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
+                           pa_features_207, sizeof(pa_features_207))));
     }
 
     if (chip->ram_size) {
-- 
2.42.0

Re: [PATCH 07/13] ppc/pnv: Permit ibm,pa-features set per machine variant

[Cédric Le Goater]

On 3/11/24 19:51, Nicholas Piggin wrote:
> This allows different pa-features for powernv8/9/10.
> 
> Cc: "Cédric Le Goater" <clg@kaod.org>
> Cc: "Frédéric Barrat" <fbarrat@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

The features could be a chip class attribute instead.

Thanks,

C.


> ---
>   hw/ppc/pnv.c | 41 +++++++++++++++++++++++++++++------------
>   1 file changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index aa9786e970..52d964f77a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -133,7 +133,7 @@ static int get_cpus_node(void *fdt)
>    * device tree, used in XSCOM to address cores and in interrupt
>    * servers.
>    */
> -static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> +static int pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>   {
>       PowerPCCPU *cpu = pc->threads[0];
>       CPUState *cs = CPU(cpu);
> @@ -149,11 +149,6 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>       uint32_t cpufreq = 1000000000;
>       uint32_t page_sizes_prop[64];
>       size_t page_sizes_prop_size;
> -    const uint8_t pa_features[] = { 24, 0,
> -                                    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
> -                                    0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> -                                    0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> -                                    0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
>       int offset;
>       char *nodename;
>       int cpus_offset = get_cpus_node(fdt);
> @@ -236,15 +231,14 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>                              page_sizes_prop, page_sizes_prop_size)));
>       }
>   
> -    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> -                       pa_features, sizeof(pa_features))));
> -
>       /* Build interrupt servers properties */
>       for (i = 0; i < smt_threads; i++) {
>           servers_prop[i] = cpu_to_be32(pc->pir + i);
>       }
>       _FDT((fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
>                          servers_prop, sizeof(*servers_prop) * smt_threads)));
> +
> +    return offset;
>   }
>   
>   static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
> @@ -299,6 +293,17 @@ PnvChip *pnv_chip_add_phb(PnvChip *chip, PnvPHB *phb)
>       return chip;
>   }
>   
> +/*
> + * Same as spapr pa_features_207 except pnv always enables CI largepages bit.
> + * HTM is always enabled because TCG does implement HTM, it's just a
> + * degenerate implementation.
> + */
> +static const uint8_t pa_features_207[] = { 24, 0,
> +                 0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0,
> +                 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                 0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> +                 0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
> +
>   static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
>   {
>       static const char compat[] = "ibm,power8-xscom\0ibm,xscom";
> @@ -311,8 +316,12 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
>   
>       for (i = 0; i < chip->nr_cores; i++) {
>           PnvCore *pnv_core = chip->cores[i];
> +        int offset;
> +
> +        offset = pnv_dt_core(chip, pnv_core, fdt);
>   
> -        pnv_dt_core(chip, pnv_core, fdt);
> +        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> +                           pa_features_207, sizeof(pa_features_207))));
>   
>           /* Interrupt Control Presenters (ICP). One per core. */
>           pnv_dt_icp(chip, fdt, pnv_core->pir, CPU_CORE(pnv_core)->nr_threads);
> @@ -335,8 +344,12 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
>   
>       for (i = 0; i < chip->nr_cores; i++) {
>           PnvCore *pnv_core = chip->cores[i];
> +        int offset;
>   
> -        pnv_dt_core(chip, pnv_core, fdt);
> +        offset = pnv_dt_core(chip, pnv_core, fdt);
> +
> +        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> +                           pa_features_207, sizeof(pa_features_207))));
>       }
>   
>       if (chip->ram_size) {
> @@ -358,8 +371,12 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
>   
>       for (i = 0; i < chip->nr_cores; i++) {
>           PnvCore *pnv_core = chip->cores[i];
> +        int offset;
> +
> +        offset = pnv_dt_core(chip, pnv_core, fdt);
>   
> -        pnv_dt_core(chip, pnv_core, fdt);
> +        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> +                           pa_features_207, sizeof(pa_features_207))));
>       }
>   
>       if (chip->ram_size) {

[PATCH 08/13] ppc/pnv: Set POWER9, POWER10 ibm,pa-features bits

[Nicholas Piggin]

Copy the pa-features arrays from spapr, adjusting slightly as
described in comments.

Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: "Frédéric Barrat" <fbarrat@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/ppc/spapr.c |  1 +
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 52d964f77a..3e30c08420 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -332,6 +332,35 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
     }
 }
 
+/*
+ * Same as spapr pa_features_300 except pnv always enables CI largepages bit.
+ */
+static const uint8_t pa_features_300[] = { 66, 0,
+    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
+    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
+    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
+    /* 6: DS207 */
+    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
+    /* 16: Vector */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
+    /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 18 - 23 */
+    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
+    /* 32: LE atomic, 34: EBB + ext EBB */
+    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+    /* 40: Radix MMU */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
+    /* 42: PM, 44: PC RA, 46: SC vec'd */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
+    /* 48: SIMD, 50: QP BFP, 52: String */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
+    /* 54: DecFP, 56: DecI, 58: SHA */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
+    /* 60: NM atomic, 62: RNG */
+    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
+};
+
 static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
 {
     static const char compat[] = "ibm,power9-xscom\0ibm,xscom";
@@ -349,7 +378,7 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
         offset = pnv_dt_core(chip, pnv_core, fdt);
 
         _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
-                           pa_features_207, sizeof(pa_features_207))));
+                           pa_features_300, sizeof(pa_features_300))));
     }
 
     if (chip->ram_size) {
@@ -359,6 +388,40 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
     pnv_dt_lpc(chip, fdt, 0, PNV9_LPCM_BASE(chip), PNV9_LPCM_SIZE);
 }
 
+/*
+ * Same as spapr pa_features_31 except pnv always enables CI largepages bit,
+ * always disables copy/paste.
+ */
+static const uint8_t pa_features_31[] = { 74, 0,
+    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
+    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
+    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
+    /* 6: DS207 */
+    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
+    /* 16: Vector */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
+    /* 18: Vec. Scalar, 20: Vec. XOR */
+    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
+    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
+    /* 32: LE atomic, 34: EBB + ext EBB */
+    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+    /* 40: Radix MMU */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
+    /* 42: PM, 44: PC RA, 46: SC vec'd */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
+    /* 48: SIMD, 50: QP BFP, 52: String */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
+    /* 54: DecFP, 56: DecI, 58: SHA */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
+    /* 60: NM atomic, 62: RNG */
+    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
+    /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
+    0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
+    /* 72: [P]HASHCHK */
+    0x80, 0x00,                         /* 72 - 73 */
+};
+
 static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
 {
     static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
@@ -376,7 +439,7 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
         offset = pnv_dt_core(chip, pnv_core, fdt);
 
         _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
-                           pa_features_207, sizeof(pa_features_207))));
+                           pa_features_31, sizeof(pa_features_31))));
     }
 
     if (chip->ram_size) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 128bfe11a8..b53c13e037 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -233,6 +233,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
                                  PowerPCCPU *cpu,
                                  void *fdt, int offset)
 {
+    /* These should be kept in sync with pnv */
     uint8_t pa_features_206[] = { 6, 0,
         0xf6, 0x1f, 0xc7, 0x00, 0x00, 0xc0 };
     uint8_t pa_features_207[] = { 24, 0,
-- 
2.42.0

Re: [PATCH 08/13] ppc/pnv: Set POWER9, POWER10 ibm,pa-features bits

[Cédric Le Goater]

On 3/11/24 19:51, Nicholas Piggin wrote:
> Copy the pa-features arrays from spapr, adjusting slightly as
> described in comments.
> 
> Cc: "Cédric Le Goater" <clg@kaod.org>
> Cc: "Frédéric Barrat" <fbarrat@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/pnv.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
>   hw/ppc/spapr.c |  1 +
>   2 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 52d964f77a..3e30c08420 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -332,6 +332,35 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
>       }
>   }
>   
> +/*
> + * Same as spapr pa_features_300 except pnv always enables CI largepages bit.
> + */
> +static const uint8_t pa_features_300[] = { 66, 0,
> +    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> +    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> +    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> +    /* 6: DS207 */
> +    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> +    /* 16: Vector */
> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> +    /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 18 - 23 */
> +    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> +    /* 32: LE atomic, 34: EBB + ext EBB */
> +    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> +    /* 40: Radix MMU */
> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> +    /* 42: PM, 44: PC RA, 46: SC vec'd */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> +    /* 48: SIMD, 50: QP BFP, 52: String */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> +    /* 54: DecFP, 56: DecI, 58: SHA */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> +    /* 60: NM atomic, 62: RNG */
> +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> +};
> +
>   static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
>   {
>       static const char compat[] = "ibm,power9-xscom\0ibm,xscom";
> @@ -349,7 +378,7 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
>           offset = pnv_dt_core(chip, pnv_core, fdt);
>   
>           _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> -                           pa_features_207, sizeof(pa_features_207))));
> +                           pa_features_300, sizeof(pa_features_300))));
>       }
>   
>       if (chip->ram_size) {
> @@ -359,6 +388,40 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
>       pnv_dt_lpc(chip, fdt, 0, PNV9_LPCM_BASE(chip), PNV9_LPCM_SIZE);
>   }
>   
> +/*
> + * Same as spapr pa_features_31 except pnv always enables CI largepages bit,
> + * always disables copy/paste.
> + */
> +static const uint8_t pa_features_31[] = { 74, 0,
> +    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> +    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> +    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> +    /* 6: DS207 */
> +    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> +    /* 16: Vector */
> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> +    /* 18: Vec. Scalar, 20: Vec. XOR */
> +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
> +    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> +    /* 32: LE atomic, 34: EBB + ext EBB */
> +    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> +    /* 40: Radix MMU */
> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> +    /* 42: PM, 44: PC RA, 46: SC vec'd */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> +    /* 48: SIMD, 50: QP BFP, 52: String */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> +    /* 54: DecFP, 56: DecI, 58: SHA */
> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> +    /* 60: NM atomic, 62: RNG */
> +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> +    /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
> +    0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
> +    /* 72: [P]HASHCHK */
> +    0x80, 0x00,                         /* 72 - 73 */
> +};
> +
>   static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
>   {
>       static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
> @@ -376,7 +439,7 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
>           offset = pnv_dt_core(chip, pnv_core, fdt);
>   
>           _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> -                           pa_features_207, sizeof(pa_features_207))));
> +                           pa_features_31, sizeof(pa_features_31))));
>       }
>   
>       if (chip->ram_size) {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 128bfe11a8..b53c13e037 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -233,6 +233,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>                                    PowerPCCPU *cpu,
>                                    void *fdt, int offset)
>   {
> +    /* These should be kept in sync with pnv */

yes. In that case, the array definition should be moved under target/ppc/.
May be under PowerPCCPUClass ?

Thanks,

C.

>       uint8_t pa_features_206[] = { 6, 0,
>           0xf6, 0x1f, 0xc7, 0x00, 0x00, 0xc0 };
>       uint8_t pa_features_207[] = { 24, 0,

Re: [PATCH 08/13] ppc/pnv: Set POWER9, POWER10 ibm,pa-features bits

[Nicholas Piggin]

On Tue Mar 12, 2024 at 6:06 PM AEST, Cédric Le Goater wrote:
> On 3/11/24 19:51, Nicholas Piggin wrote:
> > Copy the pa-features arrays from spapr, adjusting slightly as
> > described in comments.
> > 
> > Cc: "Cédric Le Goater" <clg@kaod.org>
> > Cc: "Frédéric Barrat" <fbarrat@linux.ibm.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   hw/ppc/pnv.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
> >   hw/ppc/spapr.c |  1 +
> >   2 files changed, 66 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 52d964f77a..3e30c08420 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -332,6 +332,35 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
> >       }
> >   }
> >   
> > +/*
> > + * Same as spapr pa_features_300 except pnv always enables CI largepages bit.
> > + */
> > +static const uint8_t pa_features_300[] = { 66, 0,
> > +    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> > +    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> > +    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> > +    /* 6: DS207 */
> > +    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> > +    /* 16: Vector */
> > +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> > +    /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 18 - 23 */
> > +    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> > +    /* 32: LE atomic, 34: EBB + ext EBB */
> > +    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> > +    /* 40: Radix MMU */
> > +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> > +    /* 42: PM, 44: PC RA, 46: SC vec'd */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> > +    /* 48: SIMD, 50: QP BFP, 52: String */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> > +    /* 54: DecFP, 56: DecI, 58: SHA */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> > +    /* 60: NM atomic, 62: RNG */
> > +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> > +};
> > +
> >   static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
> >   {
> >       static const char compat[] = "ibm,power9-xscom\0ibm,xscom";
> > @@ -349,7 +378,7 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
> >           offset = pnv_dt_core(chip, pnv_core, fdt);
> >   
> >           _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> > -                           pa_features_207, sizeof(pa_features_207))));
> > +                           pa_features_300, sizeof(pa_features_300))));
> >       }
> >   
> >       if (chip->ram_size) {
> > @@ -359,6 +388,40 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
> >       pnv_dt_lpc(chip, fdt, 0, PNV9_LPCM_BASE(chip), PNV9_LPCM_SIZE);
> >   }
> >   
> > +/*
> > + * Same as spapr pa_features_31 except pnv always enables CI largepages bit,
> > + * always disables copy/paste.
> > + */
> > +static const uint8_t pa_features_31[] = { 74, 0,
> > +    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
> > +    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
> > +    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
> > +    /* 6: DS207 */
> > +    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
> > +    /* 16: Vector */
> > +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
> > +    /* 18: Vec. Scalar, 20: Vec. XOR */
> > +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
> > +    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
> > +    /* 32: LE atomic, 34: EBB + ext EBB */
> > +    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
> > +    /* 40: Radix MMU */
> > +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
> > +    /* 42: PM, 44: PC RA, 46: SC vec'd */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
> > +    /* 48: SIMD, 50: QP BFP, 52: String */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
> > +    /* 54: DecFP, 56: DecI, 58: SHA */
> > +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> > +    /* 60: NM atomic, 62: RNG */
> > +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
> > +    /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
> > +    0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
> > +    /* 72: [P]HASHCHK */
> > +    0x80, 0x00,                         /* 72 - 73 */
> > +};
> > +
> >   static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
> >   {
> >       static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
> > @@ -376,7 +439,7 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
> >           offset = pnv_dt_core(chip, pnv_core, fdt);
> >   
> >           _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
> > -                           pa_features_207, sizeof(pa_features_207))));
> > +                           pa_features_31, sizeof(pa_features_31))));
> >       }
> >   
> >       if (chip->ram_size) {
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 128bfe11a8..b53c13e037 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -233,6 +233,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> >                                    PowerPCCPU *cpu,
> >                                    void *fdt, int offset)
> >   {
> > +    /* These should be kept in sync with pnv */
>
> yes. In that case, the array definition should be moved under target/ppc/.
> May be under PowerPCCPUClass ?

Yeah PowerPCCPUClass might be a good idea, although I'm not quite
decided whether it's best to just store the arrays there, or make a
list of features in another format and have a builder function to
turn that into the dt array. There's also a few differences between
spapr and pnv that I haven't worked out a nice way to handle yet.
I have a pi-features property to add too which is similar.

So yes this is a bit ugly but we're already duplicating and open coding
arrays so I'd like to just get this in to fix the missing P10 bits,
and refactor it afterwards.

Thanks,
Nick

Re: [PATCH 08/13] ppc/pnv: Set POWER9, POWER10 ibm,pa-features bits

[Cédric Le Goater]

On 3/12/24 09:54, Nicholas Piggin wrote:
> On Tue Mar 12, 2024 at 6:06 PM AEST, Cédric Le Goater wrote:
>> On 3/11/24 19:51, Nicholas Piggin wrote:
>>> Copy the pa-features arrays from spapr, adjusting slightly as
>>> described in comments.
>>>
>>> Cc: "Cédric Le Goater" <clg@kaod.org>
>>> Cc: "Frédéric Barrat" <fbarrat@linux.ibm.com>
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>>    hw/ppc/pnv.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>>    hw/ppc/spapr.c |  1 +
>>>    2 files changed, 66 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>>> index 52d964f77a..3e30c08420 100644
>>> --- a/hw/ppc/pnv.c
>>> +++ b/hw/ppc/pnv.c
>>> @@ -332,6 +332,35 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
>>>        }
>>>    }
>>>    
>>> +/*
>>> + * Same as spapr pa_features_300 except pnv always enables CI largepages bit.
>>> + */
>>> +static const uint8_t pa_features_300[] = { 66, 0,
>>> +    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
>>> +    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
>>> +    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
>>> +    /* 6: DS207 */
>>> +    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
>>> +    /* 16: Vector */
>>> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
>>> +    /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 18 - 23 */
>>> +    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
>>> +    /* 32: LE atomic, 34: EBB + ext EBB */
>>> +    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
>>> +    /* 40: Radix MMU */
>>> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
>>> +    /* 42: PM, 44: PC RA, 46: SC vec'd */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
>>> +    /* 48: SIMD, 50: QP BFP, 52: String */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
>>> +    /* 54: DecFP, 56: DecI, 58: SHA */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
>>> +    /* 60: NM atomic, 62: RNG */
>>> +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>>> +};
>>> +
>>>    static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
>>>    {
>>>        static const char compat[] = "ibm,power9-xscom\0ibm,xscom";
>>> @@ -349,7 +378,7 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
>>>            offset = pnv_dt_core(chip, pnv_core, fdt);
>>>    
>>>            _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
>>> -                           pa_features_207, sizeof(pa_features_207))));
>>> +                           pa_features_300, sizeof(pa_features_300))));
>>>        }
>>>    
>>>        if (chip->ram_size) {
>>> @@ -359,6 +388,40 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
>>>        pnv_dt_lpc(chip, fdt, 0, PNV9_LPCM_BASE(chip), PNV9_LPCM_SIZE);
>>>    }
>>>    
>>> +/*
>>> + * Same as spapr pa_features_31 except pnv always enables CI largepages bit,
>>> + * always disables copy/paste.
>>> + */
>>> +static const uint8_t pa_features_31[] = { 74, 0,
>>> +    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
>>> +    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
>>> +    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
>>> +    /* 6: DS207 */
>>> +    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
>>> +    /* 16: Vector */
>>> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
>>> +    /* 18: Vec. Scalar, 20: Vec. XOR */
>>> +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
>>> +    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
>>> +    /* 32: LE atomic, 34: EBB + ext EBB */
>>> +    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
>>> +    /* 40: Radix MMU */
>>> +    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
>>> +    /* 42: PM, 44: PC RA, 46: SC vec'd */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
>>> +    /* 48: SIMD, 50: QP BFP, 52: String */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
>>> +    /* 54: DecFP, 56: DecI, 58: SHA */
>>> +    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
>>> +    /* 60: NM atomic, 62: RNG */
>>> +    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>>> +    /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
>>> +    0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
>>> +    /* 72: [P]HASHCHK */
>>> +    0x80, 0x00,                         /* 72 - 73 */
>>> +};
>>> +
>>>    static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
>>>    {
>>>        static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
>>> @@ -376,7 +439,7 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
>>>            offset = pnv_dt_core(chip, pnv_core, fdt);
>>>    
>>>            _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
>>> -                           pa_features_207, sizeof(pa_features_207))));
>>> +                           pa_features_31, sizeof(pa_features_31))));
>>>        }
>>>    
>>>        if (chip->ram_size) {
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 128bfe11a8..b53c13e037 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -233,6 +233,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>>>                                     PowerPCCPU *cpu,
>>>                                     void *fdt, int offset)
>>>    {
>>> +    /* These should be kept in sync with pnv */
>>
>> yes. In that case, the array definition should be moved under target/ppc/.
>> May be under PowerPCCPUClass ?
> 
> Yeah PowerPCCPUClass might be a good idea, although I'm not quite
> decided whether it's best to just store the arrays there, or make a
> list of features in another format and have a builder function to
> turn that into the dt array. 

That would be very nice to have. The pa_features array is cryptic and
error prone.

> There's also a few differences between
> spapr and pnv that I haven't worked out a nice way to handle yet.
> I have a pi-features property to add too which is similar.
> 
> So yes this is a bit ugly but we're already duplicating and open coding
> arrays so I'd like to just get this in to fix the missing P10 bits,
> and refactor it afterwards.

OK.

Thanks,

C.

[PATCH 09/13] target/ppc: Prevent supervisor from modifying MSR[ME]

[Nicholas Piggin]

Prevent guest state modifying the MSR[ME] bit. Per ISA:

  An attempt to modify MSRME in privileged but non-hypervisor state is
  ignored (i.e., the bit is not changed).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/helper_regs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 410b39c231..25258986e3 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -264,6 +264,11 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
         value &= ~MSR_HVB;
         value |= env->msr & MSR_HVB;
     }
+    /* Attempt to modify MSR[ME] in guest state is ignored */
+    if (is_book3s_arch2x(env) && !(env->msr & MSR_HVB)) {
+        value &= ~(1 << MSR_ME);
+        value |= env->msr & (1 << MSR_ME);
+    }
     if ((value ^ env->msr) & (R_MSR_IR_MASK | R_MSR_DR_MASK)) {
         cpu_interrupt_exittb(cs);
     }
-- 
2.42.0

Re: [PATCH 09/13] target/ppc: Prevent supervisor from modifying MSR[ME]

[Harsh Prateek Bora]

On 3/12/24 00:21, Nicholas Piggin wrote:
> Prevent guest state modifying the MSR[ME] bit. Per ISA:
> 
>    An attempt to modify MSRME in privileged but non-hypervisor state is

s/MSRME/MSR[ME] ?

>    ignored (i.e., the bit is not changed).
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   target/ppc/helper_regs.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 410b39c231..25258986e3 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -264,6 +264,11 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
>           value &= ~MSR_HVB;
>           value |= env->msr & MSR_HVB;
>       }
> +    /* Attempt to modify MSR[ME] in guest state is ignored */
> +    if (is_book3s_arch2x(env) && !(env->msr & MSR_HVB)) {
> +        value &= ~(1 << MSR_ME);
> +        value |= env->msr & (1 << MSR_ME);
> +    }

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

>       if ((value ^ env->msr) & (R_MSR_IR_MASK | R_MSR_DR_MASK)) {
>           cpu_interrupt_exittb(cs);
>       }

Re: [PATCH 09/13] target/ppc: Prevent supervisor from modifying MSR[ME]

[Nicholas Piggin]

On Tue Mar 12, 2024 at 8:27 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 3/12/24 00:21, Nicholas Piggin wrote:
> > Prevent guest state modifying the MSR[ME] bit. Per ISA:
> > 
> >    An attempt to modify MSRME in privileged but non-hypervisor state is
>
> s/MSRME/MSR[ME] ?

Yes, thanks.

>
> >    ignored (i.e., the bit is not changed).
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   target/ppc/helper_regs.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> > index 410b39c231..25258986e3 100644
> > --- a/target/ppc/helper_regs.c
> > +++ b/target/ppc/helper_regs.c
> > @@ -264,6 +264,11 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
> >           value &= ~MSR_HVB;
> >           value |= env->msr & MSR_HVB;
> >       }
> > +    /* Attempt to modify MSR[ME] in guest state is ignored */
> > +    if (is_book3s_arch2x(env) && !(env->msr & MSR_HVB)) {
> > +        value &= ~(1 << MSR_ME);
> > +        value |= env->msr & (1 << MSR_ME);
> > +    }
>
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>
> >       if ((value ^ env->msr) & (R_MSR_IR_MASK | R_MSR_DR_MASK)) {
> >           cpu_interrupt_exittb(cs);
> >       }

[PATCH 10/13] spapr: set MSR[ME] and MSR[FP] on client entry

[Nicholas Piggin]

The initial MSR state for PAPR specifies MSR[ME] and MSR[FP] are set.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr_cpu_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 50523ead25..f3b01b0801 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -42,6 +42,8 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
      * as 32bit (MSR_SF=0) in "8.2.1. Initial Register Values".
      */
     env->msr &= ~(1ULL << MSR_SF);
+    env->msr |= (1ULL << MSR_ME) | (1ULL << MSR_FP);
+
     env->spr[SPR_HIOR] = 0;
 
     lpcr = env->spr[SPR_LPCR];
-- 
2.42.0

Re: [PATCH 10/13] spapr: set MSR[ME] and MSR[FP] on client entry

[Harsh Prateek Bora]

On 3/12/24 00:21, Nicholas Piggin wrote:
> The initial MSR state for PAPR specifies MSR[ME] and MSR[FP] are set.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

It would be good to mention PAPR section numbers suggesting the same.
Anyways,

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> ---
>   hw/ppc/spapr_cpu_core.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 50523ead25..f3b01b0801 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -42,6 +42,8 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>        * as 32bit (MSR_SF=0) in "8.2.1. Initial Register Values".
>        */
>       env->msr &= ~(1ULL << MSR_SF);
> +    env->msr |= (1ULL << MSR_ME) | (1ULL << MSR_FP);
> +
>       env->spr[SPR_HIOR] = 0;
>   
>       lpcr = env->spr[SPR_LPCR];

Re: [PATCH 10/13] spapr: set MSR[ME] and MSR[FP] on client entry

[Nicholas Piggin]

On Tue Mar 12, 2024 at 8:03 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 3/12/24 00:21, Nicholas Piggin wrote:
> > The initial MSR state for PAPR specifies MSR[ME] and MSR[FP] are set.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
> It would be good to mention PAPR section numbers suggesting the same.

I'll see if I can find it and put it in a comment.

Thanks,
Nick

> Anyways,
>
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>
> > ---
> >   hw/ppc/spapr_cpu_core.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 50523ead25..f3b01b0801 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -42,6 +42,8 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
> >        * as 32bit (MSR_SF=0) in "8.2.1. Initial Register Values".
> >        */
> >       env->msr &= ~(1ULL << MSR_SF);
> > +    env->msr |= (1ULL << MSR_ME) | (1ULL << MSR_FP);
> > +
> >       env->spr[SPR_HIOR] = 0;
> >   
> >       lpcr = env->spr[SPR_LPCR];

[PATCH 11/13] target/ppc: Make checkstop actually stop the system

[Nicholas Piggin]

checkstop state does not halt the system, interrupts continue to be
serviced, and other CPUs run. Make it stop the machine with
qemu_system_guest_panicked.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/excp_helper.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 98952de267..9aca4a1489 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -19,6 +19,8 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 #include "qemu/log.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/runstate.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "internal.h"
@@ -432,15 +434,22 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env)
         return;
     }
 
+    /*
+     * This stops the machine and logs CPU state without killing QEMU
+     * (like cpu_abort()) so the machine can still be debugged (because
+     * it is often a guest error).
+     */
+
     /* Machine check exception is not enabled. Enter checkstop state. */
     fprintf(stderr, "Machine check while not allowed. "
             "Entering checkstop state\n");
     if (qemu_log_separate()) {
         qemu_log("Machine check while not allowed. "
                  "Entering checkstop state\n");
-    }
-    cs->halted = 1;
-    cpu_interrupt_exittb(cs);
+
+    qemu_system_guest_panicked(NULL);
+
+    cpu_loop_exit_noexc(cs);
 }
 
 static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
-- 
2.42.0

[PATCH 12/13] target/ppc: improve checkstop logging

[Nicholas Piggin]

Change the logging not to print to stderr as well, because a
checkstop is a guest error (or perhaps a simulated machine error)
rather than a QEMU error, so send it to the log.

Update the checkstop message, and log CPU registers too.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/excp_helper.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 9aca4a1489..ad78d027e9 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -429,6 +429,7 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu, target_ulong vector,
 static void powerpc_mcheck_checkstop(CPUPPCState *env)
 {
     CPUState *cs = env_cpu(env);
+    FILE *f;
 
     if (FIELD_EX64(env->msr, MSR, ME)) {
         return;
@@ -440,12 +441,13 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env)
      * it is often a guest error).
      */
 
-    /* Machine check exception is not enabled. Enter checkstop state. */
-    fprintf(stderr, "Machine check while not allowed. "
-            "Entering checkstop state\n");
-    if (qemu_log_separate()) {
-        qemu_log("Machine check while not allowed. "
-                 "Entering checkstop state\n");
+    f = qemu_log_trylock();
+    if (f) {
+        fprintf(f, "Entering checkstop state: "
+                   "machine check with MSR[ME]=0\n");
+        cpu_dump_state(cs, f, CPU_DUMP_FPU | CPU_DUMP_CCOP);
+        qemu_log_unlock(f);
+    }
 
     qemu_system_guest_panicked(NULL);
 
-- 
2.42.0

[PATCH 13/13] target/ppc: Implement attn instruction on BookS 64-bit processors

[Nicholas Piggin]

attn is an implementation-specific instruction that on POWER (and G5/
970) can be enabled with a HID bit (disabled = illegal), and executing
it causes the host processor to stop and the service processor to be
notified. Generally used for debugging.

Implement attn and make it checkstop the system, which should be good
enough for QEMU debugging.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h         | 16 +++++++-
 target/ppc/helper.h      |  4 ++
 target/ppc/cpu_init.c    | 82 +++++++++++++++++++++++++++++++++++++---
 target/ppc/excp_helper.c | 59 +++++++++++++++++++----------
 target/ppc/translate.c   | 11 ++++++
 5 files changed, 145 insertions(+), 27 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0133da4e07..4fb5bd2e6e 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1353,6 +1353,9 @@ struct CPUArchState {
     /* Power management */
     int (*check_pow)(CPUPPCState *env);
 
+    /* attn instruction enable */
+    int (*check_attn)(CPUPPCState *env);
+
 #if !defined(CONFIG_USER_ONLY)
     void *load_info;  /* holds boot loading state */
 #endif
@@ -1500,6 +1503,7 @@ struct PowerPCCPUClass {
     int n_host_threads;
     void (*init_proc)(CPUPPCState *env);
     int  (*check_pow)(CPUPPCState *env);
+    int  (*check_attn)(CPUPPCState *env);
 };
 
 ObjectClass *ppc_cpu_class_by_name(const char *name);
@@ -2287,6 +2291,8 @@ void ppc_compat_add_property(Object *obj, const char *name,
 #define HID0_NAP            (1 << 22)           /* pre-2.06 */
 #define HID0_HILE           PPC_BIT(19) /* POWER8 */
 #define HID0_POWER9_HILE    PPC_BIT(4)
+#define HID0_ENABLE_ATTN    PPC_BIT(31) /* POWER8 */
+#define HID0_POWER9_ENABLE_ATTN PPC_BIT(3)
 
 /*****************************************************************************/
 /* PowerPC Instructions types definitions                                    */
@@ -2483,6 +2489,8 @@ enum {
     PPC2_MEM_LWSYNC    = 0x0000000000200000ULL,
     /* ISA 2.06 BCD assist instructions                                      */
     PPC2_BCDA_ISA206   = 0x0000000000400000ULL,
+    /* attn instruction found in IBM POWER (including 970)                   */
+    PPC2_ATTN          = 0x0000000000800000ULL,
 
 #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
                         PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
@@ -2492,7 +2500,7 @@ enum {
                         PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
                         PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206 | \
                         PPC2_ISA300 | PPC2_ISA310 | PPC2_MEM_LWSYNC | \
-                        PPC2_BCDA_ISA206)
+                        PPC2_BCDA_ISA206 | PPC2_ATTN)
 };
 
 /*****************************************************************************/
@@ -2991,6 +2999,12 @@ static inline int check_pow_nocheck(CPUPPCState *env)
     return 1;
 }
 
+/* attn enable check                                                         */
+static inline int check_attn_none(CPUPPCState *env)
+{
+    return 0;
+}
+
 /*****************************************************************************/
 /* PowerPC implementations definitions                                       */
 
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 86f97ee1e7..5a66b35f26 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -819,3 +819,7 @@ DEF_HELPER_4(DSCLIQ, void, env, fprp, fprp, i32)
 
 DEF_HELPER_1(tbegin, void, env)
 DEF_HELPER_FLAGS_1(fixup_thrm, TCG_CALL_NO_RWG, void, env)
+
+#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+DEF_HELPER_1(attn, noreturn, env)
+#endif
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d7e84a2f40..aac095e5fd 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -2107,6 +2107,26 @@ static int check_pow_hid0_74xx(CPUPPCState *env)
     return 0;
 }
 
+#if defined(TARGET_PPC64)
+static int check_attn_hid0(CPUPPCState *env)
+{
+    if (env->spr[SPR_HID0] & HID0_ENABLE_ATTN) {
+        return 1;
+    }
+
+    return 0;
+}
+
+static int check_attn_hid0_power9(CPUPPCState *env)
+{
+    if (env->spr[SPR_HID0] & HID0_POWER9_ENABLE_ATTN) {
+        return 1;
+    }
+
+    return 0;
+}
+#endif
+
 static void init_proc_405(CPUPPCState *env)
 {
     register_40x_sprs(env);
@@ -2138,6 +2158,7 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 405";
     pcc->init_proc = init_proc_405;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_DCR | PPC_WRTEE |
                        PPC_CACHE | PPC_CACHE_ICBI | PPC_40x_ICBT |
@@ -2210,6 +2231,7 @@ POWERPC_FAMILY(440EP)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 440 EP";
     pcc->init_proc = init_proc_440EP;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
                        PPC_FLOAT | PPC_FLOAT_FRES | PPC_FLOAT_FSEL |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -2248,6 +2270,7 @@ POWERPC_FAMILY(460EX)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 460 EX";
     pcc->init_proc = init_proc_440EP;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
                        PPC_FLOAT | PPC_FLOAT_FRES | PPC_FLOAT_FSEL |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -2308,6 +2331,7 @@ POWERPC_FAMILY(440GP)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 440 GP";
     pcc->init_proc = init_proc_440GP;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
                        PPC_DCR | PPC_DCRX | PPC_WRTEE | PPC_MFAPIDI |
                        PPC_CACHE | PPC_CACHE_ICBI |
@@ -2382,6 +2406,7 @@ POWERPC_FAMILY(440x5)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 440x5";
     pcc->init_proc = init_proc_440x5;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
                        PPC_DCR | PPC_WRTEE | PPC_RFMCI |
                        PPC_CACHE | PPC_CACHE_ICBI |
@@ -2417,6 +2442,7 @@ POWERPC_FAMILY(440x5wDFPU)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 440x5 with double precision FPU";
     pcc->init_proc = init_proc_440x5;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
                        PPC_FLOAT | PPC_FLOAT_FSQRT |
                        PPC_FLOAT_STFIWX |
@@ -2465,6 +2491,7 @@ POWERPC_FAMILY(MPC5xx)(ObjectClass *oc, void *data)
     dc->desc = "Freescale 5xx cores (aka RCPU)";
     pcc->init_proc = init_proc_MPC5xx;
     pcc->check_pow = check_pow_none;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING |
                        PPC_MEM_EIEIO | PPC_MEM_SYNC |
                        PPC_CACHE_ICBI | PPC_FLOAT | PPC_FLOAT_STFIWX |
@@ -2507,6 +2534,7 @@ POWERPC_FAMILY(MPC8xx)(ObjectClass *oc, void *data)
     dc->desc = "Freescale 8xx cores (aka PowerQUICC)";
     pcc->init_proc = init_proc_MPC8xx;
     pcc->check_pow = check_pow_none;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING  |
                        PPC_MEM_EIEIO | PPC_MEM_SYNC |
                        PPC_CACHE_ICBI | PPC_MFTB;
@@ -2557,6 +2585,7 @@ POWERPC_FAMILY(G2)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC G2";
     pcc->init_proc = init_proc_G2;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_STFIWX |
@@ -2595,6 +2624,7 @@ POWERPC_FAMILY(G2LE)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC G2LE";
     pcc->init_proc = init_proc_G2;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_STFIWX |
@@ -2741,6 +2771,7 @@ POWERPC_FAMILY(e200)(ObjectClass *oc, void *data)
     dc->desc = "e200 core";
     pcc->init_proc = init_proc_e200;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     /*
      * XXX: unimplemented instructions:
      * dcblc
@@ -3029,6 +3060,7 @@ POWERPC_FAMILY(e500v1)(ObjectClass *oc, void *data)
     dc->desc = "e500v1 core";
     pcc->init_proc = init_proc_e500v1;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL |
                        PPC_SPE | PPC_SPE_SINGLE |
                        PPC_WRTEE | PPC_RFDI |
@@ -3072,6 +3104,7 @@ POWERPC_FAMILY(e500v2)(ObjectClass *oc, void *data)
     dc->desc = "e500v2 core";
     pcc->init_proc = init_proc_e500v2;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL |
                        PPC_SPE | PPC_SPE_SINGLE | PPC_SPE_DOUBLE |
                        PPC_WRTEE | PPC_RFDI |
@@ -3115,6 +3148,7 @@ POWERPC_FAMILY(e500mc)(ObjectClass *oc, void *data)
     dc->desc = "e500mc core";
     pcc->init_proc = init_proc_e500mc;
     pcc->check_pow = check_pow_none;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_MFTB |
                        PPC_WRTEE | PPC_RFDI | PPC_RFMCI |
                        PPC_CACHE | PPC_CACHE_LOCK | PPC_CACHE_ICBI |
@@ -3161,6 +3195,7 @@ POWERPC_FAMILY(e5500)(ObjectClass *oc, void *data)
     dc->desc = "e5500 core";
     pcc->init_proc = init_proc_e5500;
     pcc->check_pow = check_pow_none;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_MFTB |
                        PPC_WRTEE | PPC_RFDI | PPC_RFMCI |
                        PPC_CACHE | PPC_CACHE_LOCK | PPC_CACHE_ICBI |
@@ -3209,6 +3244,7 @@ POWERPC_FAMILY(e6500)(ObjectClass *oc, void *data)
     dc->desc = "e6500 core";
     pcc->init_proc = init_proc_e6500;
     pcc->check_pow = check_pow_none;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_MFTB |
                        PPC_WRTEE | PPC_RFDI | PPC_RFMCI |
                        PPC_CACHE | PPC_CACHE_LOCK | PPC_CACHE_ICBI |
@@ -3271,6 +3307,7 @@ POWERPC_FAMILY(603)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 603";
     pcc->init_proc = init_proc_603;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3310,6 +3347,7 @@ POWERPC_FAMILY(603E)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 603e";
     pcc->init_proc = init_proc_603;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3355,6 +3393,7 @@ POWERPC_FAMILY(e300)(ObjectClass *oc, void *data)
     dc->desc = "e300 core";
     pcc->init_proc = init_proc_e300;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_STFIWX |
@@ -3410,6 +3449,7 @@ POWERPC_FAMILY(604)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 604";
     pcc->init_proc = init_proc_604;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3455,6 +3495,7 @@ POWERPC_FAMILY(604E)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 604E";
     pcc->init_proc = init_proc_604E;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3511,6 +3552,7 @@ POWERPC_FAMILY(740)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 740";
     pcc->init_proc = init_proc_740;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3576,6 +3618,7 @@ POWERPC_FAMILY(750)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 750";
     pcc->init_proc = init_proc_750;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3722,6 +3765,7 @@ POWERPC_FAMILY(750cl)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 750 CL";
     pcc->init_proc = init_proc_750cl;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     /*
      * XXX: not implemented:
      * cache lock instructions:
@@ -3829,6 +3873,7 @@ POWERPC_FAMILY(750cx)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 750CX";
     pcc->init_proc = init_proc_750cx;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3901,6 +3946,7 @@ POWERPC_FAMILY(750fx)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 750FX";
     pcc->init_proc = init_proc_750fx;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -3973,6 +4019,7 @@ POWERPC_FAMILY(750gx)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 750GX";
     pcc->init_proc = init_proc_750gx;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -4032,6 +4079,7 @@ POWERPC_FAMILY(745)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 745";
     pcc->init_proc = init_proc_745;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -4077,6 +4125,7 @@ POWERPC_FAMILY(755)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 755";
     pcc->init_proc = init_proc_755;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX |
@@ -4143,6 +4192,7 @@ POWERPC_FAMILY(7400)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 7400 (aka G4)";
     pcc->init_proc = init_proc_7400;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -4222,6 +4272,7 @@ POWERPC_FAMILY(7410)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 7410 (aka G4)";
     pcc->init_proc = init_proc_7410;
     pcc->check_pow = check_pow_hid0;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -4322,6 +4373,7 @@ POWERPC_FAMILY(7440)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 7440 (aka G4)";
     pcc->init_proc = init_proc_7440;
     pcc->check_pow = check_pow_hid0_74xx;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -4444,6 +4496,7 @@ POWERPC_FAMILY(7450)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 7450 (aka G4)";
     pcc->init_proc = init_proc_7450;
     pcc->check_pow = check_pow_hid0_74xx;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -4573,6 +4626,7 @@ POWERPC_FAMILY(7445)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 7445 (aka G4)";
     pcc->init_proc = init_proc_7445;
     pcc->check_pow = check_pow_hid0_74xx;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -4704,6 +4758,7 @@ POWERPC_FAMILY(7455)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 7455 (aka G4)";
     pcc->init_proc = init_proc_7455;
     pcc->check_pow = check_pow_hid0_74xx;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -4855,6 +4910,7 @@ POWERPC_FAMILY(7457)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 7457 (aka G4)";
     pcc->init_proc = init_proc_7457;
     pcc->check_pow = check_pow_hid0_74xx;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -4989,6 +5045,7 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC e600";
     pcc->init_proc = init_proc_e600;
     pcc->check_pow = check_pow_hid0_74xx;
+    pcc->check_attn = check_attn_none;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -5872,6 +5929,7 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
     dc->desc = "PowerPC 970";
     pcc->init_proc = init_proc_970;
     pcc->check_pow = check_pow_970;
+    pcc->check_attn = check_attn_hid0;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -5881,7 +5939,7 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
                        PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
                        PPC_64B | PPC_ALTIVEC |
                        PPC_SEGMENT_64B | PPC_SLBI;
-    pcc->insns_flags2 = PPC2_FP_CVT_S64 | PPC2_MEM_LWSYNC;
+    pcc->insns_flags2 = PPC2_FP_CVT_S64 | PPC2_MEM_LWSYNC | PPC2_ATTN;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_VR) |
                     (1ull << MSR_POW) |
@@ -5947,6 +6005,7 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
     dc->desc = "POWER5+";
     pcc->init_proc = init_proc_power5plus;
     pcc->check_pow = check_pow_970;
+    pcc->check_attn = check_attn_hid0;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
                        PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
                        PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
@@ -5958,7 +6017,7 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
                        PPC_64B |
                        PPC_POPCNTB |
                        PPC_SEGMENT_64B | PPC_SLBI;
-    pcc->insns_flags2 = PPC2_FP_CVT_S64 | PPC2_MEM_LWSYNC;
+    pcc->insns_flags2 = PPC2_FP_CVT_S64 | PPC2_MEM_LWSYNC | PPC2_ATTN;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_VR) |
                     (1ull << MSR_POW) |
@@ -6054,6 +6113,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
     pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_hid0;
     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 |
@@ -6071,7 +6131,8 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
                         PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
                         PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
                         PPC2_FP_TST_ISA206 | PPC2_FP_CVT_S64 |
-                        PPC2_PM_ISA206 | PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
+                        PPC2_PM_ISA206 | PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206 |
+                        PPC2_ATTN;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_VR) |
                     (1ull << MSR_VSX) |
@@ -6191,6 +6252,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
     pcc->init_proc = init_proc_POWER8;
     pcc->check_pow = check_pow_nocheck;
+    pcc->check_attn = check_attn_hid0;
     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 |
@@ -6211,7 +6273,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
                         PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
                         PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
                         PPC2_TM | PPC2_PM_ISA206 | PPC2_MEM_LWSYNC |
-                        PPC2_BCDA_ISA206;
+                        PPC2_BCDA_ISA206 | PPC2_ATTN;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_HV) |
                     (1ull << MSR_TM) |
@@ -6380,6 +6442,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
                          PCR_COMPAT_2_05;
     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 |
@@ -6400,7 +6463,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
                         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;
+                        PPC2_BCDA_ISA206 | PPC2_ATTN;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_HV) |
                     (1ull << MSR_TM) |
@@ -6554,6 +6617,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
                          PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
     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 |
@@ -6574,7 +6638,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
                         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;
+                        PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206 | PPC2_ATTN;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_HV) |
                     (1ull << MSR_TM) |
@@ -6791,6 +6855,11 @@ static void init_ppc_proc(PowerPCCPU *cpu)
         warn_report("no power management check handler registered."
                     " Attempt QEMU to crash very soon !");
     }
+
+    if (env->check_attn == NULL) {
+        warn_report("no attn check handler registered."
+                    " Attempt QEMU to crash very soon !");
+    }
 }
 
 
@@ -7254,6 +7323,7 @@ static void ppc_cpu_instance_init(Object *obj)
     env->flags = pcc->flags;
     env->bfd_mach = pcc->bfd_mach;
     env->check_pow = pcc->check_pow;
+    env->check_attn = pcc->check_attn;
 
     /*
      * Mark HV mode as supported if the CPU has an MSR_HV bit in the
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ad78d027e9..e4e760449b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -188,7 +188,45 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
              env->error_code);
 }
 
+/*
+ * This stops the machine and logs CPU state without killing QEMU (like
+ * cpu_abort()) because it is often a guest error as opposed to a QEMU error,
+ * so the machine can still be debugged.
+ */
+static G_NORETURN void powerpc_checkstop(CPUPPCState *env, const char *reason)
+{
+    CPUState *cs = env_cpu(env);
+    FILE *f;
+
+    /*
+     * This stops the machine and logs CPU state without killing QEMU
+     * (like cpu_abort()) so the machine can still be debugged (because
+     * it is often a guest error).
+     */
+
+    f = qemu_log_trylock();
+    if (f) {
+        fprintf(f, "Entering checkstop state: %s\n", reason);
+        cpu_dump_state(cs, f, CPU_DUMP_FPU | CPU_DUMP_CCOP);
+        qemu_log_unlock(f);
+    }
+
+    qemu_system_guest_panicked(NULL);
+
+    cpu_loop_exit_noexc(cs);
+}
+
 #if defined(TARGET_PPC64)
+void helper_attn(CPUPPCState *env)
+{
+    if ((*env->check_attn)(env)) {
+        powerpc_checkstop(env, "host executed attn");
+    } else {
+        raise_exception_err(env, POWERPC_EXCP_HV_EMU,
+                            POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL);
+    }
+}
+
 static int powerpc_reset_wakeup(CPUPPCState *env, int excp, target_ulong *msr)
 {
     /* We no longer are in a PM state */
@@ -428,30 +466,11 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu, target_ulong vector,
 
 static void powerpc_mcheck_checkstop(CPUPPCState *env)
 {
-    CPUState *cs = env_cpu(env);
-    FILE *f;
-
     if (FIELD_EX64(env->msr, MSR, ME)) {
         return;
     }
 
-    /*
-     * This stops the machine and logs CPU state without killing QEMU
-     * (like cpu_abort()) so the machine can still be debugged (because
-     * it is often a guest error).
-     */
-
-    f = qemu_log_trylock();
-    if (f) {
-        fprintf(f, "Entering checkstop state: "
-                   "machine check with MSR[ME]=0\n");
-        cpu_dump_state(cs, f, CPU_DUMP_FPU | CPU_DUMP_CCOP);
-        qemu_log_unlock(f);
-    }
-
-    qemu_system_guest_panicked(NULL);
-
-    cpu_loop_exit_noexc(cs);
+    powerpc_checkstop(env, "machine check with MSR[ME]=0");
 }
 
 static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 28fc7791af..c2a3bdc3e6 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6482,6 +6482,16 @@ static void gen_dform3D(DisasContext *ctx)
 }
 
 #if defined(TARGET_PPC64)
+/* attn */
+static void gen_attn(DisasContext *ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    GEN_PRIV(ctx);
+#else
+    gen_helper_attn(tcg_env);
+#endif
+}
+
 /* brd */
 static void gen_brd(DisasContext *ctx)
 {
@@ -6513,6 +6523,7 @@ static void gen_brh(DisasContext *ctx)
 
 static opcode_t opcodes[] = {
 #if defined(TARGET_PPC64)
+GEN_HANDLER_E(attn, 0x00, 0x00, 0x08, 0xFFFFFDFF, PPC_NONE, PPC2_ATTN),
 GEN_HANDLER_E(brd, 0x1F, 0x1B, 0x05, 0x0000F801, PPC_NONE, PPC2_ISA310),
 GEN_HANDLER_E(brw, 0x1F, 0x1B, 0x04, 0x0000F801, PPC_NONE, PPC2_ISA310),
 GEN_HANDLER_E(brh, 0x1F, 0x1B, 0x06, 0x0000F801, PPC_NONE, PPC2_ISA310),
-- 
2.42.0

Re: [PATCH 00/13] misc ppc patches

[Philippe Mathieu-Daudé]

On 11/3/24 19:51, Nicholas Piggin wrote:
> Here's a bunch of patches that have been on the list in various
> forms, but haven't made it upstream yet. I would like to get
> them merged for 9.0 if possible. The checkstop and attn ones are
> bigger but they're not so important so I could defer them.

Yes please! Series LGTM but I don't have enough PPC knowledge
to give a proper R-b, so I'll let more familiar developers.

Regards,

Phil.