[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_inst
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet |
Date: |
Wed, 30 Oct 2013 13:30:17 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Marcel Apfelbaum <address@hidden> writes:
> On Tue, 2013-10-29 at 17:08 +0100, 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.
>>
>> 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's already set for Bonito, grackle, i440FX, and raven. Document
>> why.
>>
>> Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch,
>> pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
>> uni-north-internal-pci, uni-north-pci, and versatile_pci_host.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> hw/mips/gt64xxx_pci.c | 6 ++++++
>> hw/pci-bridge/dec.c | 6 ++++++
>> hw/pci-host/apb.c | 6 ++++++
>> hw/pci-host/bonito.c | 6 +++++-
>> hw/pci-host/grackle.c | 6 +++++-
>> hw/pci-host/piix.c | 6 +++++-
>> hw/pci-host/ppce500.c | 5 +++++
>> hw/pci-host/prep.c | 6 +++++-
>> hw/pci-host/q35.c | 5 +++++
>> hw/pci-host/uninorth.c | 24 ++++++++++++++++++++++++
>> hw/pci-host/versatile.c | 6 ++++++
>> hw/ppc/ppc4xx_pci.c | 5 +++++
>> hw/sh4/sh_pci.c | 6 ++++++
>> 13 files changed, 89 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
>> index 3da2e67..6398514 100644
>> --- a/hw/mips/gt64xxx_pci.c
>> +++ b/hw/mips/gt64xxx_pci.c
>> @@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
>> static void gt64120_pci_class_init(ObjectClass *klass, void *data)
>> {
>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>
>> k->init = gt64120_pci_init;
>> k->vendor_id = PCI_VENDOR_ID_MARVELL;
>> 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.
>> + */
>> + dc->cannot_instantiate_with_device_add_yet = true;
> I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
Correct.
> What do you think about a different approach: check class_id
> in parent class init func and set the flag according to it?
> It corresponds to your idea of changing only sysbus base class.
> Here we don't have a "natural" base class, but we can use class_id.
> What do you think?
My understanding of QOM is rather limited, so take the following with
due skepticism.
I'm afraid the parent's class_init() runs before the child's, and
therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
class_init().
To factor common initialization code, I figure I'd have to splice in an
abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
bridge types such as this one. Might make sense, but it's a bit more
than I bargained for in this series :)
- Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work, (continued)
[Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet, armbru, 2013/10/29
[Qemu-devel] [PATCH v2 09/10] isa: Clean up use of cannot_instantiate_with_device_add_yet, armbru, 2013/10/29
[Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet, armbru, 2013/10/29
[Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet, armbru, 2013/10/29