Message ID | 1612777258-23354-1-git-send-email-bmeng.cn@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | target/ppc: Add E500 L2CSR0 write helper | expand |
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,
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
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.
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
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,