Re: [Qemu-devel] [PATCH microblaze v1 1/1] Microblaze: Convert Microblaze-pic handling to GPIOs

[Peter Crosthwaite]

On Fri, Jan 10, 2014 at 4:41 PM, Alistair Francis
<address@hidden> wrote:
> This patch uses inbound GPIO lines (IRQ and FIR) for
> interrupts instead of using the old pic_cpu method,
> which doesn't correspond to real hardware.
>
> This creates the CPU's inbound IRQ and FIR GPIO lines and
> updates the Microblaze boards to use this new method.
>

Suggested-by: Peter Crosthwaite <address@hidden>

> Signed-off-by: Alistair Francis <address@hidden>
> ---
> Similar to what Peter Maydell did to convert ARM-pic to
> GPIOs this converts Microblaze-pic to GPIOs.
>
>  hw/microblaze/petalogix_ml605_mmu.c      |    9 ++----
>  hw/microblaze/petalogix_s3adsp1800_mmu.c |    9 ++----
>  hw/microblaze/pic_cpu.c                  |   47 
> ------------------------------
>  hw/microblaze/pic_cpu.h                  |    8 -----
>  target-microblaze/cpu.c                  |   21 +++++++++++++
>  target-microblaze/cpu.h                  |    4 ++
>  6 files changed, 31 insertions(+), 67 deletions(-)
>  delete mode 100644 hw/microblaze/pic_cpu.c
>  delete mode 100644 hw/microblaze/pic_cpu.h
>
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
> b/hw/microblaze/petalogix_ml605_mmu.c
> index 10970e0..1a87756 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -39,7 +39,6 @@
>  #include "hw/ssi.h"
>
>  #include "boot.h"
> -#include "pic_cpu.h"
>
>  #include "hw/stream.h"
>
> @@ -82,20 +81,18 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
>      Object *ds, *cs;
>      MicroBlazeCPU *cpu;
>      SysBusDevice *busdev;
> -    CPUMBState *env;
>      DriveInfo *dinfo;
>      int i;
>      hwaddr ddr_base = MEMORY_BASEADDR;
>      MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1);
>      MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
> -    qemu_irq irq[32], *cpu_irq;
> +    qemu_irq irq[32];
>
>      /* init CPUs */
>      if (cpu_model == NULL) {
>          cpu_model = "microblaze";
>      }
>      cpu = cpu_mb_init(cpu_model);
> -    env = &cpu->env;
>
>      /* Attach emulated BRAM through the LMB.  */
>      memory_region_init_ram(phys_lmb_bram, NULL, "petalogix_ml605.lmb_bram",
> @@ -117,8 +114,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
>                            2, 0x89, 0x18, 0x0000, 0x0, 0);
>
>
> -    cpu_irq = microblaze_pic_init_cpu(env);
> -    dev = xilinx_intc_create(INTC_BASEADDR, cpu_irq[0], 4);
> +    dev = xilinx_intc_create(INTC_BASEADDR, qdev_get_gpio_in(DEVICE(cpu),
> +                             MB_CPU_IRQ), 4);
>      for (i = 0; i < 32; i++) {
>          irq[i] = qdev_get_gpio_in(dev, i);
>      }
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c 
> b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index ec6489c..f500215 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -35,7 +35,6 @@
>  #include "exec/address-spaces.h"
>
>  #include "boot.h"
> -#include "pic_cpu.h"
>
>  #define LMB_BRAM_SIZE  (128 * 1024)
>  #define FLASH_SIZE     (16 * 1024 * 1024)
> @@ -63,13 +62,12 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
>      const char *cpu_model = args->cpu_model;
>      DeviceState *dev;
>      MicroBlazeCPU *cpu;
> -    CPUMBState *env;
>      DriveInfo *dinfo;
>      int i;
>      hwaddr ddr_base = MEMORY_BASEADDR;
>      MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1);
>      MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
> -    qemu_irq irq[32], *cpu_irq;
> +    qemu_irq irq[32];
>      MemoryRegion *sysmem = get_system_memory();
>
>      /* init CPUs */
> @@ -77,7 +75,6 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
>          cpu_model = "microblaze";
>      }
>      cpu = cpu_mb_init(cpu_model);
> -    env = &cpu->env;
>
>      /* Attach emulated BRAM through the LMB.  */
>      memory_region_init_ram(phys_lmb_bram, NULL,
> @@ -96,8 +93,8 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
>                            FLASH_SIZE >> 16,
>                            1, 0x89, 0x18, 0x0000, 0x0, 1);
>
> -    cpu_irq = microblaze_pic_init_cpu(env);
> -    dev = xilinx_intc_create(INTC_BASEADDR, cpu_irq[0], 0xA);
> +    dev = xilinx_intc_create(INTC_BASEADDR, qdev_get_gpio_in(DEVICE(cpu),
> +                             MB_CPU_IRQ), 0xA);

A minor nit here, an argument list should not newline through a nested
() if it can be avoided so something like:

