Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
Hi Thomas,
On 9/24/19 1:06 PM, Thomas Huth wrote:
> On 24/09/2019 13.02, Auger Eric wrote:
>> Hi Thomas,
>>
>> On 9/23/19 8:09 PM, Thomas Huth wrote:
>>> On 23/09/2019 16.31, Auger Eric wrote:
Hi Thomas,
On 9/21/19 5:04 PM, Thomas Huth wrote:
> We are going to make CONFIG_ARM_V7M optional, so the related cortex-m
> CPUs should only be created if the switch is enabled. This can best
> be done if the code resides in a separate file, thus move the related
> functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M
> is enabled.
>
> Signed-off-by: Thomas Huth
> ---
> target/arm/Makefile.objs | 1 +
> target/arm/cpu.c | 146 -
> target/arm/v7m.c | 193 +++
> 3 files changed, 194 insertions(+), 146 deletions(-)
> create mode 100644 target/arm/v7m.c
>>> [...]
> diff --git a/target/arm/v7m.c b/target/arm/v7m.c
> new file mode 100644
> index 00..505043febe
> --- /dev/null
> +++ b/target/arm/v7m.c
> @@ -0,0 +1,193 @@
> +/*
> + * ARM v7m helpers.
> + *
> + * This code is licensed under the GNU GPL v2 or later.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/qemu-print.h"
> +#include "qemu-common.h"
> +#include "target/arm/idau.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "cpu.h"
> +#include "internals.h"
> +#include "exec/exec-all.h"
> +#include "hw/qdev-properties.h"
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/loader.h"
> +#include "hw/boards.h"
> +#endif
> +#include "sysemu/sysemu.h"
> +#include "sysemu/tcg.h"
> +#include "sysemu/hw_accel.h"
> +#include "disas/capstone.h"
> +#include "fpu/softfloat.h"
I guess some of those headers are not needed.
>>>
>>> Yeah, I just copy-n-pasted from the source file ... I'll check what can
>>> be omitted (if this patch series has a chance at all...)
>>>
> +
> +#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
> +
> +static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
> interrupt_request)
> +{
> +CPUClass *cc = CPU_GET_CLASS(cs);
> +ARMCPU *cpu = ARM_CPU(cs);
> +CPUARMState *env = >env;
> +bool ret = false;
> +
> +/*
> + * ARMv7-M interrupt masking works differently than -A or -R.
> + * There is no FIQ/IRQ distinction. Instead of I and F bits
> + * masking FIQ and IRQ interrupts, an exception is taken only
> + * if it is higher priority than the current execution priority
> + * (which depends on state like BASEPRI, FAULTMASK and the
> + * currently active exception).
> + */
> +if (interrupt_request & CPU_INTERRUPT_HARD
> +&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
so what is the status wrt m_helper.c which stays unconditionally
compiled. m_helper functions seem to called from target/arm/translate.c
mostly. Have you abandoned the stub idea. It may be confusing to have 2
different helper files. At least a comment explaining where a new helper
shall go may be useful.
>>>
>>> All the HELPER() functions should definitely stay in m_helper.c. They
>>> are required for linking. Or do you prefer a stub file instead? Then we
>>> could maybe make the whole m_helper.c conditional in the Makefile.objs
>>> instead.
>>
>> I was simply referring to your previous approach:
>>
>> Applying [Qemu-devel] [RFC PATCH 3/3] target/arm: Make m_helper.c
>> optional via CONFIG_ARM_V7M seems to fix the issue
>> https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00333.html
>>
>> It seems to work fine as it removes the call to armv7m_nvic.c functions
>> so no need to move the interrupt controller code?
>
> Yes, but then we either need stubs in a separate file, or have to put
> lots of #ifdefs into translate.c ... none of those solutions seem to be
> really perfect :-(
Yep, the separate stub file looked cleaner to me.
> Anyway, Philippe is currently respinning his series (I think), so I'll
> postpone my work now to avoid to interfere with him.
OK
Thanks
Eric
>
> Thomas
>
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
On 24/09/2019 13.02, Auger Eric wrote:
> Hi Thomas,
>
> On 9/23/19 8:09 PM, Thomas Huth wrote:
>> On 23/09/2019 16.31, Auger Eric wrote:
>>> Hi Thomas,
>>>
>>> On 9/21/19 5:04 PM, Thomas Huth wrote:
We are going to make CONFIG_ARM_V7M optional, so the related cortex-m
CPUs should only be created if the switch is enabled. This can best
be done if the code resides in a separate file, thus move the related
functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M
is enabled.
Signed-off-by: Thomas Huth
---
target/arm/Makefile.objs | 1 +
target/arm/cpu.c | 146 -
target/arm/v7m.c | 193 +++
3 files changed, 194 insertions(+), 146 deletions(-)
create mode 100644 target/arm/v7m.c
>> [...]
diff --git a/target/arm/v7m.c b/target/arm/v7m.c
new file mode 100644
index 00..505043febe
--- /dev/null
+++ b/target/arm/v7m.c
@@ -0,0 +1,193 @@
+/*
+ * ARM v7m helpers.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/qemu-print.h"
+#include "qemu-common.h"
+#include "target/arm/idau.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "cpu.h"
+#include "internals.h"
+#include "exec/exec-all.h"
+#include "hw/qdev-properties.h"
+#if !defined(CONFIG_USER_ONLY)
+#include "hw/loader.h"
+#include "hw/boards.h"
+#endif
+#include "sysemu/sysemu.h"
+#include "sysemu/tcg.h"
+#include "sysemu/hw_accel.h"
+#include "disas/capstone.h"
+#include "fpu/softfloat.h"
>>>
>>> I guess some of those headers are not needed.
>>
>> Yeah, I just copy-n-pasted from the source file ... I'll check what can
>> be omitted (if this patch series has a chance at all...)
>>
+
+#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
+
+static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
+{
+CPUClass *cc = CPU_GET_CLASS(cs);
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+bool ret = false;
+
+/*
+ * ARMv7-M interrupt masking works differently than -A or -R.
+ * There is no FIQ/IRQ distinction. Instead of I and F bits
+ * masking FIQ and IRQ interrupts, an exception is taken only
+ * if it is higher priority than the current execution priority
+ * (which depends on state like BASEPRI, FAULTMASK and the
+ * currently active exception).
+ */
+if (interrupt_request & CPU_INTERRUPT_HARD
+&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
>>>
>>> so what is the status wrt m_helper.c which stays unconditionally
>>> compiled. m_helper functions seem to called from target/arm/translate.c
>>> mostly. Have you abandoned the stub idea. It may be confusing to have 2
>>> different helper files. At least a comment explaining where a new helper
>>> shall go may be useful.
>>
>> All the HELPER() functions should definitely stay in m_helper.c. They
>> are required for linking. Or do you prefer a stub file instead? Then we
>> could maybe make the whole m_helper.c conditional in the Makefile.objs
>> instead.
>
> I was simply referring to your previous approach:
>
> Applying [Qemu-devel] [RFC PATCH 3/3] target/arm: Make m_helper.c
> optional via CONFIG_ARM_V7M seems to fix the issue
> https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00333.html
>
> It seems to work fine as it removes the call to armv7m_nvic.c functions
> so no need to move the interrupt controller code?
Yes, but then we either need stubs in a separate file, or have to put
lots of #ifdefs into translate.c ... none of those solutions seem to be
really perfect :-(
Anyway, Philippe is currently respinning his series (I think), so I'll
postpone my work now to avoid to interfere with him.
Thomas
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
Hi Thomas,
On 9/23/19 8:09 PM, Thomas Huth wrote:
> On 23/09/2019 16.31, Auger Eric wrote:
>> Hi Thomas,
>>
>> On 9/21/19 5:04 PM, Thomas Huth wrote:
>>> We are going to make CONFIG_ARM_V7M optional, so the related cortex-m
>>> CPUs should only be created if the switch is enabled. This can best
>>> be done if the code resides in a separate file, thus move the related
>>> functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M
>>> is enabled.
>>>
>>> Signed-off-by: Thomas Huth
>>> ---
>>> target/arm/Makefile.objs | 1 +
>>> target/arm/cpu.c | 146 -
>>> target/arm/v7m.c | 193 +++
>>> 3 files changed, 194 insertions(+), 146 deletions(-)
>>> create mode 100644 target/arm/v7m.c
> [...]
>>> diff --git a/target/arm/v7m.c b/target/arm/v7m.c
>>> new file mode 100644
>>> index 00..505043febe
>>> --- /dev/null
>>> +++ b/target/arm/v7m.c
>>> @@ -0,0 +1,193 @@
>>> +/*
>>> + * ARM v7m helpers.
>>> + *
>>> + * This code is licensed under the GNU GPL v2 or later.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qemu/qemu-print.h"
>>> +#include "qemu-common.h"
>>> +#include "target/arm/idau.h"
>>> +#include "qemu/module.h"
>>> +#include "qapi/error.h"
>>> +#include "qapi/visitor.h"
>>> +#include "cpu.h"
>>> +#include "internals.h"
>>> +#include "exec/exec-all.h"
>>> +#include "hw/qdev-properties.h"
>>> +#if !defined(CONFIG_USER_ONLY)
>>> +#include "hw/loader.h"
>>> +#include "hw/boards.h"
>>> +#endif
>>> +#include "sysemu/sysemu.h"
>>> +#include "sysemu/tcg.h"
>>> +#include "sysemu/hw_accel.h"
>>> +#include "disas/capstone.h"
>>> +#include "fpu/softfloat.h"
>>
>> I guess some of those headers are not needed.
>
> Yeah, I just copy-n-pasted from the source file ... I'll check what can
> be omitted (if this patch series has a chance at all...)
>
>>> +
>>> +#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
>>> +
>>> +static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>>> +{
>>> +CPUClass *cc = CPU_GET_CLASS(cs);
>>> +ARMCPU *cpu = ARM_CPU(cs);
>>> +CPUARMState *env = >env;
>>> +bool ret = false;
>>> +
>>> +/*
>>> + * ARMv7-M interrupt masking works differently than -A or -R.
>>> + * There is no FIQ/IRQ distinction. Instead of I and F bits
>>> + * masking FIQ and IRQ interrupts, an exception is taken only
>>> + * if it is higher priority than the current execution priority
>>> + * (which depends on state like BASEPRI, FAULTMASK and the
>>> + * currently active exception).
>>> + */
>>> +if (interrupt_request & CPU_INTERRUPT_HARD
>>> +&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
>>
>> so what is the status wrt m_helper.c which stays unconditionally
>> compiled. m_helper functions seem to called from target/arm/translate.c
>> mostly. Have you abandoned the stub idea. It may be confusing to have 2
>> different helper files. At least a comment explaining where a new helper
>> shall go may be useful.
>
> All the HELPER() functions should definitely stay in m_helper.c. They
> are required for linking. Or do you prefer a stub file instead? Then we
> could maybe make the whole m_helper.c conditional in the Makefile.objs
> instead.
I was simply referring to your previous approach:
Applying [Qemu-devel] [RFC PATCH 3/3] target/arm: Make m_helper.c
optional via CONFIG_ARM_V7M seems to fix the issue
https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00333.html
It seems to work fine as it removes the call to armv7m_nvic.c functions
so no need to move the interrupt controller code?
Thanks
Eric
>
> However, there's one thing I currently don't quite understand in this
> code (since I'm not an ARM guy, sorry) : There are references to "v8" in
> m_helper.c, too. Is that related to a separate CPU type, ie. should the
> v8 code also be available when CONFIG_ARM_V7M is disabled? Or can the
> code in m_helper.c be disabled completely if CONFIG_ARM_V7M is not set?
>
> Thomas
>
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
On 9/23/19 4:34 PM, Peter Maydell wrote: > On Sat, 21 Sep 2019 at 16:04, Thomas Huth wrote: >> >> We are going to make CONFIG_ARM_V7M optional, so the related cortex-m >> CPUs should only be created if the switch is enabled. This can best >> be done if the code resides in a separate file, thus move the related >> functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M >> is enabled. >> >> Signed-off-by: Thomas Huth >> --- >> target/arm/Makefile.objs | 1 + >> target/arm/cpu.c | 146 - >> target/arm/v7m.c | 193 +++ >> 3 files changed, 194 insertions(+), 146 deletions(-) >> create mode 100644 target/arm/v7m.c > > Calling the new file something with 'cpu' in it would help > to convey that it does the same kinds of things as > cpu.c and cpu64.c. Maybe cpu-m.c or cpu-v7m.c ? I agree to your other comment: From a CONFIG switch point of view I don't think it's worth being able to #ifdef out the various flavours of M-profile individually. So I'm in favor of using cpu-m.c.
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
On Mon, 23 Sep 2019 at 19:51, Thomas Huth wrote: > It also sounds like CONFIG_ARM_V7M should rather be renamed to > CONFIG_ARM_MPROFILE or something similar? Depends whether it's visible to end-users or not. If it is, a different name is probably more helpful; if it's just a symbol used in the QEMU source code/makefiles/etc then you might as well stick with the V7M naming convention we have for consistency with the C files. thanks -- PMM
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
On 23/09/2019 20.45, Peter Maydell wrote: > On Mon, 23 Sep 2019 at 19:09, Thomas Huth wrote: >> However, there's one thing I currently don't quite understand in this >> code (since I'm not an ARM guy, sorry) : There are references to "v8" in >> m_helper.c, too. Is that related to a separate CPU type, ie. should the >> v8 code also be available when CONFIG_ARM_V7M is disabled? Or can the >> code in m_helper.c be disabled completely if CONFIG_ARM_V7M is not set? > > QEMU's naming conventions here is a bit confusing, for > historical reasons. > > Architecturally what we have is: > * "M-profile" -- this is the flavour of Arm architecture for > microcontrollers; it has some big differences from A and R > profile (eg the exception mechanism is different and it has > a built-in NVIC interrupt controller). All the Cortex-M > CPUs are M-profile > * "Arm-v7M" -- this is the v7 flavour of the M-profile > architecture, eg Cortex-M3. > * "Arm-v6M" -- this looks like it ought to mean "v6 flavour > of M-profile", but if you look at what features it has it's > more like "cut down version of v7M" (fewer instructions, cut > down exception model, etc, but some things which on A-profile > don't appear until v7A are present in v6M). Cortex-M0 and -M1. > * "Arm-v8M" -- v8 flavour of M-profile. The big change here > is support for TrustZone. Cortex-M33. v8M comes in two > sub-profiles: "mainline", which has all the features like v7M, > and "baseline", which is cut-down in the same way v6M is a > cut-down v7M. > > In QEMU, we implemented Cortex-M3 first, and then added -M0 and > -M33 later. So mostly our function naming convention uses > "v7m" when it means "any M-profile"; a few v8M-specific > functions use a "v8m" prefix. Everything in m_helper.c is > M-profile specific; most of it is used by all M-profile cores, > and a few bits are v8M-only or v7M-and-v8M-mainline only. > > From a CONFIG switch point of view I don't think it's > worth being able to #ifdef out the various flavours of > M-profile individually. Ok, thanks a lot for the explanation! It's much clearer to me now. So I think it likely would be best to disable the whole m_helper.c code instead of introducing a new file like v7m.c (but that likely requires a stub file instead or some #ifdeffing in translate.c). It also sounds like CONFIG_ARM_V7M should rather be renamed to CONFIG_ARM_MPROFILE or something similar? Thomas
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
On Mon, 23 Sep 2019 at 19:09, Thomas Huth wrote: > However, there's one thing I currently don't quite understand in this > code (since I'm not an ARM guy, sorry) : There are references to "v8" in > m_helper.c, too. Is that related to a separate CPU type, ie. should the > v8 code also be available when CONFIG_ARM_V7M is disabled? Or can the > code in m_helper.c be disabled completely if CONFIG_ARM_V7M is not set? QEMU's naming conventions here is a bit confusing, for historical reasons. Architecturally what we have is: * "M-profile" -- this is the flavour of Arm architecture for microcontrollers; it has some big differences from A and R profile (eg the exception mechanism is different and it has a built-in NVIC interrupt controller). All the Cortex-M CPUs are M-profile * "Arm-v7M" -- this is the v7 flavour of the M-profile architecture, eg Cortex-M3. * "Arm-v6M" -- this looks like it ought to mean "v6 flavour of M-profile", but if you look at what features it has it's more like "cut down version of v7M" (fewer instructions, cut down exception model, etc, but some things which on A-profile don't appear until v7A are present in v6M). Cortex-M0 and -M1. * "Arm-v8M" -- v8 flavour of M-profile. The big change here is support for TrustZone. Cortex-M33. v8M comes in two sub-profiles: "mainline", which has all the features like v7M, and "baseline", which is cut-down in the same way v6M is a cut-down v7M. In QEMU, we implemented Cortex-M3 first, and then added -M0 and -M33 later. So mostly our function naming convention uses "v7m" when it means "any M-profile"; a few v8M-specific functions use a "v8m" prefix. Everything in m_helper.c is M-profile specific; most of it is used by all M-profile cores, and a few bits are v8M-only or v7M-and-v8M-mainline only. >From a CONFIG switch point of view I don't think it's worth being able to #ifdef out the various flavours of M-profile individually. thanks -- PMM
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
On 23/09/2019 16.31, Auger Eric wrote:
> Hi Thomas,
>
> On 9/21/19 5:04 PM, Thomas Huth wrote:
>> We are going to make CONFIG_ARM_V7M optional, so the related cortex-m
>> CPUs should only be created if the switch is enabled. This can best
>> be done if the code resides in a separate file, thus move the related
>> functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M
>> is enabled.
>>
>> Signed-off-by: Thomas Huth
>> ---
>> target/arm/Makefile.objs | 1 +
>> target/arm/cpu.c | 146 -
>> target/arm/v7m.c | 193 +++
>> 3 files changed, 194 insertions(+), 146 deletions(-)
>> create mode 100644 target/arm/v7m.c
[...]
>> diff --git a/target/arm/v7m.c b/target/arm/v7m.c
>> new file mode 100644
>> index 00..505043febe
>> --- /dev/null
>> +++ b/target/arm/v7m.c
>> @@ -0,0 +1,193 @@
>> +/*
>> + * ARM v7m helpers.
>> + *
>> + * This code is licensed under the GNU GPL v2 or later.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/qemu-print.h"
>> +#include "qemu-common.h"
>> +#include "target/arm/idau.h"
>> +#include "qemu/module.h"
>> +#include "qapi/error.h"
>> +#include "qapi/visitor.h"
>> +#include "cpu.h"
>> +#include "internals.h"
>> +#include "exec/exec-all.h"
>> +#include "hw/qdev-properties.h"
>> +#if !defined(CONFIG_USER_ONLY)
>> +#include "hw/loader.h"
>> +#include "hw/boards.h"
>> +#endif
>> +#include "sysemu/sysemu.h"
>> +#include "sysemu/tcg.h"
>> +#include "sysemu/hw_accel.h"
>> +#include "disas/capstone.h"
>> +#include "fpu/softfloat.h"
>
> I guess some of those headers are not needed.
Yeah, I just copy-n-pasted from the source file ... I'll check what can
be omitted (if this patch series has a chance at all...)
>> +
>> +#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
>> +
>> +static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>> +{
>> +CPUClass *cc = CPU_GET_CLASS(cs);
>> +ARMCPU *cpu = ARM_CPU(cs);
>> +CPUARMState *env = >env;
>> +bool ret = false;
>> +
>> +/*
>> + * ARMv7-M interrupt masking works differently than -A or -R.
>> + * There is no FIQ/IRQ distinction. Instead of I and F bits
>> + * masking FIQ and IRQ interrupts, an exception is taken only
>> + * if it is higher priority than the current execution priority
>> + * (which depends on state like BASEPRI, FAULTMASK and the
>> + * currently active exception).
>> + */
>> +if (interrupt_request & CPU_INTERRUPT_HARD
>> +&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
>
> so what is the status wrt m_helper.c which stays unconditionally
> compiled. m_helper functions seem to called from target/arm/translate.c
> mostly. Have you abandoned the stub idea. It may be confusing to have 2
> different helper files. At least a comment explaining where a new helper
> shall go may be useful.
All the HELPER() functions should definitely stay in m_helper.c. They
are required for linking. Or do you prefer a stub file instead? Then we
could maybe make the whole m_helper.c conditional in the Makefile.objs
instead.
However, there's one thing I currently don't quite understand in this
code (since I'm not an ARM guy, sorry) : There are references to "v8" in
m_helper.c, too. Is that related to a separate CPU type, ie. should the
v8 code also be available when CONFIG_ARM_V7M is disabled? Or can the
code in m_helper.c be disabled completely if CONFIG_ARM_V7M is not set?
Thomas
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
On Sat, 21 Sep 2019 at 16:04, Thomas Huth wrote: > > We are going to make CONFIG_ARM_V7M optional, so the related cortex-m > CPUs should only be created if the switch is enabled. This can best > be done if the code resides in a separate file, thus move the related > functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M > is enabled. > > Signed-off-by: Thomas Huth > --- > target/arm/Makefile.objs | 1 + > target/arm/cpu.c | 146 - > target/arm/v7m.c | 193 +++ > 3 files changed, 194 insertions(+), 146 deletions(-) > create mode 100644 target/arm/v7m.c Calling the new file something with 'cpu' in it would help to convey that it does the same kinds of things as cpu.c and cpu64.c. Maybe cpu-m.c or cpu-v7m.c ? thanks -- PMM
Re: [PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
Hi Thomas,
On 9/21/19 5:04 PM, Thomas Huth wrote:
> We are going to make CONFIG_ARM_V7M optional, so the related cortex-m
> CPUs should only be created if the switch is enabled. This can best
> be done if the code resides in a separate file, thus move the related
> functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M
> is enabled.
>
> Signed-off-by: Thomas Huth
> ---
> target/arm/Makefile.objs | 1 +
> target/arm/cpu.c | 146 -
> target/arm/v7m.c | 193 +++
> 3 files changed, 194 insertions(+), 146 deletions(-)
> create mode 100644 target/arm/v7m.c
>
> diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
> index cf26c16f5f..16b9417a8b 100644
> --- a/target/arm/Makefile.objs
> +++ b/target/arm/Makefile.objs
> @@ -61,6 +61,7 @@ obj-y += translate.o op_helper.o
> obj-y += crypto_helper.o
> obj-y += iwmmxt_helper.o vec_helper.o neon_helper.o
> obj-y += m_helper.o
> +obj-$(CONFIG_ARM_V7M) += v7m.o
>
> obj-$(CONFIG_SOFTMMU) += psci.o
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index f1f9eecdc8..d5f0d4af61 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -462,31 +462,6 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
> interrupt_request)
> return ret;
> }
>
> -#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
> -static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> -{
> -CPUClass *cc = CPU_GET_CLASS(cs);
> -ARMCPU *cpu = ARM_CPU(cs);
> -CPUARMState *env = >env;
> -bool ret = false;
> -
> -/* ARMv7-M interrupt masking works differently than -A or -R.
> - * There is no FIQ/IRQ distinction. Instead of I and F bits
> - * masking FIQ and IRQ interrupts, an exception is taken only
> - * if it is higher priority than the current execution priority
> - * (which depends on state like BASEPRI, FAULTMASK and the
> - * currently active exception).
> - */
> -if (interrupt_request & CPU_INTERRUPT_HARD
> -&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
> -cs->exception_index = EXCP_IRQ;
> -cc->do_interrupt(cs);
> -ret = true;
> -}
> -return ret;
> -}
> -#endif
> -
> void arm_cpu_update_virq(ARMCPU *cpu)
> {
> /*
> @@ -1881,119 +1856,6 @@ static void arm11mpcore_initfn(Object *obj)
> cpu->reset_auxcr = 1;
> }
>
> -static void cortex_m0_initfn(Object *obj)
> -{
> -ARMCPU *cpu = ARM_CPU(obj);
> -set_feature(>env, ARM_FEATURE_V6);
> -set_feature(>env, ARM_FEATURE_M);
> -
> -cpu->midr = 0x410cc200;
> -}
> -
> -static void cortex_m3_initfn(Object *obj)
> -{
> -ARMCPU *cpu = ARM_CPU(obj);
> -set_feature(>env, ARM_FEATURE_V7);
> -set_feature(>env, ARM_FEATURE_M);
> -set_feature(>env, ARM_FEATURE_M_MAIN);
> -cpu->midr = 0x410fc231;
> -cpu->pmsav7_dregion = 8;
> -cpu->id_pfr0 = 0x0030;
> -cpu->id_pfr1 = 0x0200;
> -cpu->id_dfr0 = 0x0010;
> -cpu->id_afr0 = 0x;
> -cpu->id_mmfr0 = 0x0030;
> -cpu->id_mmfr1 = 0x;
> -cpu->id_mmfr2 = 0x;
> -cpu->id_mmfr3 = 0x;
> -cpu->isar.id_isar0 = 0x01141110;
> -cpu->isar.id_isar1 = 0x02111000;
> -cpu->isar.id_isar2 = 0x21112231;
> -cpu->isar.id_isar3 = 0x0110;
> -cpu->isar.id_isar4 = 0x01310102;
> -cpu->isar.id_isar5 = 0x;
> -cpu->isar.id_isar6 = 0x;
> -}
> -
> -static void cortex_m4_initfn(Object *obj)
> -{
> -ARMCPU *cpu = ARM_CPU(obj);
> -
> -set_feature(>env, ARM_FEATURE_V7);
> -set_feature(>env, ARM_FEATURE_M);
> -set_feature(>env, ARM_FEATURE_M_MAIN);
> -set_feature(>env, ARM_FEATURE_THUMB_DSP);
> -set_feature(>env, ARM_FEATURE_VFP4);
> -cpu->midr = 0x410fc240; /* r0p0 */
> -cpu->pmsav7_dregion = 8;
> -cpu->isar.mvfr0 = 0x10110021;
> -cpu->isar.mvfr1 = 0x1111;
> -cpu->isar.mvfr2 = 0x;
> -cpu->id_pfr0 = 0x0030;
> -cpu->id_pfr1 = 0x0200;
> -cpu->id_dfr0 = 0x0010;
> -cpu->id_afr0 = 0x;
> -cpu->id_mmfr0 = 0x0030;
> -cpu->id_mmfr1 = 0x;
> -cpu->id_mmfr2 = 0x;
> -cpu->id_mmfr3 = 0x;
> -cpu->isar.id_isar0 = 0x01141110;
> -cpu->isar.id_isar1 = 0x02111000;
> -cpu->isar.id_isar2 = 0x21112231;
> -cpu->isar.id_isar3 = 0x0110;
> -cpu->isar.id_isar4 = 0x01310102;
> -cpu->isar.id_isar5 = 0x;
> -cpu->isar.id_isar6 = 0x;
> -}
> -
> -static void cortex_m33_initfn(Object *obj)
> -{
> -ARMCPU *cpu = ARM_CPU(obj);
> -
> -set_feature(>env, ARM_FEATURE_V8);
> -set_feature(>env, ARM_FEATURE_M);
> -set_feature(>env, ARM_FEATURE_M_MAIN);
> -set_feature(>env, ARM_FEATURE_M_SECURITY);
> -set_feature(>env, ARM_FEATURE_THUMB_DSP);
> -set_feature(>env, ARM_FEATURE_VFP4);
> -cpu->midr = 0x410fd213; /* r0p3 */
> -cpu->pmsav7_dregion = 16;
> -
[PATCH 2/4] target/arm: Move cortex-m related functions to new file v7m.c
We are going to make CONFIG_ARM_V7M optional, so the related cortex-m
CPUs should only be created if the switch is enabled. This can best
be done if the code resides in a separate file, thus move the related
functions to a new file v7m.c which only gets compiled if CONFIG_ARM_V7M
is enabled.
Signed-off-by: Thomas Huth
---
target/arm/Makefile.objs | 1 +
target/arm/cpu.c | 146 -
target/arm/v7m.c | 193 +++
3 files changed, 194 insertions(+), 146 deletions(-)
create mode 100644 target/arm/v7m.c
diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
index cf26c16f5f..16b9417a8b 100644
--- a/target/arm/Makefile.objs
+++ b/target/arm/Makefile.objs
@@ -61,6 +61,7 @@ obj-y += translate.o op_helper.o
obj-y += crypto_helper.o
obj-y += iwmmxt_helper.o vec_helper.o neon_helper.o
obj-y += m_helper.o
+obj-$(CONFIG_ARM_V7M) += v7m.o
obj-$(CONFIG_SOFTMMU) += psci.o
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f1f9eecdc8..d5f0d4af61 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -462,31 +462,6 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
return ret;
}
-#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
-static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
-{
-CPUClass *cc = CPU_GET_CLASS(cs);
-ARMCPU *cpu = ARM_CPU(cs);
-CPUARMState *env = >env;
-bool ret = false;
-
-/* ARMv7-M interrupt masking works differently than -A or -R.
- * There is no FIQ/IRQ distinction. Instead of I and F bits
- * masking FIQ and IRQ interrupts, an exception is taken only
- * if it is higher priority than the current execution priority
- * (which depends on state like BASEPRI, FAULTMASK and the
- * currently active exception).
- */
-if (interrupt_request & CPU_INTERRUPT_HARD
-&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
-cs->exception_index = EXCP_IRQ;
-cc->do_interrupt(cs);
-ret = true;
-}
-return ret;
-}
-#endif
-
void arm_cpu_update_virq(ARMCPU *cpu)
{
/*
@@ -1881,119 +1856,6 @@ static void arm11mpcore_initfn(Object *obj)
cpu->reset_auxcr = 1;
}
-static void cortex_m0_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-set_feature(>env, ARM_FEATURE_V6);
-set_feature(>env, ARM_FEATURE_M);
-
-cpu->midr = 0x410cc200;
-}
-
-static void cortex_m3_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-set_feature(>env, ARM_FEATURE_V7);
-set_feature(>env, ARM_FEATURE_M);
-set_feature(>env, ARM_FEATURE_M_MAIN);
-cpu->midr = 0x410fc231;
-cpu->pmsav7_dregion = 8;
-cpu->id_pfr0 = 0x0030;
-cpu->id_pfr1 = 0x0200;
-cpu->id_dfr0 = 0x0010;
-cpu->id_afr0 = 0x;
-cpu->id_mmfr0 = 0x0030;
-cpu->id_mmfr1 = 0x;
-cpu->id_mmfr2 = 0x;
-cpu->id_mmfr3 = 0x;
-cpu->isar.id_isar0 = 0x01141110;
-cpu->isar.id_isar1 = 0x02111000;
-cpu->isar.id_isar2 = 0x21112231;
-cpu->isar.id_isar3 = 0x0110;
-cpu->isar.id_isar4 = 0x01310102;
-cpu->isar.id_isar5 = 0x;
-cpu->isar.id_isar6 = 0x;
-}
-
-static void cortex_m4_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-
-set_feature(>env, ARM_FEATURE_V7);
-set_feature(>env, ARM_FEATURE_M);
-set_feature(>env, ARM_FEATURE_M_MAIN);
-set_feature(>env, ARM_FEATURE_THUMB_DSP);
-set_feature(>env, ARM_FEATURE_VFP4);
-cpu->midr = 0x410fc240; /* r0p0 */
-cpu->pmsav7_dregion = 8;
-cpu->isar.mvfr0 = 0x10110021;
-cpu->isar.mvfr1 = 0x1111;
-cpu->isar.mvfr2 = 0x;
-cpu->id_pfr0 = 0x0030;
-cpu->id_pfr1 = 0x0200;
-cpu->id_dfr0 = 0x0010;
-cpu->id_afr0 = 0x;
-cpu->id_mmfr0 = 0x0030;
-cpu->id_mmfr1 = 0x;
-cpu->id_mmfr2 = 0x;
-cpu->id_mmfr3 = 0x;
-cpu->isar.id_isar0 = 0x01141110;
-cpu->isar.id_isar1 = 0x02111000;
-cpu->isar.id_isar2 = 0x21112231;
-cpu->isar.id_isar3 = 0x0110;
-cpu->isar.id_isar4 = 0x01310102;
-cpu->isar.id_isar5 = 0x;
-cpu->isar.id_isar6 = 0x;
-}
-
-static void cortex_m33_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-
-set_feature(>env, ARM_FEATURE_V8);
-set_feature(>env, ARM_FEATURE_M);
-set_feature(>env, ARM_FEATURE_M_MAIN);
-set_feature(>env, ARM_FEATURE_M_SECURITY);
-set_feature(>env, ARM_FEATURE_THUMB_DSP);
-set_feature(>env, ARM_FEATURE_VFP4);
-cpu->midr = 0x410fd213; /* r0p3 */
-cpu->pmsav7_dregion = 16;
-cpu->sau_sregion = 8;
-cpu->isar.mvfr0 = 0x10110021;
-cpu->isar.mvfr1 = 0x1111;
-cpu->isar.mvfr2 = 0x0040;
-cpu->id_pfr0 = 0x0030;
-cpu->id_pfr1 = 0x0210;
-cpu->id_dfr0 = 0x0020;
-cpu->id_afr0 = 0x;
-cpu->id_mmfr0 = 0x00101F40;
-cpu->id_mmfr1 = 0x;
-cpu->id_mmfr2 =
