[PATCH] target/ppc: Add E500 L2CSR0 write helper

[PATCH] target/ppc: Add E500 L2CSR0 write helper

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

There are several bits in L2CSR0 (exists in the e500mc/e5500/e6500
core) that should be self-cleared when written:

- L2FI  (L2 cache flash invalidate)
- L2FL  (L2 cache flush)
- L2LFC (L2 cache lock flash clear)

Add a write helper to emulate this behavior.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 target/ppc/cpu.h                |  6 ++++++
 target/ppc/translate_init.c.inc | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2609e40..e77911a 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1919,6 +1919,7 @@ typedef PowerPCCPU ArchCPU;
 #define SPR_750FX_HID2        (0x3F8)
 #define SPR_Exxx_L1FINV0      (0x3F8)
 #define SPR_L2CR              (0x3F9)
+#define SPR_Exxx_L2CSR0       (0x3F9)
 #define SPR_L3CR              (0x3FA)
 #define SPR_750_TDCH          (0x3FA)
 #define SPR_IABR2             (0x3FA)
@@ -1974,6 +1975,11 @@ typedef PowerPCCPU ArchCPU;
 #define   L1CSR1_ICFI   0x00000002  /* Instruction Cache Flash Invalidate */
 #define   L1CSR1_ICE    0x00000001  /* Instruction Cache Enable */
 
+/* E500 L2CSR0 */
+#define E500_L2CSR0_L2FI    (1 << 21)   /* L2 cache flash invalidate */
+#define E500_L2CSR0_L2FL    (1 << 11)   /* L2 cache flush */
+#define E500_L2CSR0_L2LFC   (1 << 10)   /* L2 cache lock flash clear */
+
 /* HID0 bits */
 #define HID0_DEEPNAP        (1 << 24)           /* pre-2.06 */
 #define HID0_DOZE           (1 << 23)           /* pre-2.06 */
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 9867d0a..3ec45cb 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -1735,6 +1735,16 @@ static void spr_write_e500_l1csr1(DisasContext *ctx, int sprn, int gprn)
     tcg_temp_free(t0);
 }
 
+static void spr_write_e500_l2csr0(DisasContext *ctx, int sprn, int gprn)
+{
+    TCGv t0 = tcg_temp_new();
+
+    tcg_gen_andi_tl(t0, cpu_gpr[gprn],
+                    ~(E500_L2CSR0_L2FI | E500_L2CSR0_L2FL | E500_L2CSR0_L2LFC));
+    gen_store_spr(sprn, t0);
+    tcg_temp_free(t0);
+}
+
 static void spr_write_booke206_mmucsr0(DisasContext *ctx, int sprn, int gprn)
 {
     gen_helper_booke206_tlbflush(cpu_env, cpu_gpr[gprn]);
@@ -5029,6 +5039,12 @@ static void init_proc_e500(CPUPPCState *env, int version)
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_generic, &spr_write_e500_l1csr1,
                  0x00000000);
