Re: [PATCH v2 19/27] i8259.c: properly cascade secondary i8259 from primary i8259

[Peter Maydell <peter.maydell@xxxxxxxxxx>]

On Fri, 16 Oct 2026 at 15:24, Mark Cave-Ayland
<mark.caveayland@xxxxxxxxxxx> wrote:
>
> Currently the cascaded output from the secondary i8259 is returned directly in
> the pass_irqs[] array, instead of being connected to the relevant input of the
> primary i8259.
>
> This is a bug fix since in a cascaded configuration on real hardware it is
> possible (although unlikely) that the cascaded input from the secondary i8259
> can be masked on the primary i8259.
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@xxxxxxxxxxx>
> ---
>  hw/intc/i8259.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
> index bfe01067b7..5f90edbe60 100644
> --- a/hw/intc/i8259.c
> +++ b/hw/intc/i8259.c
> @@ -519,7 +519,8 @@ static void i8259_pic_realize(DeviceState *dev, Error 
> **errp)
>      }
>
>      /* Wire up secondary cascade */
> -    qdev_connect_gpio_out(sec_dev, 0, s->pass_irqs[2]);
> +    qdev_connect_gpio_out(sec_dev, 0,
> +                          qdev_get_gpio_in(pri_dev, 2));

Looking at this again, I'm not sure how this is a bugfix.
Quoting patch 18, this is the code at this point, just
for context:

+    for (i = 0; i < 8; i++) {
+        s->pass_irqs[i] = qdev_get_gpio_in(pri_dev, i);
+    }
+
+    qdev_connect_gpio_out(pri_dev, 0, &s->i8259_primary_out_irq);
+
+    /* Secondary */
+    sec_dev = DEVICE(&s->i8259[1]);
+    qdev_prop_set_uint32(sec_dev, "iobase", 0xa0);
+    qdev_prop_set_uint32(sec_dev, "elcr_addr", 0x4d1);
+    qdev_prop_set_uint8(sec_dev, "elcr_mask", 0xde);
+    if (!isa_realize_and_unref(ISA_DEVICE(sec_dev), s->isabus, errp)) {
+        return;
+    }
+
+    /* Wire up secondary cascade */
+    qdev_connect_gpio_out(sec_dev, 0, s->pass_irqs[2]);
+
+    for (i = 8; i < ISA_NUM_IRQS; i++) {
+        s->pass_irqs[i] = qdev_get_gpio_in(sec_dev, i - 8);
+    }

So before this patch, we connect sec_dev's gpio output 0 to
s->pass_irqs[2]. The loop earlier in the code set that up:
        s->pass_irqs[i] = qdev_get_gpio_in(pri_dev, i);

Now we have:

> +    qdev_connect_gpio_out(sec_dev, 0,
> +                          qdev_get_gpio_in(pri_dev, 2));

Isn't that exactly the same thing?

I think it is indeed a bit clearer to directly grab the
pri_dev input rather than pulling it back out of
pass_irqs[2], but I don't think there's any behaviour
change here.

thanks
-- PMM