Re: [Qemu-devel] [PATCH arm-devs v2 8/8] arm/highbank.c: Fix MPCore periphbase name

[Peter Maydell]

On 28 November 2013 03:31, Peter Crosthwaite
<address@hidden> wrote:
> GIC_BASE_ADDR is not the base address of the GIC. Its clear from the
> code that this is the base address of the MPCore. Rename to
> MPCORE_PERIPHBASE accordingly.

"MPCore" is one of those terms I dislike because it
doesn't actually match up with the documentation's
terminology...

> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>
>  hw/arm/highbank.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index cb32325..be2cc20 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -28,11 +28,11 @@
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
>
> -#define SMP_BOOT_ADDR 0x100
> -#define SMP_BOOT_REG  0x40
> -#define GIC_BASE_ADDR 0xfff10000
> +#define SMP_BOOT_ADDR           0x100
> +#define SMP_BOOT_REG            0x40
> +#define MPCORE_PERIPHBASE       0xfff10000
>
> -#define NIRQ_GIC      160
> +#define NIRQ_GIC                160
>
>  /* Board init.  */
>
> @@ -55,7 +55,7 @@ static void hb_write_secondary(ARMCPU *cpu, const struct 
> arm_boot_info *info)
>          0xe1110001, /* tst     r1, r1 */
>          0x0afffffb, /* beq     <wfi> */
>          0xe12fff11, /* bx      r1 */
> -        GIC_BASE_ADDR      /* privbase: gic address.  */
> +        MPCORE_PERIPHBASE   /* privbase: gic address.  */

Comment no longer matches code.

More seriously, this secondary-boot code has a bunch
of hardcoded offsets for GIC registers which are
correct for the A9 but wrong for A15. Andre, did you
test Midway with an SMP boot config?

The generic hw/arm/boot code dealt with this problem
by feeding the boot code the GIC CPU interface base
address rather than the generic peripheral base address;
the offsets within the CPU i/f are the same for both
cores.

>      };
>      for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
>          smpboot[n] = tswap32(smpboot[n]);
> @@ -236,7 +236,8 @@ 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);
> +        object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar",
> +                                &err);
>          if (err) {
>              error_report("%s", error_get_pretty(err));
>              exit(1);
> @@ -287,7 +288,7 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
> cxmachines machine)
>      qdev_prop_set_uint32(dev, "num-irq", NIRQ_GIC);
>      qdev_init_nofail(dev);
>      busdev = SYS_BUS_DEVICE(dev);
> -    sysbus_mmio_map(busdev, 0, GIC_BASE_ADDR);
> +    sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
>      for (n = 0; n < smp_cpus; n++) {
>          sysbus_connect_irq(busdev, n, cpu_irq[n]);
>      }
> --
> 1.8.4.4
>

thanks
-- PMM