[PATCH 0/3] Add "num-prio-bits" property for Cortex-M devices

[PATCH 0/3] Add "num-prio-bits" property for Cortex-M devices

[Samuel Tardieu]

This patch series builds on a discussion initiated by Anton Kochkov on this
list in 2022. It allows setting the appropriate number of priority bits for
Cortex-M devices. For example, FreeRTOS checks at startup that the right
number of priority bits is available in order to guarantee its runtime
structures safety. They added a configuration option specially for QEMU
to disable this check because QEMU always use 2 bits for Cortex-M0/M0+/M1
and 8 bits for other devices.

While this change allows the number of priority bits to be properly
configured, it keeps the same default as before in order to preserve
backward compatibility.

Based-on: <20220813112559.1974427-1-anton.kochkov@proton.me>
([PATCH] hw/arm/nvic: implement "num-prio-bits" property)

Samuel Tardieu (3):
  hw/intc/armv7m_nvic: add "num-prio-bits" property
  hw/arm/armv7m: alias the NVIC "num-prio-bits" property
  hw/arm/socs: configure priority bits for existing SOCs

 hw/arm/armv7m.c        |  2 ++
 hw/arm/stellaris.c     |  2 ++
 hw/arm/stm32f100_soc.c |  1 +
 hw/arm/stm32f205_soc.c |  1 +
 hw/arm/stm32f405_soc.c |  1 +
 hw/intc/armv7m_nvic.c  | 23 ++++++++++++++++++++++-
 6 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.42.0

[PATCH 1/3] hw/intc/armv7m_nvic: add "num-prio-bits" property

[Samuel Tardieu]

Cortex-M NVIC can have a different number of priority bits.
Cortex-M0/M0+/M1 devices must use 2 or more bits, while devices based
on ARMv7m and up must use 3 or more bits.

This adds a "num-prio-bits" property which will get sensible default
values if unset (2 or 8 depending on the device). Unless a SOC
specifies the number of bits to use, the previous behavior is
maintained for backward compatibiltiy.

