diff mbox series

[01/11] armv7m: Abstract out the "load kernel" code

Message ID 1487604965-23220-2-git-send-email-peter.maydell@linaro.org
State Superseded
Headers show
Series ARMv7M: QOMify | expand

Commit Message

Peter Maydell Feb. 20, 2017, 3:35 p.m. UTC 
Abstract the "load kernel" code out of armv7m_init() into its own
function.  This includes the registration of the CPU reset function,
to parallel how we handle this for A profile cores.

We make the function public so that boards which choose to
directly instantiate an ARMv7M device object can call it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 include/hw/arm/arm.h | 12 ++++++++++++
 hw/arm/armv7m.c      | 23 ++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

-- 
2.7.4

Comments

Philippe Mathieu-Daudé Feb. 20, 2017, 4:23 p.m. UTC | #1 
On 02/20/2017 12:35 PM, Peter Maydell wrote:
> Abstract the "load kernel" code out of armv7m_init() into its own

> function.  This includes the registration of the CPU reset function,

> to parallel how we handle this for A profile cores.

>

> We make the function public so that boards which choose to

> directly instantiate an ARMv7M device object can call it.

>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


> ---

>  include/hw/arm/arm.h | 12 ++++++++++++

>  hw/arm/armv7m.c      | 23 ++++++++++++++++++-----

>  2 files changed, 30 insertions(+), 5 deletions(-)

>

> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h

> index c175c0e..a3f79d3 100644

> --- a/include/hw/arm/arm.h

> +++ b/include/hw/arm/arm.h

> @@ -26,6 +26,18 @@ typedef enum {

>  /* armv7m.c */

>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>                        const char *kernel_filename, const char *cpu_model);

> +/**

> + * armv7m_load_kernel:

> + * @cpu: CPU

> + * @kernel_filename: file to load

> + * @mem_size: mem_size: maximum image size to load

> + *

> + * Load the guest image for an ARMv7M system. This must be called by

> + * any ARMv7M board, either directly or via armv7m_init(). (This is

> + * necessary to ensure that the CPU resets correctly on system reset,

> + * as well as for kernel loading.)

> + */

> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size);

>

>  /*

>   * struct used as a parameter of the arm_load_kernel machine init

> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c

> index 0c9ca7b..b2cc6e9 100644

> --- a/hw/arm/armv7m.c

> +++ b/hw/arm/armv7m.c

> @@ -176,10 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>      ARMCPU *cpu;

>      CPUARMState *env;

>      DeviceState *nvic;

> -    int image_size;

> -    uint64_t entry;

> -    uint64_t lowaddr;

> -    int big_endian;

>

>      if (cpu_model == NULL) {

>  	cpu_model = "cortex-m3";

> @@ -199,6 +195,16 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>      qdev_init_nofail(nvic);

>      sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,

>                         qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));

> +    armv7m_load_kernel(cpu, kernel_filename, mem_size);

> +    return nvic;

> +}

> +

> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)

> +{

> +    int image_size;

> +    uint64_t entry;

> +    uint64_t lowaddr;

> +    int big_endian;

>

>  #ifdef TARGET_WORDS_BIGENDIAN

>      big_endian = 1;

> @@ -224,8 +230,15 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>          }

>      }

>

> +    /* CPU objects (unlike devices) are not automatically reset on system

> +     * reset, so we must always register a handler to do so. Unlike

> +     * A-profile CPUs, we don't need to do anything special in the

> +     * handler to arrange that it starts correctly.

> +     * This is arguably the wrong place to do this, but it matches the

> +     * way A-profile does it. Note that this means that every M profile

> +     * board must call this function!

> +     */

>      qemu_register_reset(armv7m_reset, cpu);

> -    return nvic;

>  }

>

