On Mon, Jun 18, 2012 at 05:23:32PM -0500, Anthony Liguori wrote: > On 06/18/2012 04:44 PM, Andreas Färber wrote: > >Am 18.06.2012 20:28, schrieb Michael S. Tsirkin: > >>On Sun, Jun 10, 2012 at 05:57:54PM +0200, Andreas Färber wrote: > >>>From: Andreas Färber<andreas.faer...@web.de> > >>> > >>>Allows us to access PCIHostState QOM-style with PCI_HOST() macro. > >>> > >>>Update PReP Raven PCI to derive from this type. > >>> > >>>Signed-off-by: Anthony Liguori<aligu...@us.ibm.com> > >>>Signed-off-by: Wanpeng Li<l...@linux.vnet.ibm.com> > >>>Signed-off-by: Andreas Färber<andreas.faer...@web.de> > >>>Reviewed-by: Anthony Liguori<aligu...@us.ibm.com> > >> > >>Question: this is really a pci host *bridge*. > >>We are calling this PCIHost internally for brevity > >>but QOM hierarchy is user-visible, right? > > > >That's a good question... I would say it's not user-visible today unless > >we instantiate TYPE_PCI_HOST directly, in which case its value > >"pci-host" would be visible through the "type" property that got > >introduced on qom-next. My CPU modeling for instance is based on the > >assumption that we can introduce intermediate types later as a > >user-invisible implementation detail. > > Yes, we need to formulate a support statement for the 1.2 release. > > My general thinking is: > > 1) Properties will remain ABI compatible. A property will not > change it's type or semantics over time. An example is link/child > properties. A link will always remain a link but the link subtype > made be made more specific over time. Likewise with child > properties. > > 2) A property may be removed and new properties may be added.
At will? That's a very weak statement. > Applications should always gracefully handle the non-existence of a > property. They can fail gracefully but what else can they do? > 3) Since paths are composed of properties, they are subject to the > same rules. That is, an absolute path will always have the same > semantics as long as that path is still valid. > > 4) No guarantee is made about the stability of relative paths. > > Types are really just another form of properties here so changing > the type of an object provided that we don't violate ABI rules is > okay. Let's invent internal types that we don't expect user to know about. We have x- for properties, let's do the same here. > I actually think it's okay for a typename to change even if it's > exposed by -device. If something is using -device, it ought to > probe for the existence of a type before using -device. > > Regards, > > Anthony Liguori Then it'll fail gracefully but it can't work. > > > >>>--- > >>> hw/pci_host.c | 11 +++++++++++ > >>> hw/pci_host.h | 3 +++ > >>> hw/prep_pci.c | 4 ++-- > >>> 3 files changed, 16 insertions(+), 2 deletions(-) > >>> > >>>diff --git a/hw/pci_host.c b/hw/pci_host.c > >>>index 8041778..347bfa6 100644 > >>>--- a/hw/pci_host.c > >>>+++ b/hw/pci_host.c > >>>@@ -165,4 +165,15 @@ const MemoryRegionOps pci_host_data_be_ops = { > >>> .endianness = DEVICE_BIG_ENDIAN, > >>> }; > >>> > >>>+static const TypeInfo pci_host_type_info = { > >>>+ .name = TYPE_PCI_HOST, > >>>+ .parent = TYPE_SYS_BUS_DEVICE, > >>>+ .instance_size = sizeof(PCIHostState), > >>>+}; > > > >It would in fact be better to set .abstract = true, I guess. > > > >Anyway, I think for sPAPR I translated PHB to PCI_HOST_BRIDGE already, > >so TYPE_PCI_HOST_BRIDGE "pci-host-bridge" and PCI_HOST_BRIDGE() would > >fit in nicely with the otherwise clear and verbose QOM naming. But I'd > >rather not rename PCIHostState everywhere... do you agree? Or would you > >want to have it as PCIHostBridge or PCIHostBridgeState for consistency? > > > >If we use TYPE_PCI_HOST_BRIDGE I should also add _BRIDGE for some of the > >derived types, but we can't change the user-visible "-pcihost" type name > >there for backwards compatibility. > > > >Any further wishes? Should the second patch be split up in some way? > > > >Andreas > > > >>>+ > >>>+static void pci_host_register_types(void) > >>>+{ > >>>+ type_register_static(&pci_host_type_info); > >>>+} > >>> > >>>+type_init(pci_host_register_types) > >[snip] > >