[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v2][PATCH 2/5] hw:pci-host:piix: split i440fx_init



On 2014/7/31 17:53, Michael S. Tsirkin wrote:
On Thu, Jul 31, 2014 at 05:26:41PM +0800, Chen, Tiejun wrote:
On 2014/7/31 17:10, Michael S. Tsirkin wrote:
On Thu, Jul 31, 2014 at 02:31:36PM +0800, Tiejun Chen wrote:
We'd like to split i440fx_init and then we can share something
with other stuff.

Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>

I think this is too much work for very little benefit.
Just pass const char *type to i440fx_init.

You know we will introduce that faked PCIe device represented that PCH
later,

Later when? On top of this patch series? Would like to see it all
before applying this ...

so how to distinguish them? Are you saying I should check the type
like this?

if(Xen-Type)
{}
else
{}


No! Put the code in init function for the respective class,
pass type as an argument:

----
i440fx: make types configurable at run-time

Xen wants to supply a different pci and host devices,
inheriting i440fx devices. Make types configurable.

Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>

Looks this is not enough as well since after we use patch #3 to introduce that host bridge,

@@ -93,6 +93,9 @@ typedef struct PIIX3State {
 #define I440FX_PCI_DEVICE(obj) \
     OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)

+#define XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE(obj) \
+ OBJECT_CHECK(PCII440FXState, (obj), TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE)
+
 struct PCII440FXState {
     /*< private >*/
     PCIDevice parent_obj;

Inside i440fx_init(), we still need to replace that macro, I440FX_PCI_DEVICE, with XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE in xenigd case.

So looks we'd better split that i440fx_init() to address this point.

Thanks
Tiejun


-->

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index be8fdfe..86f295a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -230,7 +230,11 @@ extern int no_hpet;
  struct PCII440FXState;
  typedef struct PCII440FXState PCII440FXState;

-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
+#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
+#define TYPE_I440FX_PCI_DEVICE "i440FX"
+
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+                    PCII440FXState **pi440fx_state, int *piix_devfn,
                      ISABus **isa_bus, qemu_irq *pic,
                      MemoryRegion *address_space_mem,
                      MemoryRegion *address_space_io,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 31125b7..e0979cd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -194,7 +194,9 @@ static void pc_init1(MachineState *machine,
      }

      if (pci_enabled) {
-        pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
+        pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
+                              TYPE_I440FX_PCI_DEVICE,
+                              &i440fx_state, &piix3_devfn, &isa_bus, gsi,
                                system_memory, system_io, machine->ram_size,
                                below_4g_mem_size,
                                above_4g_mem_size,
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index e0e0946..0cd82b8 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,6 @@
   * http://download.intel.com/design/chipsets/datashts/29054901.pdf
   */

-#define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
  #define I440FX_PCI_HOST_BRIDGE(obj) \
      OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)

@@ -91,7 +90,6 @@ typedef struct PIIX3State {
      MemoryRegion rcr_mem;
  } PIIX3State;

-#define TYPE_I440FX_PCI_DEVICE "i440FX"
  #define I440FX_PCI_DEVICE(obj) \
      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)

@@ -305,7 +303,8 @@ static int i440fx_initfn(PCIDevice *dev)
      return 0;
  }

-PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+                    PCII440FXState **pi440fx_state,
                      int *piix3_devfn,
                      ISABus **isa_bus, qemu_irq *pic,
                      MemoryRegion *address_space_mem,
@@ -325,7 +324,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
      unsigned i;
      I440FXState *i440fx;

-    dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
+    dev = qdev_create(NULL, host_type);
      s = PCI_HOST_BRIDGE(dev);
      b = pci_bus_new(dev, NULL, pci_address_space,
                      address_space_io, 0, TYPE_PCI_BUS);
@@ -333,7 +332,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
      object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), 
NULL);
      qdev_init_nofail(dev);

-    d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
+    d = pci_create_simple(b, 0, pci_type);
      *pi440fx_state = I440FX_PCI_DEVICE(d);
      f = *pi440fx_state;
      f->system_memory = address_space_mem;



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.