Currently the OpenRISC SMP configuration only supports 2 cores due to
the UART IRQ routing being limited to 2 cores.  As was done in commit
1eeffbeb11 ("hw/openrisc/openrisc_sim: Use IRQ splitter when connecting
IRQ to multiple CPUs") we can use a splitter to wire more than 2 CPUs.

This patch moves serial initialization out to it's own function and
uses a splitter to connect multiple CPU irq lines to the UART.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 hw/openrisc/openrisc_sim.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index d12b3e0c5e..5bfbac00f8 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -137,6 +137,28 @@ static void openrisc_sim_ompic_init(hwaddr base, int num_cpus,
     sysbus_mmio_map(s, 0, base);
 }
 
+static void openrisc_sim_serial_init(hwaddr base, int num_cpus,
+                                     OpenRISCCPU *cpus[], int irq_pin)
+{
+    qemu_irq serial_irq;
+    int i;
+
+    if (num_cpus > 1) {
+        DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
+        qdev_prop_set_uint32(splitter, "num-lines", num_cpus);
+        qdev_realize_and_unref(splitter, NULL, &error_fatal);
+        for (i = 0; i < num_cpus; i++) {
+            qdev_connect_gpio_out(splitter, i, get_cpu_irq(cpus, i, irq_pin));
+        }
+        serial_irq = qdev_get_gpio_in(splitter, 0);
+    } else {
+        serial_irq = get_cpu_irq(cpus, 0, irq_pin);
+    }
+    serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
+                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
+}
+
+
 static void openrisc_load_kernel(ram_addr_t ram_size,
                                  const char *kernel_filename)
 {
@@ -177,7 +199,6 @@ static void openrisc_sim_init(MachineState *machine)
     const char *kernel_filename = machine->kernel_filename;
     OpenRISCCPU *cpus[2] = {};
     MemoryRegion *ram;
-    qemu_irq serial_irq;
     int n;
     unsigned int smp_cpus = machine->smp.cpus;
 
@@ -208,15 +229,10 @@ static void openrisc_sim_init(MachineState *machine)
     if (smp_cpus > 1) {
         openrisc_sim_ompic_init(or1ksim_memmap[OR1KSIM_OMPIC].base, smp_cpus,
                                 cpus, OR1KSIM_OMPIC_IRQ);
-
-        serial_irq = qemu_irq_split(get_cpu_irq(cpus, 0, OR1KSIM_UART_IRQ),
-                                    get_cpu_irq(cpus, 1, OR1KSIM_UART_IRQ));
-    } else {
-        serial_irq = get_cpu_irq(cpus, 0, OR1KSIM_UART_IRQ);
     }
 
-    serial_mm_init(get_system_memory(), or1ksim_memmap[OR1KSIM_UART].base, 0,
-                   serial_irq, 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
+    openrisc_sim_serial_init(or1ksim_memmap[OR1KSIM_UART].base, smp_cpus, cpus,
+                             OR1KSIM_UART_IRQ);
 
     openrisc_load_kernel(ram_size, kernel_filename);
 }
-- 
2.31.1

On Sat, 19 Feb 2022 at 06:42, Stafford Horne <shorne@gmail.com> wrote:
>
> Currently the OpenRISC SMP configuration only supports 2 cores due to
> the UART IRQ routing being limited to 2 cores.  As was done in commit
> 1eeffbeb11 ("hw/openrisc/openrisc_sim: Use IRQ splitter when connecting
> IRQ to multiple CPUs") we can use a splitter to wire more than 2 CPUs.
>
> This patch moves serial initialization out to it's own function and
> uses a splitter to connect multiple CPU irq lines to the UART.
>
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

On 19/2/22 07:42, Stafford Horne wrote:
> Currently the OpenRISC SMP configuration only supports 2 cores due to
> the UART IRQ routing being limited to 2 cores.  As was done in commit
> 1eeffbeb11 ("hw/openrisc/openrisc_sim: Use IRQ splitter when connecting
> IRQ to multiple CPUs") we can use a splitter to wire more than 2 CPUs.
> 
> This patch moves serial initialization out to it's own function and
> uses a splitter to connect multiple CPU irq lines to the UART.
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>   hw/openrisc/openrisc_sim.c | 32 ++++++++++++++++++++++++--------
>   1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index d12b3e0c5e..5bfbac00f8 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -137,6 +137,28 @@ static void openrisc_sim_ompic_init(hwaddr base, int num_cpus,
>       sysbus_mmio_map(s, 0, base);
>   }
>   
> +static void openrisc_sim_serial_init(hwaddr base, int num_cpus,
> +                                     OpenRISCCPU *cpus[], int irq_pin)
> +{
> +    qemu_irq serial_irq;
> +    int i;
> +
> +    if (num_cpus > 1) {
> +        DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
> +        qdev_prop_set_uint32(splitter, "num-lines", num_cpus);
> +        qdev_realize_and_unref(splitter, NULL, &error_fatal);
> +        for (i = 0; i < num_cpus; i++) {
> +            qdev_connect_gpio_out(splitter, i, get_cpu_irq(cpus, i, irq_pin));
> +        }
> +        serial_irq = qdev_get_gpio_in(splitter, 0);
> +    } else {
> +        serial_irq = get_cpu_irq(cpus, 0, irq_pin);
> +    }

Up to here the code seems a generic helper:

   or1k_cpus_connect_device(OpenRISCCPU *cpus[],
                            unsigned num_cpus,
                            unsigned irq_pin);

> +    serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
> +                   serial_hd(0), DEVICE_NATIVE_ENDIAN);

This part specific to UART.

> +}

Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

On Sun, Feb 20, 2022 at 09:06:41PM +0100, Philippe Mathieu-Daudé wrote:
> On 19/2/22 07:42, Stafford Horne wrote:
> > Currently the OpenRISC SMP configuration only supports 2 cores due to
> > the UART IRQ routing being limited to 2 cores.  As was done in commit
> > 1eeffbeb11 ("hw/openrisc/openrisc_sim: Use IRQ splitter when connecting
> > IRQ to multiple CPUs") we can use a splitter to wire more than 2 CPUs.
> > 
> > This patch moves serial initialization out to it's own function and
> > uses a splitter to connect multiple CPU irq lines to the UART.
> > 
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > ---
> >   hw/openrisc/openrisc_sim.c | 32 ++++++++++++++++++++++++--------
> >   1 file changed, 24 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> > index d12b3e0c5e..5bfbac00f8 100644
> > --- a/hw/openrisc/openrisc_sim.c
> > +++ b/hw/openrisc/openrisc_sim.c
> > @@ -137,6 +137,28 @@ static void openrisc_sim_ompic_init(hwaddr base, int num_cpus,
> >       sysbus_mmio_map(s, 0, base);
> >   }
> > +static void openrisc_sim_serial_init(hwaddr base, int num_cpus,
> > +                                     OpenRISCCPU *cpus[], int irq_pin)
> > +{
> > +    qemu_irq serial_irq;
> > +    int i;
> > +
> > +    if (num_cpus > 1) {
> > +        DeviceState *splitter = qdev_new(TYPE_SPLIT_IRQ);
> > +        qdev_prop_set_uint32(splitter, "num-lines", num_cpus);
> > +        qdev_realize_and_unref(splitter, NULL, &error_fatal);
> > +        for (i = 0; i < num_cpus; i++) {
> > +            qdev_connect_gpio_out(splitter, i, get_cpu_irq(cpus, i, irq_pin));
> > +        }
> > +        serial_irq = qdev_get_gpio_in(splitter, 0);
> > +    } else {
> > +        serial_irq = get_cpu_irq(cpus, 0, irq_pin);
> > +    }
> 
> Up to here the code seems a generic helper:
> 
>   or1k_cpus_connect_device(OpenRISCCPU *cpus[],
>                            unsigned num_cpus,
>                            unsigned irq_pin);

Right, this is similar to that used in openrisc_sim_net_init.  I thought about
sharing the code but I didn't think it worth adding helper.

The main reason for me is that openrisc_sim_net_init doesn't expose the qemu_irq
and just does sysbus_connect_irq.  While openrisc_sim_serial_init exposes the
qemu_irq.

I think a generic function would have to look like:

    qemu_irq openrisc_cpus_irq_pin_init(OpenRISCCPU *cpus[],
                                        unsigned num_cpus,
                                        unsigned irq_pin);

I would like to leave this as is for now as.

> > +    serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
> > +                   serial_hd(0), DEVICE_NATIVE_ENDIAN);
> 
> This part specific to UART.

Right.

> > +}
> 
> Anyhow,
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thank you,

Adding this to the patch as is.

-Stafford