+    if (version != fsl_e500v1 && version != fsl_e500v2) {
+        spr_register(env, SPR_Exxx_L2CSR0, "L2CSR0",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     &spr_read_generic, &spr_write_e500_l2csr0,
+                     0x00000000);
+    }
     spr_register(env, SPR_BOOKE_MCSRR0, "MCSRR0",
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_generic, &spr_write_generic,
-- 
2.7.4

Re: [PATCH] target/ppc: Add E500 L2CSR0 write helper

[David Gibson]

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

On Mon, Feb 08, 2021 at 05:40:58PM +0800, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> There are several bits in L2CSR0 (exists in the e500mc/e5500/e6500
> core) that should be self-cleared when written:
> 
> - L2FI  (L2 cache flash invalidate)
> - L2FL  (L2 cache flush)
> - L2LFC (L2 cache lock flash clear)
> 
> Add a write helper to emulate this behavior.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

IIUC, these are essentially write-only bits - they have some side
effect when written on real hardware, but won't ever be read back.  Is
that correct?  Do you have a reference to hardware docs describing
this behaviour?

I'm assuming that because we don't model the L2 cache, it's ok that
your implementation just ignores writing these bits, rather than
performing the cache operations requested?

Is that still true for the flash clear operation?

> ---
> 
>  target/ppc/cpu.h                |  6 ++++++
>  target/ppc/translate_init.c.inc | 16 ++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 2609e40..e77911a 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1919,6 +1919,7 @@ typedef PowerPCCPU ArchCPU;
>  #define SPR_750FX_HID2        (0x3F8)
>  #define SPR_Exxx_L1FINV0      (0x3F8)
>  #define SPR_L2CR              (0x3F9)
> +#define SPR_Exxx_L2CSR0       (0x3F9)
>  #define SPR_L3CR              (0x3FA)
>  #define SPR_750_TDCH          (0x3FA)
>  #define SPR_IABR2             (0x3FA)
> @@ -1974,6 +1975,11 @@ typedef PowerPCCPU ArchCPU;
>  #define   L1CSR1_ICFI   0x00000002  /* Instruction Cache Flash Invalidate */
>  #define   L1CSR1_ICE    0x00000001  /* Instruction Cache Enable */
>  
> +/* E500 L2CSR0 */
> +#define E500_L2CSR0_L2FI    (1 << 21)   /* L2 cache flash invalidate */
> +#define E500_L2CSR0_L2FL    (1 << 11)   /* L2 cache flush */
> +#define E500_L2CSR0_L2LFC   (1 << 10)   /* L2 cache lock flash clear */
> +
>  /* HID0 bits */
>  #define HID0_DEEPNAP        (1 << 24)           /* pre-2.06 */
>  #define HID0_DOZE           (1 << 23)           /* pre-2.06 */
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 9867d0a..3ec45cb 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -1735,6 +1735,16 @@ static void spr_write_e500_l1csr1(DisasContext *ctx, int sprn, int gprn)
>      tcg_temp_free(t0);
>  }
>  
> +static void spr_write_e500_l2csr0(DisasContext *ctx, int sprn, int gprn)
> +{
> +    TCGv t0 = tcg_temp_new();
> +
> +    tcg_gen_andi_tl(t0, cpu_gpr[gprn],
> +                    ~(E500_L2CSR0_L2FI | E500_L2CSR0_L2FL | E500_L2CSR0_L2LFC));
> +    gen_store_spr(sprn, t0);
> +    tcg_temp_free(t0);
> +}
> +
>  static void spr_write_booke206_mmucsr0(DisasContext *ctx, int sprn, int gprn)
>  {
>      gen_helper_booke206_tlbflush(cpu_env, cpu_gpr[gprn]);
> @@ -5029,6 +5039,12 @@ static void init_proc_e500(CPUPPCState *env, int version)
>                   SPR_NOACCESS, SPR_NOACCESS,
>                   &spr_read_generic, &spr_write_e500_l1csr1,
>                   0x00000000);
> +    if (version != fsl_e500v1 && version != fsl_e500v2) {
> +        spr_register(env, SPR_Exxx_L2CSR0, "L2CSR0",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_generic, &spr_write_e500_l2csr0,
> +                     0x00000000);
> +    }
>      spr_register(env, SPR_BOOKE_MCSRR0, "MCSRR0",
>                   SPR_NOACCESS, SPR_NOACCESS,
>                   &spr_read_generic, &spr_write_generic,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] target/ppc: Add E500 L2CSR0 write helper

[Bin Meng]

Hi David,

On Wed, Feb 10, 2021 at 9:50 AM David Gibson
<david@gibson.dropbear.id.au> wrote:
>
> On Mon, Feb 08, 2021 at 05:40:58PM +0800, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > There are several bits in L2CSR0 (exists in the e500mc/e5500/e6500
> > core) that should be self-cleared when written:
> >
> > - L2FI  (L2 cache flash invalidate)
> > - L2FL  (L2 cache flush)
> > - L2LFC (L2 cache lock flash clear)
> >
> > Add a write helper to emulate this behavior.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> IIUC, these are essentially write-only bits - they have some side
> effect when written on real hardware, but won't ever be read back.  Is
> that correct?  Do you have a reference to hardware docs describing
> this behaviour?
>

Please see https://www.nxp.com/files-static/32bit/doc/ref_manual/EREFRM.pdf,
chapter 3.11.2

> I'm assuming that because we don't model the L2 cache, it's ok that
> your implementation just ignores writing these bits, rather than
> performing the cache operations requested?

Yes, guests may read back these bits to confirm the operation is done
by hardware after writing 1 to these bits.

>
> Is that still true for the flash clear operation?

Yes.

>
> > ---
> >
> >  target/ppc/cpu.h                |  6 ++++++
> >  target/ppc/translate_init.c.inc | 16 ++++++++++++++++
> >  2 files changed, 22 insertions(+)
> >

