Re: [Qemu-devel] [PATCH 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> On 17 October 2013 14:54,  <address@hidden> wrote:
>> From: Markus Armbruster <address@hidden>
>>
>> Many PCI host bridges consist of a sysbus device and a PCI device.
>> You need both for the thing to work.  Arguably, these bridges should
>> be modelled as a single, composite devices instead of pairs of
>> seemingly independent devices you can only use together, but we're not
>> there, yet.
>
> I disagree here -- we should be using the modularity that our
> device model provides, not arbitrarily squashing things together
> into single objects just because we've foolishly exposed to the
> end user direct command line access to "create any random object
> whatsoever even if it doesn't make sense".

I'm afraid I didn't express myself clearly.  I'm not advocating
*squashing* these components together.  I'm saying that if A and B can
only be used wired together, there should be a C composed of A, B and
the necessary wiring, and that C is what actually gets put on the board
by configuration.

>> Since the sysbus part can't be instantiated with device_add, yet,
>> permitting it with the PCI part is useless.  We shouldn't offer
>> useless options to the user, so let's set
>> cannot_instantiate_with_device_add_yet for them.
>
> It doesn't make sense to allow the user to create the on-PCI-bus
> representation of the host controller anyway even if they could
> device_add the host controller proper: creating the host controller
> will always automatically create the on-PCI-bus part.

Technically, a device_add i440FX-pcihost doesn't automatically create
i440FX *now*.

I suspect we're arguing only about what exact kind of crazy device_add
of the PCI-facing part of the PCI host bridge is.  Assuming we actually
agree it's crazy in *today's* state of things, does it matter what kind
of crazy it is?  If it doesn't matter, perhaps you could give me a hint
on how to rephrase the commit message.

>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -1157,6 +1157,11 @@ static void
>> gt64120_pci_class_init(ObjectClass *klass, void *data)
>>      k->device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
>>      k->revision = 0x10;
>>      k->class_id = PCI_CLASS_BRIDGE_HOST;
>> +    /*
>> +     * PCI-facing part of the host bridge, not usable without the
>> +     * host-facing part, which can't be device_add'ed, yet.
>> +     */
>> +    k->parent_class.cannot_instantiate_with_device_add_yet = true;
>
> Please don't directly access parent_class -- you should be using
> the proper QOM cast macros to get the DeviceClass pointer.

Will fix, thanks!