[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH 13/15] ppc4xx: Add more PLB registers

From: BALATON Zoltan
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH 13/15] ppc4xx: Add more PLB registers
Date: Mon, 21 Aug 2017 00:12:02 +0200 (CEST)
User-agent: Alpine 2.02 (LMD 1266 2009-07-14)

On Sun, 20 Aug 2017, Philippe Mathieu-Daudé wrote:
On 08/20/2017 02:23 PM, BALATON Zoltan wrote:
These registers are present in 440 SoCs (and maybe in others too) and
U-Boot accesses them when printing register info. We don't emulate
these but add them to avoid crashing when they are read or written.

Your code isn't incorrect but it doesn't sound the right way to fix your problem. Your firmware shouldn't *crash* on unimplemented dcr.

The firmware shouldn't crash but it's just isn't written expecting missing DCRs (and we aim to run the board's patched U-Boot version as it is because it is needed by the OSes running on the board). QEMU makes it crash by raising an exception for unknown DCRs, I think in target/ppc/timebase_helper.c:150

Looking at ppc_dcr_read() I see that *valp isn't updated on unimp dcrn, while the dcr_read_plb() callback you are using return 0 on unimp (with an "avoid gcc warning" misleading comment).

What is the hardware behavior on implemented dcr? return 0? In that case this should be used in ppc_dcr_read(), also adding some qemu_log_mask(LOG_UNIMP,...) log entry there.

Of course the right way would be to emulate what these registers do on hardware but I don't know what the hardware does and for a lot of DCRs the firmware just writes some values to initialise the hadware which is not needed on QEMU so we can just do read zero, ignore writes which we are doing here. (That's what I was trying to say in the commit message too.) Actually these registers aren't even used by the firmware, only included in register dumps so this should be safe until we find something tries to use it.



Signed-off-by: BALATON Zoltan <address@hidden>
  hw/ppc/ppc405_uc.c | 12 +++++++++---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index e621d0a..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;
@@ -179,9 +182,12 @@ 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);

reply via email to

[Prev in Thread] Current Thread [Next in Thread]