Re: [Qemu-devel] [PATCH v3 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet

[Markus Armbruster]

Marcel Apfelbaum <address@hidden> writes:

> On Wed, 2013-10-30 at 17:28 +0100, address@hidden wrote:
>> From: Markus Armbruster <address@hidden>
>> 
>> A PIIX3/PIIX4 southbridge has multiple functions.  We model each
>> function as a separate qdev.  Two of them need some special wiring set
>> up in pc_init1() or mips_malta_init() to work: the ISA bridge at 01.0,
>> and the SMBus controller at 01.3.
>> 
>> The IDE controller at 01.1 (piix3-ide, piix3-ide-xen, piix4-ide) has
>> always had cannot_instantiate_with_device_add_yet set, but there is no
>> obvious reason why device_add could not work for them.  Drop it.
> Question:
> piix3-ide (and probably piix4-ide) has io ports hard coded in
> pci_piix_init_ports.
> PIIX3/PIIX4 already has a piix3-ide that uses the io ports.
> Adding more piix3-ide devices would work? 
> What am I missing?

If I understand physical hardware correctly, the southbridge routes the
legacy ISA IDE ports to the first suitable device.  We don't model that
correctly, and end up with all IDE controller device models claiming
them.

There's a similar case in PATCH 09/10: i8042.  It also has hardcoded ISA
ports.  Nevertheless, I drop cannot_instantiate_with_device_add_yet with
the following rationale:

    * i8042: drop, even though its I/O base is hardcoded (because you
      could conceivably still add one to a board that has none) [...]

Same argument applies here: I figure you could add a piix3-ide to a
board that has no IDE controller.

General rule: when in doubt, cannot_instantiate_with_device_add_yet
stays off, because that's the way Anthony wants it (if I understand him
correctly).

> Another question:
> It seems that piix3-ide it was meant to be a function within PIIX3,

Correct.

> if so, we need a cannot_instantiate_with_device_add_ever?

I agree with you that there's something missing here, but I don't think
it's cannot_instantiate_with_device_add_ever :)

It would be nice if we'd model PCI functions and PCI devices properly.
If we did, we wouldn't let users plug a function into a PCI bus.
Instead, we'd let them combine functions into devices, and plug devices
into slots[*].  But it's not what we have now.

In the current state of affairs, we approximate "combine functions into
devices" by letting users plug a bunch of function device models into
the same PCI slot, with different function addresses.  That requires
cannot_instantiate_with_device_add_ever = false.

Users generally don't plug southbridge IDE controller functions, because
a board with PCI generally has a southbridge containing one already.
But as long as a user *could* plug it successfully into *some* board,
cannot_instantiate_with_device_add_ever stays off as per the general
rule above.

Previous discussion:
https://lists.nongnu.org/archive/html/qemu-devel/2013-10/msg02000.html

> Hope it helped,

Thinking reviewers are always appreciated!


[*] Then hot plug of multi-function devices could be made to work.