[PATCH] hw/ppc/ppc400_bamboo: Set dcr-base correctly when creating UIC

[PATCH] hw/ppc/ppc400_bamboo: Set dcr-base correctly when creating UIC

[Peter Maydell]

In commit 0270d74ef8862350 we switched from ppcuic_init() to directly
creating the UIC device, but I missed that the Bamboo's UIC has a
non-standard DCR base register value (0xc0 rather than the default
of 0x30). This made Linux panic early in the boot process.

Specify the correct dcr-base property value when creating the UIC.

Fixes: 0270d74ef8862350
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
With this fix Nathan's test case goes on to eventually hit
a QEMU assert:
qemu-system-ppc: ../../hw/pci/pci.c:255: pci_bus_change_irq_level: Assertion `irq_num >= 0' failed.
but it was doing that on 5.2 as well.
---
 hw/ppc/ppc440_bamboo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index b156bcb9990..2c7a578ba73 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -198,6 +198,7 @@ static void bamboo_init(MachineState *machine)
     uicdev = qdev_new(TYPE_PPC_UIC);
     uicsbd = SYS_BUS_DEVICE(uicdev);
 
+    qdev_prop_set_uint32(uicdev, "dcr-base", 0xc0);
     object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
                              &error_fatal);
     sysbus_realize_and_unref(uicsbd, &error_fatal);
-- 
2.20.1

Re: [PATCH] hw/ppc/ppc400_bamboo: Set dcr-base correctly when creating UIC

[BALATON Zoltan]

On Mon, 11 Jan 2021, Peter Maydell wrote:
> In commit 0270d74ef8862350 we switched from ppcuic_init() to directly
> creating the UIC device, but I missed that the Bamboo's UIC has a
> non-standard DCR base register value (0xc0 rather than the default
> of 0x30). This made Linux panic early in the boot process.
>
> Specify the correct dcr-base property value when creating the UIC.
>
> Fixes: 0270d74ef8862350
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reported-by?

> ---
> With this fix Nathan's test case goes on to eventually hit
> a QEMU assert:
> qemu-system-ppc: ../../hw/pci/pci.c:255: pci_bus_change_irq_level: Assertion `irq_num >= 0' failed.
> but it was doing that on 5.2 as well.
> ---
> hw/ppc/ppc440_bamboo.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index b156bcb9990..2c7a578ba73 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -198,6 +198,7 @@ static void bamboo_init(MachineState *machine)
>     uicdev = qdev_new(TYPE_PPC_UIC);
>     uicsbd = SYS_BUS_DEVICE(uicdev);
>
> +    qdev_prop_set_uint32(uicdev, "dcr-base", 0xc0);

This fixes Bamboo but not virtex and 405 which seem to have same problem 
as I've just shown in replies to those patches. So maybe this is better 
fixed by changing default value in ppc-uic.c to 0xc0 then. You say in 
commit message that 0xc0 is non-standard but most boards seem to use that 
than the default you have now. I don't know if there's a standard by the 
way or every CPU implementation just puts DCRs where they want.

Regards,
BALATON Zoltan

>     object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
>                              &error_fatal);
>     sysbus_realize_and_unref(uicsbd, &error_fatal);
> -- 
> 2.20.1
>
>
>

Re: [PATCH] hw/ppc/ppc400_bamboo: Set dcr-base correctly when creating UIC

[Peter Maydell]

On Mon, 11 Jan 2021 at 19:19, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Mon, 11 Jan 2021, Peter Maydell wrote:
> > In commit 0270d74ef8862350 we switched from ppcuic_init() to directly
> > creating the UIC device, but I missed that the Bamboo's UIC has a
> > non-standard DCR base register value (0xc0 rather than the default
> > of 0x30). This made Linux panic early in the boot process.
> >
> > Specify the correct dcr-base property value when creating the UIC.
> >
> > Fixes: 0270d74ef8862350
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reported-by?

Yes, sorry --

Reported-by: Nathan Chancellor <natechancellor@gmail.com>

> > diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> > index b156bcb9990..2c7a578ba73 100644
> > --- a/hw/ppc/ppc440_bamboo.c
> > +++ b/hw/ppc/ppc440_bamboo.c
> > @@ -198,6 +198,7 @@ static void bamboo_init(MachineState *machine)
> >     uicdev = qdev_new(TYPE_PPC_UIC);
> >     uicsbd = SYS_BUS_DEVICE(uicdev);
> >
> > +    qdev_prop_set_uint32(uicdev, "dcr-base", 0xc0);
>
> This fixes Bamboo but not virtex and 405 which seem to have same problem
> as I've just shown in replies to those patches. So maybe this is better
> fixed by changing default value in ppc-uic.c to 0xc0 then. You say in
> commit message that 0xc0 is non-standard but most boards seem to use that
> than the default you have now. I don't know if there's a standard by the
> way or every CPU implementation just puts DCRs where they want.

My intention when I wrote the code was just to set the default value of
the property on the device to the value that most of the users seemed to
need -- I don't know if there's any actual standard. It sounds
like the actual bug is that I put in the wrong default value by
accident.

For the QEMU boards we have with UICs, they all use 0xc0 --
the only special case is sam460ex because it has four UICs,
so they go at 0xc0, 0xd0, 0xe0, 0xf0.

New patch coming up that fixes the default property value.

thanks
-- PMM