[PATCH v2 06.5/18] hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function

[Philippe Mathieu-Daudé]

On 20/2/23 10:52, Philippe Mathieu-Daudé wrote:
> On 20/2/23 10:10, Gerd Hoffmann wrote:
>> On Mon, Feb 20, 2023 at 09:00:44AM +0100, Philippe Mathieu-Daudé wrote:
>>> In order to allow Frankenstein uses such plugging a PIIX3
>>> IDE function on a ICH9 chipset (which already exposes AHCI
>>> ports...) as:
>>>
>>>    $ qemu-system-x86_64 -M q35 -device piix3-ide
>>>
>>> add a kludge to automatically wires the IDE IRQs on an ISA
>>> bus exposed by a PCI-to-ISA bridge (usually function #0).
>>> Restrict this kludge to the PIIX3.
>>
>> Well.  On physical hardware you have a config switch in the bios
>> setup which turns off sata and enables ide instead (i.e. the
>> chipset implements both and can be configured to expose the one
>> or the other).
>>
>> If we want support ide for q35 we should IMHO do something simliar
>> instead of making piix-ide user-pluggable.  We already have -machine
>> q35,sata={on,off}.  We could extend that somehow, by adding
>> ide={on,off}, or by using storage={sata,ide,off} instead.
>>
>> This has been discussed now and then in the past and the usual
>> conclusion was that there is little reason to implement that given
>> that you can just use the 'pc' machine type.  For guests that old
>> that they can't handle sata storage this is usually the better fit
>> anyway ...
> 
> I think we might not using the same words, but agree on the goal :)
> 
> Since this has been discussed in the past, I suppose some users
> want this config available. Why are they using this convoluted
> config? Could it be due to lack of good documentation?
> 
> Do we really need a storage={sata,ide,off} flag? I don't see its
> value. Help cloud users to have a sane config?
> 
> (old) boards exist with both IDE/SATA and we might want to emulate
> some of them, but IMO such boards should be well modeled (Either
> in C or later in declarative language) but not automagically created
> from CLI.
> 
> IOW:
> 
> - using PIIX on Q35 (or any machine exposing a PCI bus) is
>    acceptable, but the chipset should be instantiated as a whole.
>    So to be clear I'm not against "-M q35 -device piix3", we should
>    keep that working.
> 
> - we shouldn't try to maintain such Frankenstein corner cases which
>    force us to add complex hacks, hard to remove, which limit us
>    in implementing new features and cost us a lot of maintenance /
>    testing time.
> 
> So I propose we deprecate this config so we can keep going forward.
> 
> (More generally I'd like to not allow instantiating part of chipset).

So there is a design problem here.

- ICH expose ISA bridge (fn0) with ISA IRQs.
- internal ICH SATA fn wires the IRQs 14/15

if we plug a cripple piix3-ide function which lookup for fn0 (ISA
bridge) to wire its IRQs 14/15, we end up having 2 devices able
to raise/lower IRQs while in the BQL iothread.
IOW, one device raise an IRQ, while the other lower the same IRQ...
thus the 1st device IRQ is acked from HW and missed from guest SW.

Daniel tested such config (2 drives used concurrently, one on IDE
and one on SATA) and reported "lost interrupt" from dmesg.

I haven't investigated using a SplitIrq object, but this doesn't
match real hw.

If user want to suffer from poor quality, we should find a different
(valid) config to add IDE drives to Q35 machine. Or maybe it is
a documentation problem, and we should redirect the users to the
best config.

Ref about similar problems:
https://lore.kernel.org/qemu-devel/b8d457d1-40b1-adb5-a2ac-98070f62ac1e@eik.bme.hu/
https://lore.kernel.org/qemu-devel/Y%2FSu8eB2nIO0INOX@redhat.com/

PD: For the remote-pci it is different because it is based on
PCIe which serializes MSIX IRQs, so no way to overwrite one.