diff mbox

[22/25] pci/brdige: qdevfy and initialize secondary bus and subordinate bus.

Message ID 1254514577-11896-23-git-send-email-yamahata@valinux.co.jp
State Superseded
Headers show

Commit Message

Isaku Yamahata Oct. 2, 2009, 8:16 p.m. UTC 
qdevfy pci bridge, and
initialize secondary bus and subordinate bus in configuration space.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/apb_pci.c |    6 ++-
 hw/pci.c     |  110 +++++++++++++++++++++++++++++++++++++++++++---------------
 hw/pci.h     |   12 ++++++
 3 files changed, 98 insertions(+), 30 deletions(-)

Comments

Michael S. Tsirkin Oct. 4, 2009, 11:04 a.m. UTC | #1 
On Sat, Oct 03, 2009 at 05:16:14AM +0900, Isaku Yamahata wrote:
> qdevfy pci bridge, and
> initialize secondary bus and subordinate bus in configuration space.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/apb_pci.c |    6 ++-
>  hw/pci.c     |  110 +++++++++++++++++++++++++++++++++++++++++++---------------
>  hw/pci.h     |   12 ++++++
>  3 files changed, 98 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index 00165e5..5ccb654 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -238,10 +238,12 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
>                                           0, 32);
>      pci_create_simple(d->host_state.bus, 0, "pbm");
>      /* APB secondary busses */
> -    *bus2 = pci_bridge_init(d->host_state.bus, 8, PCI_VENDOR_ID_SUN,
> +    *bus2 = pci_bridge_init(d->host_state.bus,
> +                            PCI_DEVFN(1, 0), PCI_VENDOR_ID_SUN,
>                              PCI_DEVICE_ID_SUN_SIMBA, 1, 1, pci_apb_map_irq,
>                              "Advanced PCI Bus secondary bridge 1");
> -    *bus3 = pci_bridge_init(d->host_state.bus, 9, PCI_VENDOR_ID_SUN,
> +    *bus3 = pci_bridge_init(d->host_state.bus,
> +                            PCI_DEVFN(1, 1), PCI_VENDOR_ID_SUN,
>                              PCI_DEVICE_ID_SUN_SIMBA, 2, 2, pci_apb_map_irq,
>                              "Advanced PCI Bus secondary bridge 2");
>  
> diff --git a/hw/pci.c b/hw/pci.c
> index b708d71..cc5738a 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -133,12 +133,17 @@ PCIBus *pci_find_host_bus(int domain)
>      return NULL;
>  }
>  
> +static int pci_bus_get_instance_id(void)
> +{
> +    static int nbus = 0;
> +    return nbus++;
> +}
> +

I have not looked into this deeply, but looks scary.
How does this interact with e.g. hotplug?
How are these id's used? What if they overflow?
If you just want a random number, let user pass it in?

>  PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>                           void *irq_opaque, int devfn_min, int nirq)
>  {
>      PCIBus *bus;
> -    static int nbus = 0;
>  
>      bus = FROM_QBUS(PCIBus, qbus_create(&pci_bus_info, parent, name));
>      bus->set_irq = set_irq;
> @@ -154,7 +159,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>      QLIST_INIT(&bus->child);
>      pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
>  
> -    vmstate_register(nbus++, &vmstate_pcibus, bus);
> +    vmstate_register(pci_bus_get_instance_id(), &vmstate_pcibus, bus);
>      qemu_register_reset(pci_bus_reset, bus);
>      return bus;
>  }
> @@ -162,7 +167,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>  static PCIBus *pci_register_secondary_bus(PCIBus *parent,
>                                            PCIDevice *dev,
>                                            pci_map_irq_fn map_irq,
> -                                          const char *name)
> +                                          const char *name, int devfn_min)
>  {
>      PCIBus *bus;
>  
> @@ -172,8 +177,10 @@ static PCIBus *pci_register_secondary_bus(PCIBus *parent,
>  
>      bus->bus_num = dev->config[PCI_SECONDARY_BUS];
>      bus->sub_bus = dev->config[PCI_SUBORDINATE_BUS];
> +    bus->devfn_min = devfn_min;
>      QLIST_INIT(&bus->child);
>      QLIST_INSERT_HEAD(&parent->child, bus, sibling);
> +    vmstate_register(pci_bus_get_instance_id(), &vmstate_pcibus, bus);
>      return bus;
>  }
>  
> @@ -1154,7 +1161,7 @@ typedef struct {
>      PCIBus *bus;
>  } PCIBridge;
>  
> -static void pci_bridge_write_config(PCIDevice *d,
> +void pci_bridge_write_config(PCIDevice *d,
>                               uint32_t address, uint32_t val, int len)
>  {
>      PCIBridge *s = (PCIBridge *)d;
> @@ -1213,35 +1220,82 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, int slot, int function)
>      return bus->devices[PCI_DEVFN(slot, function)];
>  }
>  
> -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
> -                        uint8_t sec_bus, uint8_t sub_bus,
> -                        pci_map_irq_fn map_irq, const char *name)
> +int pci_bridge_initfn(PCIDevice *pci_dev)
>  {
> -    PCIBridge *s;
> -    s = (PCIBridge *)pci_register_device(bus, name, sizeof(PCIBridge),
> -                                         devfn, NULL, pci_bridge_write_config);
> -
> -    pci_config_set_vendor_id(s->dev.config, vid);
> -    pci_config_set_device_id(s->dev.config, did);
> -
> -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
> -    s->dev.config[0x05] = 0x00;
> -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> -    s->dev.config[0x07] = 0x00; // status = fast devsel
> -    s->dev.config[0x08] = 0x00; // revision
> -    s->dev.config[0x09] = 0x00; // programming i/f
> -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
> -    s->dev.config[0x0D] = 0x10; // latency_timer
> -    s->dev.config[PCI_HEADER_TYPE] =
> +    uint8_t *pci_conf;
> +
> +    pci_conf = pci_dev->config;

This seems to just cause churn.

> +    pci_conf[0x04] = 0x06; // command = bus master, pci mem
> +    pci_conf[0x05] = 0x00;
> +    pci_conf[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> +    pci_conf[0x07] = 0x00; // status = fast devsel
> +    pci_conf[0x08] = 0x00; // revision
> +    pci_conf[0x09] = 0x00; // programming i/f
> +    pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_PCI);
> +    pci_conf[0x0D] = 0x10; // latency_timer
> +    pci_conf[PCI_HEADER_TYPE] =
>          PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
> -    s->dev.config[0x1E] = 0xa0; // secondary status
> +    pci_conf[0x18] = pci_dev->bus->bus_num;// primary bus number
> +    /* secondary bus and subordinate bus will be set by the caller */
> +    pci_conf[0x1E] = 0xa0; // secondary status

If we are touching this code, maybe clean it up?
- use symbolic constants
- most registers are probably inited sanely by default already (e.g. 0)
- get rid of // comments

> +

kill empty line.

> +    /* vid/did will be set later */

later when?

> +
> +    return 0;
> +}
> +
> +#define PCI_BRIDGE_DEFAULT      "default PCI to PCI bridge"
> +static PCIDeviceInfo pci_bridge_info_default = {
> +    .qdev.name = PCI_BRIDGE_DEFAULT,
> +    .qdev.size = sizeof(PCIBridge),
> +    .config_write = pci_bridge_write_config,
> +    .init = pci_bridge_initfn,
> +    .pcie = 0,
> +};
> +
> +static void pci_bridge_register_device(void)
> +{
> +    pci_qdev_register(&pci_bridge_info_default);
> +}
> +device_init(pci_bridge_register_device);
> +
> +PCIBus *pci_bridge_create_simple(PCIBus *bus, int devfn,
> +                                 uint16_t vid, uint16_t did,
> +                                 uint8_t sec_bus, uint8_t sub_bus,
> +                                 pci_map_irq_fn map_irq, const char *bus_name,
> +                                 const char *name)
> +{
> +    DeviceState *qdev;
> +    uint8_t* pci_conf;
> +    PCIDevice *d;
> +    PCIBridge *br;
> +
> +    qdev = qdev_create(&bus->qbus, name);
> +
> +    qdev_prop_set_uint32(qdev, "addr", devfn);

What is this "addr" property?

> +
> +    qdev_init(qdev);
> +
> +    d = DO_UPCAST(PCIDevice, qdev, qdev);
> +    pci_conf = d->config;

this variable is not worth having

> +    pci_config_set_vendor_id(pci_conf, vid);
> +    pci_config_set_device_id(pci_conf, did);
>  
>      assert(sec_bus <= sub_bus);
> -    s->dev.config[PCI_SECONDARY_BUS] = sec_bus;
> -    s->dev.config[PCI_SUBORDINATE_BUS] = sub_bus;
> +    pci_set_byte(d->config + PCI_SECONDARY_BUS, sec_bus);
> +    pci_set_byte(d->config + PCI_SUBORDINATE_BUS, sub_bus);

We don't particularly have to replace single byte
accesses by pci_set_byte: it's there just so that
if code uses others like pci_set_word, it can look
uniform. This function doesn't though.

> -    s->bus = pci_register_secondary_bus(bus, &s->dev, map_irq, name);
> -    return s->bus;
> +    br = DO_UPCAST(PCIBridge, dev, d);
> +    br->bus = pci_register_secondary_bus(bus, d, map_irq, name, 0);
> +    return br->bus;
> +}
> +
> +PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
> +                        uint16_t did, uint8_t sec_bus, uint8_t sub_bus,
> +                        pci_map_irq_fn map_irq, const char *name)
> +{
> +    return pci_bridge_create_simple(bus, devfn, vid, did, sec_bus, sub_bus,
> +                                    map_irq, name, PCI_BRIDGE_DEFAULT);
>  }
>  
>  static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
> diff --git a/hw/pci.h b/hw/pci.h
> index 645dacd..be241b4 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -175,6 +175,8 @@ static inline int pci_bar_is_64bit(const PCIIORegion *r)
>  /* Header type 1 (PCI-to-PCI bridges) */
>  #define PCI_SUBORDINATE_BUS     0x1a    /* Highest bus number behind the bridge */
>  #define PCI_ROM_ADDRESS1        0x38    /* Same as PCI_ROM_ADDRESS, but for htype 1 */
> +#define PCI_SUBORDINATE_BUS     0x1a    /* Highest bus number behind the bridge */

