qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v2 06.5/18] hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function
Date: Mon, 27 Feb 2023 23:26:57 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.8.0

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:
b8d457d1-40b1-adb5-a2ac-98070f62ac1e@eik.bme.hu/">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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]