Re: [Qemu-devel] [PATCH v2 1/3] hw/arm: extract ARM M Profile base class from ARMv7-M

2018-07-11 Thread Peter Maydell
On 11 July 2018 at 13:51, Stefan Hajnoczi  wrote:
> On Thu, Jul 05, 2018 at 04:49:22PM +0100, Peter Maydell wrote:
>> On 5 July 2018 at 16:45, Peter Maydell  wrote:
>> > On 30 June 2018 at 10:13, Stefan Hajnoczi  wrote:
>> >> The ARMv7-M code is largely similar to what other M Profile CPUs need.
>> >> Extract the common M Profile aspects into the ARMMProfileState base
>> >> class.  ARMv6-M will inherit from this class in the following patch.
>> >>
>> >> It might be possible to make ARMv6-M the base class of ARMv7-M, but it
>> >> seems cleaner to have an M Profile base class instead of saying an
>> >> "ARMv7-M is an ARMv6-M".
>> >>
>> >> Signed-off-by: Stefan Hajnoczi 
>> >
>> > This makes sense, I guess (though it currently leaves us in the
>> > odd position that we have separate a object for v6m, but the v7m
>> > object handles both v7m and v8m...)
>>
>> ...though I guess the counter-argument is that the only thing that
>> the v7m object is doing that v6m doesn't want is creating
>> the bitbanding device, and in fact bitbanding is optional in v7m
>> (you can configure a Cortex-M3 without it). So maybe we should
>> instead just have a QOM property to let you turn off the
>> bitbanding ?
>
> Okay, we can do that.  So how about a single ARMMProfileState class for
> v6, v7, and v8?

Yes, I think that makes sense. (A lot of our code says 'v7m' when
it really means 'm profile in general' for historical reasons. I'm
not too worried about trying to tidy that up.)

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 1/3] hw/arm: extract ARM M Profile base class from ARMv7-M

2018-07-11 Thread Stefan Hajnoczi
On Thu, Jul 05, 2018 at 04:49:22PM +0100, Peter Maydell wrote:
> On 5 July 2018 at 16:45, Peter Maydell  wrote:
> > On 30 June 2018 at 10:13, Stefan Hajnoczi  wrote:
> >> The ARMv7-M code is largely similar to what other M Profile CPUs need.
> >> Extract the common M Profile aspects into the ARMMProfileState base
> >> class.  ARMv6-M will inherit from this class in the following patch.
> >>
> >> It might be possible to make ARMv6-M the base class of ARMv7-M, but it
> >> seems cleaner to have an M Profile base class instead of saying an
> >> "ARMv7-M is an ARMv6-M".
> >>
> >> Signed-off-by: Stefan Hajnoczi 
> >
> > This makes sense, I guess (though it currently leaves us in the
> > odd position that we have separate a object for v6m, but the v7m
> > object handles both v7m and v8m...)
> 
> ...though I guess the counter-argument is that the only thing that
> the v7m object is doing that v6m doesn't want is creating
> the bitbanding device, and in fact bitbanding is optional in v7m
> (you can configure a Cortex-M3 without it). So maybe we should
> instead just have a QOM property to let you turn off the
> bitbanding ?

Okay, we can do that.  So how about a single ARMMProfileState class for
v6, v7, and v8?

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 1/3] hw/arm: extract ARM M Profile base class from ARMv7-M

2018-07-05 Thread Peter Maydell
On 5 July 2018 at 16:45, Peter Maydell  wrote:
> On 30 June 2018 at 10:13, Stefan Hajnoczi  wrote:
>> The ARMv7-M code is largely similar to what other M Profile CPUs need.
>> Extract the common M Profile aspects into the ARMMProfileState base
>> class.  ARMv6-M will inherit from this class in the following patch.
>>
>> It might be possible to make ARMv6-M the base class of ARMv7-M, but it
>> seems cleaner to have an M Profile base class instead of saying an
>> "ARMv7-M is an ARMv6-M".
>>
>> Signed-off-by: Stefan Hajnoczi 
>
> This makes sense, I guess (though it currently leaves us in the
> odd position that we have separate a object for v6m, but the v7m
> object handles both v7m and v8m...)

...though I guess the counter-argument is that the only thing that
the v7m object is doing that v6m doesn't want is creating
the bitbanding device, and in fact bitbanding is optional in v7m
(you can configure a Cortex-M3 without it). So maybe we should
instead just have a QOM property to let you turn off the
bitbanding ?

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 1/3] hw/arm: extract ARM M Profile base class from ARMv7-M

2018-07-05 Thread Peter Maydell
On 30 June 2018 at 10:13, Stefan Hajnoczi  wrote:
> The ARMv7-M code is largely similar to what other M Profile CPUs need.
> Extract the common M Profile aspects into the ARMMProfileState base
> class.  ARMv6-M will inherit from this class in the following patch.
>
> It might be possible to make ARMv6-M the base class of ARMv7-M, but it
> seems cleaner to have an M Profile base class instead of saying an
> "ARMv7-M is an ARMv6-M".
>
> Signed-off-by: Stefan Hajnoczi 

This makes sense, I guess (though it currently leaves us in the
odd position that we have separate a object for v6m, but the v7m
object handles both v7m and v8m...)

