[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the
From: |
Markus Armbruster |
Subject: |
Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine |
Date: |
Thu, 29 Apr 2021 08:08:21 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Stefan Hajnoczi <stefanha@redhat.com> writes:
> On Wed, Apr 28, 2021 at 04:18:17PM +0200, Markus Armbruster wrote:
>> Stefan Hajnoczi <stefanha@redhat.com> writes:
[...]
>> > The approach in this patch is okay but we should keep in mind it only
>> > solves piix3-ide. ISA provides a non-qdev backdoor API and there may be
>> > more instances of this type of bug.
>> >
>> > A qdev fix would address the root cause and make it possible to drop the
>> > backdoor API, but that's probably too much work for little benefit.
>>
>> What do you mean by backdoor API? Global @isabus?
>
> Yes. It's also strange that isa_get_irq(ISADevice *dev, unsigned isairq)
> accepts dev = NULL as a valid argument.
@isabus is static in hw/isa/isa-bus.c. Uses:
* Limit isa_bus_new() to one ISA bus. Arbitrary restriction; multiple
ISA buses could work with suitable memory mapping and IRQ wiring.
"Single ISA bus" assumptions could of course hide elsewhere in the
code.
* Implied argument to isa_get_irq(), isa_register_ioport(),
isa_register_portio_list(), isa_address_space(),
isa_address_space_io().
isa_get_irq() asserts that a non-null @dev is a child of @isabus.
This means we don't actually need @isabus, except when @dev is null.
I suspect two separate functions would be cleaner: one taking an
ISABus * argument, and a wrapper taking an ISADevice * argument.
isa_address_space() and isa_address_space_io() work the same, less the
assertion.
isa_register_ioport() and isa_register_portio_list() take a non-null
@dev argument. They don't actually need @isabus.
To eliminate global @isabus, we need to fix up the callers passing null
@dev. Clean solution: plumb the ISABus returned by isa_bus_new() to the
call sites. Where that's impractical, we can also get it from QOM, like
build_dsdt_microvm() does.
- [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Thomas Huth, 2021/04/16
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Philippe Mathieu-Daudé, 2021/04/27
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Stefan Hajnoczi, 2021/04/27
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, John Snow, 2021/04/27
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Philippe Mathieu-Daudé, 2021/04/27
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, John Snow, 2021/04/27
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Stefan Hajnoczi, 2021/04/28
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Markus Armbruster, 2021/04/28
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Stefan Hajnoczi, 2021/04/28
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine,
Markus Armbruster <=
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Stefan Hajnoczi, 2021/04/28
- Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Markus Armbruster, 2021/04/28
Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, John Snow, 2021/04/27
Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine, Stefan Hajnoczi, 2021/04/28