[PATCH 0/2] Fix PSIHB interrupts init PQ state

[PATCH 0/2] Fix PSIHB interrupts init PQ state

[Frederic Barrat]

On P9 and 10, the real hardware defines the PQ state of the PSIHB
interrupts to be 0b00. Qemu defaults to 0b01.

It doesn't matter to skiboot, which doesn't rely on it and explicitly
masks the interrupts during intialization. But this patch fixes it,
just in case some other hypervisor checks it.


Frederic Barrat (2):
  pnv/xive: Add property on xive sources to define PQ state on reset
  pnv/psi: Initialize the PSIHB interrupts to match hardware

 hw/intc/xive.c        | 8 ++++++--
 hw/ppc/pnv_psi.c      | 2 ++
 include/hw/ppc/xive.h | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.41.0

[PATCH 1/2] pnv/xive: Add property on xive sources to define PQ state on reset

[Frederic Barrat]

The PQ state of a xive interrupt is always initialized to Q=1, which
means the interrupt is disabled. Since a xive source can be embedded
in many objects, this patch adds a property to allow that behavior to
be refined if needed.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/intc/xive.c        | 8 ++++++--
 include/hw/ppc/xive.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 84c079b034..c553b32638 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1232,8 +1232,7 @@ static void xive_source_reset(void *dev)
 
     /* Do not clear the LSI bitmap */
 
-    /* PQs are initialized to 0b01 (Q=1) which corresponds to "ints off" */
-    memset(xsrc->status, XIVE_ESB_OFF, xsrc->nr_irqs);
+    memset(xsrc->status, xsrc->pq_init, xsrc->nr_irqs);
 }
 
 static void xive_source_realize(DeviceState *dev, Error **errp)
@@ -1287,6 +1286,11 @@ static Property xive_source_properties[] = {
     DEFINE_PROP_UINT64("flags", XiveSource, esb_flags, 0),
     DEFINE_PROP_UINT32("nr-irqs", XiveSource, nr_irqs, 0),
     DEFINE_PROP_UINT32("shift", XiveSource, esb_shift, XIVE_ESB_64K_2PAGE),
+    /*
+     * By default, PQs are initialized to 0b01 (Q=1) which corresponds
+     * to "ints off"
+     */
+    DEFINE_PROP_UINT8("pq-init", XiveSource, pq_init, XIVE_ESB_OFF),
     DEFINE_PROP_LINK("xive", XiveSource, xive, TYPE_XIVE_NOTIFIER,
                      XiveNotifier *),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 3dfb06e002..7969f06bcf 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -187,6 +187,7 @@ struct XiveSource {
 
     /* PQ bits and LSI assertion bit */
     uint8_t         *status;
+    uint8_t         pq_init; /* PQ state on reset */
 
     /* ESB memory region */
     uint64_t        esb_flags;
-- 
2.41.0

[PATCH 2/2] pnv/psi: Initialize the PSIHB interrupts to match hardware

[Frederic Barrat]

On P9/P10, the PSIHB interrupts are initialized with a PQ state of
0b01, i.e. interrupts are disabled. However the real hardware
initializes them to 0b00 for the PSIHB. This patch updates it, in case
an hypervisor is in the mood of checking it.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/ppc/pnv_psi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 46da58dff8..f232935d27 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -863,6 +863,8 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error **errp)
     object_property_set_int(OBJECT(xsrc), "nr-irqs", PSIHB9_NUM_IRQS,
                             &error_fatal);
     object_property_set_link(OBJECT(xsrc), "xive", OBJECT(psi), &error_abort);
+    object_property_set_int(OBJECT(xsrc), "pq-init", XIVE_ESB_RESET,
+                            &error_abort);
     if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
         return;
     }
-- 
2.41.0

Re: [PATCH 1/2] pnv/xive: Add property on xive sources to define PQ state on reset

[Cédric Le Goater]

On 6/30/23 16:42, Frederic Barrat wrote:
> The PQ state of a xive interrupt is always initialized to Q=1, which
> means the interrupt is disabled. Since a xive source can be embedded
> in many objects, this patch adds a property to allow that behavior to
> be refined if needed.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>

Looks good but I would rather call the property "reset-pq" and the
attribute reset_pq. Makes more sense since it is a "reset" value.

Thanks,

C.


> ---
>   hw/intc/xive.c        | 8 ++++++--
>   include/hw/ppc/xive.h | 1 +
>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 84c079b034..c553b32638 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1232,8 +1232,7 @@ static void xive_source_reset(void *dev)
>   
>       /* Do not clear the LSI bitmap */
>   
> -    /* PQs are initialized to 0b01 (Q=1) which corresponds to "ints off" */
> -    memset(xsrc->status, XIVE_ESB_OFF, xsrc->nr_irqs);
> +    memset(xsrc->status, xsrc->pq_init, xsrc->nr_irqs);
>   }
>   
>   static void xive_source_realize(DeviceState *dev, Error **errp)
> @@ -1287,6 +1286,11 @@ static Property xive_source_properties[] = {
>       DEFINE_PROP_UINT64("flags", XiveSource, esb_flags, 0),
>       DEFINE_PROP_UINT32("nr-irqs", XiveSource, nr_irqs, 0),
>       DEFINE_PROP_UINT32("shift", XiveSource, esb_shift, XIVE_ESB_64K_2PAGE),
> +    /*
> +     * By default, PQs are initialized to 0b01 (Q=1) which corresponds
> +     * to "ints off"
> +     */
> +    DEFINE_PROP_UINT8("pq-init", XiveSource, pq_init, XIVE_ESB_OFF),
>       DEFINE_PROP_LINK("xive", XiveSource, xive, TYPE_XIVE_NOTIFIER,
>                        XiveNotifier *),
>       DEFINE_PROP_END_OF_LIST(),
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 3dfb06e002..7969f06bcf 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -187,6 +187,7 @@ struct XiveSource {
>   
>       /* PQ bits and LSI assertion bit */
>       uint8_t         *status;
> +    uint8_t         pq_init; /* PQ state on reset */
>   
>       /* ESB memory region */
>       uint64_t        esb_flags;

Re: [PATCH 2/2] pnv/psi: Initialize the PSIHB interrupts to match hardware

[Cédric Le Goater]

On 6/30/23 16:42, Frederic Barrat wrote:
> On P9/P10, the PSIHB interrupts are initialized with a PQ state of
> 0b01, i.e. interrupts are disabled. However the real hardware
> initializes them to 0b00 for the PSIHB. This patch updates it, in case
> an hypervisor is in the mood of checking it.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/ppc/pnv_psi.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 46da58dff8..f232935d27 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -863,6 +863,8 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error **errp)
>       object_property_set_int(OBJECT(xsrc), "nr-irqs", PSIHB9_NUM_IRQS,
>                               &error_fatal);
>       object_property_set_link(OBJECT(xsrc), "xive", OBJECT(psi), &error_abort);
> +    object_property_set_int(OBJECT(xsrc), "pq-init", XIVE_ESB_RESET,
> +                            &error_abort);
>       if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
>           return;
>       }