+    dev = xilinx_intc_create(INTC_BASEADDR,
+                             qdev_get_gpio_in(DEVICE(cpu), MB_CPU_IRQ), 0xA);

But this will also conflict with my MB cleanups, so me or Edgar can
rebase depending on who goes in first. So I don't think we bother with
respin as we can trivially fix formatting at that time.

Reviewed-by: Peter Crosthwaite <address@hidden>

Regards,
Peter

>      for (i = 0; i < 32; i++) {
>          irq[i] = qdev_get_gpio_in(dev, i);
>      }
> diff --git a/hw/microblaze/pic_cpu.c b/hw/microblaze/pic_cpu.c
> deleted file mode 100644
> index 16902f7..0000000
> --- a/hw/microblaze/pic_cpu.c
> +++ /dev/null
> @@ -1,47 +0,0 @@
> -/*
> - * QEMU MicroBlaze CPU interrupt wrapper logic.
> - *
> - * Copyright (c) 2009 Edgar E. Iglesias, Axis Communications AB.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> - * of this software and associated documentation files (the "Software"), to 
> deal
> - * in the Software without restriction, including without limitation the 
> rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -#include "hw/hw.h"
> -#include "pic_cpu.h"
> -
> -#define D(x)
> -
> -static void microblaze_pic_cpu_handler(void *opaque, int irq, int level)
> -{
> -    MicroBlazeCPU *cpu = opaque;
> -    CPUState *cs = CPU(cpu);
> -    int type = irq ? CPU_INTERRUPT_NMI : CPU_INTERRUPT_HARD;
> -
> -    if (level) {
> -        cpu_interrupt(cs, type);
> -    } else {
> -        cpu_reset_interrupt(cs, type);
> -    }
> -}
> -
> -qemu_irq *microblaze_pic_init_cpu(CPUMBState *env)
> -{
> -    return qemu_allocate_irqs(microblaze_pic_cpu_handler, 
> mb_env_get_cpu(env),
> -                              2);
> -}
> diff --git a/hw/microblaze/pic_cpu.h b/hw/microblaze/pic_cpu.h
> deleted file mode 100644
> index 43090a4..0000000
> --- a/hw/microblaze/pic_cpu.h
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -#ifndef MICROBLAZE_PIC_CPU_H
> -#define MICROBLAZE_PIC_CPU_H
> -
> -#include "qemu-common.h"
> -
> -qemu_irq *microblaze_pic_init_cpu(CPUMBState *env);
> -
> -#endif /*  MICROBLAZE_PIC_CPU_H */
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 0ef9aa4..f108c0b 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -4,6 +4,7 @@
>   * Copyright (c) 2009 Edgar E. Iglesias
>   * Copyright (c) 2009-2012 PetaLogix Qld Pty Ltd.
>   * Copyright (c) 2012 SUSE LINUX Products GmbH
> + * Copyright (c) 2009 Edgar E. Iglesias, Axis Communications AB.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -33,6 +34,21 @@ static void mb_cpu_set_pc(CPUState *cs, vaddr value)
>      cpu->env.sregs[SR_PC] = value;
>  }
>
> +#ifndef CONFIG_USER_ONLY
> +static void microblaze_cpu_set_irq(void *opaque, int irq, int level)
> +{
> +    MicroBlazeCPU *cpu = opaque;
> +    CPUState *cs = CPU(cpu);
> +    int type = irq ? CPU_INTERRUPT_NMI : CPU_INTERRUPT_HARD;
> +
> +    if (level) {
> +        cpu_interrupt(cs, type);
> +    } else {
> +        cpu_reset_interrupt(cs, type);
> +    }
> +}
> +#endif
> +
>  /* CPUClass::reset() */
>  static void mb_cpu_reset(CPUState *s)
>  {
> @@ -111,6 +127,11 @@ static void mb_cpu_initfn(Object *obj)
>
>      set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
>
> +#ifndef CONFIG_USER_ONLY
> +    /* Inbound IRQ and FIR lines */
> +    qdev_init_gpio_in(DEVICE(cpu), microblaze_cpu_set_irq, 2);
> +#endif
> +
>      if (tcg_enabled() && !tcg_initialized) {
>          tcg_initialized = true;
>          mb_tcg_init();
> diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
> index e1415f0..1df014e 100644
> --- a/target-microblaze/cpu.h
> +++ b/target-microblaze/cpu.h
> @@ -48,6 +48,10 @@ typedef struct CPUMBState CPUMBState;
>  /* MicroBlaze-specific interrupt pending bits.  */
>  #define CPU_INTERRUPT_NMI       CPU_INTERRUPT_TGT_EXT_3
>
> +/* Meanings of the MBCPU object's two inbound GPIO lines */
> +#define MB_CPU_IRQ 0
> +#define MB_CPU_FIR 1
> +
>  /* Register aliases. R0 - R15 */
>  #define R_SP     1
>  #define SR_PC    0
> --
> 1.7.1
>
>