On Fri, 16 Oct 2026 at 15:24, Mark Cave-Ayland
<[email protected]> 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 <[email protected]>
> ---
> 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
