[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 ...

I will send this with other IGD stuff after this series is fine to you since its just creating a simple PCIe device.

I think you should know this whole story since as you guys discussed we don't fix that PCH at 1f.0. So it may be like this,

static int create_pseudo_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
{

        struct PCIDevice *dev;

        char rid;

        /* We havt to use a simple PCI device to fake this ISA bridge
         * to avoid making some confusion to BIOS and ACPI.
         */
        dev = pci_create(bus, -1, "pseudo-intel-pch-isa-bridge");

        qdev_init_nofail(&dev->qdev);
        pci_config_set_vendor_id(dev->config, XEN_SUBSYSTEM_ID));
                                                ^
                                I don't remember this exactly.
        pci_config_set_device_id(dev->config, hdev->device_id);

        return 0;
}


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:

If you mean we don't introduce any "if/else", I still don't understand how to insert such that function above, could you show this exactly?

Tiejun


----
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>

-->

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®.