>  /**
> - * armv7m_load_kernel:
> + * arm_m_profile_load_kernel:
>   * @cpu: CPU
>   * @kernel_filename: file to load
> - * @mem_size: mem_size: maximum image size to load
> + * @mem_size: maximum image size to load
>   *
> - * Load the guest image for an ARMv7M system. This must be called by
> - * any ARMv7M board. (This is necessary to ensure that the CPU resets
> + * Load the guest image for an ARM M Profile system. This must be called by
> + * any ARM M Profile board. (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);
> +void arm_m_profile_load_kernel(ARMCPU *cpu, const char *kernel_filename,
> +   int mem_size);

This is presumably just a code movement and rename, but it's a bit
hard to confirm that. Could you split it out into its own patch, please?

thanks
-- PMM



[Qemu-devel] [PATCH v2 1/3] hw/arm: extract ARM M Profile base class from ARMv7-M

2018-06-30 Thread Stefan Hajnoczi
The ARMv7-M code is largely similar to what other M Profile CPUs need.
Extract the common M Profile aspects into the ARMMProfileState base
class.  ARMv6-M will inherit from this class in the following patch.

It might be possible to make ARMv6-M the base class of ARMv7-M, but it
seems cleaner to have an M Profile base class instead of saying an
"ARMv7-M is an ARMv6-M".

Signed-off-by: Stefan Hajnoczi 
---
 hw/arm/Makefile.objs|   1 +
 include/hw/arm/arm-m-profile.h  |  65 +++
 include/hw/arm/arm.h|  11 +-
 include/hw/arm/armv7m.h |  22 +---
 hw/arm/arm-m-profile.c  | 192 
 hw/arm/armv7m.c | 166 ++-
 hw/arm/mps2-tz.c|   3 +-
 hw/arm/mps2.c   |   4 +-
 hw/arm/msf2-soc.c   |   4 +-
 hw/arm/msf2-som.c   |   4 +-
 hw/arm/netduino2.c  |   4 +-
 hw/arm/stellaris.c  |   3 +-
 default-configs/arm-softmmu.mak |   1 +
 13 files changed, 314 insertions(+), 166 deletions(-)
 create mode 100644 include/hw/arm/arm-m-profile.h
 create mode 100644 hw/arm/arm-m-profile.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index d51fcecaf2..2c43d34c64 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -16,6 +16,7 @@ obj-$(CONFIG_STRONGARM) += collie.o
 obj-$(CONFIG_VERSATILE) += vexpress.o versatilepb.o
 obj-$(CONFIG_ZYNQ) += xilinx_zynq.o
 
+obj-$(CONFIG_ARM_M_PROFILE) += arm-m-profile.o
 obj-$(CONFIG_ARM_V7M) += armv7m.o
 obj-$(CONFIG_EXYNOS4) += exynos4210.o
 obj-$(CONFIG_PXA2XX) += pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
diff --git a/include/hw/arm/arm-m-profile.h b/include/hw/arm/arm-m-profile.h
new file mode 100644
index 00..a9c3300411
--- /dev/null
+++ b/include/hw/arm/arm-m-profile.h
@@ -0,0 +1,65 @@
+/*
+ * ARM M Profile CPU base class
+ *
+ * Copyright (c) 2017 Linaro Ltd
+ * Written by Peter Maydell 
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * This code is licensed under the GPL version 2 or later.
+ */
+
+#ifndef HW_ARM_ARM_M_PROFILE_H
+#define HW_ARM_ARM_M_PROFILE_H
+
+#include "hw/sysbus.h"
+#include "hw/intc/armv7m_nvic.h"
+
+#define TYPE_ARM_M_PROFILE "arm-m-profile"
+#define ARM_M_PROFILE(obj) OBJECT_CHECK(ARMMProfileState, (obj), \
+TYPE_ARM_M_PROFILE)
+#define ARM_M_PROFILE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(ARMMProfileClass, (klass), TYPE_ARM_M_PROFILE)
+#define ARM_M_PROFILE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(ARMMProfileClass, (obj), TYPE_ARM_M_PROFILE)
+
+/* ARM M Profile container object.
+ * + Unnamed GPIO input lines: external IRQ lines for the NVIC
+ * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
+ * + Property "cpu-type": CPU type to instantiate
+ * + Property "num-irq": number of external IRQ lines
+ * + Property "memory": MemoryRegion defining the physical address space
+ *   that CPU accesses see. (The NVIC and other CPU-internal devices will be
+ *   automatically layered on top of this view.)
+ */
+typedef struct {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+NVICState nvic;
+ARMCPU *cpu;
+
+/* MemoryRegion we pass to the CPU, with our devices layered on
+ * top of the ones the board provides in board_memory.
+ */
+MemoryRegion container;
+
+/* Properties */
+char *cpu_type;
+/* MemoryRegion the board provides to us (with its devices, RAM, etc) */
+MemoryRegion *board_memory;
+} ARMMProfileState;
+
+typedef struct {
+/*< private >*/
+SysBusDeviceClass parent_class;
+
+/*< public >*/
+/**
+ * Initialize the CPU object, for example by setting properties, before it
+ * gets realized.  May be NULL.
+ */
+void (*cpu_init)(ARMMProfileState *s, Error **errp);
+} ARMMProfileClass;
+
+#endif
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index ffed39252d..2b919e57ee 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -24,16 +24,17 @@ typedef enum {
 } arm_endianness;
 
 /**
- * armv7m_load_kernel:
+ * arm_m_profile_load_kernel:
  * @cpu: CPU
  * @kernel_filename: file to load
- * @mem_size: mem_size: maximum image size to load
+ * @mem_size: maximum image size to load
  *
- * Load the guest image for an ARMv7M system. This must be called by
- * any ARMv7M board. (This is necessary to ensure that the CPU resets
+ * Load the guest image for an ARM M Profile system. This must be called by
+ * any ARM M Profile board. (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);
+void arm_m_profile_load_kernel(ARMCPU *cpu, const char *kernel_filename,
+   int mem_size);
 
 /* arm_boot.c */
 struct arm_boot_info {
diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index 78308d1484..c9b6ab37e7 100644
---