On 28 November 2013 03:29, Peter Crosthwaite
<peter.crosthwa...@xilinx.com> wrote:
> Fix the CBAR initialisation by using the newly defined static property.
> CBAR is now set before realization, so the intended value is now
> actually used.
>
> So I have kinda tested this. I booted an ARM kernel on Highbank with the
> stock Highbank DTB. It doesnt boot (and I will be doing something
> wrong), but before this patch I got this:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at
> /workspaces/pcrost/public/linux2.git/arch/arm/mm/ioremap.c:301
> __arm_ioremap_pfn_caller+0x180/0x198()
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W
> 3.13.0-rc1-next-20131126-dirty #2
> [<c0015164>] (unwind_backtrace) from [<c00118c0>] (show_stack+0x10/0x14)
> [<c00118c0>] (show_stack) from [<c02bd5fc>] (dump_stack+0x78/0x90)
> [<c02bd5fc>] (dump_stack) from [<c001f110>] (warn_slowpath_common+0x68/0x84)
> [<c001f110>] (warn_slowpath_common) from [<c001f1f4>]
> (warn_slowpath_null+0x1c/0x24)
> [<c001f1f4>] (warn_slowpath_null) from [<c0017c6c>]
> (__arm_ioremap_pfn_caller+0x180/0x198)
> [<c0017c6c>] (__arm_ioremap_pfn_caller) from [<c0017cd8>]
> (__arm_ioremap_caller+0x54/0x5c)
> [<c0017cd8>] (__arm_ioremap_caller) from [<c0017d10>]
> (__arm_ioremap+0x18/0x1c)
> [<c0017d10>] (__arm_ioremap) from [<c03913c0>] (highbank_init_irq+0x34/0x8c)
> [<c03913c0>] (highbank_init_irq) from [<c038c228>] (init_IRQ+0x28/0x2c)
> [<c038c228>] (init_IRQ) from [<c03899ec>] (start_kernel+0x234/0x398)
> [<c03899ec>] (start_kernel) from [<00008074>] (0x8074)
> ---[ end trace 3406ff24bd97382f ]---
>
> Which dissappears with this patch.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> ---
> changed since v1:
> use error report rather than fprintf(stderr
>
> hw/arm/highbank.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index 1d19d8f..cb32325 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -236,14 +236,16 @@ static void calxeda_init(QEMUMachineInitArgs *args,
> enum cxmachines machine)
>
> cpu = ARM_CPU(object_new(object_class_get_name(oc)));
>
> + object_property_set_int(OBJECT(cpu), GIC_BASE_ADDR, "reset-cbar",
> &err);
> + if (err) {
> + error_report("%s", error_get_pretty(err));
> + exit(1);
> + }
It's kind of sad that we need five lines of code just to
say "set this property on this object which we should
know at compile time to exist" . (I was about to argue we
should just assert, but since we don't refuse to start with
wacky user-provided cpu-model strings we can't quite get
away with that.)
More significantly, this will break midway, because
cortex-a15 doesn't have this property. Fortunately, the
actual A15 does have a CBAR register so you can just
add a patch to set the feature bit for it.
Caution, this means our semantics for reset-cbar
are "actual reset value of register", which is not
the same as "base address of peripherals", because
for A15 the register has
[31:15] bits 31:15 of base-addr
[14:8] reserved, zero
[7:0] bits 39:32 of base-addr
which makes a difference if you were nutty enough to
put the GIC above the 4GB boundary.
(As with the real hardware, setting this config property
doesn't actually change where the GIC & friends live
in the address space, incidentally.)
> object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> if (err) {
> error_report("%s", error_get_pretty(err));
> exit(1);
> }
> -
> - /* This will become a QOM property eventually */
> - cpu->reset_cbar = GIC_BASE_ADDR;
> cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
> }
thanks
-- PMM
