On Mon, Oct 18, 2021 at 12:10:04PM +0200, Philippe Mathieu-Daudé wrote:
On 10/18/21 11:51, BALATON Zoltan wrote:
On Mon, 18 Oct 2021, Philippe Mathieu-Daudé wrote:
On 10/18/21 03:36, BALATON Zoltan wrote:
Cache the pointer to PCI function 0 (ISA bridge, that this IDE device
has to use for IRQs) in the PCIIDEState and pass that as the opaque
data for the interrupt handler to eliminate both the need to look up
function 0 at every interrupt and also a QOM type cast of the opaque
pointer as that's also expensive (mainly due to qom-debug being
enabled by default).
Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ide/via.c | 11 ++++++-----
include/hw/ide/pci.h | 1 +
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 82def819c4..30566bc409 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -104,15 +104,15 @@ static void bmdma_setup_bar(PCIIDEState *d)
static void via_ide_set_irq(void *opaque, int n, int level)
{
- PCIDevice *d = PCI_DEVICE(opaque);
+ PCIIDEState *d = opaque;
if (level) {
- d->config[0x70 + n * 8] |= 0x80;
+ d->parent_obj.config[0x70 + n * 8] |= 0x80;
} else {
- d->config[0x70 + n * 8] &= ~0x80;
+ d->parent_obj.config[0x70 + n * 8] &= ~0x80;
}
Better not to access parent_obj, so I'd rather keep the previous
code as it. The rest is OK, thanks. If you don't want to respin
I can fix and take via mips-next.
Why not? If it's OK to access other fields why not parent_obj? That
avoids the QOM cast PCI_DEVICE(opaque) or PCI_DEVICE(d) after this
patch. We know it's a PCIIDEState and has PCIDevice parent_obj field
because we set the opaque pointer when adding this callback so I think
this works and is the less expensive way. But feel free to change it any
way you like and use it that way. I'd keep it as it is.
My understanding of QOM is we shouldn't access internal states that
way, because 1/ this makes object refactors harder and 2/ this is
not the style/example we want in the codebase, but it doesn't seem
documented, so Cc'ing Markus/Eduardo for confirmation.
`PCI_DEVICE(d)` is preferred instead `of d.parent_obj` (parent_obj is
just a QOM implementation detail). If there are real performance
reasons to avoid it, we need numbers to support that.
Also, note that `OBJECT_CHECK(obj)` is just `return obj` if
CONFIG_QOM_CAST_DEBUG is disabled.