On Sun, 17 Oct 2021, Philippe Mathieu-Daudé wrote:
On 10/15/21 03:06, BALATON Zoltan wrote:
Use via_isa_set_irq() which better encapsulates irq handling in the
vt82xx model and avoids using isa_get_irq() that has a comment saying
it should not be used.
Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
hw/ide/via.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 94cc2142c7..252d18f4ac 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -29,7 +29,7 @@
#include "migration/vmstate.h"
#include "qemu/module.h"
#include "sysemu/dma.h"
-
+#include "hw/isa/vt82c686.h"
#include "hw/ide/pci.h"
#include "trace.h"
@@ -112,7 +112,7 @@ static void via_ide_set_irq(void *opaque, int n, int level)
d->config[0x70 + n * 8] &= ~0x80;
}
- qemu_set_irq(isa_get_irq(NULL, 14 + n), level);
+ via_isa_set_irq(pci_get_function_0(d), 14 + n, level);
Since pci_get_function_0() is expensive, we should cache
'PCIDevice *func0' in PCIIDEState, setting the pointer in
via_ide_realize(). Do you mind sending a follow-up patch?
On the other hand, IMO PCIIDEState should be about PCI IDE stuff so to
keep this clean this would need subclassing it to VIAIDEState and put the
func0 pointer there. But then we probably need to cast between VIAIDE and
PCIIDE and likely we're back to the same level of expensiveness. The main
source why of pci_get_function_0 is expensive is probably the QOM cast to
PCI_BUS in pci_get_bus() the rest is just pointer and array index
dereferences which should not be too bad. And the reason why QOM casts are
expensive is because we have --enable-qom-debug enabled by default. I've
tried to send a patch before to disable this on release builds and only
enable it with --enable-debug or when explicitly asked but it was rejected
saying that these asserts are useful. Maybe we can just live with
qemu_set_irq and not bother and drop this series. (You can cherry pick the
first patch removing code duplication from via isa if you want.)
Regards,
BALATON Zoltan
