On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
> From: Markus Armbruster <arm...@redhat.com>
>
> 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 <arm...@redhat.com>
> ---
> 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.
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?
Marcel
> }
>
> static const TypeInfo gt64120_pci_info = {
> diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c
> index e5e3be8..a6ca940 100644
> --- a/hw/pci-bridge/dec.c
> +++ b/hw/pci-bridge/dec.c
> @@ -116,6 +116,7 @@ static int dec_21154_pci_host_init(PCIDevice *d)
> static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = dec_21154_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_DEC;
> @@ -123,6 +124,11 @@ static void dec_21154_pci_host_class_init(ObjectClass
> *klass, void *data)
> k->revision = 0x02;
> k->class_id = PCI_CLASS_BRIDGE_PCI;
> k->is_bridge = 1;
> + /*
> + * 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;
> }
>
> static const TypeInfo dec_21154_pci_host_info = {
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 92f289f..1b399dd 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -516,11 +516,17 @@ static int pbm_pci_host_init(PCIDevice *d)
> static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = pbm_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_SUN;
> k->device_id = PCI_DEVICE_ID_SUN_SABRE;
> 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;
> }
>
> static const TypeInfo pbm_pci_host_info = {
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index bfb9820..902441f 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -806,8 +806,12 @@ static void bonito_class_init(ObjectClass *klass, void
> *data)
> k->revision = 0x01;
> k->class_id = PCI_CLASS_BRIDGE_HOST;
> dc->desc = "Host bridge";
> - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
> */
> dc->vmsd = &vmstate_bonito;
> + /*
> + * 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;
> }
>
> static const TypeInfo bonito_info = {
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index c178375..7d95821 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -130,7 +130,11 @@ static void grackle_pci_class_init(ObjectClass *klass,
> void *data)
> k->device_id = PCI_DEVICE_ID_MOTOROLA_MPC106;
> k->revision = 0x00;
> k->class_id = PCI_CLASS_BRIDGE_HOST;
> - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
> */
> + /*
> + * 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;
> }
>
> static const TypeInfo grackle_pci_info = {
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 21ffe97..8089fd6 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -698,8 +698,12 @@ static void i440fx_class_init(ObjectClass *klass, void
> *data)
> k->revision = 0x02;
> k->class_id = PCI_CLASS_BRIDGE_HOST;
> dc->desc = "Host bridge";
> - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
> */
> dc->vmsd = &vmstate_i440fx;
> + /*
> + * 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;
> }
>
> static const TypeInfo i440fx_info = {
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index f00793d..c80b7cb 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -387,6 +387,11 @@ static void e500_host_bridge_class_init(ObjectClass
> *klass, void *data)
> k->device_id = PCI_DEVICE_ID_MPC8533E;
> k->class_id = PCI_CLASS_PROCESSOR_POWERPC;
> dc->desc = "Host bridge";
> + /*
> + * 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;
> }
>
> static const TypeInfo e500_host_bridge_info = {
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index ebc40c6..042dc8f 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -198,7 +198,11 @@ static void raven_class_init(ObjectClass *klass, void
> *data)
> k->class_id = PCI_CLASS_BRIDGE_HOST;
> dc->desc = "PReP Host Bridge - Motorola Raven";
> dc->vmsd = &vmstate_raven;
> - dc->cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
> */
> + /*
> + * 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;
> }
>
> static const TypeInfo raven_info = {
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index ad703a4..4dd75c6 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -390,6 +390,11 @@ static void mch_class_init(ObjectClass *klass, void
> *data)
> k->device_id = PCI_DEVICE_ID_INTEL_Q35_MCH;
> k->revision = MCH_HOST_BRIDGE_REVISION_DEFAULT;
> 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;
> }
>
> static const TypeInfo mch_info = {
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index 91530cd..ae04be5 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -351,12 +351,18 @@ static int unin_internal_pci_host_init(PCIDevice *d)
> static void unin_main_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = unin_main_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_PCI;
> k->revision = 0x00;
> 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;
> }
>
> static const TypeInfo unin_main_pci_host_info = {
> @@ -369,12 +375,18 @@ static const TypeInfo unin_main_pci_host_info = {
> static void u3_agp_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = u3_agp_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> k->device_id = PCI_DEVICE_ID_APPLE_U3_AGP;
> k->revision = 0x00;
> 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;
> }
>
> static const TypeInfo u3_agp_pci_host_info = {
> @@ -387,12 +399,18 @@ static const TypeInfo u3_agp_pci_host_info = {
> static void unin_agp_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = unin_agp_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_AGP;
> k->revision = 0x00;
> 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;
> }
>
> static const TypeInfo unin_agp_pci_host_info = {
> @@ -405,12 +423,18 @@ static const TypeInfo unin_agp_pci_host_info = {
> static void unin_internal_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = unin_internal_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_APPLE;
> k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_I_PCI;
> k->revision = 0x00;
> 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;
> }
>
> static const TypeInfo unin_internal_pci_host_info = {
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 6b28929..71ff0de 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -467,11 +467,17 @@ static int versatile_pci_host_init(PCIDevice *d)
> static void versatile_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = versatile_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_XILINX;
> k->device_id = PCI_DEVICE_ID_XILINX_XC2VP30;
> k->class_id = PCI_CLASS_PROCESSOR_CO;
> + /*
> + * 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;
> }
>
> static const TypeInfo versatile_pci_host_info = {
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index d2d6f65..4cb7851 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -380,6 +380,11 @@ static void ppc4xx_host_bridge_class_init(ObjectClass
> *klass, void *data)
> k->vendor_id = PCI_VENDOR_ID_IBM;
> k->device_id = PCI_DEVICE_ID_IBM_440GX;
> k->class_id = PCI_CLASS_BRIDGE_OTHER;
> + /*
> + * 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;
> }
>
> static const TypeInfo ppc4xx_host_bridge_info = {
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index e81176a..a2f6d9e 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -162,10 +162,16 @@ static int sh_pci_host_init(PCIDevice *d)
> static void sh_pci_host_class_init(ObjectClass *klass, void *data)
> {
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
>
> k->init = sh_pci_host_init;
> k->vendor_id = PCI_VENDOR_ID_HITACHI;
> k->device_id = PCI_DEVICE_ID_HITACHI_SH7751R;
> + /*
> + * 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;
> }
>
> static const TypeInfo sh_pci_host_info = {
