Re: [Qemu-devel] [PATCH V6 for-2.3 13/26] hw/pci-host: introduce TYPE_PCI_HOST_BRIDGE_SNOOPED interface

[Marcel Apfelbaum]

On 04/27/2015 03:14 PM, Michael S. Tsirkin wrote:
> On Thu, Mar 19, 2015 at 08:52:48PM +0200, Marcel Apfelbaum wrote:
> > TYPE_PCI_HOST_BRIDGE_SNOOPED is a special case of host bridge
> > whose configuration registers are snooped by other host bridges
> > to complete their configuration cycles.
> >
> > The interface exposes a list of snooping host bridges that
> > shall be used by the hosts implementing this interface
> > in order to emulate a snooping mechanism.
> >
> > The way that the snooping hosts are registered or how
> > the snooping is implemented are out of the interface scope,
> > it only provides a way to determine if a host bridge has
> > snooping hosts and list them.
>
>
>
> > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > ---
> >   hw/pci/pci_host.c         |  8 ++++++++
> >   include/hw/pci/pci_host.h | 24 ++++++++++++++++++++++++
> >   2 files changed, 32 insertions(+)
> >
> > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> > index 87180c8..288e74c 100644
> > --- a/hw/pci/pci_host.c
> > +++ b/hw/pci/pci_host.c
> > @@ -180,6 +180,12 @@ static const TypeInfo pci_main_host_interface_info = {
> >       .parent        = TYPE_INTERFACE,
> >   };
> >
> > +static const TypeInfo pci_host_bridge_snooped_interface_info = {
> > +    .name = TYPE_PCI_HOST_BRIDGE_SNOOPED,
> > +    .parent = TYPE_INTERFACE,
> > +    .class_size = sizeof(PCIHostBridgeSnoopedClass),
> > +};
> > +
> >   static const TypeInfo pci_host_type_info = {
> >       .name = TYPE_PCI_HOST_BRIDGE,
> >       .parent = TYPE_SYS_BUS_DEVICE,
> > @@ -191,7 +197,9 @@ static const TypeInfo pci_host_type_info = {
> >   static void pci_host_register_types(void)
> >   {
> >       type_register_static(&pci_main_host_interface_info);
> > +    type_register_static(&pci_host_bridge_snooped_interface_info);
> >       type_register_static(&pci_host_type_info);
> > +
>
> extra empty line
Will take care of it, thanks.

> >   }
> >
> >   type_init(pci_host_register_types)
> > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> > index 3c72e26..a041919 100644
> > --- a/include/hw/pci/pci_host.h
> > +++ b/include/hw/pci/pci_host.h
> > @@ -63,6 +63,30 @@ typedef struct PCIHostBridgeClass {
> >       const char *(*root_bus_path)(PCIHostState *, PCIBus *);
> >   } PCIHostBridgeClass;
> >
> > +/**
> > + * A special case of host bridge whose configuration registers
> > + * are snooped by other host bridges to complete their
> > + * configuration cycles.
> > + */
> > +#define TYPE_PCI_HOST_BRIDGE_SNOOPED "pci-host-bridge-snooped"
> > +#define TYPE_PCI_HOST_BRIDGE_SNOOPED_CLASS(klass) \
> > +     OBJECT_CLASS_CHECK(PCIHostBridgeSnoopedClass, (klass), \
> > +                        TYPE_PCI_HOST_BRIDGE_SNOOPED)
> > +#define PCI_HOST_BRIDGE_SNOOPED_GET_CLASS(obj) \
> > +     OBJECT_GET_CLASS(PCIHostBridgeSnoopedClass, (obj), \
> > +                      TYPE_PCI_HOST_BRIDGE_SNOOPED)
> > +#define PCI_HOST_BRIDGE_SNOOPED(obj) \
> > +     INTERFACE_CHECK(PCIHostState, (obj), \
> > +                     TYPE_PCI_HOST_BRIDGE_SNOOPED)
> > +
> > +typedef struct PCIHostBridgeSnoopedClass {
> > +    /* <private> */
> > +    InterfaceClass parent_class;
> > +
> > +    /* <public> */
> > +    GPtrArray *(*snooping_hosts)(PCIHostState *);
>
> Why not just add GPtrArray here directly, and add an API to
> register/deregister?
The interface concentrates on usage, letting the way to fill the snooping
hosts to the specific implementation.

Thanks,
Marcel

> > +} PCIHostBridgeSnoopedClass;
> > +
> >   /* common internal helpers for PCI/PCIe hosts, cut off overflows */
> >   void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
> >                                     uint32_t limit, uint32_t val, uint32_t len);
> > --
> > 2.1.0