diff mbox

[RFC,11/12] ppc4xx: Export ECB and PLB emulation

Message ID 675e091387248b49c97ff99ef07eb23ed316aefb.1502643878.git.balaton@eik.bme.hu
State New
Headers show

Commit Message

BALATON Zoltan Aug. 13, 2017, 5:04 p.m. UTC
These devices are found in some other ppc4xx cores too.
Also add some more PLB registers for 460EX.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc405.h    |  3 +++
 hw/ppc/ppc405_uc.c | 16 +++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

Comments

David Gibson Aug. 14, 2017, 4:44 a.m. UTC | #1
On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:
> These devices are found in some other ppc4xx cores too.

Elsewhere in the series you moved things that were used elsewhere out
of 405_uc, why not do that here as well?

> Also add some more PLB registers for 460EX.

Separate patch for that please, it's logically unrelated.  Plus the
commit message should say what these registers are, where they appear,
why do we need them?

> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/ppc405.h    |  3 +++
>  hw/ppc/ppc405_uc.c | 16 +++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
> index a9ffc87..7ed25cf 100644
> --- a/hw/ppc/ppc405.h
> +++ b/hw/ppc/ppc405.h
> @@ -59,6 +59,9 @@ struct ppc4xx_bd_info_t {
>  ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
>                                  uint32_t flags);
>  
> +void ppc4xx_plb_init(CPUPPCState *env);
> +void ppc405_ebc_init(CPUPPCState *env);
> +
>  CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
>                          MemoryRegion ram_memories[4],
>                          hwaddr ram_bases[4],
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index 8f44cb4..8e58065 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -105,9 +105,12 @@ ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
>  /*****************************************************************************/
>  /* Peripheral local bus arbitrer */
>  enum {
> -    PLB0_BESR = 0x084,
> -    PLB0_BEAR = 0x086,
> -    PLB0_ACR  = 0x087,
> +    PLB3A0_ACR = 0x077,
> +    PLB4A0_ACR = 0x081,
> +    PLB0_BESR  = 0x084,
> +    PLB0_BEAR  = 0x086,
> +    PLB0_ACR   = 0x087,
> +    PLB4A1_ACR = 0x089,
>  };
>  
>  typedef struct ppc4xx_plb_t ppc4xx_plb_t;
> @@ -174,14 +177,17 @@ static void ppc4xx_plb_reset (void *opaque)
>      plb->besr = 0x00000000;
>  }
>  
> -static void ppc4xx_plb_init(CPUPPCState *env)
> +void ppc4xx_plb_init(CPUPPCState *env)
>  {
>      ppc4xx_plb_t *plb;
>  
>      plb = g_malloc0(sizeof(ppc4xx_plb_t));
> +    ppc_dcr_register(env, PLB3A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
> +    ppc_dcr_register(env, PLB4A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
>      ppc_dcr_register(env, PLB0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
>      ppc_dcr_register(env, PLB0_BEAR, plb, &dcr_read_plb, &dcr_write_plb);
>      ppc_dcr_register(env, PLB0_BESR, plb, &dcr_read_plb, &dcr_write_plb);
> +    ppc_dcr_register(env, PLB4A1_ACR, plb, &dcr_read_plb, &dcr_write_plb);
>      qemu_register_reset(ppc4xx_plb_reset, plb);
>  }
>  
> @@ -585,7 +591,7 @@ static void ebc_reset (void *opaque)
>      ebc->cfg = 0x80400000;
>  }
>  
> -static void ppc405_ebc_init(CPUPPCState *env)
> +void ppc405_ebc_init(CPUPPCState *env)
>  {
>      ppc4xx_ebc_t *ebc;
>
BALATON Zoltan Aug. 14, 2017, 11:06 a.m. UTC | #2
On Mon, 14 Aug 2017, David Gibson wrote:
> On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:
>> These devices are found in some other ppc4xx cores too.

Thanks for the quick review, hope more will follow for other patches 
too maybe also from others.

> Elsewhere in the series you moved things that were used elsewhere out
> of 405_uc, why not do that here as well?

I've moved those because those devices needed to be modified extensively 
while these two in this patch are used basically unchanged (they may not 
even be completely correct for 460EX but seem to work well enough) so I've 
left them here. I could add this explanation to the commit message or 
maybe move them to ppc4xx_devs to make it clearer that they are not only 
used by 405 emulation.

>> Also add some more PLB registers for 460EX.
>
> Separate patch for that please, it's logically unrelated.  Plus the
> commit message should say what these registers are, where they appear,
> why do we need them?

OK. I don't really know what these registers are. I guess we need them 
because U-Boot or other guests poke them but they are only added to avoid 
the crash, not really emulated (just read zero, ignore write) for now.

Regards,
BALATON Zoltan
David Gibson Aug. 18, 2017, 6:11 a.m. UTC | #3
On Mon, Aug 14, 2017 at 01:06:42PM +0200, BALATON Zoltan wrote:
> On Mon, 14 Aug 2017, David Gibson wrote:
> > On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:
> > > These devices are found in some other ppc4xx cores too.
> 
> Thanks for the quick review, hope more will follow for other patches too
> maybe also from others.
> 
> > Elsewhere in the series you moved things that were used elsewhere out
> > of 405_uc, why not do that here as well?
> 
> I've moved those because those devices needed to be modified extensively
> while these two in this patch are used basically unchanged (they may not
> even be completely correct for 460EX but seem to work well enough) so I've
> left them here. I could add this explanation to the commit message or maybe
> move them to ppc4xx_devs to make it clearer that they are not only used by
> 405 emulation.

I think move them.  Probably to their own file - I think smaller files
are usually going to be more readable than one big file with heaps of
4xx devices.

> > > Also add some more PLB registers for 460EX.
> > 
> > Separate patch for that please, it's logically unrelated.  Plus the
> > commit message should say what these registers are, where they appear,
> > why do we need them?
> 
> OK. I don't really know what these registers are. I guess we need them
> because U-Boot or other guests poke them but they are only added to avoid
> the crash, not really emulated (just read zero, ignore write) for
> now.

Ok, so say that in the commit message.
diff mbox

Patch

diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index a9ffc87..7ed25cf 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -59,6 +59,9 @@  struct ppc4xx_bd_info_t {
 ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
                                 uint32_t flags);
 
+void ppc4xx_plb_init(CPUPPCState *env);
+void ppc405_ebc_init(CPUPPCState *env);
+
 CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
                         MemoryRegion ram_memories[4],
                         hwaddr ram_bases[4],
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 8f44cb4..8e58065 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -105,9 +105,12 @@  ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, ppc4xx_bd_info_t *bd,
 /*****************************************************************************/
 /* Peripheral local bus arbitrer */
 enum {
-    PLB0_BESR = 0x084,
-    PLB0_BEAR = 0x086,
-    PLB0_ACR  = 0x087,
+    PLB3A0_ACR = 0x077,
+    PLB4A0_ACR = 0x081,
+    PLB0_BESR  = 0x084,
+    PLB0_BEAR  = 0x086,
+    PLB0_ACR   = 0x087,
+    PLB4A1_ACR = 0x089,
 };
 
 typedef struct ppc4xx_plb_t ppc4xx_plb_t;
@@ -174,14 +177,17 @@  static void ppc4xx_plb_reset (void *opaque)
     plb->besr = 0x00000000;
 }
 
-static void ppc4xx_plb_init(CPUPPCState *env)
+void ppc4xx_plb_init(CPUPPCState *env)
 {
     ppc4xx_plb_t *plb;
 
     plb = g_malloc0(sizeof(ppc4xx_plb_t));
+    ppc_dcr_register(env, PLB3A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
+    ppc_dcr_register(env, PLB4A0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
     ppc_dcr_register(env, PLB0_ACR, plb, &dcr_read_plb, &dcr_write_plb);
     ppc_dcr_register(env, PLB0_BEAR, plb, &dcr_read_plb, &dcr_write_plb);
     ppc_dcr_register(env, PLB0_BESR, plb, &dcr_read_plb, &dcr_write_plb);
+    ppc_dcr_register(env, PLB4A1_ACR, plb, &dcr_read_plb, &dcr_write_plb);
     qemu_register_reset(ppc4xx_plb_reset, plb);
 }
 
@@ -585,7 +591,7 @@  static void ebc_reset (void *opaque)
     ebc->cfg = 0x80400000;
 }
 
-static void ppc405_ebc_init(CPUPPCState *env)
+void ppc405_ebc_init(CPUPPCState *env)
 {
     ppc4xx_ebc_t *ebc;