>  static Property bitband_properties[] = {

>
Alistair Francis Feb. 21, 2017, 11:35 a.m. UTC | #2 
On Mon, Feb 20, 2017 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/20/2017 12:35 PM, Peter Maydell wrote:

>>

>> Abstract the "load kernel" code out of armv7m_init() into its own

>> function.  This includes the registration of the CPU reset function,

>> to parallel how we handle this for A profile cores.

>>

>> We make the function public so that boards which choose to

>> directly instantiate an ARMv7M device object can call it.

>>

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>

>

> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>


Thanks,

Alistair

>

>

>> ---

>>  include/hw/arm/arm.h | 12 ++++++++++++

>>  hw/arm/armv7m.c      | 23 ++++++++++++++++++-----

>>  2 files changed, 30 insertions(+), 5 deletions(-)

>>

>> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h

>> index c175c0e..a3f79d3 100644

>> --- a/include/hw/arm/arm.h

>> +++ b/include/hw/arm/arm.h

>> @@ -26,6 +26,18 @@ typedef enum {

>>  /* armv7m.c */

>>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int

>> num_irq,

>>                        const char *kernel_filename, const char

>> *cpu_model);

>> +/**

>> + * armv7m_load_kernel:

>> + * @cpu: CPU

>> + * @kernel_filename: file to load

>> + * @mem_size: mem_size: maximum image size to load

>> + *

>> + * Load the guest image for an ARMv7M system. This must be called by

>> + * any ARMv7M board, either directly or via armv7m_init(). (This is

>> + * necessary to ensure that the CPU resets correctly on system reset,

>> + * as well as for kernel loading.)

>> + */

>> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int

>> mem_size);

>>

>>  /*

>>   * struct used as a parameter of the arm_load_kernel machine init

>> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c

>> index 0c9ca7b..b2cc6e9 100644

>> --- a/hw/arm/armv7m.c

>> +++ b/hw/arm/armv7m.c

>> @@ -176,10 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory,

>> int mem_size, int num_irq,

>>      ARMCPU *cpu;

>>      CPUARMState *env;

>>      DeviceState *nvic;

>> -    int image_size;

>> -    uint64_t entry;

>> -    uint64_t lowaddr;

>> -    int big_endian;

>>

>>      if (cpu_model == NULL) {

>>         cpu_model = "cortex-m3";

>> @@ -199,6 +195,16 @@ DeviceState *armv7m_init(MemoryRegion *system_memory,

>> int mem_size, int num_irq,

>>      qdev_init_nofail(nvic);

>>      sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,

>>                         qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));

>> +    armv7m_load_kernel(cpu, kernel_filename, mem_size);

>> +    return nvic;

>> +}

>> +

>> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int

>> mem_size)

>> +{

>> +    int image_size;

>> +    uint64_t entry;

>> +    uint64_t lowaddr;

>> +    int big_endian;

>>

>>  #ifdef TARGET_WORDS_BIGENDIAN

>>      big_endian = 1;

>> @@ -224,8 +230,15 @@ DeviceState *armv7m_init(MemoryRegion *system_memory,

>> int mem_size, int num_irq,

>>          }

>>      }

>>

>> +    /* CPU objects (unlike devices) are not automatically reset on system

>> +     * reset, so we must always register a handler to do so. Unlike

>> +     * A-profile CPUs, we don't need to do anything special in the

>> +     * handler to arrange that it starts correctly.

>> +     * This is arguably the wrong place to do this, but it matches the

>> +     * way A-profile does it. Note that this means that every M profile

>> +     * board must call this function!

>> +     */

>>      qemu_register_reset(armv7m_reset, cpu);

>> -    return nvic;

>>  }

>>

>>  static Property bitband_properties[] = {

>>

>
Alex Bennée Feb. 28, 2017, 1:57 p.m. UTC | #3 
Peter Maydell <peter.maydell@linaro.org> writes:

> Abstract the "load kernel" code out of armv7m_init() into its own

> function.  This includes the registration of the CPU reset function,

> to parallel how we handle this for A profile cores.

>

> We make the function public so that boards which choose to

> directly instantiate an ARMv7M device object can call it.

>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> ---

>  include/hw/arm/arm.h | 12 ++++++++++++

>  hw/arm/armv7m.c      | 23 ++++++++++++++++++-----

>  2 files changed, 30 insertions(+), 5 deletions(-)

>

> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h

> index c175c0e..a3f79d3 100644

> --- a/include/hw/arm/arm.h

> +++ b/include/hw/arm/arm.h

> @@ -26,6 +26,18 @@ typedef enum {

>  /* armv7m.c */

>  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>                        const char *kernel_filename, const char *cpu_model);

> +/**

> + * armv7m_load_kernel:

> + * @cpu: CPU

> + * @kernel_filename: file to load

> + * @mem_size: mem_size: maximum image size to load

> + *

> + * Load the guest image for an ARMv7M system. This must be called by

> + * any ARMv7M board, either directly or via armv7m_init(). (This is

> + * necessary to ensure that the CPU resets correctly on system reset,

> + * as well as for kernel loading.)

> + */

> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size);

