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

[Marcel Apfelbaum]

On Thu, 2013-10-31 at 11:29 +0100, Markus Armbruster wrote:
> 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.
Thanks for the explanation, with this rule in mind:

    Reviewed-by: Marcel Apfelbaum <address@hidden>

Thanks,
Marcel

> 
> 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.