| Message ID | cdfb3c5a42e505450f6803124f27856434c5b298.1677628524.git.balaton@eik.bme.hu |
|---|---|
| State | New |
| Headers | show |
| Series | Pegasos2 fixes and audio output support | expand |
On Wed, 1 Mar 2023, BALATON Zoltan wrote: > This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 > keeping the rename of a state field but reverting other cahanges which > break interrupts on pegasos2. I've found this with just booting the MorphOS iso which now hangs without this revert when trying to read from the ide device. I think I've mentioned that I've also tried this way first but then ended up adding this because it was needed in a review of the patch earlier but I can't find that message now. For now it seems the easiest is to revert this and think about it later. Regards, BALATON Zoltan > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/isa/vt82c686.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c > index f4c40965cd..01e0148967 100644 > --- a/hw/isa/vt82c686.c > +++ b/hw/isa/vt82c686.c > @@ -598,15 +598,23 @@ void via_isa_set_irq(PCIDevice *d, int n, int level) > qemu_set_irq(s->isa_irqs_in[n], level); > } > > +static void via_isa_request_i8259_irq(void *opaque, int irq, int level) > +{ > + ViaISAState *s = opaque; > + qemu_set_irq(s->cpu_intr, level); > +} > + > static void via_isa_realize(PCIDevice *d, Error **errp) > { > ViaISAState *s = VIA_ISA(d); > DeviceState *dev = DEVICE(d); > PCIBus *pci_bus = pci_get_bus(d); > + qemu_irq *isa_irq; > ISABus *isa_bus; > int i; > > qdev_init_gpio_out(dev, &s->cpu_intr, 1); > + isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1); > isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d), > errp); > > @@ -614,7 +622,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp) > return; > } > > - s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr); > + s->isa_irqs_in = i8259_init(isa_bus, *isa_irq); > isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in); > i8254_pit_init(isa_bus, 0x40, 0, NULL); > i8257_dma_init(isa_bus, 0); >
Am 1. März 2023 00:33:28 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >On Wed, 1 Mar 2023, BALATON Zoltan wrote: >> This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 >> keeping the rename of a state field but reverting other cahanges which >> break interrupts on pegasos2. > >I've found this with just booting the MorphOS iso which now hangs without this revert when trying to read from the ide device. I think I've mentioned that I've also tried this way first but then ended up adding this because it was needed in a review of the patch earlier but I can't find that message now. For now it seems the easiest is to revert this and think about it later. It looks like Philippe's patch should work, at least in theory. Why does the indirection work while it doesn't without? > >Regards, >BALATON Zoltan > >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/isa/vt82c686.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c >> index f4c40965cd..01e0148967 100644 >> --- a/hw/isa/vt82c686.c >> +++ b/hw/isa/vt82c686.c >> @@ -598,15 +598,23 @@ void via_isa_set_irq(PCIDevice *d, int n, int level) >> qemu_set_irq(s->isa_irqs_in[n], level); >> } >> >> +static void via_isa_request_i8259_irq(void *opaque, int irq, int level) >> +{ >> + ViaISAState *s = opaque; >> + qemu_set_irq(s->cpu_intr, level); >> +} >> + >> static void via_isa_realize(PCIDevice *d, Error **errp) >> { >> ViaISAState *s = VIA_ISA(d); >> DeviceState *dev = DEVICE(d); >> PCIBus *pci_bus = pci_get_bus(d); >> + qemu_irq *isa_irq; >> ISABus *isa_bus; >> int i; >> >> qdev_init_gpio_out(dev, &s->cpu_intr, 1); >> + isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1); >> isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d), >> errp); >> >> @@ -614,7 +622,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp) >> return; >> } >> >> - s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr); >> + s->isa_irqs_in = i8259_init(isa_bus, *isa_irq); >> isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in); >> i8254_pit_init(isa_bus, 0x40, 0, NULL); >> i8257_dma_init(isa_bus, 0); >>
On Wed, 1 Mar 2023, Bernhard Beschow wrote: > Am 1. März 2023 00:33:28 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >> On Wed, 1 Mar 2023, BALATON Zoltan wrote: >>> This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 >>> keeping the rename of a state field but reverting other cahanges which >>> break interrupts on pegasos2. >> >> I've found this with just booting the MorphOS iso which now hangs without this revert when trying to read from the ide device. I think I've mentioned that I've also tried this way first but then ended up adding this because it was needed in a review of the patch earlier but I can't find that message now. For now it seems the easiest is to revert this and think about it later. > > It looks like Philippe's patch should work, at least in theory. Why does the indirection work while it doesn't without? I have eno idea. I've found this before because I've also thought it should work in the simpler way but found it doesn't, that's why I've added the indirection which I saw was the way other similar devices also did it. We're now too close to the freeze to have another try so just chose to revert it for now then we can find out later what's going on. Regards, BALATON Zoltan > >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> hw/isa/vt82c686.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c >>> index f4c40965cd..01e0148967 100644 >>> --- a/hw/isa/vt82c686.c >>> +++ b/hw/isa/vt82c686.c >>> @@ -598,15 +598,23 @@ void via_isa_set_irq(PCIDevice *d, int n, int level) >>> qemu_set_irq(s->isa_irqs_in[n], level); >>> } >>> >>> +static void via_isa_request_i8259_irq(void *opaque, int irq, int level) >>> +{ >>> + ViaISAState *s = opaque; >>> + qemu_set_irq(s->cpu_intr, level); >>> +} >>> + >>> static void via_isa_realize(PCIDevice *d, Error **errp) >>> { >>> ViaISAState *s = VIA_ISA(d); >>> DeviceState *dev = DEVICE(d); >>> PCIBus *pci_bus = pci_get_bus(d); >>> + qemu_irq *isa_irq; >>> ISABus *isa_bus; >>> int i; >>> >>> qdev_init_gpio_out(dev, &s->cpu_intr, 1); >>> + isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1); >>> isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d), >>> errp); >>> >>> @@ -614,7 +622,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp) >>> return; >>> } >>> >>> - s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr); >>> + s->isa_irqs_in = i8259_init(isa_bus, *isa_irq); >>> isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in); >>> i8254_pit_init(isa_bus, 0x40, 0, NULL); >>> i8257_dma_init(isa_bus, 0); >>> > >
On 1/3/23 01:33, BALATON Zoltan wrote: > On Wed, 1 Mar 2023, BALATON Zoltan wrote: >> This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 >> keeping the rename of a state field but reverting other cahanges which >> break interrupts on pegasos2. > > I've found this with just booting the MorphOS iso which now hangs > without this revert when trying to read from the ide device. Can you add an Avocado test booting the MorphOS iso? > I think > I've mentioned that I've also tried this way first but then ended up > adding this because it was needed in a review of the patch earlier but I > can't find that message now. For now it seems the easiest is to revert > this and think about it later. > > Regards, > BALATON Zoltan > >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/isa/vt82c686.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-)
On 2/3/23 11:41, Philippe Mathieu-Daudé wrote: > On 1/3/23 01:33, BALATON Zoltan wrote: >> On Wed, 1 Mar 2023, BALATON Zoltan wrote: >>> This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 >>> keeping the rename of a state field but reverting other cahanges which >>> break interrupts on pegasos2. >> >> I've found this with just booting the MorphOS iso which now hangs >> without this revert when trying to read from the ide device. > > Can you add an Avocado test booting the MorphOS iso? Arf I search on the list and someone already did that ;) https://lore.kernel.org/qemu-devel/20210515134555.307404-3-f4bug@amsat.org/
On Thu, 2 Mar 2023, Philippe Mathieu-Daudé wrote: > On 1/3/23 01:33, BALATON Zoltan wrote: >> On Wed, 1 Mar 2023, BALATON Zoltan wrote: >>> This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 >>> keeping the rename of a state field but reverting other cahanges which >>> break interrupts on pegasos2. >> >> I've found this with just booting the MorphOS iso which now hangs without >> this revert when trying to read from the ide device. > > Can you add an Avocado test booting the MorphOS iso? I think you had a patch for that before, so you remember where to find it? I can't test that though as I don't have avocado and it does not come with my distro. So maybe if you can revive your patch and test it that might work better. Regards, BALATON Zoltan >> I think I've mentioned that I've also tried this way first but then ended >> up adding this because it was needed in a review of the patch earlier but I >> can't find that message now. For now it seems the easiest is to revert this >> and think about it later. >> >> Regards, >> BALATON Zoltan >> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> hw/isa/vt82c686.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) > >
On 2/3/23 13:37, BALATON Zoltan wrote: > On Thu, 2 Mar 2023, Philippe Mathieu-Daudé wrote: >> On 1/3/23 01:33, BALATON Zoltan wrote: >>> On Wed, 1 Mar 2023, BALATON Zoltan wrote: >>>> This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 >>>> keeping the rename of a state field but reverting other cahanges which >>>> break interrupts on pegasos2. >>> >>> I've found this with just booting the MorphOS iso which now hangs >>> without this revert when trying to read from the ide device. >> >> Can you add an Avocado test booting the MorphOS iso? > > I think you had a patch for that before, so you remember where to find > it? I can't test that though as I don't have avocado and it does not > come with my distro. So maybe if you can revive your patch and test it > that might work better. It doesn't work on Darwin/macOS neither sigh :( A few users also reported / complained about this there. Waiting for this since 15months or more now... https://github.com/avocado-framework/avocado/issues/5138 https://github.com/avocado-framework/avocado/issues/4888 Personally I ended running it on a remote Linux, this is unfortunate but ¯\_(ツ)_/¯
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index f4c40965cd..01e0148967 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -598,15 +598,23 @@ void via_isa_set_irq(PCIDevice *d, int n, int level) qemu_set_irq(s->isa_irqs_in[n], level); } +static void via_isa_request_i8259_irq(void *opaque, int irq, int level) +{ + ViaISAState *s = opaque; + qemu_set_irq(s->cpu_intr, level); +} + static void via_isa_realize(PCIDevice *d, Error **errp) { ViaISAState *s = VIA_ISA(d); DeviceState *dev = DEVICE(d); PCIBus *pci_bus = pci_get_bus(d); + qemu_irq *isa_irq; ISABus *isa_bus; int i; qdev_init_gpio_out(dev, &s->cpu_intr, 1); + isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1); isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d), errp); @@ -614,7 +622,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp) return; } - s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr); + s->isa_irqs_in = i8259_init(isa_bus, *isa_irq); isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in); i8254_pit_init(isa_bus, 0x40, 0, NULL); i8257_dma_init(isa_bus, 0);
This partially reverts commit bb98e0f59cde846666d9fddc60ae74ef7ddfca17 keeping the rename of a state field but reverting other cahanges which break interrupts on pegasos2. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/isa/vt82c686.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)