Re: [PATCH] hw/ide/ahci: Check for PCI device once in ahci_init()

[Philippe Mathieu-Daudé]

On 3/12/24 07:46, Bernhard Beschow wrote:
> Am 21. November 2024 10:01:52 UTC schrieb "Philippe Mathieu-Daudé" 
> <philmd@linaro.org>:
> > object_dynamic_cast() is expensive; IRQ helpers are certainly
> > a bad place to call it. Since the device type won't change at
> > runtime, resolve it once when the AHCI context is initialized
> > in ahci_init().
> >
> > Reported-by: Peter Xu <peterx@redhat.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > include/hw/ide/ahci.h |  2 +-
> > hw/ide/ahci.c         | 17 +++++------------
> > 2 files changed, 6 insertions(+), 13 deletions(-)


> > @@ -196,13 +192,9 @@ static void ahci_irq_raise(AHCIState *s)
> >
> > static void ahci_irq_lower(AHCIState *s)
> > {
> > -    DeviceState *dev_state = s->container;
> > -    PCIDevice *pci_dev = (PCIDevice *) object_dynamic_cast(OBJECT(dev_state),
> > -                                                           TYPE_PCI_DEVICE);
> > -
> >      trace_ahci_irq_lower(s);
> >
> > -    if (!pci_dev || !msi_enabled(pci_dev)) {
> > +    if (!s->pci_dev || !msi_enabled(s->pci_dev)) {
> >          qemu_irq_lower(s->irq);
> >      }
> > }
>
> By always triggering the "irq" property, it might be possible to push out the above two 
> methods to the caller, i.e. the parent PCI device. This would make this device model independent 
> from PCI, essentially turning it into an "IP block". At the same time this eliminates the 
> need for the dynamic casts and AFAICS would also fix the missing PCI dependency in the Kconfig 
> file. I could send a patch.

Oh. Please go ahead, that is appreciated!

> Best regards,
> Bernhard