Regards,
Bin

Re: [PATCH] target/ppc: Add E500 L2CSR0 write helper

[David Gibson]

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

On Wed, Feb 10, 2021 at 09:53:53AM +0800, Bin Meng wrote:
> Hi David,
> 
> On Wed, Feb 10, 2021 at 9:50 AM David Gibson
> <david@gibson.dropbear.id.au> wrote:
> >
> > On Mon, Feb 08, 2021 at 05:40:58PM +0800, Bin Meng wrote:
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > There are several bits in L2CSR0 (exists in the e500mc/e5500/e6500
> > > core) that should be self-cleared when written:
> > >
> > > - L2FI  (L2 cache flash invalidate)
> > > - L2FL  (L2 cache flush)
> > > - L2LFC (L2 cache lock flash clear)
> > >
> > > Add a write helper to emulate this behavior.
> > >
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > IIUC, these are essentially write-only bits - they have some side
> > effect when written on real hardware, but won't ever be read back.  Is
> > that correct?  Do you have a reference to hardware docs describing
> > this behaviour?
> >
> 
> Please see https://www.nxp.com/files-static/32bit/doc/ref_manual/EREFRM.pdf,
> chapter 3.11.2

Ah, thanks.  So these actually don't operate quite how I was
suggesting - they are readable, and return 1 until the operation is
completed.

So what you're effectively doing here is simulating the cache
operations completing instantly - which is correct because we don't
model the cache.

Can you please clarify that in your commit message, including the
pointer to the chip doc.

> > I'm assuming that because we don't model the L2 cache, it's ok that
> > your implementation just ignores writing these bits, rather than
> > performing the cache operations requested?
> 
> Yes, guests may read back these bits to confirm the operation is done
> by hardware after writing 1 to these bits.
> 
> >
> > Is that still true for the flash clear operation?
> 
> Yes.

Ah, yes, I see.  The name made me think this might be something like
dcbz, which has visible effects on architected state.  This is just
clearing cache locks, which we don't model in any case.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] target/ppc: Add E500 L2CSR0 write helper

[Bin Meng]

Hi David,

On Wed, Feb 10, 2021 at 10:09 AM David Gibson
<david@gibson.dropbear.id.au> wrote:
>
> On Wed, Feb 10, 2021 at 09:53:53AM +0800, Bin Meng wrote:
> > Hi David,
> >
> > On Wed, Feb 10, 2021 at 9:50 AM David Gibson
> > <david@gibson.dropbear.id.au> wrote:
> > >
> > > On Mon, Feb 08, 2021 at 05:40:58PM +0800, Bin Meng wrote:
> > > > From: Bin Meng <bin.meng@windriver.com>
> > > >
> > > > There are several bits in L2CSR0 (exists in the e500mc/e5500/e6500
> > > > core) that should be self-cleared when written:
> > > >
> > > > - L2FI  (L2 cache flash invalidate)
> > > > - L2FL  (L2 cache flush)
> > > > - L2LFC (L2 cache lock flash clear)
> > > >
> > > > Add a write helper to emulate this behavior.
> > > >
> > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > >
> > > IIUC, these are essentially write-only bits - they have some side
> > > effect when written on real hardware, but won't ever be read back.  Is
> > > that correct?  Do you have a reference to hardware docs describing
> > > this behaviour?
> > >
> >
> > Please see https://www.nxp.com/files-static/32bit/doc/ref_manual/EREFRM.pdf,
> > chapter 3.11.2
>
> Ah, thanks.  So these actually don't operate quite how I was
> suggesting - they are readable, and return 1 until the operation is
> completed.
>
> So what you're effectively doing here is simulating the cache
> operations completing instantly - which is correct because we don't
> model the cache.
>
> Can you please clarify that in your commit message, including the
> pointer to the chip doc.

Sure, will do in v2.

>
> > > I'm assuming that because we don't model the L2 cache, it's ok that
> > > your implementation just ignores writing these bits, rather than
> > > performing the cache operations requested?
> >
> > Yes, guests may read back these bits to confirm the operation is done
> > by hardware after writing 1 to these bits.
> >
> > >
> > > Is that still true for the flash clear operation?
> >
> > Yes.
>
> Ah, yes, I see.  The name made me think this might be something like
> dcbz, which has visible effects on architected state.  This is just
> clearing cache locks, which we don't model in any case.
>

Thanks for the review.

Regards,
Bin