diff mbox

[v5,3/3] convert pci-host to QOM

Message ID 1343046959-6659-4-git-send-email-liwanp@linux.vnet.ibm.com
State New
Headers show

Commit Message

Wanpeng Li July 23, 2012, 12:35 p.m. UTC 
From: Anthony Liguori <aliguori@us.ibm.com>

makes pci_host a proper QOM type.

Changelog:
* against Andreas pci_host branch
* make host bridge TypeInfos const
* use PCI_HOST_BRIDGE() where appropriate 

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>

---
 hw/i440fx.c   |    6 +++---
 hw/pc.c       |    2 +-
 hw/pci_host.c |   14 ++++++++++++++
 hw/pci_host.h |    2 ++
 hw/piix3.c    |    4 ++--
 5 files changed, 22 insertions(+), 6 deletions(-)

Comments

Andreas Färber July 23, 2012, 12:57 p.m. UTC | #1 
Am 23.07.2012 14:35, schrieb Wanpeng Li:
> From: Anthony Liguori <aliguori@us.ibm.com>
> 
> makes pci_host a proper QOM type.
> 
> Changelog:
> * against Andreas pci_host branch
> * make host bridge TypeInfos const
> * use PCI_HOST_BRIDGE() where appropriate 
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
> 
> ---
>  hw/i440fx.c   |    6 +++---
>  hw/pc.c       |    2 +-
>  hw/pci_host.c |   14 ++++++++++++++
>  hw/pci_host.h |    2 ++
>  hw/piix3.c    |    4 ++--
>  5 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i440fx.c b/hw/i440fx.c
> index 720a25a..fdf040b 100644
> --- a/hw/i440fx.c
> +++ b/hw/i440fx.c
> @@ -191,7 +191,7 @@ static const VMStateDescription vmstate_i440fx_pmc = {
>  static int i440fx_realize(SysBusDevice *dev)
>  {
>      I440FXState *s = I440FX(dev);
> -    PCIHostState *h = PCI_HOST(s);
> +    PCIHostState *h = PCI_HOST_BRIDGE(s);
>      int bios_size, isa_bios_size;
>      char *filename;
>      int ret;

Either there's a miscommunication or a technical error: My branch surely
is using PCI_HOST_BRIDGE(), so these PCI_HOST -> PCI_HOST_BRIDGE changes
look bogus. Did you make sure each patch compiles?

> @@ -401,7 +401,7 @@ static void i440fx_pmc_class_init(ObjectClass *klass, void *data)
>      dc->vmsd = &vmstate_i440fx_pmc;
>  }
>  
> -static TypeInfo i440fx_pmc_info = {
> +static const TypeInfo i440fx_pmc_info = {
>      .name          = TYPE_I440FX_PMC,
>      .parent        = TYPE_PCI_DEVICE,
>      .instance_size = sizeof(I440FXPMCState),
> @@ -418,7 +418,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
>      dc->no_user = 1;
>  }
>  
> -static TypeInfo i440fx_info = {
> +static const TypeInfo i440fx_info = {
>      .name          = TYPE_I440FX,
>      .parent        = TYPE_PCI_HOST_BRIDGE,
>      .instance_size = sizeof(I440FXState),

Patch 1/3 does not have const, patch 2/3 adds new TypeInfos without const.

So my guess is you've not rebased this on my pci-host branch [1] but
onto something else? If they're not against master it's advisable to
mark patches [PATCH treename xx/nn] btw, for clarity.

For the new/changed TypeInfos please add const from the start.

Regards,
Andreas

[1] http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/pci-host
git://repo.or.cz/qemu/afaerber.git pci-host

> diff --git a/hw/pc.c b/hw/pc.c
> index d9a0443..f095109 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1217,7 +1217,7 @@ static PCIBus *i440fx_init(I440FXPMCState **pi440fx_state, int *piix3_devfn,
>      PCIHostState *h;
>  
>      s = I440FX(object_new(TYPE_I440FX));
> -    h = PCI_HOST(s);
> +    h = PCI_HOST_BRIDGE(s);
>  
>      /* FIXME make a properties */
>      h->address_space = address_space_mem;
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 3950e94..4e10042 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -165,11 +165,25 @@ const MemoryRegionOps pci_host_data_be_ops = {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
> +{
> +    object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
> +}
> +
> +static void pci_host_initfn(Object *obj)
> +{
> +    PCIHostState *s = PCI_HOST_BRIDGE(obj);
> +
> +    object_property_add_link(obj, "mmio", "memory-region",
> +                            (Object **)&s->address_space, NULL);
> +}
> +
>  static const TypeInfo pci_host_type_info = {
>      .name = TYPE_PCI_HOST_BRIDGE,
>      .parent = TYPE_SYS_BUS_DEVICE,
>      .abstract = true,
>      .instance_size = sizeof(PCIHostState),
> +    .instance_init = pci_host_initfn,
>  };
>  
>  static void pci_host_register_types(void)
> diff --git a/hw/pci_host.h b/hw/pci_host.h
> index 4b9c300..9f28728 100644
> --- a/hw/pci_host.h
> +++ b/hw/pci_host.h
> @@ -54,6 +54,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
>  void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
>  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
>  
> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
> +
>  extern const MemoryRegionOps pci_host_conf_le_ops;
>  extern const MemoryRegionOps pci_host_conf_be_ops;
>  extern const MemoryRegionOps pci_host_data_le_ops;
> diff --git a/hw/piix3.c b/hw/piix3.c
> index eca6ec8..3b69b15 100644
> --- a/hw/piix3.c
> +++ b/hw/piix3.c
> @@ -204,7 +204,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
>      k->class_id     = PCI_CLASS_BRIDGE_ISA;
>  }
>  
> -static TypeInfo piix3_info = {
> +static const TypeInfo piix3_info = {
>      .name          = TYPE_PIIX3,
>      .parent        = TYPE_PCI_DEVICE,
>      .instance_size = sizeof(PIIX3State),
> @@ -219,7 +219,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
>      k->config_write = piix3_write_config_xen;
>  };
>  
> -static TypeInfo piix3_xen_info = {
> +static const TypeInfo piix3_xen_info = {
>      .name          = "PIIX3-xen",
>      .parent        = TYPE_PIIX3,
>      .instance_size = sizeof(PIIX3State),
>
Wanpeng Li July 23, 2012, 1:27 p.m. UTC | #2 
On Mon, Jul 23, 2012 at 02:57:20PM +0200, Andreas Färber wrote:
>Am 23.07.2012 14:35, schrieb Wanpeng Li:
>> From: Anthony Liguori <aliguori@us.ibm.com>
>> 
>> makes pci_host a proper QOM type.
>> 
>> Changelog:
>> * against Andreas pci_host branch
>> * make host bridge TypeInfos const
>> * use PCI_HOST_BRIDGE() where appropriate 
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>> 
>> ---
>>  hw/i440fx.c   |    6 +++---
>>  hw/pc.c       |    2 +-
>>  hw/pci_host.c |   14 ++++++++++++++
>>  hw/pci_host.h |    2 ++
>>  hw/piix3.c    |    4 ++--
>>  5 files changed, 22 insertions(+), 6 deletions(-)
>> 
>> diff --git a/hw/i440fx.c b/hw/i440fx.c
>> index 720a25a..fdf040b 100644
>> --- a/hw/i440fx.c
>> +++ b/hw/i440fx.c
>> @@ -191,7 +191,7 @@ static const VMStateDescription vmstate_i440fx_pmc = {
>>  static int i440fx_realize(SysBusDevice *dev)
>>  {
>>      I440FXState *s = I440FX(dev);
>> -    PCIHostState *h = PCI_HOST(s);
>> +    PCIHostState *h = PCI_HOST_BRIDGE(s);
>>      int bios_size, isa_bios_size;
>>      char *filename;
>>      int ret;
>
>Either there's a miscommunication or a technical error: My branch surely
>is using PCI_HOST_BRIDGE(), so these PCI_HOST -> PCI_HOST_BRIDGE changes
>look bogus. Did you make sure each patch compiles?
The third patch against your pci-host branch which you hope to, the three 
patches can overall compile success, but not each patch compile success. 
Because PATCH 1/3 should take advantage of PCI_HOST_BRIDGE(), and
PATCH 2/3 also should take advantage of PATCH 1/3.

>
>> @@ -401,7 +401,7 @@ static void i440fx_pmc_class_init(ObjectClass *klass, void *data)
>>      dc->vmsd = &vmstate_i440fx_pmc;
>>  }
>>  
>> -static TypeInfo i440fx_pmc_info = {
>> +static const TypeInfo i440fx_pmc_info = {
>>      .name          = TYPE_I440FX_PMC,
>>      .parent        = TYPE_PCI_DEVICE,
>>      .instance_size = sizeof(I440FXPMCState),
>> @@ -418,7 +418,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
>>      dc->no_user = 1;
>>  }
>>  
>> -static TypeInfo i440fx_info = {
>> +static const TypeInfo i440fx_info = {
>>      .name          = TYPE_I440FX,
>>      .parent        = TYPE_PCI_HOST_BRIDGE,
>>      .instance_size = sizeof(I440FXState),
>
>Patch 1/3 does not have const, patch 2/3 adds new TypeInfos without const.

Patch 2/3 doesn't add any new TypeInfos, where you see the new
TypeInfos you mentioned.

>

>So my guess is you've not rebased this on my pci-host branch [1] but

No, I rebase the third patch against your pci_host branch. Since your patch 

pci: Derive PCI host bridges from TYPE_PCI_HOST_BRIDGE
Some typedef'ed their state to PCIHostState. Use a proper struct,
and use PCIHostState and PCI_HOST_BRIDGE() where appropriate.

Signed-off-by: Andreas Färber <afaerber@suse.de>
 
want to use PCI_HOST_BRIDGE() where appropriate, so I add them to Patch
1/3 codes.

Regards,
Wanpeng Li
>onto something else? If they're not against master it's advisable to
>mark patches [PATCH treename xx/nn] btw, for clarity.
>
>For the new/changed TypeInfos please add const from the start.
>
>Regards,
>Andreas
>
>[1] http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/pci-host
>git://repo.or.cz/qemu/afaerber.git pci-host
>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index d9a0443..f095109 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -1217,7 +1217,7 @@ static PCIBus *i440fx_init(I440FXPMCState **pi440fx_state, int *piix3_devfn,
>>      PCIHostState *h;
>>  
>>      s = I440FX(object_new(TYPE_I440FX));
>> -    h = PCI_HOST(s);
>> +    h = PCI_HOST_BRIDGE(s);
>>  
>>      /* FIXME make a properties */
>>      h->address_space = address_space_mem;
>> diff --git a/hw/pci_host.c b/hw/pci_host.c
>> index 3950e94..4e10042 100644
>> --- a/hw/pci_host.c
>> +++ b/hw/pci_host.c
>> @@ -165,11 +165,25 @@ const MemoryRegionOps pci_host_data_be_ops = {
>>      .endianness = DEVICE_BIG_ENDIAN,
>>  };
>>  
>> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
>> +{
>> +    object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
>> +}
>> +
>> +static void pci_host_initfn(Object *obj)
>> +{
>> +    PCIHostState *s = PCI_HOST_BRIDGE(obj);
>> +
>> +    object_property_add_link(obj, "mmio", "memory-region",
>> +                            (Object **)&s->address_space, NULL);
>> +}
>> +
>>  static const TypeInfo pci_host_type_info = {
>>      .name = TYPE_PCI_HOST_BRIDGE,
>>      .parent = TYPE_SYS_BUS_DEVICE,
>>      .abstract = true,
>>      .instance_size = sizeof(PCIHostState),
>> +    .instance_init = pci_host_initfn,
>>  };
>>  
>>  static void pci_host_register_types(void)
>> diff --git a/hw/pci_host.h b/hw/pci_host.h
>> index 4b9c300..9f28728 100644
>> --- a/hw/pci_host.h
>> +++ b/hw/pci_host.h
>> @@ -54,6 +54,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
>>  void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
>>  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
>>  
>> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
>> +
>>  extern const MemoryRegionOps pci_host_conf_le_ops;
>>  extern const MemoryRegionOps pci_host_conf_be_ops;
>>  extern const MemoryRegionOps pci_host_data_le_ops;
>> diff --git a/hw/piix3.c b/hw/piix3.c
>> index eca6ec8..3b69b15 100644
>> --- a/hw/piix3.c
>> +++ b/hw/piix3.c
>> @@ -204,7 +204,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
>>      k->class_id     = PCI_CLASS_BRIDGE_ISA;
>>  }
>>  
>> -static TypeInfo piix3_info = {
>> +static const TypeInfo piix3_info = {
>>      .name          = TYPE_PIIX3,
>>      .parent        = TYPE_PCI_DEVICE,
>>      .instance_size = sizeof(PIIX3State),
>> @@ -219,7 +219,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
>>      k->config_write = piix3_write_config_xen;
>>  };
>>  
>> -static TypeInfo piix3_xen_info = {
>> +static const TypeInfo piix3_xen_info = {
>>      .name          = "PIIX3-xen",
>>      .parent        = TYPE_PIIX3,
>>      .instance_size = sizeof(PIIX3State),
>> 
>
>
>-- 
>SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
>
diff mbox

Patch

diff --git a/hw/i440fx.c b/hw/i440fx.c
index 720a25a..fdf040b 100644
--- a/hw/i440fx.c
+++ b/hw/i440fx.c
@@ -191,7 +191,7 @@  static const VMStateDescription vmstate_i440fx_pmc = {
 static int i440fx_realize(SysBusDevice *dev)
 {
     I440FXState *s = I440FX(dev);
-    PCIHostState *h = PCI_HOST(s);
+    PCIHostState *h = PCI_HOST_BRIDGE(s);
     int bios_size, isa_bios_size;
     char *filename;
     int ret;
@@ -401,7 +401,7 @@  static void i440fx_pmc_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_i440fx_pmc;
 }
 
-static TypeInfo i440fx_pmc_info = {
+static const TypeInfo i440fx_pmc_info = {
     .name          = TYPE_I440FX_PMC,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(I440FXPMCState),
@@ -418,7 +418,7 @@  static void i440fx_class_init(ObjectClass *klass, void *data)
     dc->no_user = 1;
 }
 
-static TypeInfo i440fx_info = {
+static const TypeInfo i440fx_info = {
     .name          = TYPE_I440FX,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(I440FXState),
diff --git a/hw/pc.c b/hw/pc.c
index d9a0443..f095109 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1217,7 +1217,7 @@  static PCIBus *i440fx_init(I440FXPMCState **pi440fx_state, int *piix3_devfn,
     PCIHostState *h;
 
     s = I440FX(object_new(TYPE_I440FX));
-    h = PCI_HOST(s);
+    h = PCI_HOST_BRIDGE(s);
 
     /* FIXME make a properties */
     h->address_space = address_space_mem;
diff --git a/hw/pci_host.c b/hw/pci_host.c
index 3950e94..4e10042 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -165,11 +165,25 @@  const MemoryRegionOps pci_host_data_be_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
+{
+    object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
+}
+
+static void pci_host_initfn(Object *obj)
+{
+    PCIHostState *s = PCI_HOST_BRIDGE(obj);
+
+    object_property_add_link(obj, "mmio", "memory-region",
+                            (Object **)&s->address_space, NULL);
+}
+
 static const TypeInfo pci_host_type_info = {
     .name = TYPE_PCI_HOST_BRIDGE,
     .parent = TYPE_SYS_BUS_DEVICE,
     .abstract = true,
     .instance_size = sizeof(PCIHostState),
+    .instance_init = pci_host_initfn,
 };
 
 static void pci_host_register_types(void)
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 4b9c300..9f28728 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -54,6 +54,8 @@  uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
+
 extern const MemoryRegionOps pci_host_conf_le_ops;
 extern const MemoryRegionOps pci_host_conf_be_ops;
 extern const MemoryRegionOps pci_host_data_le_ops;
diff --git a/hw/piix3.c b/hw/piix3.c
index eca6ec8..3b69b15 100644
--- a/hw/piix3.c
+++ b/hw/piix3.c
@@ -204,7 +204,7 @@  static void piix3_class_init(ObjectClass *klass, void *data)
     k->class_id     = PCI_CLASS_BRIDGE_ISA;
 }
 
-static TypeInfo piix3_info = {
+static const TypeInfo piix3_info = {
     .name          = TYPE_PIIX3,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PIIX3State),
@@ -219,7 +219,7 @@  static void piix3_xen_class_init(ObjectClass *klass, void *data)
     k->config_write = piix3_write_config_xen;
 };
 
-static TypeInfo piix3_xen_info = {
+static const TypeInfo piix3_xen_info = {
     .name          = "PIIX3-xen",
     .parent        = TYPE_PIIX3,
     .instance_size = sizeof(PIIX3State),