[PATCH] ppc/spapr: re-assert IRQs during event-scan if there are pending

Laurent Vivier posted 1 patch 4 days ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201015210318.117386-1-lvivier@redhat.com
Maintainers: David Gibson <david@gibson.dropbear.id.au>
hw/ppc/spapr_events.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

[PATCH] ppc/spapr: re-assert IRQs during event-scan if there are pending

Posted by Laurent Vivier 4 days ago
If we hotplug a CPU during the first second of the kernel boot,
the IRQ can be sent to the kernel while the RTAS event handler
is not installed. The event is queued, but the kernel doesn't
collect it and ignores the new CPU.

As the code relies on edge-triggered IRQ, we can re-assert it
during the event-scan RTAS call if there are still pending
events (as it is already done in check-exception).

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/ppc/spapr_events.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 1069d0197b4f..1add53547ec3 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -1000,10 +1000,22 @@ static void event_scan(PowerPCCPU *cpu, SpaprMachineState *spapr,
                        target_ulong args,
                        uint32_t nret, target_ulong rets)
 {
+    int i;
     if (nargs != 4 || nret != 1) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
+
+    for (i = 0; i < EVENT_CLASS_MAX; i++) {
+        if (rtas_event_log_contains(EVENT_CLASS_MASK(i))) {
+            const SpaprEventSource *source =
+                spapr_event_sources_get_source(spapr->event_sources, i);
+
+            g_assert(source->enabled);
+            qemu_irq_pulse(spapr_qirq(spapr, source->irq));
+        }
+    }
+
     rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND);
 }
 
-- 
2.26.2


Re: [PATCH] ppc/spapr: re-assert IRQs during event-scan if there are pending

Posted by David Gibson 4 days ago
On Thu, Oct 15, 2020 at 11:03:18PM +0200, Laurent Vivier wrote:
> If we hotplug a CPU during the first second of the kernel boot,
> the IRQ can be sent to the kernel while the RTAS event handler
> is not installed. The event is queued, but the kernel doesn't
> collect it and ignores the new CPU.
> 
> As the code relies on edge-triggered IRQ, we can re-assert it
> during the event-scan RTAS call if there are still pending
> events (as it is already done in check-exception).
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Applied to ppc-for-5.2.

> ---
>  hw/ppc/spapr_events.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 1069d0197b4f..1add53547ec3 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -1000,10 +1000,22 @@ static void event_scan(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                         target_ulong args,
>                         uint32_t nret, target_ulong rets)
>  {
> +    int i;
>      if (nargs != 4 || nret != 1) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
> +
> +    for (i = 0; i < EVENT_CLASS_MAX; i++) {
> +        if (rtas_event_log_contains(EVENT_CLASS_MASK(i))) {
> +            const SpaprEventSource *source =
> +                spapr_event_sources_get_source(spapr->event_sources, i);
> +
> +            g_assert(source->enabled);
> +            qemu_irq_pulse(spapr_qirq(spapr, source->irq));
> +        }
> +    }
> +
>      rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND);
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] ppc/spapr: re-assert IRQs during event-scan if there are pending

Posted by Greg Kurz 4 days ago
On Thu, 15 Oct 2020 23:03:18 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> If we hotplug a CPU during the first second of the kernel boot,
> the IRQ can be sent to the kernel while the RTAS event handler
> is not installed. The event is queued, but the kernel doesn't
> collect it and ignores the new CPU.
> 
> As the code relies on edge-triggered IRQ, we can re-assert it
> during the event-scan RTAS call if there are still pending
> events (as it is already done in check-exception).
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

Any BugId to share ?

>  hw/ppc/spapr_events.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 1069d0197b4f..1add53547ec3 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -1000,10 +1000,22 @@ static void event_scan(PowerPCCPU *cpu, SpaprMachineState *spapr,
>                         target_ulong args,
>                         uint32_t nret, target_ulong rets)
>  {
> +    int i;
>      if (nargs != 4 || nret != 1) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
> +
> +    for (i = 0; i < EVENT_CLASS_MAX; i++) {
> +        if (rtas_event_log_contains(EVENT_CLASS_MASK(i))) {
> +            const SpaprEventSource *source =
> +                spapr_event_sources_get_source(spapr->event_sources, i);
> +
> +            g_assert(source->enabled);
> +            qemu_irq_pulse(spapr_qirq(spapr, source->irq));
> +        }
> +    }
> +

This looks good but any reason for not putting this in a function called by
both event_scan() and check_exception() ?

Anyway, this can be done as a follow-up:

Reviewed-by: Greg Kurz <groug@kaod.org>

>      rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND);
>  }
>