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

[David Gibson]

On Sun, Aug 20, 2017 at 06:58:52PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Zoltan,
> 
> 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.
> 
> 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).

Hrm.. I thought accessing a bad DCR led to a 0x700 exception rather
than just returning a dummy value.

> 
> 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.
> 
> Regards,
> 
> Phil.
> 
> > 
> > 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);
> >   }
> > 
> 

-- 
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

signature.asc
Description: PGP signature