seems to be duplicate

> +
>

kill empty line
  
>  /* Size of the standard PCI config header */
>  #define PCI_CONFIG_HEADER_SIZE 0x40
> @@ -304,6 +306,16 @@ void pci_info(Monitor *mon);
>  PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
>                          uint8_t sec_bus, uint8_t sub_bus,
>                          pci_map_irq_fn map_irq, const char *name);
> +PCIBus *pci_bridge_create_simple(PCIBus *bus, int devfn,
> +                                 uint16_t vid, uint16_t did,
> +                                 uint8_t sec_bus, uint8_t sub_bus,
> +                                 pci_map_irq_fn map_irq, const char *bus_name,
> +                                 const char *name);
> +void pci_bridge_write_config(PCIDevice *d,
> +                             uint32_t address, uint32_t val, int len);
> +int pci_bridge_initfn(PCIDevice *pci_dev);
> +void pci_bridge_set_map_irq(PCIBus *bus, pci_map_irq_fn map_irq);
> +
>  
>  static inline void
>  pci_set_byte(uint8_t *config, uint8_t val)
> -- 
> 1.6.0.2
> 
>
Isaku Yamahata Oct. 5, 2009, 9:51 a.m. UTC | #2 
On Sun, Oct 04, 2009 at 01:04:02PM +0200, Michael S. Tsirkin wrote:
> On Sat, Oct 03, 2009 at 05:16:14AM +0900, Isaku Yamahata wrote:
> > qdevfy pci bridge, and
> > initialize secondary bus and subordinate bus in configuration space.
> > 
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > ---
> >  hw/apb_pci.c |    6 ++-
> >  hw/pci.c     |  110 +++++++++++++++++++++++++++++++++++++++++++---------------
> >  hw/pci.h     |   12 ++++++
> >  3 files changed, 98 insertions(+), 30 deletions(-)
> > 
> > diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> > index 00165e5..5ccb654 100644
> > --- a/hw/apb_pci.c
> > +++ b/hw/apb_pci.c
> > @@ -238,10 +238,12 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
> >                                           0, 32);
> >      pci_create_simple(d->host_state.bus, 0, "pbm");
> >      /* APB secondary busses */
> > -    *bus2 = pci_bridge_init(d->host_state.bus, 8, PCI_VENDOR_ID_SUN,
> > +    *bus2 = pci_bridge_init(d->host_state.bus,
> > +                            PCI_DEVFN(1, 0), PCI_VENDOR_ID_SUN,
> >                              PCI_DEVICE_ID_SUN_SIMBA, 1, 1, pci_apb_map_irq,
> >                              "Advanced PCI Bus secondary bridge 1");
> > -    *bus3 = pci_bridge_init(d->host_state.bus, 9, PCI_VENDOR_ID_SUN,
> > +    *bus3 = pci_bridge_init(d->host_state.bus,
> > +                            PCI_DEVFN(1, 1), PCI_VENDOR_ID_SUN,
> >                              PCI_DEVICE_ID_SUN_SIMBA, 2, 2, pci_apb_map_irq,
> >                              "Advanced PCI Bus secondary bridge 2");
> >  
> > diff --git a/hw/pci.c b/hw/pci.c
> > index b708d71..cc5738a 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -133,12 +133,17 @@ PCIBus *pci_find_host_bus(int domain)
> >      return NULL;
> >  }
> >  
> > +static int pci_bus_get_instance_id(void)
> > +{
> > +    static int nbus = 0;
> > +    return nbus++;
> > +}
> > +
> 
> I have not looked into this deeply, but looks scary.
> How does this interact with e.g. hotplug?
> How are these id's used? What if they overflow?
> If you just want a random number, let user pass it in?

Those fear sounds reasonable. I'll leave them untouched.


> >  PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> >                           pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> >                           void *irq_opaque, int devfn_min, int nirq)
> >  {
> >      PCIBus *bus;
> > -    static int nbus = 0;
> >  
> >      bus = FROM_QBUS(PCIBus, qbus_create(&pci_bus_info, parent, name));
> >      bus->set_irq = set_irq;
> > @@ -154,7 +159,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> >      QLIST_INIT(&bus->child);
> >      pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
> >  
> > -    vmstate_register(nbus++, &vmstate_pcibus, bus);
> > +    vmstate_register(pci_bus_get_instance_id(), &vmstate_pcibus, bus);
> >      qemu_register_reset(pci_bus_reset, bus);
> >      return bus;
> >  }
> > @@ -162,7 +167,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> >  static PCIBus *pci_register_secondary_bus(PCIBus *parent,
> >                                            PCIDevice *dev,
> >                                            pci_map_irq_fn map_irq,
> > -                                          const char *name)
> > +                                          const char *name, int devfn_min)
> >  {
> >      PCIBus *bus;
> >  
> > @@ -172,8 +177,10 @@ static PCIBus *pci_register_secondary_bus(PCIBus *parent,
> >  
> >      bus->bus_num = dev->config[PCI_SECONDARY_BUS];
> >      bus->sub_bus = dev->config[PCI_SUBORDINATE_BUS];
> > +    bus->devfn_min = devfn_min;
> >      QLIST_INIT(&bus->child);
> >      QLIST_INSERT_HEAD(&parent->child, bus, sibling);
> > +    vmstate_register(pci_bus_get_instance_id(), &vmstate_pcibus, bus);
> >      return bus;
> >  }
> >  
> > @@ -1154,7 +1161,7 @@ typedef struct {
> >      PCIBus *bus;
> >  } PCIBridge;
> >  
> > -static void pci_bridge_write_config(PCIDevice *d,
> > +void pci_bridge_write_config(PCIDevice *d,
> >                               uint32_t address, uint32_t val, int len)
> >  {
> >      PCIBridge *s = (PCIBridge *)d;
> > @@ -1213,35 +1220,82 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, int slot, int function)
> >      return bus->devices[PCI_DEVFN(slot, function)];
> >  }
> >  
> > -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
> > -                        uint8_t sec_bus, uint8_t sub_bus,
> > -                        pci_map_irq_fn map_irq, const char *name)
> > +int pci_bridge_initfn(PCIDevice *pci_dev)
> >  {
> > -    PCIBridge *s;
> > -    s = (PCIBridge *)pci_register_device(bus, name, sizeof(PCIBridge),
> > -                                         devfn, NULL, pci_bridge_write_config);
> > -
> > -    pci_config_set_vendor_id(s->dev.config, vid);
> > -    pci_config_set_device_id(s->dev.config, did);
> > -
> > -    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
> > -    s->dev.config[0x05] = 0x00;
> > -    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> > -    s->dev.config[0x07] = 0x00; // status = fast devsel
> > -    s->dev.config[0x08] = 0x00; // revision
> > -    s->dev.config[0x09] = 0x00; // programming i/f
> > -    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
> > -    s->dev.config[0x0D] = 0x10; // latency_timer
> > -    s->dev.config[PCI_HEADER_TYPE] =
> > +    uint8_t *pci_conf;
> > +
> > +    pci_conf = pci_dev->config;
> 
> This seems to just cause churn.
> 
> > +    pci_conf[0x04] = 0x06; // command = bus master, pci mem
> > +    pci_conf[0x05] = 0x00;
> > +    pci_conf[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
> > +    pci_conf[0x07] = 0x00; // status = fast devsel
> > +    pci_conf[0x08] = 0x00; // revision
> > +    pci_conf[0x09] = 0x00; // programming i/f
> > +    pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_PCI);
> > +    pci_conf[0x0D] = 0x10; // latency_timer
> > +    pci_conf[PCI_HEADER_TYPE] =
> >          PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
> > -    s->dev.config[0x1E] = 0xa0; // secondary status
> > +    pci_conf[0x18] = pci_dev->bus->bus_num;// primary bus number
> > +    /* secondary bus and subordinate bus will be set by the caller */
> > +    pci_conf[0x1E] = 0xa0; // secondary status
> 
> If we are touching this code, maybe clean it up?
> - use symbolic constants
> - most registers are probably inited sanely by default already (e.g. 0)
> - get rid of // comments

Will do.


> > +
> 
> kill empty line.
> 
> > +    /* vid/did will be set later */
> 
> later when?
> 
> > +
> > +    return 0;
> > +}
> > +
> > +#define PCI_BRIDGE_DEFAULT      "default PCI to PCI bridge"
> > +static PCIDeviceInfo pci_bridge_info_default = {
> > +    .qdev.name = PCI_BRIDGE_DEFAULT,
> > +    .qdev.size = sizeof(PCIBridge),
> > +    .config_write = pci_bridge_write_config,
> > +    .init = pci_bridge_initfn,
> > +    .pcie = 0,
> > +};
> > +
> > +static void pci_bridge_register_device(void)
> > +{
> > +    pci_qdev_register(&pci_bridge_info_default);
> > +}
> > +device_init(pci_bridge_register_device);
> > +
> > +PCIBus *pci_bridge_create_simple(PCIBus *bus, int devfn,
> > +                                 uint16_t vid, uint16_t did,
> > +                                 uint8_t sec_bus, uint8_t sub_bus,
> > +                                 pci_map_irq_fn map_irq, const char *bus_name,
> > +                                 const char *name)
> > +{
> > +    DeviceState *qdev;
> > +    uint8_t* pci_conf;
> > +    PCIDevice *d;
> > +    PCIBridge *br;
> > +
> > +    qdev = qdev_create(&bus->qbus, name);
> > +
> > +    qdev_prop_set_uint32(qdev, "addr", devfn);
> 
> What is this "addr" property?
> 
> > +
> > +    qdev_init(qdev);
> > +
> > +    d = DO_UPCAST(PCIDevice, qdev, qdev);
> > +    pci_conf = d->config;
> 
> this variable is not worth having
> 
> > +    pci_config_set_vendor_id(pci_conf, vid);
> > +    pci_config_set_device_id(pci_conf, did);
> >  
> >      assert(sec_bus <= sub_bus);
> > -    s->dev.config[PCI_SECONDARY_BUS] = sec_bus;
> > -    s->dev.config[PCI_SUBORDINATE_BUS] = sub_bus;
> > +    pci_set_byte(d->config + PCI_SECONDARY_BUS, sec_bus);
> > +    pci_set_byte(d->config + PCI_SUBORDINATE_BUS, sub_bus);
> 
> We don't particularly have to replace single byte
> accesses by pci_set_byte: it's there just so that
> if code uses others like pci_set_word, it can look
> uniform. This function doesn't though.
> 
> > -    s->bus = pci_register_secondary_bus(bus, &s->dev, map_irq, name);
> > -    return s->bus;
> > +    br = DO_UPCAST(PCIBridge, dev, d);
> > +    br->bus = pci_register_secondary_bus(bus, d, map_irq, name, 0);
> > +    return br->bus;
> > +}
> > +
> > +PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
> > +                        uint16_t did, uint8_t sec_bus, uint8_t sub_bus,
> > +                        pci_map_irq_fn map_irq, const char *name)
> > +{
> > +    return pci_bridge_create_simple(bus, devfn, vid, did, sec_bus, sub_bus,
> > +                                    map_irq, name, PCI_BRIDGE_DEFAULT);
> >  }
> >  
> >  static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 645dacd..be241b4 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -175,6 +175,8 @@ static inline int pci_bar_is_64bit(const PCIIORegion *r)
> >  /* Header type 1 (PCI-to-PCI bridges) */
> >  #define PCI_SUBORDINATE_BUS     0x1a    /* Highest bus number behind the bridge */
> >  #define PCI_ROM_ADDRESS1        0x38    /* Same as PCI_ROM_ADDRESS, but for htype 1 */
> > +#define PCI_SUBORDINATE_BUS     0x1a    /* Highest bus number behind the bridge */
> 
> seems to be duplicate
> 
> > +
> >
> 
> kill empty line
>   
> >  /* Size of the standard PCI config header */
> >  #define PCI_CONFIG_HEADER_SIZE 0x40
> > @@ -304,6 +306,16 @@ void pci_info(Monitor *mon);
> >  PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
> >                          uint8_t sec_bus, uint8_t sub_bus,
> >                          pci_map_irq_fn map_irq, const char *name);
> > +PCIBus *pci_bridge_create_simple(PCIBus *bus, int devfn,
> > +                                 uint16_t vid, uint16_t did,
> > +                                 uint8_t sec_bus, uint8_t sub_bus,
> > +                                 pci_map_irq_fn map_irq, const char *bus_name,
> > +                                 const char *name);
> > +void pci_bridge_write_config(PCIDevice *d,
> > +                             uint32_t address, uint32_t val, int len);
> > +int pci_bridge_initfn(PCIDevice *pci_dev);
> > +void pci_bridge_set_map_irq(PCIBus *bus, pci_map_irq_fn map_irq);
> > +
> >  
> >  static inline void
> >  pci_set_byte(uint8_t *config, uint8_t val)
>
diff mbox

