Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
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 bb98e0f59cde84d9fddc60ae74ef7ddfca17 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 ¯\_(ツ)_/¯
Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
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 bb98e0f59cde84d9fddc60ae74ef7ddfca17 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 --- hw/isa/vt82c686.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-)
Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
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 bb98e0f59cde84d9fddc60ae74ef7ddfca17 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-f4...@amsat.org/
Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
On 1/3/23 01:33, BALATON Zoltan wrote: On Wed, 1 Mar 2023, BALATON Zoltan wrote: This partially reverts commit bb98e0f59cde84d9fddc60ae74ef7ddfca17 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 --- hw/isa/vt82c686.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-)
Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
On Wed, 1 Mar 2023, Bernhard Beschow wrote:
Am 1. März 2023 00:33:28 UTC schrieb BALATON Zoltan :
On Wed, 1 Mar 2023, BALATON Zoltan wrote:
This partially reverts commit bb98e0f59cde84d9fddc60ae74ef7ddfca17
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
---
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, >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);
Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
Am 1. März 2023 00:33:28 UTC schrieb BALATON Zoltan :
>On Wed, 1 Mar 2023, BALATON Zoltan wrote:
>> This partially reverts commit bb98e0f59cde84d9fddc60ae74ef7ddfca17
>> 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
>> ---
>> 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, >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);
>>
Re: [PATCH v5 2/7] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
On Wed, 1 Mar 2023, BALATON Zoltan wrote:
This partially reverts commit bb98e0f59cde84d9fddc60ae74ef7ddfca17
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
---
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, >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);
