* [Qemu-devel] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel()
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
@ 2018-07-25 8:59 ` Stefan Hajnoczi
2018-07-27 4:52 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-07-30 17:30 ` [Qemu-devel] " Peter Maydell
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE Stefan Hajnoczi
` (6 subsequent siblings)
7 siblings, 2 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 8:59 UTC (permalink / raw)
To: qemu-devel
Cc: mail, Alistair Francis, Peter Maydell, ilg, qemu-arm,
Julia Suvorova, Subbaraya Sundeep, Su Hang, Steffen Gortz, jim,
Joel Stanley, Stefan Hajnoczi
ARMv6-M and ARMv8-M need the same kernel loading functionality as
ARMv7-M. Rename armv7m_load_kernel() to arm_m_profile_load_kernel() so
it's clear that this function isn't specific to ARMv7-M.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/arm/Makefile.objs | 1 +
include/hw/arm/arm.h | 11 +++--
hw/arm/arm-m-profile.c | 81 +++++++++++++++++++++++++++++++++
hw/arm/armv7m.c | 60 ------------------------
hw/arm/mps2-tz.c | 3 +-
hw/arm/mps2.c | 4 +-
hw/arm/msf2-som.c | 4 +-
hw/arm/netduino2.c | 4 +-
hw/arm/stellaris.c | 3 +-
default-configs/arm-softmmu.mak | 1 +
10 files changed, 99 insertions(+), 73 deletions(-)
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.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/hw/arm/arm-m-profile.c b/hw/arm/arm-m-profile.c
new file mode 100644
index 0000000000..262706ed62
--- /dev/null
+++ b/hw/arm/arm-m-profile.c
@@ -0,0 +1,81 @@
+/*
+ * ARM M Profile System emulation.
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * Copyright (c) 2006-2007 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "hw/sysbus.h"
+#include "hw/arm/arm.h"
+#include "hw/loader.h"
+#include "elf.h"
+#include "sysemu/qtest.h"
+#include "qemu/error-report.h"
+#include "exec/address-spaces.h"
+
+static void arm_m_profile_reset(void *opaque)
+{
+ ARMCPU *cpu = opaque;
+
+ cpu_reset(CPU(cpu));
+}
+
+void arm_m_profile_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
+{
+ int image_size;
+ uint64_t entry;
+ uint64_t lowaddr;
+ int big_endian;
+ AddressSpace *as;
+ int asidx;
+ CPUState *cs = CPU(cpu);
+
+#ifdef TARGET_WORDS_BIGENDIAN
+ big_endian = 1;
+#else
+ big_endian = 0;
+#endif
+
+ if (!kernel_filename && !qtest_enabled()) {
+ error_report("Guest image must be specified (using -kernel)");
+ exit(1);
+ }
+
+ if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
+ asidx = ARMASIdx_S;
+ } else {
+ asidx = ARMASIdx_NS;
+ }
+ as = cpu_get_address_space(cs, asidx);
+
+ if (kernel_filename) {
+ image_size = load_elf_as(kernel_filename, NULL, NULL, &entry, &lowaddr,
+ NULL, big_endian, EM_ARM, 1, 0, as);
+ if (image_size < 0) {
+ image_size = load_image_targphys_as(kernel_filename, 0,
+ mem_size, as);
+ lowaddr = 0;
+ }
+ if (image_size < 0) {
+ error_report("Could not load kernel '%s'", kernel_filename);
+ exit(1);
+ }
+ }
+
+ /* 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(arm_m_profile_reset, cpu);
+}
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 6b07666057..7405a1ec69 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -258,66 +258,6 @@ static const TypeInfo armv7m_info = {
.class_init = armv7m_class_init,
};
-static void armv7m_reset(void *opaque)
-{
- ARMCPU *cpu = opaque;
-
- cpu_reset(CPU(cpu));
-}
-
-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;
- AddressSpace *as;
- int asidx;
- CPUState *cs = CPU(cpu);
-
-#ifdef TARGET_WORDS_BIGENDIAN
- big_endian = 1;
-#else
- big_endian = 0;
-#endif
-
- if (!kernel_filename && !qtest_enabled()) {
- error_report("Guest image must be specified (using -kernel)");
- exit(1);
- }
-
- if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
- asidx = ARMASIdx_S;
- } else {
- asidx = ARMASIdx_NS;
- }
- as = cpu_get_address_space(cs, asidx);
-
- if (kernel_filename) {
- image_size = load_elf_as(kernel_filename, NULL, NULL, &entry, &lowaddr,
- NULL, big_endian, EM_ARM, 1, 0, as);
- if (image_size < 0) {
- image_size = load_image_targphys_as(kernel_filename, 0,
- mem_size, as);
- lowaddr = 0;
- }
- if (image_size < 0) {
- error_report("Could not load kernel '%s'", kernel_filename);
- exit(1);
- }
- }
-
- /* 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);
-}
-
static Property bitband_properties[] = {
DEFINE_PROP_UINT32("base", BitBandState, base, 0),
DEFINE_PROP_LINK("source-memory", BitBandState, source_memory,
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 22180c56fb..af10ee0cc9 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -487,7 +487,8 @@ static void mps2tz_common_init(MachineState *machine)
create_unimplemented_device("FPGA NS PC", 0x48007000, 0x1000);
- armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0x400000);
+ arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+ 0x400000);
}
static void mps2tz_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index c3946da317..bcc7070104 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -315,8 +315,8 @@ static void mps2_common_init(MachineState *machine)
system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
- armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
- 0x400000);
+ arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+ 0x400000);
}
static void mps2_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
index 2432b5e935..cb21ced472 100644
--- a/hw/arm/msf2-som.c
+++ b/hw/arm/msf2-som.c
@@ -91,8 +91,8 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
cs_line = qdev_get_gpio_in_named(spi_flash, SSI_GPIO_CS, 0);
sysbus_connect_irq(SYS_BUS_DEVICE(&soc->spi[0]), 1, cs_line);
- armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
- soc->envm_size);
+ arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+ soc->envm_size);
}
static void emcraft_sf2_machine_init(MachineClass *mc)
diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index f936017d4a..e18d377f94 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -37,8 +37,8 @@ static void netduino2_init(MachineState *machine)
qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
- armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
- FLASH_SIZE);
+ arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+ FLASH_SIZE);
}
static void netduino2_machine_init(MachineClass *mc)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index dc521b4a5a..68e52367c0 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1440,7 +1440,8 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
create_unimplemented_device("hibernation", 0x400fc000, 0x1000);
create_unimplemented_device("flash-control", 0x400fd000, 0x1000);
- armv7m_load_kernel(ARM_CPU(first_cpu), ms->kernel_filename, flash_size);
+ arm_m_profile_load_kernel(ARM_CPU(first_cpu), ms->kernel_filename,
+ flash_size);
}
/* FIXME: Figure out how to generate these from stellaris_boards. */
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 834d45cfaf..e704cb6e34 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -48,6 +48,7 @@ CONFIG_ARM11MPCORE=y
CONFIG_A9MPCORE=y
CONFIG_A15MPCORE=y
+CONFIG_ARM_M_PROFILE=y
CONFIG_ARM_V7M=y
CONFIG_NETDUINO2=y
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel()
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel() Stefan Hajnoczi
@ 2018-07-27 4:52 ` Philippe Mathieu-Daudé
2018-07-30 17:30 ` [Qemu-devel] " Peter Maydell
1 sibling, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-27 4:52 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Peter Maydell, jim, mail, Su Hang, ilg, Alistair Francis,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
> ARMv6-M and ARMv8-M need the same kernel loading functionality as
> ARMv7-M. Rename armv7m_load_kernel() to arm_m_profile_load_kernel() so
> it's clear that this function isn't specific to ARMv7-M.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/arm/Makefile.objs | 1 +
> include/hw/arm/arm.h | 11 +++--
> hw/arm/arm-m-profile.c | 81 +++++++++++++++++++++++++++++++++
> hw/arm/armv7m.c | 60 ------------------------
> hw/arm/mps2-tz.c | 3 +-
> hw/arm/mps2.c | 4 +-
> hw/arm/msf2-som.c | 4 +-
> hw/arm/netduino2.c | 4 +-
> hw/arm/stellaris.c | 3 +-
> default-configs/arm-softmmu.mak | 1 +
> 10 files changed, 99 insertions(+), 73 deletions(-)
> 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.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/hw/arm/arm-m-profile.c b/hw/arm/arm-m-profile.c
> new file mode 100644
> index 0000000000..262706ed62
> --- /dev/null
> +++ b/hw/arm/arm-m-profile.c
> @@ -0,0 +1,81 @@
> +/*
> + * ARM M Profile System emulation.
> + *
> + * Copyright (C) 2018 Red Hat, Inc.
> + *
> + * Copyright (c) 2006-2007 CodeSourcery.
> + * Written by Paul Brook
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "hw/sysbus.h"
> +#include "hw/arm/arm.h"
> +#include "hw/loader.h"
> +#include "elf.h"
> +#include "sysemu/qtest.h"
> +#include "qemu/error-report.h"
> +#include "exec/address-spaces.h"
> +
> +static void arm_m_profile_reset(void *opaque)
> +{
> + ARMCPU *cpu = opaque;
> +
> + cpu_reset(CPU(cpu));
> +}
> +
> +void arm_m_profile_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
> +{
> + int image_size;
> + uint64_t entry;
> + uint64_t lowaddr;
> + int big_endian;
> + AddressSpace *as;
> + int asidx;
> + CPUState *cs = CPU(cpu);
> +
> +#ifdef TARGET_WORDS_BIGENDIAN
> + big_endian = 1;
> +#else
> + big_endian = 0;
> +#endif
> +
> + if (!kernel_filename && !qtest_enabled()) {
> + error_report("Guest image must be specified (using -kernel)");
> + exit(1);
> + }
> +
> + if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
> + asidx = ARMASIdx_S;
> + } else {
> + asidx = ARMASIdx_NS;
> + }
> + as = cpu_get_address_space(cs, asidx);
> +
> + if (kernel_filename) {
> + image_size = load_elf_as(kernel_filename, NULL, NULL, &entry, &lowaddr,
> + NULL, big_endian, EM_ARM, 1, 0, as);
> + if (image_size < 0) {
> + image_size = load_image_targphys_as(kernel_filename, 0,
> + mem_size, as);
> + lowaddr = 0;
> + }
> + if (image_size < 0) {
> + error_report("Could not load kernel '%s'", kernel_filename);
> + exit(1);
> + }
> + }
> +
> + /* 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(arm_m_profile_reset, cpu);
> +}
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 6b07666057..7405a1ec69 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -258,66 +258,6 @@ static const TypeInfo armv7m_info = {
> .class_init = armv7m_class_init,
> };
>
> -static void armv7m_reset(void *opaque)
> -{
> - ARMCPU *cpu = opaque;
> -
> - cpu_reset(CPU(cpu));
> -}
> -
> -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;
> - AddressSpace *as;
> - int asidx;
> - CPUState *cs = CPU(cpu);
> -
> -#ifdef TARGET_WORDS_BIGENDIAN
> - big_endian = 1;
> -#else
> - big_endian = 0;
> -#endif
> -
> - if (!kernel_filename && !qtest_enabled()) {
> - error_report("Guest image must be specified (using -kernel)");
> - exit(1);
> - }
> -
> - if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
> - asidx = ARMASIdx_S;
> - } else {
> - asidx = ARMASIdx_NS;
> - }
> - as = cpu_get_address_space(cs, asidx);
> -
> - if (kernel_filename) {
> - image_size = load_elf_as(kernel_filename, NULL, NULL, &entry, &lowaddr,
> - NULL, big_endian, EM_ARM, 1, 0, as);
> - if (image_size < 0) {
> - image_size = load_image_targphys_as(kernel_filename, 0,
> - mem_size, as);
> - lowaddr = 0;
> - }
> - if (image_size < 0) {
> - error_report("Could not load kernel '%s'", kernel_filename);
> - exit(1);
> - }
> - }
> -
> - /* 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);
> -}
> -
> static Property bitband_properties[] = {
> DEFINE_PROP_UINT32("base", BitBandState, base, 0),
> DEFINE_PROP_LINK("source-memory", BitBandState, source_memory,
> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> index 22180c56fb..af10ee0cc9 100644
> --- a/hw/arm/mps2-tz.c
> +++ b/hw/arm/mps2-tz.c
> @@ -487,7 +487,8 @@ static void mps2tz_common_init(MachineState *machine)
>
> create_unimplemented_device("FPGA NS PC", 0x48007000, 0x1000);
>
> - armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0x400000);
> + arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> + 0x400000);
> }
>
> static void mps2tz_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
> index c3946da317..bcc7070104 100644
> --- a/hw/arm/mps2.c
> +++ b/hw/arm/mps2.c
> @@ -315,8 +315,8 @@ static void mps2_common_init(MachineState *machine)
>
> system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
>
> - armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> - 0x400000);
> + arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> + 0x400000);
> }
>
> static void mps2_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
> index 2432b5e935..cb21ced472 100644
> --- a/hw/arm/msf2-som.c
> +++ b/hw/arm/msf2-som.c
> @@ -91,8 +91,8 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
> cs_line = qdev_get_gpio_in_named(spi_flash, SSI_GPIO_CS, 0);
> sysbus_connect_irq(SYS_BUS_DEVICE(&soc->spi[0]), 1, cs_line);
>
> - armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> - soc->envm_size);
> + arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> + soc->envm_size);
> }
>
> static void emcraft_sf2_machine_init(MachineClass *mc)
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index f936017d4a..e18d377f94 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -37,8 +37,8 @@ static void netduino2_init(MachineState *machine)
> qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
> object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
>
> - armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> - FLASH_SIZE);
> + arm_m_profile_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> + FLASH_SIZE);
> }
>
> static void netduino2_machine_init(MachineClass *mc)
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index dc521b4a5a..68e52367c0 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -1440,7 +1440,8 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
> create_unimplemented_device("hibernation", 0x400fc000, 0x1000);
> create_unimplemented_device("flash-control", 0x400fd000, 0x1000);
>
> - armv7m_load_kernel(ARM_CPU(first_cpu), ms->kernel_filename, flash_size);
> + arm_m_profile_load_kernel(ARM_CPU(first_cpu), ms->kernel_filename,
> + flash_size);
> }
>
> /* FIXME: Figure out how to generate these from stellaris_boards. */
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 834d45cfaf..e704cb6e34 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -48,6 +48,7 @@ CONFIG_ARM11MPCORE=y
> CONFIG_A9MPCORE=y
> CONFIG_A15MPCORE=y
>
> +CONFIG_ARM_M_PROFILE=y
> CONFIG_ARM_V7M=y
> CONFIG_NETDUINO2=y
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel()
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel() Stefan Hajnoczi
2018-07-27 4:52 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-07-30 17:30 ` Peter Maydell
1 sibling, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2018-07-30 17:30 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Steffen Görtz, Alistair Francis,
Liviu Ionescu, qemu-arm, Julia Suvorova, Subbaraya Sundeep,
Su Hang, Steffen Gortz, Jim Mussared, Joel Stanley
On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> ARMv6-M and ARMv8-M need the same kernel loading functionality as
> ARMv7-M. Rename armv7m_load_kernel() to arm_m_profile_load_kernel() so
> it's clear that this function isn't specific to ARMv7-M.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/arm/Makefile.objs | 1 +
> include/hw/arm/arm.h | 11 +++--
> hw/arm/arm-m-profile.c | 81 +++++++++++++++++++++++++++++++++
> hw/arm/armv7m.c | 60 ------------------------
> hw/arm/mps2-tz.c | 3 +-
> hw/arm/mps2.c | 4 +-
> hw/arm/msf2-som.c | 4 +-
> hw/arm/netduino2.c | 4 +-
> hw/arm/stellaris.c | 3 +-
> default-configs/arm-softmmu.mak | 1 +
> 10 files changed, 99 insertions(+), 73 deletions(-)
> create mode 100644 hw/arm/arm-m-profile.c
Another rename that shows up in the patch as "delete the old
one and have a copy somewhere else". Maybe this can be fixed
with suitable git rename-detection options?
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel() Stefan Hajnoczi
@ 2018-07-25 8:59 ` Stefan Hajnoczi
2018-07-27 4:53 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-07-30 17:29 ` [Qemu-devel] " Peter Maydell
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile Stefan Hajnoczi
` (5 subsequent siblings)
7 siblings, 2 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 8:59 UTC (permalink / raw)
To: qemu-devel
Cc: mail, Alistair Francis, Peter Maydell, ilg, qemu-arm,
Julia Suvorova, Subbaraya Sundeep, Su Hang, Steffen Gortz, jim,
Joel Stanley, Stefan Hajnoczi
The TYPE_ARMV7M class is really a container for an ARM M Profile CPU,
NVIC, and related pieces. It can also be used for ARMv6-M and ARMv8-M.
Rename the class since it is not exclusive to ARMv7-M.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/arm/Makefile.objs | 1 -
include/hw/arm/{armv7m.h => arm-m-profile.h} | 37 ++-
include/hw/arm/iotkit.h | 4 +-
include/hw/arm/msf2-soc.h | 4 +-
include/hw/arm/stm32f205_soc.h | 4 +-
hw/arm/arm-m-profile.c | 271 +++++++++++++++++
hw/arm/armv7m.c | 290 -------------------
hw/arm/iotkit.c | 2 +-
hw/arm/mps2-tz.c | 1 -
hw/arm/mps2.c | 6 +-
hw/arm/msf2-soc.c | 2 +-
hw/arm/stellaris.c | 4 +-
hw/arm/stm32f205_soc.c | 2 +-
13 files changed, 313 insertions(+), 315 deletions(-)
rename include/hw/arm/{armv7m.h => arm-m-profile.h} (65%)
delete mode 100644 hw/arm/armv7m.c
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 2c43d34c64..b1e4f8f006 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -17,7 +17,6 @@ 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
obj-$(CONFIG_DIGIC) += digic.o
diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/arm-m-profile.h
similarity index 65%
rename from include/hw/arm/armv7m.h
rename to include/hw/arm/arm-m-profile.h
index 78308d1484..ea496d9b88 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/arm-m-profile.h
@@ -1,14 +1,16 @@
/*
- * ARMv7M CPU object
+ * ARM M Profile CPU class
*
* Copyright (c) 2017 Linaro Ltd
* Written by Peter Maydell <peter.maydell@linaro.org>
*
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
* This code is licensed under the GPL version 2 or later.
*/
-#ifndef HW_ARM_ARMV7M_H
-#define HW_ARM_ARMV7M_H
+#ifndef HW_ARM_ARM_M_PROFILE_H
+#define HW_ARM_ARM_M_PROFILE_H
#include "hw/sysbus.h"
#include "hw/intc/armv7m_nvic.h"
@@ -28,12 +30,17 @@ typedef struct {
MemoryRegion *source_memory;
} BitBandState;
-#define TYPE_ARMV7M "armv7m"
-#define ARMV7M(obj) OBJECT_CHECK(ARMv7MState, (obj), TYPE_ARMV7M)
-
#define ARMV7M_NUM_BITBANDS 2
-/* ARMv7M container object.
+#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
@@ -44,7 +51,7 @@ typedef struct {
* + Property "idau": IDAU interface (forwarded to CPU object)
* + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
*/
-typedef struct ARMv7MState {
+typedef struct {
/*< private >*/
SysBusDevice parent_obj;
/*< public >*/
@@ -63,6 +70,18 @@ typedef struct ARMv7MState {
MemoryRegion *board_memory;
Object *idau;
uint32_t init_svtor;
-} ARMv7MState;
+} 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/iotkit.h b/include/hw/arm/iotkit.h
index 2cddde55dd..8b672c2b6c 100644
--- a/include/hw/arm/iotkit.h
+++ b/include/hw/arm/iotkit.h
@@ -51,7 +51,7 @@
#define IOTKIT_H
#include "hw/sysbus.h"
-#include "hw/arm/armv7m.h"
+#include "hw/arm/arm-m-profile.h"
#include "hw/misc/iotkit-secctl.h"
#include "hw/misc/tz-ppc.h"
#include "hw/misc/tz-mpc.h"
@@ -74,7 +74,7 @@ typedef struct IoTKit {
SysBusDevice parent_obj;
/*< public >*/
- ARMv7MState armv7m;
+ ARMMProfileState armv7m;
IoTKitSecCtl secctl;
TZPPC apb_ppc0;
TZPPC apb_ppc1;
diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
index 3cfe5c76ee..36d47db274 100644
--- a/include/hw/arm/msf2-soc.h
+++ b/include/hw/arm/msf2-soc.h
@@ -25,7 +25,7 @@
#ifndef HW_ARM_MSF2_SOC_H
#define HW_ARM_MSF2_SOC_H
-#include "hw/arm/armv7m.h"
+#include "hw/arm/arm-m-profile.h"
#include "hw/timer/mss-timer.h"
#include "hw/misc/msf2-sysreg.h"
#include "hw/ssi/mss-spi.h"
@@ -48,7 +48,7 @@ typedef struct MSF2State {
SysBusDevice parent_obj;
/*< public >*/
- ARMv7MState armv7m;
+ ARMMProfileState armv7m;
char *cpu_type;
char *part_name;
diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
index 922a733f88..03b2e0a79d 100644
--- a/include/hw/arm/stm32f205_soc.h
+++ b/include/hw/arm/stm32f205_soc.h
@@ -31,7 +31,7 @@
#include "hw/adc/stm32f2xx_adc.h"
#include "hw/or-irq.h"
#include "hw/ssi/stm32f2xx_spi.h"
-#include "hw/arm/armv7m.h"
+#include "hw/arm/arm-m-profile.h"
#define TYPE_STM32F205_SOC "stm32f205-soc"
#define STM32F205_SOC(obj) \
@@ -54,7 +54,7 @@ typedef struct STM32F205State {
char *cpu_type;
- ARMv7MState armv7m;
+ ARMMProfileState armv7m;
STM32F2XXSyscfgState syscfg;
STM32F2XXUsartState usart[STM_NUM_USARTS];
diff --git a/hw/arm/arm-m-profile.c b/hw/arm/arm-m-profile.c
index 262706ed62..b7dd2370d4 100644
--- a/hw/arm/arm-m-profile.c
+++ b/hw/arm/arm-m-profile.c
@@ -10,6 +10,8 @@
*/
#include "qemu/osdep.h"
+#include "hw/arm/arm-m-profile.h"
+#include "qapi/error.h"
#include "qemu-common.h"
#include "cpu.h"
#include "hw/sysbus.h"
@@ -20,6 +22,244 @@
#include "qemu/error-report.h"
#include "exec/address-spaces.h"
+/* Bitbanded IO. Each word corresponds to a single bit. */
+
+/* Get the byte address of the real memory for a bitband access. */
+static inline hwaddr bitband_addr(BitBandState *s, hwaddr offset)
+{
+ return s->base | (offset & 0x1ffffff) >> 5;
+}
+
+static MemTxResult bitband_read(void *opaque, hwaddr offset,
+ uint64_t *data, unsigned size, MemTxAttrs attrs)
+{
+ BitBandState *s = opaque;
+ uint8_t buf[4];
+ MemTxResult res;
+ int bitpos, bit;
+ hwaddr addr;
+
+ assert(size <= 4);
+
+ /* Find address in underlying memory and round down to multiple of size */
+ addr = bitband_addr(s, offset) & (-size);
+ res = address_space_read(&s->source_as, addr, attrs, buf, size);
+ if (res) {
+ return res;
+ }
+ /* Bit position in the N bytes read... */
+ bitpos = (offset >> 2) & ((size * 8) - 1);
+ /* ...converted to byte in buffer and bit in byte */
+ bit = (buf[bitpos >> 3] >> (bitpos & 7)) & 1;
+ *data = bit;
+ return MEMTX_OK;
+}
+
+static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned size, MemTxAttrs attrs)
+{
+ BitBandState *s = opaque;
+ uint8_t buf[4];
+ MemTxResult res;
+ int bitpos, bit;
+ hwaddr addr;
+
+ assert(size <= 4);
+
+ /* Find address in underlying memory and round down to multiple of size */
+ addr = bitband_addr(s, offset) & (-size);
+ res = address_space_read(&s->source_as, addr, attrs, buf, size);
+ if (res) {
+ return res;
+ }
+ /* Bit position in the N bytes read... */
+ bitpos = (offset >> 2) & ((size * 8) - 1);
+ /* ...converted to byte in buffer and bit in byte */
+ bit = 1 << (bitpos & 7);
+ if (value & 1) {
+ buf[bitpos >> 3] |= bit;
+ } else {
+ buf[bitpos >> 3] &= ~bit;
+ }
+ return address_space_write(&s->source_as, addr, attrs, buf, size);
+}
+
+static const MemoryRegionOps bitband_ops = {
+ .read_with_attrs = bitband_read,
+ .write_with_attrs = bitband_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .impl.min_access_size = 1,
+ .impl.max_access_size = 4,
+ .valid.min_access_size = 1,
+ .valid.max_access_size = 4,
+};
+
+static void bitband_init(Object *obj)
+{
+ BitBandState *s = BITBAND(obj);
+ SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+
+ memory_region_init_io(&s->iomem, obj, &bitband_ops, s,
+ "bitband", 0x02000000);
+ sysbus_init_mmio(dev, &s->iomem);
+}
+
+static void bitband_realize(DeviceState *dev, Error **errp)
+{
+ BitBandState *s = BITBAND(dev);
+
+ if (!s->source_memory) {
+ error_setg(errp, "source-memory property not set");
+ return;
+ }
+
+ address_space_init(&s->source_as, s->source_memory, "bitband-source");
+}
+
+/* Board init. */
+
+static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
+ 0x20000000, 0x40000000
+};
+
+static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
+ 0x22000000, 0x42000000
+};
+
+static void arm_m_profile_instance_init(Object *obj)
+{
+ ARMMProfileState *s = ARM_M_PROFILE(obj);
+ int i;
+
+ /* Can't init the cpu here, we don't yet know which model to use */
+
+ memory_region_init(&s->container, obj, "arm-m-profile-container", UINT64_MAX);
+
+ sysbus_init_child_obj(obj, "nvnic", &s->nvic, sizeof(s->nvic), TYPE_NVIC);
+ object_property_add_alias(obj, "num-irq",
+ OBJECT(&s->nvic), "num-irq", &error_abort);
+
+ for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+ sysbus_init_child_obj(obj, "bitband[*]", &s->bitband[i],
+ sizeof(s->bitband[i]), TYPE_BITBAND);
+ }
+}
+
+static void arm_m_profile_realize(DeviceState *dev, Error **errp)
+{
+ ARMMProfileState *s = ARM_M_PROFILE(dev);
+ SysBusDevice *sbd;
+ Error *err = NULL;
+ int i;
+
+ if (!s->board_memory) {
+ error_setg(errp, "memory property was not set");
+ return;
+ }
+
+ memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
+
+ s->cpu = ARM_CPU(object_new(s->cpu_type));
+
+ object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
+ &error_abort);
+ if (object_property_find(OBJECT(s->cpu), "idau", NULL)) {
+ object_property_set_link(OBJECT(s->cpu), s->idau, "idau", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+ }
+ if (object_property_find(OBJECT(s->cpu), "init-svtor", NULL)) {
+ object_property_set_uint(OBJECT(s->cpu), s->init_svtor,
+ "init-svtor", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+ }
+
+ /* Tell the CPU where the NVIC is; it will fail realize if it doesn't
+ * have one.
+ */
+ s->cpu->env.nvic = &s->nvic;
+
+ object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ /* Note that we must realize the NVIC after the CPU */
+ object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ /* Alias the NVIC's input and output GPIOs as our own so the board
+ * code can wire them up. (We do this in realize because the
+ * NVIC doesn't create the input GPIO array until realize.)
+ */
+ qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
+ qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
+
+ /* Wire the NVIC up to the CPU */
+ sbd = SYS_BUS_DEVICE(&s->nvic);
+ sysbus_connect_irq(sbd, 0,
+ qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
+
+ memory_region_add_subregion(&s->container, 0xe000e000,
+ sysbus_mmio_get_region(sbd, 0));
+
+ for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+ Object *obj = OBJECT(&s->bitband[i]);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
+
+ object_property_set_int(obj, bitband_input_addr[i], "base", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+ object_property_set_link(obj, OBJECT(s->board_memory),
+ "source-memory", &error_abort);
+ object_property_set_bool(obj, true, "realized", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&s->container, bitband_output_addr[i],
+ sysbus_mmio_get_region(sbd, 0));
+ }
+}
+
+static Property arm_m_profile_properties[] = {
+ DEFINE_PROP_STRING("cpu-type", ARMMProfileState, cpu_type),
+ DEFINE_PROP_LINK("memory", ARMMProfileState, board_memory,
+ TYPE_MEMORY_REGION, MemoryRegion *),
+ DEFINE_PROP_LINK("idau", ARMMProfileState, idau,
+ TYPE_IDAU_INTERFACE, Object *),
+ DEFINE_PROP_UINT32("init-svtor", ARMMProfileState, init_svtor, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void arm_m_profile_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = arm_m_profile_realize;
+ dc->props = arm_m_profile_properties;
+}
+
+static const TypeInfo arm_m_profile_info = {
+ .name = TYPE_ARM_M_PROFILE,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(ARMMProfileState),
+ .instance_init = arm_m_profile_instance_init,
+ .class_init = arm_m_profile_class_init,
+};
+
static void arm_m_profile_reset(void *opaque)
{
ARMCPU *cpu = opaque;
@@ -79,3 +319,34 @@ void arm_m_profile_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem
*/
qemu_register_reset(arm_m_profile_reset, cpu);
}
+
+static Property bitband_properties[] = {
+ DEFINE_PROP_UINT32("base", BitBandState, base, 0),
+ DEFINE_PROP_LINK("source-memory", BitBandState, source_memory,
+ TYPE_MEMORY_REGION, MemoryRegion *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void bitband_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = bitband_realize;
+ dc->props = bitband_properties;
+}
+
+static const TypeInfo bitband_info = {
+ .name = TYPE_BITBAND,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(BitBandState),
+ .instance_init = bitband_init,
+ .class_init = bitband_class_init,
+};
+
+static void arm_m_profile_register_types(void)
+{
+ type_register_static(&bitband_info);
+ type_register_static(&arm_m_profile_info);
+}
+
+type_init(arm_m_profile_register_types)
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
deleted file mode 100644
index 7405a1ec69..0000000000
--- a/hw/arm/armv7m.c
+++ /dev/null
@@ -1,290 +0,0 @@
-/*
- * ARMV7M System emulation.
- *
- * Copyright (c) 2006-2007 CodeSourcery.
- * Written by Paul Brook
- *
- * This code is licensed under the GPL.
- */
-
-#include "qemu/osdep.h"
-#include "hw/arm/armv7m.h"
-#include "qapi/error.h"
-#include "qemu-common.h"
-#include "cpu.h"
-#include "hw/sysbus.h"
-#include "hw/arm/arm.h"
-#include "hw/loader.h"
-#include "elf.h"
-#include "sysemu/qtest.h"
-#include "qemu/error-report.h"
-#include "exec/address-spaces.h"
-#include "target/arm/idau.h"
-
-/* Bitbanded IO. Each word corresponds to a single bit. */
-
-/* Get the byte address of the real memory for a bitband access. */
-static inline hwaddr bitband_addr(BitBandState *s, hwaddr offset)
-{
- return s->base | (offset & 0x1ffffff) >> 5;
-}
-
-static MemTxResult bitband_read(void *opaque, hwaddr offset,
- uint64_t *data, unsigned size, MemTxAttrs attrs)
-{
- BitBandState *s = opaque;
- uint8_t buf[4];
- MemTxResult res;
- int bitpos, bit;
- hwaddr addr;
-
- assert(size <= 4);
-
- /* Find address in underlying memory and round down to multiple of size */
- addr = bitband_addr(s, offset) & (-size);
- res = address_space_read(&s->source_as, addr, attrs, buf, size);
- if (res) {
- return res;
- }
- /* Bit position in the N bytes read... */
- bitpos = (offset >> 2) & ((size * 8) - 1);
- /* ...converted to byte in buffer and bit in byte */
- bit = (buf[bitpos >> 3] >> (bitpos & 7)) & 1;
- *data = bit;
- return MEMTX_OK;
-}
-
-static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
- unsigned size, MemTxAttrs attrs)
-{
- BitBandState *s = opaque;
- uint8_t buf[4];
- MemTxResult res;
- int bitpos, bit;
- hwaddr addr;
-
- assert(size <= 4);
-
- /* Find address in underlying memory and round down to multiple of size */
- addr = bitband_addr(s, offset) & (-size);
- res = address_space_read(&s->source_as, addr, attrs, buf, size);
- if (res) {
- return res;
- }
- /* Bit position in the N bytes read... */
- bitpos = (offset >> 2) & ((size * 8) - 1);
- /* ...converted to byte in buffer and bit in byte */
- bit = 1 << (bitpos & 7);
- if (value & 1) {
- buf[bitpos >> 3] |= bit;
- } else {
- buf[bitpos >> 3] &= ~bit;
- }
- return address_space_write(&s->source_as, addr, attrs, buf, size);
-}
-
-static const MemoryRegionOps bitband_ops = {
- .read_with_attrs = bitband_read,
- .write_with_attrs = bitband_write,
- .endianness = DEVICE_NATIVE_ENDIAN,
- .impl.min_access_size = 1,
- .impl.max_access_size = 4,
- .valid.min_access_size = 1,
- .valid.max_access_size = 4,
-};
-
-static void bitband_init(Object *obj)
-{
- BitBandState *s = BITBAND(obj);
- SysBusDevice *dev = SYS_BUS_DEVICE(obj);
-
- memory_region_init_io(&s->iomem, obj, &bitband_ops, s,
- "bitband", 0x02000000);
- sysbus_init_mmio(dev, &s->iomem);
-}
-
-static void bitband_realize(DeviceState *dev, Error **errp)
-{
- BitBandState *s = BITBAND(dev);
-
- if (!s->source_memory) {
- error_setg(errp, "source-memory property not set");
- return;
- }
-
- address_space_init(&s->source_as, s->source_memory, "bitband-source");
-}
-
-/* Board init. */
-
-static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
- 0x20000000, 0x40000000
-};
-
-static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
- 0x22000000, 0x42000000
-};
-
-static void armv7m_instance_init(Object *obj)
-{
- ARMv7MState *s = ARMV7M(obj);
- int i;
-
- /* Can't init the cpu here, we don't yet know which model to use */
-
- memory_region_init(&s->container, obj, "armv7m-container", UINT64_MAX);
-
- sysbus_init_child_obj(obj, "nvnic", &s->nvic, sizeof(s->nvic), TYPE_NVIC);
- object_property_add_alias(obj, "num-irq",
- OBJECT(&s->nvic), "num-irq", &error_abort);
-
- for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
- sysbus_init_child_obj(obj, "bitband[*]", &s->bitband[i],
- sizeof(s->bitband[i]), TYPE_BITBAND);
- }
-}
-
-static void armv7m_realize(DeviceState *dev, Error **errp)
-{
- ARMv7MState *s = ARMV7M(dev);
- SysBusDevice *sbd;
- Error *err = NULL;
- int i;
-
- if (!s->board_memory) {
- error_setg(errp, "memory property was not set");
- return;
- }
-
- memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
-
- s->cpu = ARM_CPU(object_new(s->cpu_type));
-
- object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
- &error_abort);
- if (object_property_find(OBJECT(s->cpu), "idau", NULL)) {
- object_property_set_link(OBJECT(s->cpu), s->idau, "idau", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
- }
- if (object_property_find(OBJECT(s->cpu), "init-svtor", NULL)) {
- object_property_set_uint(OBJECT(s->cpu), s->init_svtor,
- "init-svtor", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
- }
-
- /* Tell the CPU where the NVIC is; it will fail realize if it doesn't
- * have one.
- */
- s->cpu->env.nvic = &s->nvic;
-
- object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
-
- /* Note that we must realize the NVIC after the CPU */
- object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
-
- /* Alias the NVIC's input and output GPIOs as our own so the board
- * code can wire them up. (We do this in realize because the
- * NVIC doesn't create the input GPIO array until realize.)
- */
- qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
- qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
-
- /* Wire the NVIC up to the CPU */
- sbd = SYS_BUS_DEVICE(&s->nvic);
- sysbus_connect_irq(sbd, 0,
- qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
-
- memory_region_add_subregion(&s->container, 0xe000e000,
- sysbus_mmio_get_region(sbd, 0));
-
- for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
- Object *obj = OBJECT(&s->bitband[i]);
- SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
-
- object_property_set_int(obj, bitband_input_addr[i], "base", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
- object_property_set_link(obj, OBJECT(s->board_memory),
- "source-memory", &error_abort);
- object_property_set_bool(obj, true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
-
- memory_region_add_subregion(&s->container, bitband_output_addr[i],
- sysbus_mmio_get_region(sbd, 0));
- }
-}
-
-static Property armv7m_properties[] = {
- DEFINE_PROP_STRING("cpu-type", ARMv7MState, cpu_type),
- DEFINE_PROP_LINK("memory", ARMv7MState, board_memory, TYPE_MEMORY_REGION,
- MemoryRegion *),
- DEFINE_PROP_LINK("idau", ARMv7MState, idau, TYPE_IDAU_INTERFACE, Object *),
- DEFINE_PROP_UINT32("init-svtor", ARMv7MState, init_svtor, 0),
- DEFINE_PROP_END_OF_LIST(),
-};
-
-static void armv7m_class_init(ObjectClass *klass, void *data)
-{
- DeviceClass *dc = DEVICE_CLASS(klass);
-
- dc->realize = armv7m_realize;
- dc->props = armv7m_properties;
-}
-
-static const TypeInfo armv7m_info = {
- .name = TYPE_ARMV7M,
- .parent = TYPE_SYS_BUS_DEVICE,
- .instance_size = sizeof(ARMv7MState),
- .instance_init = armv7m_instance_init,
- .class_init = armv7m_class_init,
-};
-
-static Property bitband_properties[] = {
- DEFINE_PROP_UINT32("base", BitBandState, base, 0),
- DEFINE_PROP_LINK("source-memory", BitBandState, source_memory,
- TYPE_MEMORY_REGION, MemoryRegion *),
- DEFINE_PROP_END_OF_LIST(),
-};
-
-static void bitband_class_init(ObjectClass *klass, void *data)
-{
- DeviceClass *dc = DEVICE_CLASS(klass);
-
- dc->realize = bitband_realize;
- dc->props = bitband_properties;
-}
-
-static const TypeInfo bitband_info = {
- .name = TYPE_BITBAND,
- .parent = TYPE_SYS_BUS_DEVICE,
- .instance_size = sizeof(BitBandState),
- .instance_init = bitband_init,
- .class_init = bitband_class_init,
-};
-
-static void armv7m_register_types(void)
-{
- type_register_static(&bitband_info);
- type_register_static(&armv7m_info);
-}
-
-type_init(armv7m_register_types)
diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c
index c76d3ed743..bdf854c8b2 100644
--- a/hw/arm/iotkit.c
+++ b/hw/arm/iotkit.c
@@ -111,7 +111,7 @@ static void iotkit_init(Object *obj)
memory_region_init(&s->container, obj, "iotkit-container", UINT64_MAX);
sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
- TYPE_ARMV7M);
+ TYPE_ARM_M_PROFILE);
qdev_prop_set_string(DEVICE(&s->armv7m), "cpu-type",
ARM_CPU_TYPE_NAME("cortex-m33"));
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index af10ee0cc9..114632c94b 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -34,7 +34,6 @@
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "hw/arm/arm.h"
-#include "hw/arm/armv7m.h"
#include "hw/or-irq.h"
#include "hw/boards.h"
#include "exec/address-spaces.h"
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index bcc7070104..912e1297d5 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -26,7 +26,7 @@
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "hw/arm/arm.h"
-#include "hw/arm/armv7m.h"
+#include "hw/arm/arm-m-profile.h"
#include "hw/or-irq.h"
#include "hw/boards.h"
#include "exec/address-spaces.h"
@@ -52,7 +52,7 @@ typedef struct {
typedef struct {
MachineState parent;
- ARMv7MState armv7m;
+ ARMMProfileState armv7m;
MemoryRegion psram;
MemoryRegion ssram1;
MemoryRegion ssram1_m;
@@ -172,7 +172,7 @@ static void mps2_common_init(MachineState *machine)
g_assert_not_reached();
}
- object_initialize(&mms->armv7m, sizeof(mms->armv7m), TYPE_ARMV7M);
+ object_initialize(&mms->armv7m, sizeof(mms->armv7m), TYPE_ARM_M_PROFILE);
armv7m = DEVICE(&mms->armv7m);
qdev_set_parent_bus(armv7m, sysbus_get_default());
switch (mmc->fpga_type) {
diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index dbefade644..09cdc61f44 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -69,7 +69,7 @@ static void m2sxxx_soc_initfn(Object *obj)
int i;
sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
- TYPE_ARMV7M);
+ TYPE_ARM_M_PROFILE);
sysbus_init_child_obj(obj, "sysreg", &s->sysreg, sizeof(s->sysreg),
TYPE_MSF2_SYSREG);
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 68e52367c0..42785f5bd1 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -20,7 +20,7 @@
#include "qemu/log.h"
#include "exec/address-spaces.h"
#include "sysemu/sysemu.h"
-#include "hw/arm/armv7m.h"
+#include "hw/arm/arm-m-profile.h"
#include "hw/char/pl011.h"
#include "hw/misc/unimp.h"
#include "cpu.h"
@@ -1301,7 +1301,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
&error_fatal);
memory_region_add_subregion(system_memory, 0x20000000, sram);
- nvic = qdev_create(NULL, TYPE_ARMV7M);
+ nvic = qdev_create(NULL, TYPE_ARM_M_PROFILE);
qdev_prop_set_uint32(nvic, "num-irq", NUM_IRQ_LINES);
qdev_prop_set_string(nvic, "cpu-type", ms->cpu_type);
object_property_set_link(OBJECT(nvic), OBJECT(get_system_memory()),
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index c486d06a8b..7de1474ade 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -50,7 +50,7 @@ static void stm32f205_soc_initfn(Object *obj)
int i;
sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
- TYPE_ARMV7M);
+ TYPE_ARM_M_PROFILE);
sysbus_init_child_obj(obj, "syscfg", &s->syscfg, sizeof(s->syscfg),
TYPE_STM32F2XX_SYSCFG);
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE Stefan Hajnoczi
@ 2018-07-27 4:53 ` Philippe Mathieu-Daudé
2018-07-30 17:29 ` [Qemu-devel] " Peter Maydell
1 sibling, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-27 4:53 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Peter Maydell, jim, mail, Su Hang, ilg, Alistair Francis,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
> The TYPE_ARMV7M class is really a container for an ARM M Profile CPU,
> NVIC, and related pieces. It can also be used for ARMv6-M and ARMv8-M.
> Rename the class since it is not exclusive to ARMv7-M.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/arm/Makefile.objs | 1 -
> include/hw/arm/{armv7m.h => arm-m-profile.h} | 37 ++-
> include/hw/arm/iotkit.h | 4 +-
> include/hw/arm/msf2-soc.h | 4 +-
> include/hw/arm/stm32f205_soc.h | 4 +-
> hw/arm/arm-m-profile.c | 271 +++++++++++++++++
> hw/arm/armv7m.c | 290 -------------------
> hw/arm/iotkit.c | 2 +-
> hw/arm/mps2-tz.c | 1 -
> hw/arm/mps2.c | 6 +-
> hw/arm/msf2-soc.c | 2 +-
> hw/arm/stellaris.c | 4 +-
> hw/arm/stm32f205_soc.c | 2 +-
> 13 files changed, 313 insertions(+), 315 deletions(-)
> rename include/hw/arm/{armv7m.h => arm-m-profile.h} (65%)
> delete mode 100644 hw/arm/armv7m.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 2c43d34c64..b1e4f8f006 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -17,7 +17,6 @@ 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
> obj-$(CONFIG_DIGIC) += digic.o
> diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/arm-m-profile.h
> similarity index 65%
> rename from include/hw/arm/armv7m.h
> rename to include/hw/arm/arm-m-profile.h
> index 78308d1484..ea496d9b88 100644
> --- a/include/hw/arm/armv7m.h
> +++ b/include/hw/arm/arm-m-profile.h
> @@ -1,14 +1,16 @@
> /*
> - * ARMv7M CPU object
> + * ARM M Profile CPU class
> *
> * Copyright (c) 2017 Linaro Ltd
> * Written by Peter Maydell <peter.maydell@linaro.org>
> *
> + * Copyright (C) 2018 Red Hat, Inc.
> + *
> * This code is licensed under the GPL version 2 or later.
> */
>
> -#ifndef HW_ARM_ARMV7M_H
> -#define HW_ARM_ARMV7M_H
> +#ifndef HW_ARM_ARM_M_PROFILE_H
> +#define HW_ARM_ARM_M_PROFILE_H
>
> #include "hw/sysbus.h"
> #include "hw/intc/armv7m_nvic.h"
> @@ -28,12 +30,17 @@ typedef struct {
> MemoryRegion *source_memory;
> } BitBandState;
>
> -#define TYPE_ARMV7M "armv7m"
> -#define ARMV7M(obj) OBJECT_CHECK(ARMv7MState, (obj), TYPE_ARMV7M)
> -
> #define ARMV7M_NUM_BITBANDS 2
>
> -/* ARMv7M container object.
> +#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
> @@ -44,7 +51,7 @@ typedef struct {
> * + Property "idau": IDAU interface (forwarded to CPU object)
> * + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
> */
> -typedef struct ARMv7MState {
> +typedef struct {
> /*< private >*/
> SysBusDevice parent_obj;
> /*< public >*/
> @@ -63,6 +70,18 @@ typedef struct ARMv7MState {
> MemoryRegion *board_memory;
> Object *idau;
> uint32_t init_svtor;
> -} ARMv7MState;
> +} 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/iotkit.h b/include/hw/arm/iotkit.h
> index 2cddde55dd..8b672c2b6c 100644
> --- a/include/hw/arm/iotkit.h
> +++ b/include/hw/arm/iotkit.h
> @@ -51,7 +51,7 @@
> #define IOTKIT_H
>
> #include "hw/sysbus.h"
> -#include "hw/arm/armv7m.h"
> +#include "hw/arm/arm-m-profile.h"
> #include "hw/misc/iotkit-secctl.h"
> #include "hw/misc/tz-ppc.h"
> #include "hw/misc/tz-mpc.h"
> @@ -74,7 +74,7 @@ typedef struct IoTKit {
> SysBusDevice parent_obj;
>
> /*< public >*/
> - ARMv7MState armv7m;
> + ARMMProfileState armv7m;
> IoTKitSecCtl secctl;
> TZPPC apb_ppc0;
> TZPPC apb_ppc1;
> diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
> index 3cfe5c76ee..36d47db274 100644
> --- a/include/hw/arm/msf2-soc.h
> +++ b/include/hw/arm/msf2-soc.h
> @@ -25,7 +25,7 @@
> #ifndef HW_ARM_MSF2_SOC_H
> #define HW_ARM_MSF2_SOC_H
>
> -#include "hw/arm/armv7m.h"
> +#include "hw/arm/arm-m-profile.h"
> #include "hw/timer/mss-timer.h"
> #include "hw/misc/msf2-sysreg.h"
> #include "hw/ssi/mss-spi.h"
> @@ -48,7 +48,7 @@ typedef struct MSF2State {
> SysBusDevice parent_obj;
> /*< public >*/
>
> - ARMv7MState armv7m;
> + ARMMProfileState armv7m;
>
> char *cpu_type;
> char *part_name;
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> index 922a733f88..03b2e0a79d 100644
> --- a/include/hw/arm/stm32f205_soc.h
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -31,7 +31,7 @@
> #include "hw/adc/stm32f2xx_adc.h"
> #include "hw/or-irq.h"
> #include "hw/ssi/stm32f2xx_spi.h"
> -#include "hw/arm/armv7m.h"
> +#include "hw/arm/arm-m-profile.h"
>
> #define TYPE_STM32F205_SOC "stm32f205-soc"
> #define STM32F205_SOC(obj) \
> @@ -54,7 +54,7 @@ typedef struct STM32F205State {
>
> char *cpu_type;
>
> - ARMv7MState armv7m;
> + ARMMProfileState armv7m;
>
> STM32F2XXSyscfgState syscfg;
> STM32F2XXUsartState usart[STM_NUM_USARTS];
> diff --git a/hw/arm/arm-m-profile.c b/hw/arm/arm-m-profile.c
> index 262706ed62..b7dd2370d4 100644
> --- a/hw/arm/arm-m-profile.c
> +++ b/hw/arm/arm-m-profile.c
> @@ -10,6 +10,8 @@
> */
>
> #include "qemu/osdep.h"
> +#include "hw/arm/arm-m-profile.h"
> +#include "qapi/error.h"
> #include "qemu-common.h"
> #include "cpu.h"
> #include "hw/sysbus.h"
> @@ -20,6 +22,244 @@
> #include "qemu/error-report.h"
> #include "exec/address-spaces.h"
>
> +/* Bitbanded IO. Each word corresponds to a single bit. */
> +
> +/* Get the byte address of the real memory for a bitband access. */
> +static inline hwaddr bitband_addr(BitBandState *s, hwaddr offset)
> +{
> + return s->base | (offset & 0x1ffffff) >> 5;
> +}
> +
> +static MemTxResult bitband_read(void *opaque, hwaddr offset,
> + uint64_t *data, unsigned size, MemTxAttrs attrs)
> +{
> + BitBandState *s = opaque;
> + uint8_t buf[4];
> + MemTxResult res;
> + int bitpos, bit;
> + hwaddr addr;
> +
> + assert(size <= 4);
> +
> + /* Find address in underlying memory and round down to multiple of size */
> + addr = bitband_addr(s, offset) & (-size);
> + res = address_space_read(&s->source_as, addr, attrs, buf, size);
> + if (res) {
> + return res;
> + }
> + /* Bit position in the N bytes read... */
> + bitpos = (offset >> 2) & ((size * 8) - 1);
> + /* ...converted to byte in buffer and bit in byte */
> + bit = (buf[bitpos >> 3] >> (bitpos & 7)) & 1;
> + *data = bit;
> + return MEMTX_OK;
> +}
> +
> +static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size, MemTxAttrs attrs)
> +{
> + BitBandState *s = opaque;
> + uint8_t buf[4];
> + MemTxResult res;
> + int bitpos, bit;
> + hwaddr addr;
> +
> + assert(size <= 4);
> +
> + /* Find address in underlying memory and round down to multiple of size */
> + addr = bitband_addr(s, offset) & (-size);
> + res = address_space_read(&s->source_as, addr, attrs, buf, size);
> + if (res) {
> + return res;
> + }
> + /* Bit position in the N bytes read... */
> + bitpos = (offset >> 2) & ((size * 8) - 1);
> + /* ...converted to byte in buffer and bit in byte */
> + bit = 1 << (bitpos & 7);
> + if (value & 1) {
> + buf[bitpos >> 3] |= bit;
> + } else {
> + buf[bitpos >> 3] &= ~bit;
> + }
> + return address_space_write(&s->source_as, addr, attrs, buf, size);
> +}
> +
> +static const MemoryRegionOps bitband_ops = {
> + .read_with_attrs = bitband_read,
> + .write_with_attrs = bitband_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> + .impl.min_access_size = 1,
> + .impl.max_access_size = 4,
> + .valid.min_access_size = 1,
> + .valid.max_access_size = 4,
> +};
> +
> +static void bitband_init(Object *obj)
> +{
> + BitBandState *s = BITBAND(obj);
> + SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> +
> + memory_region_init_io(&s->iomem, obj, &bitband_ops, s,
> + "bitband", 0x02000000);
> + sysbus_init_mmio(dev, &s->iomem);
> +}
> +
> +static void bitband_realize(DeviceState *dev, Error **errp)
> +{
> + BitBandState *s = BITBAND(dev);
> +
> + if (!s->source_memory) {
> + error_setg(errp, "source-memory property not set");
> + return;
> + }
> +
> + address_space_init(&s->source_as, s->source_memory, "bitband-source");
> +}
> +
> +/* Board init. */
> +
> +static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> + 0x20000000, 0x40000000
> +};
> +
> +static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
> + 0x22000000, 0x42000000
> +};
> +
> +static void arm_m_profile_instance_init(Object *obj)
> +{
> + ARMMProfileState *s = ARM_M_PROFILE(obj);
> + int i;
> +
> + /* Can't init the cpu here, we don't yet know which model to use */
> +
> + memory_region_init(&s->container, obj, "arm-m-profile-container", UINT64_MAX);
> +
> + sysbus_init_child_obj(obj, "nvnic", &s->nvic, sizeof(s->nvic), TYPE_NVIC);
> + object_property_add_alias(obj, "num-irq",
> + OBJECT(&s->nvic), "num-irq", &error_abort);
> +
> + for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> + sysbus_init_child_obj(obj, "bitband[*]", &s->bitband[i],
> + sizeof(s->bitband[i]), TYPE_BITBAND);
> + }
> +}
> +
> +static void arm_m_profile_realize(DeviceState *dev, Error **errp)
> +{
> + ARMMProfileState *s = ARM_M_PROFILE(dev);
> + SysBusDevice *sbd;
> + Error *err = NULL;
> + int i;
> +
> + if (!s->board_memory) {
> + error_setg(errp, "memory property was not set");
> + return;
> + }
> +
> + memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
> +
> + s->cpu = ARM_CPU(object_new(s->cpu_type));
> +
> + object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
> + &error_abort);
> + if (object_property_find(OBJECT(s->cpu), "idau", NULL)) {
> + object_property_set_link(OBJECT(s->cpu), s->idau, "idau", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> + }
> + if (object_property_find(OBJECT(s->cpu), "init-svtor", NULL)) {
> + object_property_set_uint(OBJECT(s->cpu), s->init_svtor,
> + "init-svtor", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> + }
> +
> + /* Tell the CPU where the NVIC is; it will fail realize if it doesn't
> + * have one.
> + */
> + s->cpu->env.nvic = &s->nvic;
> +
> + object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + /* Note that we must realize the NVIC after the CPU */
> + object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + /* Alias the NVIC's input and output GPIOs as our own so the board
> + * code can wire them up. (We do this in realize because the
> + * NVIC doesn't create the input GPIO array until realize.)
> + */
> + qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
> + qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
> +
> + /* Wire the NVIC up to the CPU */
> + sbd = SYS_BUS_DEVICE(&s->nvic);
> + sysbus_connect_irq(sbd, 0,
> + qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
> +
> + memory_region_add_subregion(&s->container, 0xe000e000,
> + sysbus_mmio_get_region(sbd, 0));
> +
> + for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> + Object *obj = OBJECT(&s->bitband[i]);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> +
> + object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> + object_property_set_link(obj, OBJECT(s->board_memory),
> + "source-memory", &error_abort);
> + object_property_set_bool(obj, true, "realized", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + memory_region_add_subregion(&s->container, bitband_output_addr[i],
> + sysbus_mmio_get_region(sbd, 0));
> + }
> +}
> +
> +static Property arm_m_profile_properties[] = {
> + DEFINE_PROP_STRING("cpu-type", ARMMProfileState, cpu_type),
> + DEFINE_PROP_LINK("memory", ARMMProfileState, board_memory,
> + TYPE_MEMORY_REGION, MemoryRegion *),
> + DEFINE_PROP_LINK("idau", ARMMProfileState, idau,
> + TYPE_IDAU_INTERFACE, Object *),
> + DEFINE_PROP_UINT32("init-svtor", ARMMProfileState, init_svtor, 0),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void arm_m_profile_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = arm_m_profile_realize;
> + dc->props = arm_m_profile_properties;
> +}
> +
> +static const TypeInfo arm_m_profile_info = {
> + .name = TYPE_ARM_M_PROFILE,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(ARMMProfileState),
> + .instance_init = arm_m_profile_instance_init,
> + .class_init = arm_m_profile_class_init,
> +};
> +
> static void arm_m_profile_reset(void *opaque)
> {
> ARMCPU *cpu = opaque;
> @@ -79,3 +319,34 @@ void arm_m_profile_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem
> */
> qemu_register_reset(arm_m_profile_reset, cpu);
> }
> +
> +static Property bitband_properties[] = {
> + DEFINE_PROP_UINT32("base", BitBandState, base, 0),
> + DEFINE_PROP_LINK("source-memory", BitBandState, source_memory,
> + TYPE_MEMORY_REGION, MemoryRegion *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void bitband_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = bitband_realize;
> + dc->props = bitband_properties;
> +}
> +
> +static const TypeInfo bitband_info = {
> + .name = TYPE_BITBAND,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(BitBandState),
> + .instance_init = bitband_init,
> + .class_init = bitband_class_init,
> +};
> +
> +static void arm_m_profile_register_types(void)
> +{
> + type_register_static(&bitband_info);
> + type_register_static(&arm_m_profile_info);
> +}
> +
> +type_init(arm_m_profile_register_types)
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> deleted file mode 100644
> index 7405a1ec69..0000000000
> --- a/hw/arm/armv7m.c
> +++ /dev/null
> @@ -1,290 +0,0 @@
> -/*
> - * ARMV7M System emulation.
> - *
> - * Copyright (c) 2006-2007 CodeSourcery.
> - * Written by Paul Brook
> - *
> - * This code is licensed under the GPL.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "hw/arm/armv7m.h"
> -#include "qapi/error.h"
> -#include "qemu-common.h"
> -#include "cpu.h"
> -#include "hw/sysbus.h"
> -#include "hw/arm/arm.h"
> -#include "hw/loader.h"
> -#include "elf.h"
> -#include "sysemu/qtest.h"
> -#include "qemu/error-report.h"
> -#include "exec/address-spaces.h"
> -#include "target/arm/idau.h"
> -
> -/* Bitbanded IO. Each word corresponds to a single bit. */
> -
> -/* Get the byte address of the real memory for a bitband access. */
> -static inline hwaddr bitband_addr(BitBandState *s, hwaddr offset)
> -{
> - return s->base | (offset & 0x1ffffff) >> 5;
> -}
> -
> -static MemTxResult bitband_read(void *opaque, hwaddr offset,
> - uint64_t *data, unsigned size, MemTxAttrs attrs)
> -{
> - BitBandState *s = opaque;
> - uint8_t buf[4];
> - MemTxResult res;
> - int bitpos, bit;
> - hwaddr addr;
> -
> - assert(size <= 4);
> -
> - /* Find address in underlying memory and round down to multiple of size */
> - addr = bitband_addr(s, offset) & (-size);
> - res = address_space_read(&s->source_as, addr, attrs, buf, size);
> - if (res) {
> - return res;
> - }
> - /* Bit position in the N bytes read... */
> - bitpos = (offset >> 2) & ((size * 8) - 1);
> - /* ...converted to byte in buffer and bit in byte */
> - bit = (buf[bitpos >> 3] >> (bitpos & 7)) & 1;
> - *data = bit;
> - return MEMTX_OK;
> -}
> -
> -static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
> - unsigned size, MemTxAttrs attrs)
> -{
> - BitBandState *s = opaque;
> - uint8_t buf[4];
> - MemTxResult res;
> - int bitpos, bit;
> - hwaddr addr;
> -
> - assert(size <= 4);
> -
> - /* Find address in underlying memory and round down to multiple of size */
> - addr = bitband_addr(s, offset) & (-size);
> - res = address_space_read(&s->source_as, addr, attrs, buf, size);
> - if (res) {
> - return res;
> - }
> - /* Bit position in the N bytes read... */
> - bitpos = (offset >> 2) & ((size * 8) - 1);
> - /* ...converted to byte in buffer and bit in byte */
> - bit = 1 << (bitpos & 7);
> - if (value & 1) {
> - buf[bitpos >> 3] |= bit;
> - } else {
> - buf[bitpos >> 3] &= ~bit;
> - }
> - return address_space_write(&s->source_as, addr, attrs, buf, size);
> -}
> -
> -static const MemoryRegionOps bitband_ops = {
> - .read_with_attrs = bitband_read,
> - .write_with_attrs = bitband_write,
> - .endianness = DEVICE_NATIVE_ENDIAN,
> - .impl.min_access_size = 1,
> - .impl.max_access_size = 4,
> - .valid.min_access_size = 1,
> - .valid.max_access_size = 4,
> -};
> -
> -static void bitband_init(Object *obj)
> -{
> - BitBandState *s = BITBAND(obj);
> - SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> -
> - memory_region_init_io(&s->iomem, obj, &bitband_ops, s,
> - "bitband", 0x02000000);
> - sysbus_init_mmio(dev, &s->iomem);
> -}
> -
> -static void bitband_realize(DeviceState *dev, Error **errp)
> -{
> - BitBandState *s = BITBAND(dev);
> -
> - if (!s->source_memory) {
> - error_setg(errp, "source-memory property not set");
> - return;
> - }
> -
> - address_space_init(&s->source_as, s->source_memory, "bitband-source");
> -}
> -
> -/* Board init. */
> -
> -static const hwaddr bitband_input_addr[ARMV7M_NUM_BITBANDS] = {
> - 0x20000000, 0x40000000
> -};
> -
> -static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = {
> - 0x22000000, 0x42000000
> -};
> -
> -static void armv7m_instance_init(Object *obj)
> -{
> - ARMv7MState *s = ARMV7M(obj);
> - int i;
> -
> - /* Can't init the cpu here, we don't yet know which model to use */
> -
> - memory_region_init(&s->container, obj, "armv7m-container", UINT64_MAX);
> -
> - sysbus_init_child_obj(obj, "nvnic", &s->nvic, sizeof(s->nvic), TYPE_NVIC);
> - object_property_add_alias(obj, "num-irq",
> - OBJECT(&s->nvic), "num-irq", &error_abort);
> -
> - for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> - sysbus_init_child_obj(obj, "bitband[*]", &s->bitband[i],
> - sizeof(s->bitband[i]), TYPE_BITBAND);
> - }
> -}
> -
> -static void armv7m_realize(DeviceState *dev, Error **errp)
> -{
> - ARMv7MState *s = ARMV7M(dev);
> - SysBusDevice *sbd;
> - Error *err = NULL;
> - int i;
> -
> - if (!s->board_memory) {
> - error_setg(errp, "memory property was not set");
> - return;
> - }
> -
> - memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1);
> -
> - s->cpu = ARM_CPU(object_new(s->cpu_type));
> -
> - object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
> - &error_abort);
> - if (object_property_find(OBJECT(s->cpu), "idau", NULL)) {
> - object_property_set_link(OBJECT(s->cpu), s->idau, "idau", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> - }
> - if (object_property_find(OBJECT(s->cpu), "init-svtor", NULL)) {
> - object_property_set_uint(OBJECT(s->cpu), s->init_svtor,
> - "init-svtor", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> - }
> -
> - /* Tell the CPU where the NVIC is; it will fail realize if it doesn't
> - * have one.
> - */
> - s->cpu->env.nvic = &s->nvic;
> -
> - object_property_set_bool(OBJECT(s->cpu), true, "realized", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> -
> - /* Note that we must realize the NVIC after the CPU */
> - object_property_set_bool(OBJECT(&s->nvic), true, "realized", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> -
> - /* Alias the NVIC's input and output GPIOs as our own so the board
> - * code can wire them up. (We do this in realize because the
> - * NVIC doesn't create the input GPIO array until realize.)
> - */
> - qdev_pass_gpios(DEVICE(&s->nvic), dev, NULL);
> - qdev_pass_gpios(DEVICE(&s->nvic), dev, "SYSRESETREQ");
> -
> - /* Wire the NVIC up to the CPU */
> - sbd = SYS_BUS_DEVICE(&s->nvic);
> - sysbus_connect_irq(sbd, 0,
> - qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
> -
> - memory_region_add_subregion(&s->container, 0xe000e000,
> - sysbus_mmio_get_region(sbd, 0));
> -
> - for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> - Object *obj = OBJECT(&s->bitband[i]);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> -
> - object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> - object_property_set_link(obj, OBJECT(s->board_memory),
> - "source-memory", &error_abort);
> - object_property_set_bool(obj, true, "realized", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> -
> - memory_region_add_subregion(&s->container, bitband_output_addr[i],
> - sysbus_mmio_get_region(sbd, 0));
> - }
> -}
> -
> -static Property armv7m_properties[] = {
> - DEFINE_PROP_STRING("cpu-type", ARMv7MState, cpu_type),
> - DEFINE_PROP_LINK("memory", ARMv7MState, board_memory, TYPE_MEMORY_REGION,
> - MemoryRegion *),
> - DEFINE_PROP_LINK("idau", ARMv7MState, idau, TYPE_IDAU_INTERFACE, Object *),
> - DEFINE_PROP_UINT32("init-svtor", ARMv7MState, init_svtor, 0),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void armv7m_class_init(ObjectClass *klass, void *data)
> -{
> - DeviceClass *dc = DEVICE_CLASS(klass);
> -
> - dc->realize = armv7m_realize;
> - dc->props = armv7m_properties;
> -}
> -
> -static const TypeInfo armv7m_info = {
> - .name = TYPE_ARMV7M,
> - .parent = TYPE_SYS_BUS_DEVICE,
> - .instance_size = sizeof(ARMv7MState),
> - .instance_init = armv7m_instance_init,
> - .class_init = armv7m_class_init,
> -};
> -
> -static Property bitband_properties[] = {
> - DEFINE_PROP_UINT32("base", BitBandState, base, 0),
> - DEFINE_PROP_LINK("source-memory", BitBandState, source_memory,
> - TYPE_MEMORY_REGION, MemoryRegion *),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void bitband_class_init(ObjectClass *klass, void *data)
> -{
> - DeviceClass *dc = DEVICE_CLASS(klass);
> -
> - dc->realize = bitband_realize;
> - dc->props = bitband_properties;
> -}
> -
> -static const TypeInfo bitband_info = {
> - .name = TYPE_BITBAND,
> - .parent = TYPE_SYS_BUS_DEVICE,
> - .instance_size = sizeof(BitBandState),
> - .instance_init = bitband_init,
> - .class_init = bitband_class_init,
> -};
> -
> -static void armv7m_register_types(void)
> -{
> - type_register_static(&bitband_info);
> - type_register_static(&armv7m_info);
> -}
> -
> -type_init(armv7m_register_types)
> diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c
> index c76d3ed743..bdf854c8b2 100644
> --- a/hw/arm/iotkit.c
> +++ b/hw/arm/iotkit.c
> @@ -111,7 +111,7 @@ static void iotkit_init(Object *obj)
> memory_region_init(&s->container, obj, "iotkit-container", UINT64_MAX);
>
> sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
> - TYPE_ARMV7M);
> + TYPE_ARM_M_PROFILE);
> qdev_prop_set_string(DEVICE(&s->armv7m), "cpu-type",
> ARM_CPU_TYPE_NAME("cortex-m33"));
>
> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> index af10ee0cc9..114632c94b 100644
> --- a/hw/arm/mps2-tz.c
> +++ b/hw/arm/mps2-tz.c
> @@ -34,7 +34,6 @@
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "hw/arm/arm.h"
> -#include "hw/arm/armv7m.h"
> #include "hw/or-irq.h"
> #include "hw/boards.h"
> #include "exec/address-spaces.h"
> diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
> index bcc7070104..912e1297d5 100644
> --- a/hw/arm/mps2.c
> +++ b/hw/arm/mps2.c
> @@ -26,7 +26,7 @@
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "hw/arm/arm.h"
> -#include "hw/arm/armv7m.h"
> +#include "hw/arm/arm-m-profile.h"
> #include "hw/or-irq.h"
> #include "hw/boards.h"
> #include "exec/address-spaces.h"
> @@ -52,7 +52,7 @@ typedef struct {
> typedef struct {
> MachineState parent;
>
> - ARMv7MState armv7m;
> + ARMMProfileState armv7m;
> MemoryRegion psram;
> MemoryRegion ssram1;
> MemoryRegion ssram1_m;
> @@ -172,7 +172,7 @@ static void mps2_common_init(MachineState *machine)
> g_assert_not_reached();
> }
>
> - object_initialize(&mms->armv7m, sizeof(mms->armv7m), TYPE_ARMV7M);
> + object_initialize(&mms->armv7m, sizeof(mms->armv7m), TYPE_ARM_M_PROFILE);
> armv7m = DEVICE(&mms->armv7m);
> qdev_set_parent_bus(armv7m, sysbus_get_default());
> switch (mmc->fpga_type) {
> diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
> index dbefade644..09cdc61f44 100644
> --- a/hw/arm/msf2-soc.c
> +++ b/hw/arm/msf2-soc.c
> @@ -69,7 +69,7 @@ static void m2sxxx_soc_initfn(Object *obj)
> int i;
>
> sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
> - TYPE_ARMV7M);
> + TYPE_ARM_M_PROFILE);
>
> sysbus_init_child_obj(obj, "sysreg", &s->sysreg, sizeof(s->sysreg),
> TYPE_MSF2_SYSREG);
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 68e52367c0..42785f5bd1 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -20,7 +20,7 @@
> #include "qemu/log.h"
> #include "exec/address-spaces.h"
> #include "sysemu/sysemu.h"
> -#include "hw/arm/armv7m.h"
> +#include "hw/arm/arm-m-profile.h"
> #include "hw/char/pl011.h"
> #include "hw/misc/unimp.h"
> #include "cpu.h"
> @@ -1301,7 +1301,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
> &error_fatal);
> memory_region_add_subregion(system_memory, 0x20000000, sram);
>
> - nvic = qdev_create(NULL, TYPE_ARMV7M);
> + nvic = qdev_create(NULL, TYPE_ARM_M_PROFILE);
> qdev_prop_set_uint32(nvic, "num-irq", NUM_IRQ_LINES);
> qdev_prop_set_string(nvic, "cpu-type", ms->cpu_type);
> object_property_set_link(OBJECT(nvic), OBJECT(get_system_memory()),
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index c486d06a8b..7de1474ade 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -50,7 +50,7 @@ static void stm32f205_soc_initfn(Object *obj)
> int i;
>
> sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
> - TYPE_ARMV7M);
> + TYPE_ARM_M_PROFILE);
>
> sysbus_init_child_obj(obj, "syscfg", &s->syscfg, sizeof(s->syscfg),
> TYPE_STM32F2XX_SYSCFG);
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE Stefan Hajnoczi
2018-07-27 4:53 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-07-30 17:29 ` Peter Maydell
1 sibling, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2018-07-30 17:29 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Steffen Görtz, Alistair Francis,
Liviu Ionescu, qemu-arm, Julia Suvorova, Subbaraya Sundeep,
Su Hang, Steffen Gortz, Jim Mussared, Joel Stanley
On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The TYPE_ARMV7M class is really a container for an ARM M Profile CPU,
> NVIC, and related pieces. It can also be used for ARMv6-M and ARMv8-M.
> Rename the class since it is not exclusive to ARMv7-M.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/arm/Makefile.objs | 1 -
> include/hw/arm/{armv7m.h => arm-m-profile.h} | 37 ++-
> include/hw/arm/iotkit.h | 4 +-
> include/hw/arm/msf2-soc.h | 4 +-
> include/hw/arm/stm32f205_soc.h | 4 +-
> hw/arm/arm-m-profile.c | 271 +++++++++++++++++
> hw/arm/armv7m.c | 290 -------------------
> hw/arm/iotkit.c | 2 +-
> hw/arm/mps2-tz.c | 1 -
> hw/arm/mps2.c | 6 +-
> hw/arm/msf2-soc.c | 2 +-
> hw/arm/stellaris.c | 4 +-
> hw/arm/stm32f205_soc.c | 2 +-
> 13 files changed, 313 insertions(+), 315 deletions(-)
> rename include/hw/arm/{armv7m.h => arm-m-profile.h} (65%)
> delete mode 100644 hw/arm/armv7m.c
It would be nice if this rename showed up in the patch as
a rename, rather than a "delete everything and have a new one"...
Reviewing this is going to be painful enough to make me wonder
whether it really matters that we call this "m-profile" rather
than "v7m" (we have a lot of places that use 'v7m' as a somewhat
inaccurate name for "M profile" anyway, including a bunch
of new code I wrote that's really v8m related.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel() Stefan Hajnoczi
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE Stefan Hajnoczi
@ 2018-07-25 8:59 ` Stefan Hajnoczi
2018-07-27 4:53 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
` (2 more replies)
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model Stefan Hajnoczi
` (4 subsequent siblings)
7 siblings, 3 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 8:59 UTC (permalink / raw)
To: qemu-devel
Cc: mail, Alistair Francis, Peter Maydell, ilg, qemu-arm,
Julia Suvorova, Subbaraya Sundeep, Su Hang, Steffen Gortz, jim,
Joel Stanley, Stefan Hajnoczi
Some ARM CPUs have bitbanded IO, a memory region that allows convenient
bit access via 32-bit memory loads/stores. This eliminates the need for
read-modify-update instruction sequences.
This patch makes this optional feature a ARMMProfile qdev property,
allowing boards to choose whether they want bitbanding or not.
Status of boards:
* iotkit (Cortex M33), no bitband
* mps2 (Cortex M3), bitband
* msf2 (Cortex M3), bitband
* stellaris (Cortex M3), bitband
* stm32f205 (Cortex M3), bitband
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/arm/arm-m-profile.h | 2 ++
hw/arm/arm-m-profile.c | 38 +++++++++++++++++++---------------
hw/arm/mps2.c | 1 +
hw/arm/msf2-soc.c | 1 +
hw/arm/stellaris.c | 1 +
hw/arm/stm32f205_soc.c | 1 +
6 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/include/hw/arm/arm-m-profile.h b/include/hw/arm/arm-m-profile.h
index ea496d9b88..1eb7a5c328 100644
--- a/include/hw/arm/arm-m-profile.h
+++ b/include/hw/arm/arm-m-profile.h
@@ -50,6 +50,7 @@ typedef struct {
* devices will be automatically layered on top of this view.)
* + Property "idau": IDAU interface (forwarded to CPU object)
* + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
+ * + Property "enable-bitband": expose bitbanded IO
*/
typedef struct {
/*< private >*/
@@ -70,6 +71,7 @@ typedef struct {
MemoryRegion *board_memory;
Object *idau;
uint32_t init_svtor;
+ bool enable_bitband;
} ARMMProfileState;
typedef struct {
diff --git a/hw/arm/arm-m-profile.c b/hw/arm/arm-m-profile.c
index b7dd2370d4..8bafd6602d 100644
--- a/hw/arm/arm-m-profile.c
+++ b/hw/arm/arm-m-profile.c
@@ -212,25 +212,27 @@ static void arm_m_profile_realize(DeviceState *dev, Error **errp)
memory_region_add_subregion(&s->container, 0xe000e000,
sysbus_mmio_get_region(sbd, 0));
- for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
- Object *obj = OBJECT(&s->bitband[i]);
- SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
+ if (s->enable_bitband) {
+ for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+ Object *obj = OBJECT(&s->bitband[i]);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
- object_property_set_int(obj, bitband_input_addr[i], "base", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
- object_property_set_link(obj, OBJECT(s->board_memory),
- "source-memory", &error_abort);
- object_property_set_bool(obj, true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
+ object_property_set_int(obj, bitband_input_addr[i], "base", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+ object_property_set_link(obj, OBJECT(s->board_memory),
+ "source-memory", &error_abort);
+ object_property_set_bool(obj, true, "realized", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
- memory_region_add_subregion(&s->container, bitband_output_addr[i],
- sysbus_mmio_get_region(sbd, 0));
+ memory_region_add_subregion(&s->container, bitband_output_addr[i],
+ sysbus_mmio_get_region(sbd, 0));
+ }
}
}
@@ -241,6 +243,8 @@ static Property arm_m_profile_properties[] = {
DEFINE_PROP_LINK("idau", ARMMProfileState, idau,
TYPE_IDAU_INTERFACE, Object *),
DEFINE_PROP_UINT32("init-svtor", ARMMProfileState, init_svtor, 0),
+ DEFINE_PROP_BOOL("enable-bitband", ARMMProfileState,
+ enable_bitband, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index 912e1297d5..b232162dbd 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -186,6 +186,7 @@ static void mps2_common_init(MachineState *machine)
g_assert_not_reached();
}
qdev_prop_set_string(armv7m, "cpu-type", machine->cpu_type);
+ qdev_prop_set_bit(armv7m, "enable-bitband", true);
object_property_set_link(OBJECT(&mms->armv7m), OBJECT(system_memory),
"memory", &error_abort);
object_property_set_bool(OBJECT(&mms->armv7m), true, "realized",
diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index 09cdc61f44..fb5d16338f 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -117,6 +117,7 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
armv7m = DEVICE(&s->armv7m);
qdev_prop_set_uint32(armv7m, "num-irq", 81);
qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
+ qdev_prop_set_bit(armv7m, "enable-bitband", true);
object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
"memory", &error_abort);
object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 42785f5bd1..cb22684ffe 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1304,6 +1304,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
nvic = qdev_create(NULL, TYPE_ARM_M_PROFILE);
qdev_prop_set_uint32(nvic, "num-irq", NUM_IRQ_LINES);
qdev_prop_set_string(nvic, "cpu-type", ms->cpu_type);
+ qdev_prop_set_bit(nvic, "enable-bitband", true);
object_property_set_link(OBJECT(nvic), OBJECT(get_system_memory()),
"memory", &error_abort);
/* This will exit with an error if the user passed us a bad cpu_type */
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 7de1474ade..f78b89d205 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -109,6 +109,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
armv7m = DEVICE(&s->armv7m);
qdev_prop_set_uint32(armv7m, "num-irq", 96);
qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
+ qdev_prop_set_bit(armv7m, "enable-bitband", true);
object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
"memory", &error_abort);
object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile Stefan Hajnoczi
@ 2018-07-27 4:53 ` Philippe Mathieu-Daudé
2018-07-30 17:33 ` [Qemu-devel] " Peter Maydell
2018-07-31 17:18 ` Peter Maydell
2 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-27 4:53 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Peter Maydell, jim, mail, Su Hang, ilg, Alistair Francis,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
> Some ARM CPUs have bitbanded IO, a memory region that allows convenient
> bit access via 32-bit memory loads/stores. This eliminates the need for
> read-modify-update instruction sequences.
>
> This patch makes this optional feature a ARMMProfile qdev property,
> allowing boards to choose whether they want bitbanding or not.
>
> Status of boards:
> * iotkit (Cortex M33), no bitband
> * mps2 (Cortex M3), bitband
> * msf2 (Cortex M3), bitband
> * stellaris (Cortex M3), bitband
> * stm32f205 (Cortex M3), bitband
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/arm/arm-m-profile.h | 2 ++
> hw/arm/arm-m-profile.c | 38 +++++++++++++++++++---------------
> hw/arm/mps2.c | 1 +
> hw/arm/msf2-soc.c | 1 +
> hw/arm/stellaris.c | 1 +
> hw/arm/stm32f205_soc.c | 1 +
> 6 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/arm/arm-m-profile.h b/include/hw/arm/arm-m-profile.h
> index ea496d9b88..1eb7a5c328 100644
> --- a/include/hw/arm/arm-m-profile.h
> +++ b/include/hw/arm/arm-m-profile.h
> @@ -50,6 +50,7 @@ typedef struct {
> * devices will be automatically layered on top of this view.)
> * + Property "idau": IDAU interface (forwarded to CPU object)
> * + Property "init-svtor": secure VTOR reset value (forwarded to CPU object)
> + * + Property "enable-bitband": expose bitbanded IO
> */
> typedef struct {
> /*< private >*/
> @@ -70,6 +71,7 @@ typedef struct {
> MemoryRegion *board_memory;
> Object *idau;
> uint32_t init_svtor;
> + bool enable_bitband;
> } ARMMProfileState;
>
> typedef struct {
> diff --git a/hw/arm/arm-m-profile.c b/hw/arm/arm-m-profile.c
> index b7dd2370d4..8bafd6602d 100644
> --- a/hw/arm/arm-m-profile.c
> +++ b/hw/arm/arm-m-profile.c
> @@ -212,25 +212,27 @@ static void arm_m_profile_realize(DeviceState *dev, Error **errp)
> memory_region_add_subregion(&s->container, 0xe000e000,
> sysbus_mmio_get_region(sbd, 0));
>
> - for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> - Object *obj = OBJECT(&s->bitband[i]);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
> + if (s->enable_bitband) {
> + for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
> + Object *obj = OBJECT(&s->bitband[i]);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
>
> - object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> - object_property_set_link(obj, OBJECT(s->board_memory),
> - "source-memory", &error_abort);
> - object_property_set_bool(obj, true, "realized", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - return;
> - }
> + object_property_set_int(obj, bitband_input_addr[i], "base", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> + object_property_set_link(obj, OBJECT(s->board_memory),
> + "source-memory", &error_abort);
> + object_property_set_bool(obj, true, "realized", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
>
> - memory_region_add_subregion(&s->container, bitband_output_addr[i],
> - sysbus_mmio_get_region(sbd, 0));
> + memory_region_add_subregion(&s->container, bitband_output_addr[i],
> + sysbus_mmio_get_region(sbd, 0));
> + }
> }
> }
>
> @@ -241,6 +243,8 @@ static Property arm_m_profile_properties[] = {
> DEFINE_PROP_LINK("idau", ARMMProfileState, idau,
> TYPE_IDAU_INTERFACE, Object *),
> DEFINE_PROP_UINT32("init-svtor", ARMMProfileState, init_svtor, 0),
> + DEFINE_PROP_BOOL("enable-bitband", ARMMProfileState,
> + enable_bitband, false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
> index 912e1297d5..b232162dbd 100644
> --- a/hw/arm/mps2.c
> +++ b/hw/arm/mps2.c
> @@ -186,6 +186,7 @@ static void mps2_common_init(MachineState *machine)
> g_assert_not_reached();
> }
> qdev_prop_set_string(armv7m, "cpu-type", machine->cpu_type);
> + qdev_prop_set_bit(armv7m, "enable-bitband", true);
> object_property_set_link(OBJECT(&mms->armv7m), OBJECT(system_memory),
> "memory", &error_abort);
> object_property_set_bool(OBJECT(&mms->armv7m), true, "realized",
> diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
> index 09cdc61f44..fb5d16338f 100644
> --- a/hw/arm/msf2-soc.c
> +++ b/hw/arm/msf2-soc.c
> @@ -117,6 +117,7 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
> armv7m = DEVICE(&s->armv7m);
> qdev_prop_set_uint32(armv7m, "num-irq", 81);
> qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> + qdev_prop_set_bit(armv7m, "enable-bitband", true);
> object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
> "memory", &error_abort);
> object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 42785f5bd1..cb22684ffe 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -1304,6 +1304,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
> nvic = qdev_create(NULL, TYPE_ARM_M_PROFILE);
> qdev_prop_set_uint32(nvic, "num-irq", NUM_IRQ_LINES);
> qdev_prop_set_string(nvic, "cpu-type", ms->cpu_type);
> + qdev_prop_set_bit(nvic, "enable-bitband", true);
> object_property_set_link(OBJECT(nvic), OBJECT(get_system_memory()),
> "memory", &error_abort);
> /* This will exit with an error if the user passed us a bad cpu_type */
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 7de1474ade..f78b89d205 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -109,6 +109,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> armv7m = DEVICE(&s->armv7m);
> qdev_prop_set_uint32(armv7m, "num-irq", 96);
> qdev_prop_set_string(armv7m, "cpu-type", s->cpu_type);
> + qdev_prop_set_bit(armv7m, "enable-bitband", true);
> object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
> "memory", &error_abort);
> object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile Stefan Hajnoczi
2018-07-27 4:53 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-07-30 17:33 ` Peter Maydell
2018-07-31 17:18 ` Peter Maydell
2 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2018-07-30 17:33 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Steffen Görtz, Alistair Francis,
Liviu Ionescu, qemu-arm, Julia Suvorova, Subbaraya Sundeep,
Su Hang, Steffen Gortz, Jim Mussared, Joel Stanley
On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Some ARM CPUs have bitbanded IO, a memory region that allows convenient
> bit access via 32-bit memory loads/stores. This eliminates the need for
> read-modify-update instruction sequences.
>
> This patch makes this optional feature a ARMMProfile qdev property,
> allowing boards to choose whether they want bitbanding or not.
>
> Status of boards:
> * iotkit (Cortex M33), no bitband
> * mps2 (Cortex M3), bitband
> * msf2 (Cortex M3), bitband
> * stellaris (Cortex M3), bitband
> * stm32f205 (Cortex M3), bitband
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile Stefan Hajnoczi
2018-07-27 4:53 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-07-30 17:33 ` [Qemu-devel] " Peter Maydell
@ 2018-07-31 17:18 ` Peter Maydell
2 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2018-07-31 17:18 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Steffen Görtz, Alistair Francis,
Liviu Ionescu, qemu-arm, Julia Suvorova, Subbaraya Sundeep,
Su Hang, Steffen Gortz, Jim Mussared, Joel Stanley
On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Some ARM CPUs have bitbanded IO, a memory region that allows convenient
> bit access via 32-bit memory loads/stores. This eliminates the need for
> read-modify-update instruction sequences.
>
> This patch makes this optional feature a ARMMProfile qdev property,
> allowing boards to choose whether they want bitbanding or not.
>
> Status of boards:
> * iotkit (Cortex M33), no bitband
> * mps2 (Cortex M3), bitband
> * msf2 (Cortex M3), bitband
> * stellaris (Cortex M3), bitband
> * stm32f205 (Cortex M3), bitband
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This fixes a bug in the MPS2 boards, incidentally, where the
Ethernet controller is inaccessible because it's hidden by
the shouldn't-be-present bitbanding region...
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
` (2 preceding siblings ...)
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile Stefan Hajnoczi
@ 2018-07-25 8:59 ` Stefan Hajnoczi
2018-07-27 5:26 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-07-30 17:52 ` [Qemu-devel] " Peter Maydell
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 5/7] loader: add rom transaction API Stefan Hajnoczi
` (3 subsequent siblings)
7 siblings, 2 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 8:59 UTC (permalink / raw)
To: qemu-devel
Cc: mail, Alistair Francis, Peter Maydell, ilg, qemu-arm,
Julia Suvorova, Subbaraya Sundeep, Su Hang, Steffen Gortz, jim,
Joel Stanley, Stefan Hajnoczi
Define a "cortex-m0" ARMv6-M CPU model.
Most of the register reset values set by other CPU models are not
relevant for the cut-down ARMv6-M architecture.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
target/arm/cpu.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 3848ef46aa..7e477c0d23 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
cpu->reset_auxcr = 1;
}
+static void cortex_m0_initfn(Object *obj)
+{
+ ARMCPU *cpu = ARM_CPU(obj);
+ set_feature(&cpu->env, ARM_FEATURE_V6);
+ set_feature(&cpu->env, ARM_FEATURE_M);
+
+ cpu->midr = 0x410cc200;
+}
+
static void cortex_m3_initfn(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
@@ -1845,6 +1854,8 @@ static const ARMCPUInfo arm_cpus[] = {
{ .name = "arm1136", .initfn = arm1136_initfn },
{ .name = "arm1176", .initfn = arm1176_initfn },
{ .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
+ { .name = "cortex-m0", .initfn = cortex_m0_initfn,
+ .class_init = arm_v7m_class_init },
{ .name = "cortex-m3", .initfn = cortex_m3_initfn,
.class_init = arm_v7m_class_init },
{ .name = "cortex-m4", .initfn = cortex_m4_initfn,
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model Stefan Hajnoczi
@ 2018-07-27 5:26 ` Philippe Mathieu-Daudé
2018-07-30 17:51 ` Peter Maydell
2018-07-30 17:52 ` [Qemu-devel] " Peter Maydell
1 sibling, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-27 5:26 UTC (permalink / raw)
To: Stefan Hajnoczi, Peter Maydell
Cc: qemu-devel, jim, mail, Su Hang, ilg, Alistair Francis,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
Hi Stefan,
On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
> Define a "cortex-m0" ARMv6-M CPU model.
>
> Most of the register reset values set by other CPU models are not
> relevant for the cut-down ARMv6-M architecture.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> target/arm/cpu.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 3848ef46aa..7e477c0d23 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
> cpu->reset_auxcr = 1;
> }
>
> +static void cortex_m0_initfn(Object *obj)
> +{
> + ARMCPU *cpu = ARM_CPU(obj);
> + set_feature(&cpu->env, ARM_FEATURE_V6);
> + set_feature(&cpu->env, ARM_FEATURE_M);
What about ARM_FEATURE_THUMB2 (T32)?
Peter: Since the M0 (optionally?) supports 32x32 multiply, should this
cpu use the ARM_FEATURE_THUMB_DSP feature? Else this might trigger an
'Undefined Instruction' in disas_thumb2_insn().
And what about optional ARM_FEATURE_PMSA?
Oh this would be cortex_m0plus_initfn() for "cortex-m0-plus", ok.
> +
> + cpu->midr = 0x410cc200;
> +}
> +
> static void cortex_m3_initfn(Object *obj)
> {
> ARMCPU *cpu = ARM_CPU(obj);
> @@ -1845,6 +1854,8 @@ static const ARMCPUInfo arm_cpus[] = {
> { .name = "arm1136", .initfn = arm1136_initfn },
> { .name = "arm1176", .initfn = arm1176_initfn },
> { .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
> + { .name = "cortex-m0", .initfn = cortex_m0_initfn,
> + .class_init = arm_v7m_class_init },
What about renaming arm_v7m_class_init -> arm_m_profile_class_init (such
your patches 1 and 2)?
> { .name = "cortex-m3", .initfn = cortex_m3_initfn,
> .class_init = arm_v7m_class_init },
> { .name = "cortex-m4", .initfn = cortex_m4_initfn,
>
Regards,
Phil.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model
2018-07-27 5:26 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-07-30 17:51 ` Peter Maydell
2018-08-10 3:37 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2018-07-30 17:51 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Stefan Hajnoczi, QEMU Developers, Jim Mussared,
Steffen Görtz, Su Hang, Liviu Ionescu, Alistair Francis,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
On 27 July 2018 at 06:26, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Stefan,
>
> On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
>> Define a "cortex-m0" ARMv6-M CPU model.
>>
>> Most of the register reset values set by other CPU models are not
>> relevant for the cut-down ARMv6-M architecture.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>> target/arm/cpu.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 3848ef46aa..7e477c0d23 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
>> cpu->reset_auxcr = 1;
>> }
>>
>> +static void cortex_m0_initfn(Object *obj)
>> +{
>> + ARMCPU *cpu = ARM_CPU(obj);
>> + set_feature(&cpu->env, ARM_FEATURE_V6);
>> + set_feature(&cpu->env, ARM_FEATURE_M);
>
> What about ARM_FEATURE_THUMB2 (T32)?
No, the M0 doesn't have Thumb2. It has Thumb1 plus a tiny set
of 32-bit instructions (which we deal with specially in translate.c).
(As it happens we generally can't get to the checks on the THUMB2
feature for a v6M core, so I think but have not checked thoroughly
that it would make no difference to QEMU's behaviour whether the
feature bit was set or not.)
> Peter: Since the M0 (optionally?) supports 32x32 multiply, should this
> cpu use the ARM_FEATURE_THUMB_DSP feature? Else this might trigger an
> 'Undefined Instruction' in disas_thumb2_insn().
ARM_FEATURE_THUMB_DSP only enables checks in disas_thumb2_insn()
(and 32x32->32 multiply is not one of the insns it gates).
The only insns in that function that a v6M core can execute are
msr, mrs, dsb, dmb, isb and bl, none of which are affected by that
feature switch.
> And what about optional ARM_FEATURE_PMSA?
> Oh this would be cortex_m0plus_initfn() for "cortex-m0-plus", ok.
Yep, plain M0 has no MPU (and so we're postponing the work
of implementing the v6M PMSA, which IIRC isn't quite the
same as the v7M one).
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model
2018-07-30 17:51 ` Peter Maydell
@ 2018-08-10 3:37 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-08-10 3:37 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, QEMU Developers, Jim Mussared,
Steffen Görtz, Su Hang, Liviu Ionescu, Alistair Francis,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
On 07/30/2018 02:51 PM, Peter Maydell wrote:
> On 27 July 2018 at 06:26, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Hi Stefan,
>>
>> On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
>>> Define a "cortex-m0" ARMv6-M CPU model.
>>>
>>> Most of the register reset values set by other CPU models are not
>>> relevant for the cut-down ARMv6-M architecture.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>> target/arm/cpu.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>> index 3848ef46aa..7e477c0d23 100644
>>> --- a/target/arm/cpu.c
>>> +++ b/target/arm/cpu.c
>>> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
>>> cpu->reset_auxcr = 1;
>>> }
>>>
>>> +static void cortex_m0_initfn(Object *obj)
>>> +{
>>> + ARMCPU *cpu = ARM_CPU(obj);
>>> + set_feature(&cpu->env, ARM_FEATURE_V6);
>>> + set_feature(&cpu->env, ARM_FEATURE_M);
>>
>> What about ARM_FEATURE_THUMB2 (T32)?
>
> No, the M0 doesn't have Thumb2. It has Thumb1 plus a tiny set
> of 32-bit instructions (which we deal with specially in translate.c).
Oh I thought it as Thumb2, ok.
>
> (As it happens we generally can't get to the checks on the THUMB2
> feature for a v6M core, so I think but have not checked thoroughly
> that it would make no difference to QEMU's behaviour whether the
> feature bit was set or not.)
>
>> Peter: Since the M0 (optionally?) supports 32x32 multiply, should this
>> cpu use the ARM_FEATURE_THUMB_DSP feature? Else this might trigger an
>> 'Undefined Instruction' in disas_thumb2_insn().
>
> ARM_FEATURE_THUMB_DSP only enables checks in disas_thumb2_insn()
> (and 32x32->32 multiply is not one of the insns it gates).
> The only insns in that function that a v6M core can execute are
> msr, mrs, dsb, dmb, isb and bl, none of which are affected by that
> feature switch.
Yes.
>
>> And what about optional ARM_FEATURE_PMSA?
>> Oh this would be cortex_m0plus_initfn() for "cortex-m0-plus", ok.
>
> Yep, plain M0 has no MPU (and so we're postponing the work
> of implementing the v6M PMSA, which IIRC isn't quite the
> same as the v7M one).
Thanks for your detailed explanations :)
Regards,
Phil.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model Stefan Hajnoczi
2018-07-27 5:26 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-07-30 17:52 ` Peter Maydell
2018-08-02 6:47 ` Stefan Hajnoczi
1 sibling, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2018-07-30 17:52 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Steffen Görtz, Alistair Francis,
Liviu Ionescu, qemu-arm, Julia Suvorova, Subbaraya Sundeep,
Su Hang, Steffen Gortz, Jim Mussared, Joel Stanley
On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Define a "cortex-m0" ARMv6-M CPU model.
>
> Most of the register reset values set by other CPU models are not
> relevant for the cut-down ARMv6-M architecture.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> target/arm/cpu.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 3848ef46aa..7e477c0d23 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
> cpu->reset_auxcr = 1;
> }
>
> +static void cortex_m0_initfn(Object *obj)
> +{
> + ARMCPU *cpu = ARM_CPU(obj);
> + set_feature(&cpu->env, ARM_FEATURE_V6);
> + set_feature(&cpu->env, ARM_FEATURE_M);
> +
> + cpu->midr = 0x410cc200;
> +}
We have all the patches for turning off not-v6M bits of
behaviour either in master or in target-arm.for-3.1 already,
right?
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model
2018-07-30 17:52 ` [Qemu-devel] " Peter Maydell
@ 2018-08-02 6:47 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-08-02 6:47 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Jim Mussared, Steffen Görtz, Su Hang,
Liviu Ionescu, Alistair Francis, QEMU Developers,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]
On Mon, Jul 30, 2018 at 06:52:29PM +0100, Peter Maydell wrote:
> On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > Define a "cortex-m0" ARMv6-M CPU model.
> >
> > Most of the register reset values set by other CPU models are not
> > relevant for the cut-down ARMv6-M architecture.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > target/arm/cpu.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 3848ef46aa..7e477c0d23 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
> > cpu->reset_auxcr = 1;
> > }
> >
> > +static void cortex_m0_initfn(Object *obj)
> > +{
> > + ARMCPU *cpu = ARM_CPU(obj);
> > + set_feature(&cpu->env, ARM_FEATURE_V6);
> > + set_feature(&cpu->env, ARM_FEATURE_M);
> > +
> > + cpu->midr = 0x410cc200;
> > +}
>
> We have all the patches for turning off not-v6M bits of
> behaviour either in master or in target-arm.for-3.1 already,
> right?
Yes. I've sent this series because Julia's work is now in
target-arm.for-3.1.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 5/7] loader: add rom transaction API
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
` (3 preceding siblings ...)
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model Stefan Hajnoczi
@ 2018-07-25 8:59 ` Stefan Hajnoczi
2018-07-30 17:57 ` Peter Maydell
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader Stefan Hajnoczi
` (2 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 8:59 UTC (permalink / raw)
To: qemu-devel
Cc: mail, Alistair Francis, Peter Maydell, ilg, qemu-arm,
Julia Suvorova, Subbaraya Sundeep, Su Hang, Steffen Gortz, jim,
Joel Stanley, Stefan Hajnoczi
Image file loaders may add a series of roms. If an error occurs partway
through loading there is no easy way to drop previously added roms.
This patch adds a transaction mechanism that works like this:
rom_transaction_begin();
...call rom_add_*()...
rom_transaction_end(ok);
If ok is false then roms added in this transaction are dropped.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/loader.h | 19 +++++++++++++++++++
hw/core/loader.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index e98b84b8f9..5235f119a3 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -225,6 +225,25 @@ int rom_check_and_register_reset(void);
void rom_set_fw(FWCfgState *f);
void rom_set_order_override(int order);
void rom_reset_order_override(void);
+
+/**
+ * rom_transaction_begin:
+ *
+ * Call this before of a series of rom_add_*() calls. Call
+ * rom_transaction_end() afterwards to commit or abort. These functions are
+ * useful for undoing a series of rom_add_*() calls if image file loading fails
+ * partway through.
+ */
+void rom_transaction_begin(void);
+
+/**
+ * rom_transaction_end:
+ * @commit: true to commit added roms, false to drop added roms
+ *
+ * Call this after a series of rom_add_*() calls. See rom_transaction_begin().
+ */
+void rom_transaction_end(bool commit);
+
int rom_copy(uint8_t *dest, hwaddr addr, size_t size);
void *rom_ptr(hwaddr addr, size_t size);
void hmp_info_roms(Monitor *mon, const QDict *qdict);
diff --git a/hw/core/loader.c b/hw/core/loader.c
index bbb6e65bb5..182abcce28 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -840,6 +840,8 @@ struct Rom {
char *fw_dir;
char *fw_file;
+ bool committed;
+
hwaddr addr;
QTAILQ_ENTRY(Rom) next;
};
@@ -866,6 +868,8 @@ static void rom_insert(Rom *rom)
rom->as = &address_space_memory;
}
+ rom->committed = false;
+
/* List is ordered by load address in the same address space */
QTAILQ_FOREACH(item, &roms, next) {
if (rom_order_compare(rom, item)) {
@@ -1165,6 +1169,39 @@ void rom_reset_order_override(void)
fw_cfg_reset_order_override(fw_cfg);
}
+void rom_transaction_begin(void)
+{
+ Rom *rom;
+
+ /* Ignore ROMs added without the transaction API */
+ QTAILQ_FOREACH(rom, &roms, next) {
+ rom->committed = true;
+ }
+}
+
+void rom_transaction_end(bool commit)
+{
+ Rom *rom;
+ Rom *tmp;
+
+ QTAILQ_FOREACH_SAFE(rom, &roms, next, tmp) {
+ if (rom->committed) {
+ continue;
+ }
+ if (commit) {
+ rom->committed = true;
+ } else {
+ QTAILQ_REMOVE(&roms, rom, next);
+ g_free(rom->data);
+ g_free(rom->path);
+ g_free(rom->name);
+ g_free(rom->fw_dir);
+ g_free(rom->fw_file);
+ g_free(rom);
+ }
+ }
+}
+
static Rom *find_rom(hwaddr addr, size_t size)
{
Rom *rom;
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 5/7] loader: add rom transaction API
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 5/7] loader: add rom transaction API Stefan Hajnoczi
@ 2018-07-30 17:57 ` Peter Maydell
0 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2018-07-30 17:57 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Steffen Görtz, Alistair Francis,
Liviu Ionescu, qemu-arm, Julia Suvorova, Subbaraya Sundeep,
Su Hang, Steffen Gortz, Jim Mussared, Joel Stanley
On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Image file loaders may add a series of roms. If an error occurs partway
> through loading there is no easy way to drop previously added roms.
>
> This patch adds a transaction mechanism that works like this:
>
> rom_transaction_begin();
> ...call rom_add_*()...
> rom_transaction_end(ok);
>
> If ok is false then roms added in this transaction are dropped.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> +void rom_transaction_end(bool commit)
> +{
> + Rom *rom;
> + Rom *tmp;
> +
> + QTAILQ_FOREACH_SAFE(rom, &roms, next, tmp) {
> + if (rom->committed) {
> + continue;
> + }
> + if (commit) {
> + rom->committed = true;
> + } else {
> + QTAILQ_REMOVE(&roms, rom, next);
> + g_free(rom->data);
> + g_free(rom->path);
> + g_free(rom->name);
> + g_free(rom->fw_dir);
> + g_free(rom->fw_file);
> + g_free(rom);
Is it worth having a rom_free() function so we can
share the "free all the pointers" code between this
and the error-exit codepath at the end of rom_add_file() ?
Either way
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
` (4 preceding siblings ...)
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 5/7] loader: add rom transaction API Stefan Hajnoczi
@ 2018-07-25 8:59 ` Stefan Hajnoczi
2018-07-30 18:01 ` Peter Maydell
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 7/7] Add QTest testcase for the Intel Hexadecimal Stefan Hajnoczi
2018-07-25 9:04 ` [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
7 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 8:59 UTC (permalink / raw)
To: qemu-devel
Cc: mail, Alistair Francis, Peter Maydell, ilg, qemu-arm,
Julia Suvorova, Subbaraya Sundeep, Su Hang, Steffen Gortz, jim,
Joel Stanley, Stefan Hajnoczi
From: Su Hang <suhang16@mails.ucas.ac.cn>
This patch adds Intel Hexadecimal Object File format support to the
loader. The file format specification is available here:
http://www.piclist.com/techref/fileext/hex/intel.htm
This file format is often used with microcontrollers such as the
micro:bit, Arduino, STM32, etc. Users expect to be able to run them
directly with qemu -kernel program.hex instead of converting to ELF or
binary.
Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/loader.h | 12 ++
hw/arm/arm-m-profile.c | 3 +
hw/core/loader.c | 243 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 258 insertions(+)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 5235f119a3..3c112975f4 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -28,6 +28,18 @@ ssize_t load_image_size(const char *filename, void *addr, size_t size);
int load_image_targphys_as(const char *filename,
hwaddr addr, uint64_t max_sz, AddressSpace *as);
+/**load_targphys_hex_as:
+ * @filename: Path to the .hex file
+ * @entry: Store the entry point given by the .hex file
+ * @as: The AddressSpace to load the .hex file to. The value of
+ * address_space_memory is used if nothing is supplied here.
+ *
+ * Load a fixed .hex file into memory.
+ *
+ * Returns the size of the loaded .hex file on success, -1 otherwise.
+ */
+int load_targphys_hex_as(const char *filename, hwaddr *entry, AddressSpace *as);
+
/** load_image_targphys:
* Same as load_image_targphys_as(), but doesn't allow the caller to specify
* an AddressSpace.
diff --git a/hw/arm/arm-m-profile.c b/hw/arm/arm-m-profile.c
index 8bafd6602d..441bc1a9c2 100644
--- a/hw/arm/arm-m-profile.c
+++ b/hw/arm/arm-m-profile.c
@@ -302,6 +302,9 @@ void arm_m_profile_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem
if (kernel_filename) {
image_size = load_elf_as(kernel_filename, NULL, NULL, &entry, &lowaddr,
NULL, big_endian, EM_ARM, 1, 0, as);
+ if (image_size < 0) {
+ image_size = load_targphys_hex_as(kernel_filename, &entry, as);
+ }
if (image_size < 0) {
image_size = load_image_targphys_as(kernel_filename, 0,
mem_size, as);
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 182abcce28..9ce5f45d5c 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1323,3 +1323,246 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict)
}
}
}
+
+typedef enum HexRecord HexRecord;
+enum HexRecord {
+ DATA_RECORD = 0,
+ EOF_RECORD,
+ EXT_SEG_ADDR_RECORD,
+ START_SEG_ADDR_RECORD,
+ EXT_LINEAR_ADDR_RECORD,
+ START_LINEAR_ADDR_RECORD,
+};
+
+#define DATA_FIELD_MAX_LEN 0xff
+#define LEN_EXCEPT_DATA 0x5
+/* 0x5 = sizeof(byte_count) + sizeof(address) + sizeof(record_type) +
+ * sizeof(checksum) */
+typedef struct {
+ uint8_t byte_count;
+ uint16_t address;
+ uint8_t record_type;
+ uint8_t data[DATA_FIELD_MAX_LEN];
+ uint8_t checksum;
+} HexLine;
+
+/* return 0 or -1 if error */
+static bool parse_record(HexLine *line, uint8_t *our_checksum, const uint8_t c,
+ uint32_t *index, const bool in_process)
+{
+ /* +-------+---------------+-------+---------------------+--------+
+ * | byte | |record | | |
+ * | count | address | type | data |checksum|
+ * +-------+---------------+-------+---------------------+--------+
+ * ^ ^ ^ ^ ^ ^
+ * |1 byte | 2 bytes |1 byte | 0-255 bytes | 1 byte |
+ */
+ uint8_t value = 0;
+ uint32_t idx = *index;
+ /* ignore space */
+ if (g_ascii_isspace(c)) {
+ return true;
+ }
+ if (!g_ascii_isxdigit(c) || !in_process) {
+ return false;
+ }
+ value = g_ascii_xdigit_value(c);
+ value = idx & 0x1 ? value & 0xf : value << 4;
+ if (idx < 2) {
+ line->byte_count |= value;
+ } else if (2 <= idx && idx < 6) {
+ line->address <<= 4;
+ line->address += g_ascii_xdigit_value(c);
+ } else if (6 <= idx && idx < 8) {
+ line->record_type |= value;
+ } else if (8 <= idx && idx < 8 + 2 * line->byte_count) {
+ line->data[(idx - 8) >> 1] |= value;
+ } else if (8 + 2 * line->byte_count <= idx &&
+ idx < 10 + 2 * line->byte_count) {
+ line->checksum |= value;
+ } else {
+ return false;
+ }
+ *our_checksum += value;
+ ++(*index);
+ return true;
+}
+
+typedef struct {
+ const char *filename;
+ HexLine line;
+ uint8_t *bin_buf;
+ hwaddr *start_addr;
+ int total_size;
+ uint32_t next_address_to_write;
+ uint32_t current_address;
+ uint32_t current_rom_index;
+ uint32_t rom_start_address;
+ AddressSpace *as;
+} HexParser;
+
+/* return size or -1 if error */
+static int handle_record_type(HexParser *parser)
+{
+ HexLine *line = &(parser->line);
+ switch (line->record_type) {
+ case DATA_RECORD:
+ parser->current_address =
+ (parser->next_address_to_write & 0xffff0000) | line->address;
+ /* verify this is a contiguous block of memory */
+ if (parser->current_address != parser->next_address_to_write) {
+ if (parser->current_rom_index != 0) {
+ rom_add_blob_fixed_as(parser->filename, parser->bin_buf,
+ parser->current_rom_index,
+ parser->rom_start_address, parser->as);
+ }
+ parser->rom_start_address = parser->current_address;
+ parser->current_rom_index = 0;
+ }
+
+ /* copy from line buffer to output bin_buf */
+ memcpy(parser->bin_buf + parser->current_rom_index, line->data,
+ line->byte_count);
+ parser->current_rom_index += line->byte_count;
+ parser->total_size += line->byte_count;
+ /* save next address to write */
+ parser->next_address_to_write =
+ parser->current_address + line->byte_count;
+ break;
+
+ case EOF_RECORD:
+ if (parser->current_rom_index != 0) {
+ rom_add_blob_fixed_as(parser->filename, parser->bin_buf,
+ parser->current_rom_index,
+ parser->rom_start_address, parser->as);
+ }
+ return parser->total_size;
+ case EXT_SEG_ADDR_RECORD:
+ case EXT_LINEAR_ADDR_RECORD:
+ if (line->byte_count != 2 && line->address != 0) {
+ return -1;
+ }
+
+ if (parser->current_rom_index != 0) {
+ rom_add_blob_fixed_as(parser->filename, parser->bin_buf,
+ parser->current_rom_index,
+ parser->rom_start_address, parser->as);
+ }
+
+ /* save next address to write,
+ * in case of non-contiguous block of memory */
+ parser->next_address_to_write =
+ line->record_type == EXT_SEG_ADDR_RECORD
+ ? ((line->data[0] << 12) | (line->data[1] << 4))
+ : ((line->data[0] << 24) | (line->data[1] << 16));
+ parser->rom_start_address = parser->next_address_to_write;
+ parser->current_rom_index = 0;
+ break;
+
+ case START_SEG_ADDR_RECORD:
+ if (line->byte_count != 4 && line->address != 0) {
+ return -1;
+ }
+
+ /* x86 16-bit CS:IP segmented addressing */
+ *(parser->start_addr) = (((line->data[0] << 8) | line->data[1]) << 4) |
+ (line->data[2] << 8) | line->data[3];
+ break;
+
+ case START_LINEAR_ADDR_RECORD:
+ if (line->byte_count != 4 && line->address != 0) {
+ return -1;
+ }
+
+ *(parser->start_addr) = (line->data[0] << 24) | (line->data[1] << 16) |
+ (line->data[2] << 8) | (line->data[3]);
+ break;
+
+ default:
+ return -1;
+ }
+
+ return parser->total_size;
+}
+
+/* return size or -1 if error */
+static int parse_hex_blob(const char *filename, hwaddr *addr, uint8_t *hex_blob,
+ size_t hex_blob_size, AddressSpace *as)
+{
+ bool in_process = false; /* avoid re-enter and
+ * check whether record begin with ':' */
+ uint8_t *end = hex_blob + hex_blob_size;
+ uint8_t our_checksum = 0;
+ uint32_t record_index = 0;
+ HexParser parser = {
+ .filename = filename,
+ .bin_buf = g_malloc(hex_blob_size),
+ .start_addr = addr,
+ .as = as,
+ };
+
+ rom_transaction_begin();
+
+ for (; hex_blob < end; ++hex_blob) {
+ switch (*hex_blob) {
+ case '\r':
+ case '\n':
+ if (!in_process) {
+ break;
+ }
+
+ in_process = false;
+ if ((LEN_EXCEPT_DATA + parser.line.byte_count) * 2 !=
+ record_index ||
+ our_checksum != 0) {
+ parser.total_size = -1;
+ goto out;
+ }
+
+ if (handle_record_type(&parser) == -1) {
+ parser.total_size = -1;
+ goto out;
+ }
+ break;
+
+ /* start of a new record. */
+ case ':':
+ memset(&parser.line, 0, sizeof(HexLine));
+ in_process = true;
+ record_index = 0;
+ break;
+
+ /* decoding lines */
+ default:
+ if (!parse_record(&parser.line, &our_checksum, *hex_blob,
+ &record_index, in_process)) {
+ parser.total_size = -1;
+ goto out;
+ }
+ break;
+ }
+ }
+
+out:
+ g_free(parser.bin_buf);
+ rom_transaction_end(parser.total_size != -1);
+ return parser.total_size;
+}
+
+/* return size or -1 if error */
+int load_targphys_hex_as(const char *filename, hwaddr *entry, AddressSpace *as)
+{
+ gsize hex_blob_size;
+ gchar *hex_blob;
+ int total_size = 0;
+
+ if (!g_file_get_contents(filename, &hex_blob, &hex_blob_size, NULL)) {
+ return -1;
+ }
+
+ total_size = parse_hex_blob(filename, entry, (uint8_t *)hex_blob,
+ hex_blob_size, as);
+
+ g_free(hex_blob);
+ return total_size;
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader Stefan Hajnoczi
@ 2018-07-30 18:01 ` Peter Maydell
2018-08-02 6:51 ` Stefan Hajnoczi
2018-08-02 12:43 ` Stefan Hajnoczi
0 siblings, 2 replies; 30+ messages in thread
From: Peter Maydell @ 2018-07-30 18:01 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: QEMU Developers, Steffen Görtz, Alistair Francis,
Liviu Ionescu, qemu-arm, Julia Suvorova, Subbaraya Sundeep,
Su Hang, Steffen Gortz, Jim Mussared, Joel Stanley
On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> From: Su Hang <suhang16@mails.ucas.ac.cn>
>
> This patch adds Intel Hexadecimal Object File format support to the
> loader. The file format specification is available here:
> http://www.piclist.com/techref/fileext/hex/intel.htm
>
> This file format is often used with microcontrollers such as the
> micro:bit, Arduino, STM32, etc. Users expect to be able to run them
> directly with qemu -kernel program.hex instead of converting to ELF or
> binary.
I'm still not convinced we want to add another random
special case only-works-on-one-architecture-and-some-boards
feature to the -kernel command line option.
Adding it to the "generic loader" device might be more plausible?
Or just get the user to create ELF files or plain binary files,
both of which we already handle.
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader
2018-07-30 18:01 ` Peter Maydell
@ 2018-08-02 6:51 ` Stefan Hajnoczi
2018-08-02 12:43 ` Stefan Hajnoczi
1 sibling, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-08-02 6:51 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Jim Mussared, Steffen Görtz, Su Hang,
Liviu Ionescu, Alistair Francis, QEMU Developers,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]
On Mon, Jul 30, 2018 at 07:01:53PM +0100, Peter Maydell wrote:
> On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > From: Su Hang <suhang16@mails.ucas.ac.cn>
> >
> > This patch adds Intel Hexadecimal Object File format support to the
> > loader. The file format specification is available here:
> > http://www.piclist.com/techref/fileext/hex/intel.htm
> >
> > This file format is often used with microcontrollers such as the
> > micro:bit, Arduino, STM32, etc. Users expect to be able to run them
> > directly with qemu -kernel program.hex instead of converting to ELF or
> > binary.
>
> I'm still not convinced we want to add another random
> special case only-works-on-one-architecture-and-some-boards
> feature to the -kernel command line option.
>
> Adding it to the "generic loader" device might be more plausible?
I'll look into this and let you know what I find.
> Or just get the user to create ELF files or plain binary files,
> both of which we already handle.
Plain binary definitely won't work because the .hex files contain sparse
(discontiguous) data.
But even using ELF would be very inconvenient to users since .hex is
*the* format that micro:bit IDEs and toolchains use. Our goal is for
less technical users (even kids learning to program) to use QEMU has a
micro:bit simulator, so the ability to directly use .hex files is very
important.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader
2018-07-30 18:01 ` Peter Maydell
2018-08-02 6:51 ` Stefan Hajnoczi
@ 2018-08-02 12:43 ` Stefan Hajnoczi
2018-08-02 22:04 ` Peter Maydell
1 sibling, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-08-02 12:43 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Jim Mussared, Steffen Görtz, Su Hang,
Liviu Ionescu, Alistair Francis, QEMU Developers,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]
On Mon, Jul 30, 2018 at 07:01:53PM +0100, Peter Maydell wrote:
> On 25 July 2018 at 09:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > From: Su Hang <suhang16@mails.ucas.ac.cn>
> >
> > This patch adds Intel Hexadecimal Object File format support to the
> > loader. The file format specification is available here:
> > http://www.piclist.com/techref/fileext/hex/intel.htm
> >
> > This file format is often used with microcontrollers such as the
> > micro:bit, Arduino, STM32, etc. Users expect to be able to run them
> > directly with qemu -kernel program.hex instead of converting to ELF or
> > binary.
>
> I'm still not convinced we want to add another random
> special case only-works-on-one-architecture-and-some-boards
> feature to the -kernel command line option.
>
> Adding it to the "generic loader" device might be more plausible?
I'm not sure I understand the purpose of the generic loader.
As a user -kernel <myfile> is easier than -device
loader,file=<myfile>,cpu-num=1.
Can you explain the advantage to moving hex file loading to the generic
loader?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader
2018-08-02 12:43 ` Stefan Hajnoczi
@ 2018-08-02 22:04 ` Peter Maydell
2018-08-03 10:32 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2018-08-02 22:04 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Stefan Hajnoczi, Jim Mussared, Steffen Görtz, Su Hang,
Liviu Ionescu, Alistair Francis, QEMU Developers,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
On 2 August 2018 at 13:43, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Jul 30, 2018 at 07:01:53PM +0100, Peter Maydell wrote:
>> I'm still not convinced we want to add another random
>> special case only-works-on-one-architecture-and-some-boards
>> feature to the -kernel command line option.
>>
>> Adding it to the "generic loader" device might be more plausible?
>
> I'm not sure I understand the purpose of the generic loader.
>
> As a user -kernel <myfile> is easier than -device
> loader,file=<myfile>,cpu-num=1.
>
> Can you explain the advantage to moving hex file loading to the generic
> loader?
It means we have a command line option for loading hex files
that works for every board and every CPU architecture.
(Similarly, if you want a way to load an ELF file that
works the same way for all boards and CPUs, the generic
loader is it -- -kernel will not reliably do the job.
You can also use it to load more than one ELF file or
to load different ELF files for different CPUs, neither
of which you can do with -kernel.)
-kernel, like all our legacy short options, is, yes,
easier to use; it's also a twisted mess of different
"do what I mean" functionality that varies depending
on the guest CPU architecture and subtype, the machine
being emulated, and other random things like "did the
user also tell us to start a BIOS image". It mostly means
"run a Linux kernel", with some extras wedged in on the
side where we thought we could do it without breaking the
kernel case. Oh, and we don't document anywhere what
it actually does. I'm reluctant to add yet another layer of
"do what I mean" to it that only has an effect on a subset
of Arm boards.
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader
2018-08-02 22:04 ` Peter Maydell
@ 2018-08-03 10:32 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-08-03 10:32 UTC (permalink / raw)
To: Peter Maydell
Cc: Stefan Hajnoczi, Jim Mussared, Steffen Görtz, Su Hang,
Liviu Ionescu, Alistair Francis, QEMU Developers,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]
On Thu, Aug 02, 2018 at 11:04:46PM +0100, Peter Maydell wrote:
> On 2 August 2018 at 13:43, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > On Mon, Jul 30, 2018 at 07:01:53PM +0100, Peter Maydell wrote:
> >> I'm still not convinced we want to add another random
> >> special case only-works-on-one-architecture-and-some-boards
> >> feature to the -kernel command line option.
> >>
> >> Adding it to the "generic loader" device might be more plausible?
> >
> > I'm not sure I understand the purpose of the generic loader.
> >
> > As a user -kernel <myfile> is easier than -device
> > loader,file=<myfile>,cpu-num=1.
> >
> > Can you explain the advantage to moving hex file loading to the generic
> > loader?
>
> It means we have a command line option for loading hex files
> that works for every board and every CPU architecture.
> (Similarly, if you want a way to load an ELF file that
> works the same way for all boards and CPUs, the generic
> loader is it -- -kernel will not reliably do the job.
> You can also use it to load more than one ELF file or
> to load different ELF files for different CPUs, neither
> of which you can do with -kernel.)
>
> -kernel, like all our legacy short options, is, yes,
> easier to use; it's also a twisted mess of different
> "do what I mean" functionality that varies depending
> on the guest CPU architecture and subtype, the machine
> being emulated, and other random things like "did the
> user also tell us to start a BIOS image". It mostly means
> "run a Linux kernel", with some extras wedged in on the
> side where we thought we could do it without breaking the
> kernel case. Oh, and we don't document anywhere what
> it actually does. I'm reluctant to add yet another layer of
> "do what I mean" to it that only has an effect on a subset
> of Arm boards.
Okay, I understand better now. Thanks!
I'll resend with the generic loader instead of -kernel.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 7/7] Add QTest testcase for the Intel Hexadecimal
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
` (5 preceding siblings ...)
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader Stefan Hajnoczi
@ 2018-07-25 8:59 ` Stefan Hajnoczi
2018-07-27 4:52 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-07-25 9:04 ` [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
7 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 8:59 UTC (permalink / raw)
To: qemu-devel
Cc: mail, Alistair Francis, Peter Maydell, ilg, qemu-arm,
Julia Suvorova, Subbaraya Sundeep, Su Hang, Steffen Gortz, jim,
Joel Stanley, Stefan Hajnoczi
From: Su Hang <suhang16@mails.ucas.ac.cn>
'test.hex' file is a bare metal ARM software stored in Hexadecimal
Object Format. When it's loaded by QEMU, it will print "Hello world!\n"
on console.
`pre_store` array in 'hexloader-test.c' file, stores the binary format
of 'test.hex' file, which is used to verify correctness.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Suggested-by: Steffen Gortz <qemu.ml@steffen-goertz.de>
Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
MAINTAINERS | 6 +++
configure | 4 ++
tests/Makefile.include | 2 +
tests/hexloader-test.c | 56 ++++++++++++++++++++++++++++
tests/hex-loader-check-data/test.hex | 12 ++++++
5 files changed, 80 insertions(+)
create mode 100644 tests/hexloader-test.c
create mode 100644 tests/hex-loader-check-data/test.hex
diff --git a/MAINTAINERS b/MAINTAINERS
index 666e936812..c48d9271cf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1323,6 +1323,12 @@ F: hw/core/generic-loader.c
F: include/hw/core/generic-loader.h
F: docs/generic-loader.txt
+Intel Hexadecimal Object File Loader
+M: Su Hang <suhang16@mails.ucas.ac.cn>
+S: Maintained
+F: tests/hexloader-test.c
+F: tests/hex-loader-check-data/test.hex
+
CHRP NVRAM
M: Thomas Huth <thuth@redhat.com>
S: Maintained
diff --git a/configure b/configure
index 2a7796ea80..db97930314 100755
--- a/configure
+++ b/configure
@@ -7382,6 +7382,10 @@ for test_file in $(find $source_path/tests/acpi-test-data -type f)
do
FILES="$FILES tests/acpi-test-data$(echo $test_file | sed -e 's/.*acpi-test-data//')"
done
+for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
+do
+ FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
+done
mkdir -p $DIRS
for f in $FILES ; do
if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
diff --git a/tests/Makefile.include b/tests/Makefile.include
index a49282704e..760a0f18b6 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -386,6 +386,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
gcov-files-arm-y += hw/timer/arm_mptimer.c
check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
check-qtest-arm-y += tests/sdhci-test$(EXESUF)
+check-qtest-arm-y += tests/hexloader-test$(EXESUF)
check-qtest-aarch64-y = tests/numa-test$(EXESUF)
check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
@@ -773,6 +774,7 @@ tests/qmp-test$(EXESUF): tests/qmp-test.o
tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
tests/rtc-test$(EXESUF): tests/rtc-test.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o
+tests/hexloader-test$(EXESUF): tests/hexloader-test.o
tests/endianness-test$(EXESUF): tests/endianness-test.o
tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
new file mode 100644
index 0000000000..78b566f8b1
--- /dev/null
+++ b/tests/hexloader-test.c
@@ -0,0 +1,56 @@
+/*
+ * QTest testcase for the Intel Hexadecimal Object File Loader
+ *
+ * Authors:
+ * Su Hang <suhang16@mails.ucas.ac.cn> 2018
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+#define BIN_SIZE 146
+
+static unsigned char pre_store[BIN_SIZE] = {
+ 4, 208, 159, 229, 22, 0, 0, 235, 254, 255, 255, 234, 152, 16, 1,
+ 0, 4, 176, 45, 229, 0, 176, 141, 226, 12, 208, 77, 226, 8, 0,
+ 11, 229, 6, 0, 0, 234, 8, 48, 27, 229, 0, 32, 211, 229, 44,
+ 48, 159, 229, 0, 32, 131, 229, 8, 48, 27, 229, 1, 48, 131, 226,
+ 8, 48, 11, 229, 8, 48, 27, 229, 0, 48, 211, 229, 0, 0, 83,
+ 227, 244, 255, 255, 26, 0, 0, 160, 225, 0, 208, 139, 226, 4, 176,
+ 157, 228, 30, 255, 47, 225, 0, 16, 31, 16, 0, 72, 45, 233, 4,
+ 176, 141, 226, 8, 0, 159, 229, 230, 255, 255, 235, 0, 0, 160, 225,
+ 0, 136, 189, 232, 132, 0, 1, 0, 0, 16, 31, 16, 72, 101, 108,
+ 108, 111, 32, 119, 111, 114, 108, 100, 33, 10, 0};
+
+/* success if no crash or abort */
+static void hex_loader_test(void)
+{
+ unsigned int i;
+ unsigned char memory_content[BIN_SIZE];
+ const unsigned int base_addr = 0x00010000;
+
+ QTestState *s = qtest_startf(
+ "-M emcraft-sf2 -nographic -kernel ./tests/hex-loader-check-data/test.hex");
+
+ for (i = 0; i < BIN_SIZE; ++i) {
+ memory_content[i] = qtest_readb(s, base_addr + i);
+ g_assert_cmpuint(memory_content[i], ==, pre_store[i]);
+ }
+ qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/tmp/hex_loader", hex_loader_test);
+ ret = g_test_run();
+
+ return ret;
+}
diff --git a/tests/hex-loader-check-data/test.hex b/tests/hex-loader-check-data/test.hex
new file mode 100644
index 0000000000..7e99b452f5
--- /dev/null
+++ b/tests/hex-loader-check-data/test.hex
@@ -0,0 +1,12 @@
+:020000040001F9
+:1000000004D09FE5160000EBFEFFFFEA9810010008
+:1000100004B02DE500B08DE20CD04DE208000BE5F8
+:10002000060000EA08301BE50020D3E52C309FE5F0
+:10003000002083E508301BE5013083E208300BE542
+:1000400008301BE50030D3E5000053E3F4FFFF1A4E
+:100050000000A0E100D08BE204B09DE41EFF2FE180
+:1000600000101F1000482DE904B08DE208009FE544
+:10007000E6FFFFEB0000A0E10088BDE8840001007E
+:1000800000101F1048656C6C6F20776F726C6421D4
+:020090000A0064
+:00000001FF
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 7/7] Add QTest testcase for the Intel Hexadecimal
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 7/7] Add QTest testcase for the Intel Hexadecimal Stefan Hajnoczi
@ 2018-07-27 4:52 ` Philippe Mathieu-Daudé
2018-07-27 23:58 ` Su Hang
0 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-07-27 4:52 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel, Su Hang
Cc: Peter Maydell, jim, mail, ilg, Alistair Francis,
Subbaraya Sundeep, Steffen Gortz, qemu-arm, Joel Stanley,
Julia Suvorova
Hi Su,
On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
> From: Su Hang <suhang16@mails.ucas.ac.cn>
>
> 'test.hex' file is a bare metal ARM software stored in Hexadecimal
> Object Format. When it's loaded by QEMU, it will print "Hello world!\n"
> on console.
>
> `pre_store` array in 'hexloader-test.c' file, stores the binary format
> of 'test.hex' file, which is used to verify correctness.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Suggested-by: Steffen Gortz <qemu.ml@steffen-goertz.de>
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> MAINTAINERS | 6 +++
> configure | 4 ++
> tests/Makefile.include | 2 +
> tests/hexloader-test.c | 56 ++++++++++++++++++++++++++++
> tests/hex-loader-check-data/test.hex | 12 ++++++
> 5 files changed, 80 insertions(+)
> create mode 100644 tests/hexloader-test.c
> create mode 100644 tests/hex-loader-check-data/test.hex
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 666e936812..c48d9271cf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1323,6 +1323,12 @@ F: hw/core/generic-loader.c
> F: include/hw/core/generic-loader.h
> F: docs/generic-loader.txt
>
> +Intel Hexadecimal Object File Loader
> +M: Su Hang <suhang16@mails.ucas.ac.cn>
> +S: Maintained
> +F: tests/hexloader-test.c
> +F: tests/hex-loader-check-data/test.hex
> +
> CHRP NVRAM
> M: Thomas Huth <thuth@redhat.com>
> S: Maintained
> diff --git a/configure b/configure
> index 2a7796ea80..db97930314 100755
> --- a/configure
> +++ b/configure
> @@ -7382,6 +7382,10 @@ for test_file in $(find $source_path/tests/acpi-test-data -type f)
> do
> FILES="$FILES tests/acpi-test-data$(echo $test_file | sed -e 's/.*acpi-test-data//')"
> done
> +for test_file in $(find $source_path/tests/hex-loader-check-data -type f)
> +do
> + FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed -e 's/.*hex-loader-check-data//')"
> +done
> mkdir -p $DIRS
> for f in $FILES ; do
> if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; then
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a49282704e..760a0f18b6 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -386,6 +386,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
> gcov-files-arm-y += hw/timer/arm_mptimer.c
> check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
> check-qtest-arm-y += tests/sdhci-test$(EXESUF)
> +check-qtest-arm-y += tests/hexloader-test$(EXESUF)
>
> check-qtest-aarch64-y = tests/numa-test$(EXESUF)
> check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
> @@ -773,6 +774,7 @@ tests/qmp-test$(EXESUF): tests/qmp-test.o
> tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
> tests/rtc-test$(EXESUF): tests/rtc-test.o
> tests/m48t59-test$(EXESUF): tests/m48t59-test.o
> +tests/hexloader-test$(EXESUF): tests/hexloader-test.o
> tests/endianness-test$(EXESUF): tests/endianness-test.o
> tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
> tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
> diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
> new file mode 100644
> index 0000000000..78b566f8b1
> --- /dev/null
> +++ b/tests/hexloader-test.c
> @@ -0,0 +1,56 @@
> +/*
> + * QTest testcase for the Intel Hexadecimal Object File Loader
> + *
> + * Authors:
> + * Su Hang <suhang16@mails.ucas.ac.cn> 2018
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +
> +#define BIN_SIZE 146
> +
> +static unsigned char pre_store[BIN_SIZE] = {
> + 4, 208, 159, 229, 22, 0, 0, 235, 254, 255, 255, 234, 152, 16, 1,
> + 0, 4, 176, 45, 229, 0, 176, 141, 226, 12, 208, 77, 226, 8, 0,
> + 11, 229, 6, 0, 0, 234, 8, 48, 27, 229, 0, 32, 211, 229, 44,
> + 48, 159, 229, 0, 32, 131, 229, 8, 48, 27, 229, 1, 48, 131, 226,
> + 8, 48, 11, 229, 8, 48, 27, 229, 0, 48, 211, 229, 0, 0, 83,
> + 227, 244, 255, 255, 26, 0, 0, 160, 225, 0, 208, 139, 226, 4, 176,
> + 157, 228, 30, 255, 47, 225, 0, 16, 31, 16, 0, 72, 45, 233, 4,
> + 176, 141, 226, 8, 0, 159, 229, 230, 255, 255, 235, 0, 0, 160, 225,
> + 0, 136, 189, 232, 132, 0, 1, 0, 0, 16, 31, 16, 72, 101, 108,
> + 108, 111, 32, 119, 111, 114, 108, 100, 33, 10, 0};
Can this be:
0x04, 0xd0, 0x9f, 0xe5, 0x16, 0x00, 0x00, 0xeb, 0xfe, 0xff, 0xff,
0xea,
0x98, 0x10, 0x01, 0x00, 0x04, 0xb0, 0x2d, 0xe5, 0x00, 0xb0, 0x8d,
0xe2,
0x0c, 0xd0, 0x4d, 0xe2, 0x08, 0x00, 0x0b, 0xe5, 0x06, 0x00, 0x00,
0xea,
0x08, 0x30, 0x1b, 0xe5, 0x00, 0x20, 0xd3, 0xe5, 0x2c, 0x30, 0x9f,
0xe5,
0x00, 0x20, 0x83, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x01, 0x30, 0x83,
0xe2,
0x08, 0x30, 0x0b, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x00, 0x30, 0xd3,
0xe5,
0x00, 0x00, 0x53, 0xe3, 0xf4, 0xff, 0xff, 0x1a, 0x00, 0x00, 0xa0,
0xe1,
0x00, 0xd0, 0x8b, 0xe2, 0x04, 0xb0, 0x9d, 0xe4, 0x1e, 0xff, 0x2f,
0xe1,
0x00, 0x10, 0x1f, 0x10, 0x00, 0x48, 0x2d, 0xe9, 0x04, 0xb0, 0x8d,
0xe2,
0x08, 0x00, 0x9f, 0xe5, 0xe6, 0xff, 0xff, 0xeb, 0x00, 0x00, 0xa0,
0xe1,
0x00, 0x88, 0xbd, 0xe8, 0x84, 0x00, 0x01, 0x00, 0x00, 0x10, 0x1f,
0x10,
'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!',
'\n', '\0'
};
?
> +
> +/* success if no crash or abort */
> +static void hex_loader_test(void)
> +{
> + unsigned int i;
> + unsigned char memory_content[BIN_SIZE];
> + const unsigned int base_addr = 0x00010000;
> +
> + QTestState *s = qtest_startf(
> + "-M emcraft-sf2 -nographic -kernel ./tests/hex-loader-check-data/test.hex");
> +
> + for (i = 0; i < BIN_SIZE; ++i) {
> + memory_content[i] = qtest_readb(s, base_addr + i);
> + g_assert_cmpuint(memory_content[i], ==, pre_store[i]);
> + }
> + qtest_quit(s);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int ret;
> +
> + g_test_init(&argc, &argv, NULL);
> +
> + qtest_add_func("/tmp/hex_loader", hex_loader_test);
> + ret = g_test_run();
> +
> + return ret;
> +}
> diff --git a/tests/hex-loader-check-data/test.hex b/tests/hex-loader-check-data/test.hex
> new file mode 100644
> index 0000000000..7e99b452f5
> --- /dev/null
> +++ b/tests/hex-loader-check-data/test.hex
> @@ -0,0 +1,12 @@
> +:020000040001F9
> +:1000000004D09FE5160000EBFEFFFFEA9810010008
> +:1000100004B02DE500B08DE20CD04DE208000BE5F8
> +:10002000060000EA08301BE50020D3E52C309FE5F0
> +:10003000002083E508301BE5013083E208300BE542
> +:1000400008301BE50030D3E5000053E3F4FFFF1A4E
> +:100050000000A0E100D08BE204B09DE41EFF2FE180
> +:1000600000101F1000482DE904B08DE208009FE544
> +:10007000E6FFFFEB0000A0E10088BDE8840001007E
> +:1000800000101F1048656C6C6F20776F726C6421D4
> +:020090000A0064
> +:00000001FF
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 7/7] Add QTest testcase for the Intel Hexadecimal
2018-07-27 4:52 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-07-27 23:58 ` Su Hang
2018-08-02 6:53 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Su Hang @ 2018-07-27 23:58 UTC (permalink / raw)
To: philippe mathieu-daudé, qemu-devel
Cc: stefanha, peter.maydell, alistair, sundeep.lkml, qemu.ml, joel,
jusual, qemu-arm, jim, mail, ilg
> > +#define BIN_SIZE 146
> > +
> > +static unsigned char pre_store[BIN_SIZE] = {
> > + 4, 208, 159, 229, 22, 0, 0, 235, 254, 255, 255, 234, 152, 16, 1,
> > + 0, 4, 176, 45, 229, 0, 176, 141, 226, 12, 208, 77, 226, 8, 0,
> > + 11, 229, 6, 0, 0, 234, 8, 48, 27, 229, 0, 32, 211, 229, 44,
> > + 48, 159, 229, 0, 32, 131, 229, 8, 48, 27, 229, 1, 48, 131, 226,
> > + 8, 48, 11, 229, 8, 48, 27, 229, 0, 48, 211, 229, 0, 0, 83,
> > + 227, 244, 255, 255, 26, 0, 0, 160, 225, 0, 208, 139, 226, 4, 176,
> > + 157, 228, 30, 255, 47, 225, 0, 16, 31, 16, 0, 72, 45, 233, 4,
> > + 176, 141, 226, 8, 0, 159, 229, 230, 255, 255, 235, 0, 0, 160, 225,
> > + 0, 136, 189, 232, 132, 0, 1, 0, 0, 16, 31, 16, 72, 101, 108,
> > + 108, 111, 32, 119, 111, 114, 108, 100, 33, 10, 0};
>
> Can this be:
>
> 0x04, 0xd0, 0x9f, 0xe5, 0x16, 0x00, 0x00, 0xeb, 0xfe, 0xff, 0xff,
> 0xea,
> 0x98, 0x10, 0x01, 0x00, 0x04, 0xb0, 0x2d, 0xe5, 0x00, 0xb0, 0x8d,
> 0xe2,
> 0x0c, 0xd0, 0x4d, 0xe2, 0x08, 0x00, 0x0b, 0xe5, 0x06, 0x00, 0x00,
> 0xea,
> 0x08, 0x30, 0x1b, 0xe5, 0x00, 0x20, 0xd3, 0xe5, 0x2c, 0x30, 0x9f,
> 0xe5,
> 0x00, 0x20, 0x83, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x01, 0x30, 0x83,
> 0xe2,
> 0x08, 0x30, 0x0b, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x00, 0x30, 0xd3,
> 0xe5,
> 0x00, 0x00, 0x53, 0xe3, 0xf4, 0xff, 0xff, 0x1a, 0x00, 0x00, 0xa0,
> 0xe1,
> 0x00, 0xd0, 0x8b, 0xe2, 0x04, 0xb0, 0x9d, 0xe4, 0x1e, 0xff, 0x2f,
> 0xe1,
> 0x00, 0x10, 0x1f, 0x10, 0x00, 0x48, 0x2d, 0xe9, 0x04, 0xb0, 0x8d,
> 0xe2,
> 0x08, 0x00, 0x9f, 0xe5, 0xe6, 0xff, 0xff, 0xeb, 0x00, 0x00, 0xa0,
> 0xe1,
> 0x00, 0x88, 0xbd, 0xe8, 0x84, 0x00, 0x01, 0x00, 0x00, 0x10, 0x1f,
> 0x10,
> 'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!',
> '\n', '\0'
> };
>
> ?
Yes, good suggestion. Your version is clearer and more readable for humans.
Best,
Su Hang
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v3 7/7] Add QTest testcase for the Intel Hexadecimal
2018-07-27 23:58 ` Su Hang
@ 2018-08-02 6:53 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-08-02 6:53 UTC (permalink / raw)
To: Su Hang
Cc: philippe mathieu-daudé,
qemu-devel, peter.maydell, jim, mail, ilg, alistair,
sundeep.lkml, qemu.ml, qemu-arm, joel, stefanha, jusual
[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]
On Sat, Jul 28, 2018 at 07:58:17AM +0800, Su Hang wrote:
> > > +#define BIN_SIZE 146
> > > +
> > > +static unsigned char pre_store[BIN_SIZE] = {
> > > + 4, 208, 159, 229, 22, 0, 0, 235, 254, 255, 255, 234, 152, 16, 1,
> > > + 0, 4, 176, 45, 229, 0, 176, 141, 226, 12, 208, 77, 226, 8, 0,
> > > + 11, 229, 6, 0, 0, 234, 8, 48, 27, 229, 0, 32, 211, 229, 44,
> > > + 48, 159, 229, 0, 32, 131, 229, 8, 48, 27, 229, 1, 48, 131, 226,
> > > + 8, 48, 11, 229, 8, 48, 27, 229, 0, 48, 211, 229, 0, 0, 83,
> > > + 227, 244, 255, 255, 26, 0, 0, 160, 225, 0, 208, 139, 226, 4, 176,
> > > + 157, 228, 30, 255, 47, 225, 0, 16, 31, 16, 0, 72, 45, 233, 4,
> > > + 176, 141, 226, 8, 0, 159, 229, 230, 255, 255, 235, 0, 0, 160, 225,
> > > + 0, 136, 189, 232, 132, 0, 1, 0, 0, 16, 31, 16, 72, 101, 108,
> > > + 108, 111, 32, 119, 111, 114, 108, 100, 33, 10, 0};
> >
> > Can this be:
> >
> > 0x04, 0xd0, 0x9f, 0xe5, 0x16, 0x00, 0x00, 0xeb, 0xfe, 0xff, 0xff,
> > 0xea,
> > 0x98, 0x10, 0x01, 0x00, 0x04, 0xb0, 0x2d, 0xe5, 0x00, 0xb0, 0x8d,
> > 0xe2,
> > 0x0c, 0xd0, 0x4d, 0xe2, 0x08, 0x00, 0x0b, 0xe5, 0x06, 0x00, 0x00,
> > 0xea,
> > 0x08, 0x30, 0x1b, 0xe5, 0x00, 0x20, 0xd3, 0xe5, 0x2c, 0x30, 0x9f,
> > 0xe5,
> > 0x00, 0x20, 0x83, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x01, 0x30, 0x83,
> > 0xe2,
> > 0x08, 0x30, 0x0b, 0xe5, 0x08, 0x30, 0x1b, 0xe5, 0x00, 0x30, 0xd3,
> > 0xe5,
> > 0x00, 0x00, 0x53, 0xe3, 0xf4, 0xff, 0xff, 0x1a, 0x00, 0x00, 0xa0,
> > 0xe1,
> > 0x00, 0xd0, 0x8b, 0xe2, 0x04, 0xb0, 0x9d, 0xe4, 0x1e, 0xff, 0x2f,
> > 0xe1,
> > 0x00, 0x10, 0x1f, 0x10, 0x00, 0x48, 0x2d, 0xe9, 0x04, 0xb0, 0x8d,
> > 0xe2,
> > 0x08, 0x00, 0x9f, 0xe5, 0xe6, 0xff, 0xff, 0xeb, 0x00, 0x00, 0xa0,
> > 0xe1,
> > 0x00, 0x88, 0xbd, 0xe8, 0x84, 0x00, 0x01, 0x00, 0x00, 0x10, 0x1f,
> > 0x10,
> > 'H', 'e', 'l', 'l', 'o', ' ', 'w', 'o', 'r', 'l', 'd', '!',
> > '\n', '\0'
> > };
> >
> > ?
>
> Yes, good suggestion. Your version is clearer and more readable for humans.
Okay, I will make the change in v4.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader
2018-07-25 8:59 [Qemu-devel] [PATCH v3 0/7] arm: add Cortex M0 CPU model and hex file loader Stefan Hajnoczi
` (6 preceding siblings ...)
2018-07-25 8:59 ` [Qemu-devel] [PATCH v3 7/7] Add QTest testcase for the Intel Hexadecimal Stefan Hajnoczi
@ 2018-07-25 9:04 ` Stefan Hajnoczi
7 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2018-07-25 9:04 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Peter Maydell, Jim Mussared, Steffen Görtz,
Su Hang, Liviu Ionescu, Alistair Francis, Subbaraya Sundeep,
Steffen Gortz, qemu-arm, Joel Stanley, Julia Suvorova
Based-on: https://git.linaro.org/people/pmaydell/qemu-arm.git target-arm.for-3.1
^ permalink raw reply [flat|nested] 30+ messages in thread