Patch

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 00165e5..5ccb654 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -238,10 +238,12 @@  PCIBus *pci_apb_init(target_phys_addr_t special_base,
                                          0, 32);
     pci_create_simple(d->host_state.bus, 0, "pbm");
     /* APB secondary busses */
-    *bus2 = pci_bridge_init(d->host_state.bus, 8, PCI_VENDOR_ID_SUN,
+    *bus2 = pci_bridge_init(d->host_state.bus,
+                            PCI_DEVFN(1, 0), PCI_VENDOR_ID_SUN,
                             PCI_DEVICE_ID_SUN_SIMBA, 1, 1, pci_apb_map_irq,
                             "Advanced PCI Bus secondary bridge 1");
-    *bus3 = pci_bridge_init(d->host_state.bus, 9, PCI_VENDOR_ID_SUN,
+    *bus3 = pci_bridge_init(d->host_state.bus,
+                            PCI_DEVFN(1, 1), PCI_VENDOR_ID_SUN,
                             PCI_DEVICE_ID_SUN_SIMBA, 2, 2, pci_apb_map_irq,
                             "Advanced PCI Bus secondary bridge 2");
 
diff --git a/hw/pci.c b/hw/pci.c
index b708d71..cc5738a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -133,12 +133,17 @@  PCIBus *pci_find_host_bus(int domain)
     return NULL;
 }
 
+static int pci_bus_get_instance_id(void)
+{
+    static int nbus = 0;
+    return nbus++;
+}
+
 PCIBus *pci_register_bus(DeviceState *parent, const char *name,
                          pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                          void *irq_opaque, int devfn_min, int nirq)
 {
     PCIBus *bus;
-    static int nbus = 0;
 
     bus = FROM_QBUS(PCIBus, qbus_create(&pci_bus_info, parent, name));
     bus->set_irq = set_irq;
@@ -154,7 +159,7 @@  PCIBus *pci_register_bus(DeviceState *parent, const char *name,
     QLIST_INIT(&bus->child);
     pci_host_bus_register(0, bus); /* for now only pci domain 0 is supported */
 
-    vmstate_register(nbus++, &vmstate_pcibus, bus);
+    vmstate_register(pci_bus_get_instance_id(), &vmstate_pcibus, bus);
     qemu_register_reset(pci_bus_reset, bus);
     return bus;
 }
@@ -162,7 +167,7 @@  PCIBus *pci_register_bus(DeviceState *parent, const char *name,
 static PCIBus *pci_register_secondary_bus(PCIBus *parent,
                                           PCIDevice *dev,
                                           pci_map_irq_fn map_irq,
-                                          const char *name)
+                                          const char *name, int devfn_min)
 {
     PCIBus *bus;
 
@@ -172,8 +177,10 @@  static PCIBus *pci_register_secondary_bus(PCIBus *parent,
 
     bus->bus_num = dev->config[PCI_SECONDARY_BUS];
     bus->sub_bus = dev->config[PCI_SUBORDINATE_BUS];
+    bus->devfn_min = devfn_min;
     QLIST_INIT(&bus->child);
     QLIST_INSERT_HEAD(&parent->child, bus, sibling);
+    vmstate_register(pci_bus_get_instance_id(), &vmstate_pcibus, bus);
     return bus;
 }
 
@@ -1154,7 +1161,7 @@  typedef struct {
     PCIBus *bus;
 } PCIBridge;
 
-static void pci_bridge_write_config(PCIDevice *d,
+void pci_bridge_write_config(PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
     PCIBridge *s = (PCIBridge *)d;
@@ -1213,35 +1220,82 @@  PCIDevice *pci_find_device(PCIBus *bus, int bus_num, int slot, int function)
     return bus->devices[PCI_DEVFN(slot, function)];
 }
 
-PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
-                        uint8_t sec_bus, uint8_t sub_bus,
-                        pci_map_irq_fn map_irq, const char *name)
+int pci_bridge_initfn(PCIDevice *pci_dev)
 {
-    PCIBridge *s;
-    s = (PCIBridge *)pci_register_device(bus, name, sizeof(PCIBridge),
-                                         devfn, NULL, pci_bridge_write_config);
-
-    pci_config_set_vendor_id(s->dev.config, vid);
-    pci_config_set_device_id(s->dev.config, did);
-
-    s->dev.config[0x04] = 0x06; // command = bus master, pci mem
-    s->dev.config[0x05] = 0x00;
-    s->dev.config[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
-    s->dev.config[0x07] = 0x00; // status = fast devsel
-    s->dev.config[0x08] = 0x00; // revision
-    s->dev.config[0x09] = 0x00; // programming i/f
-    pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_PCI);
-    s->dev.config[0x0D] = 0x10; // latency_timer
-    s->dev.config[PCI_HEADER_TYPE] =
+    uint8_t *pci_conf;
+
+    pci_conf = pci_dev->config;
+    pci_conf[0x04] = 0x06; // command = bus master, pci mem
+    pci_conf[0x05] = 0x00;
+    pci_conf[0x06] = 0xa0; // status = fast back-to-back, 66MHz, no error
+    pci_conf[0x07] = 0x00; // status = fast devsel
+    pci_conf[0x08] = 0x00; // revision
+    pci_conf[0x09] = 0x00; // programming i/f
+    pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_PCI);
+    pci_conf[0x0D] = 0x10; // latency_timer
+    pci_conf[PCI_HEADER_TYPE] =
         PCI_HEADER_TYPE_MULTI_FUNCTION | PCI_HEADER_TYPE_BRIDGE; // header_type
-    s->dev.config[0x1E] = 0xa0; // secondary status
+    pci_conf[0x18] = pci_dev->bus->bus_num;// primary bus number
+    /* secondary bus and subordinate bus will be set by the caller */
+    pci_conf[0x1E] = 0xa0; // secondary status
+
+    /* vid/did will be set later */
+
+    return 0;
+}
+
+#define PCI_BRIDGE_DEFAULT      "default PCI to PCI bridge"
+static PCIDeviceInfo pci_bridge_info_default = {
+    .qdev.name = PCI_BRIDGE_DEFAULT,
+    .qdev.size = sizeof(PCIBridge),
+    .config_write = pci_bridge_write_config,
+    .init = pci_bridge_initfn,
+    .pcie = 0,
+};
+
+static void pci_bridge_register_device(void)
+{
+    pci_qdev_register(&pci_bridge_info_default);
+}
+device_init(pci_bridge_register_device);
+
+PCIBus *pci_bridge_create_simple(PCIBus *bus, int devfn,
+                                 uint16_t vid, uint16_t did,
+                                 uint8_t sec_bus, uint8_t sub_bus,
+                                 pci_map_irq_fn map_irq, const char *bus_name,
+                                 const char *name)
+{
+    DeviceState *qdev;
+    uint8_t* pci_conf;
+    PCIDevice *d;
+    PCIBridge *br;
+
+    qdev = qdev_create(&bus->qbus, name);
+
+    qdev_prop_set_uint32(qdev, "addr", devfn);
+
+    qdev_init(qdev);
+
+    d = DO_UPCAST(PCIDevice, qdev, qdev);
+    pci_conf = d->config;
+    pci_config_set_vendor_id(pci_conf, vid);
+    pci_config_set_device_id(pci_conf, did);
 
     assert(sec_bus <= sub_bus);
-    s->dev.config[PCI_SECONDARY_BUS] = sec_bus;
-    s->dev.config[PCI_SUBORDINATE_BUS] = sub_bus;
+    pci_set_byte(d->config + PCI_SECONDARY_BUS, sec_bus);
+    pci_set_byte(d->config + PCI_SUBORDINATE_BUS, sub_bus);
 
-    s->bus = pci_register_secondary_bus(bus, &s->dev, map_irq, name);
-    return s->bus;
+    br = DO_UPCAST(PCIBridge, dev, d);
+    br->bus = pci_register_secondary_bus(bus, d, map_irq, name, 0);
+    return br->bus;
+}
+
+PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid,
+                        uint16_t did, uint8_t sec_bus, uint8_t sub_bus,
+                        pci_map_irq_fn map_irq, const char *name)
+{
+    return pci_bridge_create_simple(bus, devfn, vid, did, sec_bus, sub_bus,
+                                    map_irq, name, PCI_BRIDGE_DEFAULT);
 }
 
 static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
diff --git a/hw/pci.h b/hw/pci.h
index 645dacd..be241b4 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -175,6 +175,8 @@  static inline int pci_bar_is_64bit(const PCIIORegion *r)
 /* Header type 1 (PCI-to-PCI bridges) */
 #define PCI_SUBORDINATE_BUS     0x1a    /* Highest bus number behind the bridge */
 #define PCI_ROM_ADDRESS1        0x38    /* Same as PCI_ROM_ADDRESS, but for htype 1 */
+#define PCI_SUBORDINATE_BUS     0x1a    /* Highest bus number behind the bridge */
+
 
 /* Size of the standard PCI config header */
 #define PCI_CONFIG_HEADER_SIZE 0x40
@@ -304,6 +306,16 @@  void pci_info(Monitor *mon);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
                         uint8_t sec_bus, uint8_t sub_bus,
                         pci_map_irq_fn map_irq, const char *name);
+PCIBus *pci_bridge_create_simple(PCIBus *bus, int devfn,
+                                 uint16_t vid, uint16_t did,
+                                 uint8_t sec_bus, uint8_t sub_bus,
+                                 pci_map_irq_fn map_irq, const char *bus_name,
+                                 const char *name);
+void pci_bridge_write_config(PCIDevice *d,
+                             uint32_t address, uint32_t val, int len);
+int pci_bridge_initfn(PCIDevice *pci_dev);
+void pci_bridge_set_map_irq(PCIBus *bus, pci_map_irq_fn map_irq);
+
 
 static inline void
 pci_set_byte(uint8_t *config, uint8_t val)