Signed-off-by: Samuel Tardieu <sam@rfc1149.net>
Suggested-by: Anton Kochkov <anton.kochkov@proton.me>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1122
---
 hw/intc/armv7m_nvic.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 942be7bd11..82aacd7f22 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2572,6 +2572,11 @@ static const VMStateDescription vmstate_nvic = {
 static Property props_nvic[] = {
     /* Number of external IRQ lines (so excluding the 16 internal exceptions) */
     DEFINE_PROP_UINT32("num-irq", NVICState, num_irq, 64),
+    /*
+     * Number of the maximum priority bits that can be used. 0 means
+     * to use a reasonable default.
+     */
+    DEFINE_PROP_UINT8("num-prio-bits", NVICState, num_prio_bits, 0),
     DEFINE_PROP_END_OF_LIST()
 };
 
@@ -2685,7 +2690,23 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
     /* include space for internal exception vectors */
     s->num_irq += NVIC_FIRST_IRQ;
 
-    s->num_prio_bits = arm_feature(&s->cpu->env, ARM_FEATURE_V7) ? 8 : 2;
+    if (s->num_prio_bits == 0) {
+        /*
+         * If left unspecified, use 2 bits by default on Cortex-M0/M0+/M1
+         * and 8 bits otherwise.
+         */
+        s->num_prio_bits = arm_feature(&s->cpu->env, ARM_FEATURE_V7) ? 8 : 2;
+    } else {
+        uint8_t min_prio_bits =
+            arm_feature(&s->cpu->env, ARM_FEATURE_V7) ? 3 : 2;
+        if (s->num_prio_bits < min_prio_bits || s->num_prio_bits > 8) {
+            error_setg(errp,
+                       "num-prio-bits %d is outside "
+                       "NVIC acceptable range [%d-8]",
+                       s->num_prio_bits, min_prio_bits);
+            return;
+        }
+    }
 
     /*
      * This device provides a single memory region which covers the
-- 
2.42.0

[PATCH 2/3] hw/arm/armv7m: alias the NVIC "num-prio-bits" property

[Samuel Tardieu]

Signed-off-by: Samuel Tardieu <sam@rfc1149.net>
---
 hw/arm/armv7m.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index d10abb36a8..4fda2d1d47 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -256,6 +256,8 @@ static void armv7m_instance_init(Object *obj)
     object_initialize_child(obj, "nvic", &s->nvic, TYPE_NVIC);
     object_property_add_alias(obj, "num-irq",
                               OBJECT(&s->nvic), "num-irq");
+    object_property_add_alias(obj, "num-prio-bits",
+                              OBJECT(&s->nvic), "num-prio-bits");
 
     object_initialize_child(obj, "systick-reg-ns", &s->systick[M_REG_NS],
                             TYPE_SYSTICK);
-- 
2.42.0

[PATCH 3/3] hw/arm/socs: configure priority bits for existing SOCs

[Samuel Tardieu]

Update the number of priority bits for a number of existing
SOCsaccording to their technical documentation:

- STM32F100/F205/F405: 4 bits
- Stellaris (Sandstorm/Fury): 3 bits

Signed-off-by: Samuel Tardieu <sam@rfc1149.net>
---
 hw/arm/stellaris.c     | 2 ++
 hw/arm/stm32f100_soc.c | 1 +
 hw/arm/stm32f205_soc.c | 1 +
 hw/arm/stm32f405_soc.c | 1 +
 4 files changed, 5 insertions(+)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index dd90f686bf..38981967f3 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -47,6 +47,7 @@
 #define BP_GAMEPAD   0x04
 
 #define NUM_IRQ_LINES 64
+#define NUM_PRIO_BITS 3
 
 typedef const struct {
     const char *name;
@@ -1067,6 +1068,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
 
     nvic = qdev_new(TYPE_ARMV7M);
     qdev_prop_set_uint32(nvic, "num-irq", NUM_IRQ_LINES);
+    qdev_prop_set_uint8(nvic, "num-prio-bits", NUM_PRIO_BITS);
     qdev_prop_set_string(nvic, "cpu-type", ms->cpu_type);
     qdev_prop_set_bit(nvic, "enable-bitband", true);
     qdev_connect_clock_in(nvic, "cpuclk",
diff --git a/hw/arm/stm32f100_soc.c b/hw/arm/stm32f100_soc.c
index b90d440d7a..808b783515 100644
--- a/hw/arm/stm32f100_soc.c
+++ b/hw/arm/stm32f100_soc.c
@@ -115,6 +115,7 @@ static void stm32f100_soc_realize(DeviceState *dev_soc, Error **errp)
     /* Init ARMv7m */
     armv7m = DEVICE(&s->armv7m);
     qdev_prop_set_uint32(armv7m, "num-irq", 61);
+    qdev_prop_set_uint8(armv7m, "num-prio-bits", 4);
     qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     qdev_prop_set_bit(armv7m, "enable-bitband", true);
     qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 1a548646f6..a451e21f59 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -127,6 +127,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_uint8(armv7m, "num-prio-bits", 4);
     qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     qdev_prop_set_bit(armv7m, "enable-bitband", true);
     qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
index a65bbe298d..2ad5b79a06 100644
--- a/hw/arm/stm32f405_soc.c
+++ b/hw/arm/stm32f405_soc.c
@@ -149,6 +149,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
 
     armv7m = DEVICE(&s->armv7m);
     qdev_prop_set_uint32(armv7m, "num-irq", 96);
+    qdev_prop_set_uint8(armv7m, "num-prio-bits", 4);
     qdev_prop_set_string(armv7m, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
     qdev_prop_set_bit(armv7m, "enable-bitband", true);
     qdev_connect_clock_in(armv7m, "cpuclk", s->sysclk);
-- 
2.42.0

Re: [PATCH 0/3] Add "num-prio-bits" property for Cortex-M devices

[Samuel Tardieu]

> Samuel Tardieu (3):
>   hw/intc/armv7m_nvic: add "num-prio-bits" property
>   hw/arm/armv7m: alias the NVIC "num-prio-bits" property
>   hw/arm/socs: configure priority bits for existing SOCs

Any idea to why patchew fails to apply thoses patches? The mbox at 
<https://patchew.org/QEMU/20231216182740.3305724-1-sam@rfc1149.net/> 
applies cleanly on master AFAICS.

  Sam
-- 
Samuel Tardieu

Re: [PATCH 0/3] Add "num-prio-bits" property for Cortex-M devices

[Peter Maydell]

On Sun, 17 Dec 2023 at 07:37, Samuel Tardieu <sam@rfc1149.net> wrote:
>
>
> > Samuel Tardieu (3):
> >   hw/intc/armv7m_nvic: add "num-prio-bits" property
> >   hw/arm/armv7m: alias the NVIC "num-prio-bits" property
> >   hw/arm/socs: configure priority bits for existing SOCs
>
> Any idea to why patchew fails to apply thoses patches? The mbox at
> <https://patchew.org/QEMU/20231216182740.3305724-1-sam@rfc1149.net/>
> applies cleanly on master AFAICS.

This is because you put a Based-on: tag in the cover letter.
Based-on: means "please apply this other patch first before this
series, because there is a dependency" (we use it for things
like "this patchset has to sit on top of some other cleanup
patchset I sent last week and which hasn't got into git yet").
So patchew applied Anton's original patch from 2022, and then tried
to apply your three patches on top of that, which caused a conflict.

Anyway, I just wanted to say that this patchset is on my
todo list to review but I'm not going to be able to get to
it before I break for Christmas, so I'll get back to it
in January. Thanks for the contribution!

-- PMM

Re: [PATCH 0/3] Add "num-prio-bits" property for Cortex-M devices

[Samuel Tardieu]

Peter Maydell <peter.maydell@linaro.org> writes:

>> Any idea to why patchew fails to apply thoses patches? The mbox 
>> at
>> <https://patchew.org/QEMU/20231216182740.3305724-1-sam@rfc1149.net/>
>> applies cleanly on master AFAICS.
>
> This is because you put a Based-on: tag in the cover letter.
> Based-on: means "please apply this other patch first before this
> series, because there is a dependency" (we use it for things
> like "this patchset has to sit on top of some other cleanup
> patchset I sent last week and which hasn't got into git yet").
> So patchew applied Anton's original patch from 2022, and then 
> tried
> to apply your three patches on top of that, which caused a 
> conflict.

Thanks for the explanation. I thought "Based-on:" was just a 
polite way of attributing credit to past discussions. I'll keep 
that in mind. Do you want me to resubmit it as a v2 without the 
"Based-on:" tag so that patchew gets it right?

> Anyway, I just wanted to say that this patchset is on my
> todo list to review but I'm not going to be able to get to
> it before I break for Christmas, so I'll get back to it
> in January. Thanks for the contribution!

Noted!

Best.

  Sam
-- 
Samuel Tardieu

Re: [PATCH 0/3] Add "num-prio-bits" property for Cortex-M devices

[Peter Maydell]

On Tue, 19 Dec 2023 at 19:15, Samuel Tardieu <sam@rfc1149.net> wrote:
>
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> >> Any idea to why patchew fails to apply thoses patches? The mbox
> >> at
> >> <https://patchew.org/QEMU/20231216182740.3305724-1-sam@rfc1149.net/>
> >> applies cleanly on master AFAICS.
> >
> > This is because you put a Based-on: tag in the cover letter.
> > Based-on: means "please apply this other patch first before this
> > series, because there is a dependency" (we use it for things
> > like "this patchset has to sit on top of some other cleanup
> > patchset I sent last week and which hasn't got into git yet").
> > So patchew applied Anton's original patch from 2022, and then
> > tried
> > to apply your three patches on top of that, which caused a
> > conflict.
>
> Thanks for the explanation. I thought "Based-on:" was just a
> polite way of attributing credit to past discussions. I'll keep
> that in mind. Do you want me to resubmit it as a v2 without the
> "Based-on:" tag so that patchew gets it right?

No, you don't need to do that. I can review it, and if it's
OK I can apply it to my target-arm tree, based on what we
have now.

thanks
-- PMM