[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq()
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq() |
Date: |
Sun, 17 Oct 2021 22:25:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 |
On 10/17/21 21:39, BALATON Zoltan wrote:
> 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 <balaton@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.
I expect any multi-function PCI embedding an IDE controller to route
its IRQs via Func#0, but I'm not a PCI expert.
> But then we probably need to cast between
> VIAIDE and PCIIDE and likely we're back to the same level of
> expensiveness.
realize() is called once, get_irq() multiple times.
> 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
- [PATCH 3/4] hw/usb/vt82c686-uhci-pci: Avoid using isa_get_irq(), (continued)
- [PATCH 3/4] hw/usb/vt82c686-uhci-pci: Avoid using isa_get_irq(), BALATON Zoltan, 2021/10/14
- [PATCH 4/4] via-ide: Avoid using isa_get_irq(), BALATON Zoltan, 2021/10/14
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), Philippe Mathieu-Daudé, 2021/10/17
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), BALATON Zoltan, 2021/10/17
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), Philippe Mathieu-Daudé, 2021/10/17
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), BALATON Zoltan, 2021/10/17
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), Gerd Hoffmann, 2021/10/18
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), BALATON Zoltan, 2021/10/18
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), BALATON Zoltan, 2021/10/17
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(),
Philippe Mathieu-Daudé <=
- Re: [PATCH 4/4] via-ide: Avoid using isa_get_irq(), BALATON Zoltan, 2021/10/17
[PATCH] via-ide: Set user_creatable to false, BALATON Zoltan, 2021/10/15