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

Nicholas Piggin posted 13 patches 8 months, 2 weeks ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Nicholas Piggin <npiggin@gmail.com>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH 02/13] target/ppc: POWER10 does not have transactional memory
Posted by Nicholas Piggin 8 months, 2 weeks ago
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
Posted by Harsh Prateek Bora 8 months, 2 weeks ago
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
Posted by Nicholas Piggin 8 months, 2 weeks ago
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>