Set number of priority bits property of gic as guided by machine
configuration.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/cpu/a9mpcore.c         | 2 ++
 include/hw/cpu/a9mpcore.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index 1f8bc8a..eb1e752 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -68,6 +68,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
     gicdev = DEVICE(&s->gic);
     qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
     qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
+    qdev_prop_set_uint32(gicdev, "num-prio-bits", s->n_prio_bits);
 
     /* Make the GIC's TZ support match the CPUs. We assume that
      * either all the CPUs have TZ, or none do.
@@ -167,6 +168,7 @@ static Property a9mp_priv_properties[] = {
      * Other boards may differ and should set this property appropriately.
      */
     DEFINE_PROP_UINT32("num-irq", A9MPPrivState, num_irq, 96),
+    DEFINE_PROP_UINT32("num-priority-bits", A9MPPrivState, n_prio_bits, 8),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/cpu/a9mpcore.h b/include/hw/cpu/a9mpcore.h
index 5d67ca2..4f146bd 100644
--- a/include/hw/cpu/a9mpcore.h
+++ b/include/hw/cpu/a9mpcore.h
@@ -28,6 +28,7 @@ typedef struct A9MPPrivState {
     uint32_t num_cpu;
     MemoryRegion container;
     uint32_t num_irq;
+    uint32_t n_prio_bits;
 
     A9SCUState scu;
     GICState gic;
-- 
2.7.4

On Fri, 14 Feb 2020 at 13:21, Sai Pavan Boddu
<sai.pavan.boddu@xilinx.com> wrote:
>
> Set number of priority bits property of gic as guided by machine
> configuration.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
>  hw/cpu/a9mpcore.c         | 2 ++
>  include/hw/cpu/a9mpcore.h | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
> index 1f8bc8a..eb1e752 100644
> --- a/hw/cpu/a9mpcore.c
> +++ b/hw/cpu/a9mpcore.c
> @@ -68,6 +68,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
>      gicdev = DEVICE(&s->gic);
>      qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
>      qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
> +    qdev_prop_set_uint32(gicdev, "num-prio-bits", s->n_prio_bits);
>
>      /* Make the GIC's TZ support match the CPUs. We assume that
>       * either all the CPUs have TZ, or none do.
> @@ -167,6 +168,7 @@ static Property a9mp_priv_properties[] = {
>       * Other boards may differ and should set this property appropriately.
>       */
>      DEFINE_PROP_UINT32("num-irq", A9MPPrivState, num_irq, 96),
> +    DEFINE_PROP_UINT32("num-priority-bits", A9MPPrivState, n_prio_bits, 8),
>      DEFINE_PROP_END_OF_LIST(),

You should be able to just directly pass through the property
from the GIC object by calling
    object_property_add_alias(obj, "num-priority-bits", OBJECT(&s->gic),
                              "num-priority-bits", &error_abort);
at the end of a9mp_priv_initfn().

Then you don't need to have a DEFINE_PROP* for it, or a field in
the state struct, or manually pass the value on in realize.

(We don't do this for the existing num-irq and num-cpu properties
because in those cases this device itself needs to know the
values, as well as passing them on to other devices under it.)

thanks
-- PMM

On Tue, 18 Feb 2020 at 18:16, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 14 Feb 2020 at 13:21, Sai Pavan Boddu
> <sai.pavan.boddu@xilinx.com> wrote:
> >
> > Set number of priority bits property of gic as guided by machine
> > configuration.
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > ---
> >  hw/cpu/a9mpcore.c         | 2 ++
> >  include/hw/cpu/a9mpcore.h | 1 +
> >  2 files changed, 3 insertions(+)
> >
> > diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
> > index 1f8bc8a..eb1e752 100644
> > --- a/hw/cpu/a9mpcore.c
> > +++ b/hw/cpu/a9mpcore.c
> > @@ -68,6 +68,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
> >      gicdev = DEVICE(&s->gic);
> >      qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
> >      qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
> > +    qdev_prop_set_uint32(gicdev, "num-prio-bits", s->n_prio_bits);
> >
> >      /* Make the GIC's TZ support match the CPUs. We assume that
> >       * either all the CPUs have TZ, or none do.
> > @@ -167,6 +168,7 @@ static Property a9mp_priv_properties[] = {
> >       * Other boards may differ and should set this property appropriately.
> >       */
> >      DEFINE_PROP_UINT32("num-irq", A9MPPrivState, num_irq, 96),
> > +    DEFINE_PROP_UINT32("num-priority-bits", A9MPPrivState, n_prio_bits, 8),
> >      DEFINE_PROP_END_OF_LIST(),
>
> You should be able to just directly pass through the property
> from the GIC object by calling
>     object_property_add_alias(obj, "num-priority-bits", OBJECT(&s->gic),
>                               "num-priority-bits", &error_abort);
> at the end of a9mp_priv_initfn().

On second thoughts, and having checked the manual:

All A9 CPUs have a GIC with 5 bits of priority. So we should
just set the num-priority-bits on the GIC object in a9mpcore.c,
and not expose it as a property of this container device.

That will be a behaviour change for:
 - exynos4210 boards
 - fsl-imx6 boards
 - highbank
 - realview
 - vexpress
as well as xilinx, but it is a bug fix, and I have enough test
images for those to check it doesn't completely break them.
So we should just go ahead and implement them correctly.

Similarly, the 11MPCore GIC always has 4 bits of priority,
so we could get the behaviour right for the 'realview-eb-mpcore'
board too.

thanks
-- PMM