>

>  /*

>   * struct used as a parameter of the arm_load_kernel machine init

> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c

> index 0c9ca7b..b2cc6e9 100644

> --- a/hw/arm/armv7m.c

> +++ b/hw/arm/armv7m.c

> @@ -176,10 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>      ARMCPU *cpu;

>      CPUARMState *env;

>      DeviceState *nvic;

> -    int image_size;

> -    uint64_t entry;

> -    uint64_t lowaddr;

> -    int big_endian;

>

>      if (cpu_model == NULL) {

>  	cpu_model = "cortex-m3";

> @@ -199,6 +195,16 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>      qdev_init_nofail(nvic);

>      sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,

>                         qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));

> +    armv7m_load_kernel(cpu, kernel_filename, mem_size);

> +    return nvic;

> +}

> +

> +void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)

> +{

> +    int image_size;

> +    uint64_t entry;

> +    uint64_t lowaddr;

> +    int big_endian;

>

>  #ifdef TARGET_WORDS_BIGENDIAN

>      big_endian = 1;

> @@ -224,8 +230,15 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,

>          }

>      }

>

> +    /* CPU objects (unlike devices) are not automatically reset on system

> +     * reset, so we must always register a handler to do so. Unlike

> +     * A-profile CPUs, we don't need to do anything special in the

> +     * handler to arrange that it starts correctly.

> +     * This is arguably the wrong place to do this, but it matches the

> +     * way A-profile does it. Note that this means that every M profile

> +     * board must call this function!

> +     */

>      qemu_register_reset(armv7m_reset, cpu);

> -    return nvic;

>  }

>

>  static Property bitband_properties[] = {



--
Alex Bennée
diff mbox series

Patch

diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index c175c0e..a3f79d3 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -26,6 +26,18 @@  typedef enum {
 /* armv7m.c */
 DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
                       const char *kernel_filename, const char *cpu_model);
+/**
+ * armv7m_load_kernel:
+ * @cpu: CPU
+ * @kernel_filename: file to load
+ * @mem_size: mem_size: maximum image size to load
+ *
+ * Load the guest image for an ARMv7M system. This must be called by
+ * any ARMv7M board, either directly or via armv7m_init(). (This is
+ * necessary to ensure that the CPU resets correctly on system reset,
+ * as well as for kernel loading.)
+ */
+void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size);
 
 /*
  * struct used as a parameter of the arm_load_kernel machine init
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 0c9ca7b..b2cc6e9 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -176,10 +176,6 @@  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
     ARMCPU *cpu;
     CPUARMState *env;
     DeviceState *nvic;
-    int image_size;
-    uint64_t entry;
-    uint64_t lowaddr;
-    int big_endian;
 
     if (cpu_model == NULL) {
 	cpu_model = "cortex-m3";
@@ -199,6 +195,16 @@  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
     qdev_init_nofail(nvic);
     sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
                        qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
+    armv7m_load_kernel(cpu, kernel_filename, mem_size);
+    return nvic;
+}
+
+void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
+{
+    int image_size;
+    uint64_t entry;
+    uint64_t lowaddr;
+    int big_endian;
 
 #ifdef TARGET_WORDS_BIGENDIAN
     big_endian = 1;
@@ -224,8 +230,15 @@  DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
         }
     }
 
+    /* CPU objects (unlike devices) are not automatically reset on system
+     * reset, so we must always register a handler to do so. Unlike
+     * A-profile CPUs, we don't need to do anything special in the
+     * handler to arrange that it starts correctly.
+     * This is arguably the wrong place to do this, but it matches the
+     * way A-profile does it. Note that this means that every M profile
+     * board must call this function!
+     */
     qemu_register_reset(armv7m_reset, cpu);
-    return nvic;
 }
 
 static Property bitband_properties[] = {