[Qemu-devel] [v4][PATCH 0/5] xen: introduce new machine for IGD passthrough

[Qemu-devel] [v4][PATCH 0/5] xen: introduce new machine for IGD passthrough

[Tiejun Chen]

v4:

* Rebase on latest tree
* Drop patch #2
* Regenerate patches after Michael introduce patch #1
* We need to use this pci_type as a index to reuse I440FX_PCI_DEVICE()
* Test: boot with a preinstalled winxp
  ./i386-softmmu/qemu-system-i386 -hda winxp-32.img -m 2560 -boot c -machine pc

v3:

* Drop patch #4
* Add one patch #1 from Michael
* Rebase
* In./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

v2:

* Fix some coding style
* New patch to separate i440fx_init
* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
* Based on patch #2 to regenerate
* Unify prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough like patch #3
* Test: boot with a preinstalled ubuntu 14.04
  ./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

As we discussed we need to create a separate machine to support current
IGD passthrough.

----------------------------------------------------------------
Michael S. Tsirkin (1):
      i440fx: make types configurable at run-time

Tiejun Chen (4):
      pc_init1: pass parameters just with types
      I440FX_PCI_DEVICE: add pci_type to index
      xen:hw:pci-host:piix: create host bridge to passthrough
      xen:hw:i386:pc_piix: introduce new machine for IGD passthrough

 hw/i386/pc_piix.c    | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/pci-host/piix.c   | 58 ++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/hw/i386/pc.h |  8 +++++++-
 3 files changed, 110 insertions(+), 16 deletions(-)

Thanks
Tiejun

[v4][PATCH 0/5] xen: introduce new machine for IGD passthrough

[Tiejun Chen]

v4:

* Rebase on latest tree
* Drop patch #2
* Regenerate patches after Michael introduce patch #1
* We need to use this pci_type as a index to reuse I440FX_PCI_DEVICE()
* Test: boot with a preinstalled winxp
  ./i386-softmmu/qemu-system-i386 -hda winxp-32.img -m 2560 -boot c -machine pc

v3:

* Drop patch #4
* Add one patch #1 from Michael
* Rebase
* In./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

v2:

* Fix some coding style
* New patch to separate i440fx_init
* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
* Based on patch #2 to regenerate
* Unify prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough like patch #3
* Test: boot with a preinstalled ubuntu 14.04
  ./i386-softmmu/qemu-system-i386 -hda test.img -m 2560 -boot c -machine pc

As we discussed we need to create a separate machine to support current
IGD passthrough.

----------------------------------------------------------------
Michael S. Tsirkin (1):
      i440fx: make types configurable at run-time

Tiejun Chen (4):
      pc_init1: pass parameters just with types
      I440FX_PCI_DEVICE: add pci_type to index
      xen:hw:pci-host:piix: create host bridge to passthrough
      xen:hw:i386:pc_piix: introduce new machine for IGD passthrough

 hw/i386/pc_piix.c    | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/pci-host/piix.c   | 58 ++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/hw/i386/pc.h |  8 +++++++-
 3 files changed, 110 insertions(+), 16 deletions(-)

Thanks
Tiejun

[Qemu-devel] [v4][PATCH 1/5] i440fx: make types configurable at run-time

[Tiejun Chen]

From: "Michael S. Tsirkin" <mst@redhat.com>

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

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c    | 4 +++-
 hw/pci-host/piix.c   | 9 ++++-----
 include/hw/i386/pc.h | 6 +++++-
 3 files changed, 12 insertions(+), 7 deletions(-)

v4:

* Just add From: Michael S. Tsirkin <mst@redhat.com>

v3:

* New patch from Michael

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4f22be8..bf26550 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -201,7 +201,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;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 863eefb..11fb72f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -235,7 +235,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,
-- 
1.9.1

[v4][PATCH 1/5] i440fx: make types configurable at run-time

[Tiejun Chen]

From: "Michael S. Tsirkin" <mst@redhat.com>

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

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c    | 4 +++-
 hw/pci-host/piix.c   | 9 ++++-----
 include/hw/i386/pc.h | 6 +++++-
 3 files changed, 12 insertions(+), 7 deletions(-)

v4:

* Just add From: Michael S. Tsirkin <mst@redhat.com>

v3:

* New patch from Michael

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4f22be8..bf26550 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -201,7 +201,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;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 863eefb..11fb72f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -235,7 +235,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,
-- 
1.9.1

[Qemu-devel] [v4][PATCH 2/5] pc_init1: pass parameters just with types

[Tiejun Chen]

Pass types to configure pc_init1().

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

v4:

* New patch to work for patch #1

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bf26550..2bf8046 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -74,7 +74,9 @@ static bool has_reserved_memory = true;
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      int pci_enabled,
-                     int kvmclock_enabled)
+                     int kvmclock_enabled,
+                     const char *host_type,
+                     const char *pci_type)
 {
     PCMachineState *pc_machine = PC_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
@@ -201,8 +203,8 @@ static void pc_init1(MachineState *machine,
     }
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
-                              TYPE_I440FX_PCI_DEVICE,
+        pci_bus = i440fx_init(host_type,
+                              pci_type,
                               &i440fx_state, &piix3_devfn, &isa_bus, gsi,
                               system_memory, system_io, machine->ram_size,
                               below_4g_mem_size,
@@ -303,7 +305,8 @@ static void pc_init1(MachineState *machine,
 
 static void pc_init_pci(MachineState *machine)
 {
-    pc_init1(machine, 1, 1);
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_compat_2_0(MachineState *machine)
@@ -419,7 +422,8 @@ static void pc_init_pci_1_2(MachineState *machine)
 static void pc_init_pci_no_kvmclock(MachineState *machine)
 {
     pc_compat_1_2(machine);
-    pc_init1(machine, 1, 0);
+    pc_init1(machine, 1, 0, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_init_isa(MachineState *machine)
@@ -437,7 +441,8 @@ static void pc_init_isa(MachineState *machine)
     }
     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
     enable_compat_apic_id_mode();
-    pc_init1(machine, 0, 1);
+    pc_init1(machine, 0, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 #ifdef CONFIG_XEN
-- 
1.9.1

[v4][PATCH 2/5] pc_init1: pass parameters just with types

[Tiejun Chen]

Pass types to configure pc_init1().

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

v4:

* New patch to work for patch #1

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bf26550..2bf8046 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -74,7 +74,9 @@ static bool has_reserved_memory = true;
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      int pci_enabled,
-                     int kvmclock_enabled)
+                     int kvmclock_enabled,
+                     const char *host_type,
+                     const char *pci_type)
 {
     PCMachineState *pc_machine = PC_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
@@ -201,8 +203,8 @@ static void pc_init1(MachineState *machine,
     }
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
-                              TYPE_I440FX_PCI_DEVICE,
+        pci_bus = i440fx_init(host_type,
+                              pci_type,
                               &i440fx_state, &piix3_devfn, &isa_bus, gsi,
                               system_memory, system_io, machine->ram_size,
                               below_4g_mem_size,
@@ -303,7 +305,8 @@ static void pc_init1(MachineState *machine,
 
 static void pc_init_pci(MachineState *machine)
 {
-    pc_init1(machine, 1, 1);
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_compat_2_0(MachineState *machine)
@@ -419,7 +422,8 @@ static void pc_init_pci_1_2(MachineState *machine)
 static void pc_init_pci_no_kvmclock(MachineState *machine)
 {
     pc_compat_1_2(machine);
-    pc_init1(machine, 1, 0);
+    pc_init1(machine, 1, 0, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 static void pc_init_isa(MachineState *machine)
@@ -437,7 +441,8 @@ static void pc_init_isa(MachineState *machine)
     }
     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
     enable_compat_apic_id_mode();
-    pc_init1(machine, 0, 1);
+    pc_init1(machine, 0, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_I440FX_PCI_DEVICE);
 }
 
 #ifdef CONFIG_XEN
-- 
1.9.1

[Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Tiejun Chen]

We need to use this index to reuse this macro later

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/pci-host/piix.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

v4:

* New patch to extend I440FX_PCI_DEVICE

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0cd82b8..4330599 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -90,8 +90,8 @@ typedef struct PIIX3State {
     MemoryRegion rcr_mem;
 } PIIX3State;
 
-#define I440FX_PCI_DEVICE(obj) \
-    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
+#define I440FX_PCI_DEVICE(obj, type) \
+    OBJECT_CHECK(PCII440FXState, (obj), type)
 
 struct PCII440FXState {
     /*< private >*/
@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
 static void i440fx_write_config(PCIDevice *dev,
                                 uint32_t address, uint32_t val, int len)
 {
-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
 
     /* XXX: implement SMRAM.D_LOCK */
     pci_default_write_config(dev, address, val, len);
@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
 
 static int i440fx_initfn(PCIDevice *dev)
 {
-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
 
     dev->config[I440FX_SMRAM] = 0x02;
 
@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
     qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, pci_type);
-    *pi440fx_state = I440FX_PCI_DEVICE(d);
+    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
     f = *pi440fx_state;
     f->system_memory = address_space_mem;
     f->pci_address_space = pci_address_space;
-- 
1.9.1

[v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Tiejun Chen]

We need to use this index to reuse this macro later

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/pci-host/piix.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

v4:

* New patch to extend I440FX_PCI_DEVICE

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0cd82b8..4330599 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -90,8 +90,8 @@ typedef struct PIIX3State {
     MemoryRegion rcr_mem;
 } PIIX3State;
 
-#define I440FX_PCI_DEVICE(obj) \
-    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
+#define I440FX_PCI_DEVICE(obj, type) \
+    OBJECT_CHECK(PCII440FXState, (obj), type)
 
 struct PCII440FXState {
     /*< private >*/
@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
 static void i440fx_write_config(PCIDevice *dev,
                                 uint32_t address, uint32_t val, int len)
 {
-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
 
     /* XXX: implement SMRAM.D_LOCK */
     pci_default_write_config(dev, address, val, len);
@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
 
 static int i440fx_initfn(PCIDevice *dev)
 {
-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
 
     dev->config[I440FX_SMRAM] = 0x02;
 
@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
     qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, pci_type);
-    *pi440fx_state = I440FX_PCI_DEVICE(d);
+    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
     f = *pi440fx_state;
     f->system_memory = address_space_mem;
     f->pci_address_space = pci_address_space;
-- 
1.9.1

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> We need to use this index to reuse this macro later
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>

Which index?
Most users don't need to change.
Just open-code OBJECT_CHECK where necessary, or add
a new wrapper.

> ---
>  hw/pci-host/piix.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> v4:
> 
> * New patch to extend I440FX_PCI_DEVICE
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 0cd82b8..4330599 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -90,8 +90,8 @@ typedef struct PIIX3State {
>      MemoryRegion rcr_mem;
>  } PIIX3State;
>  
> -#define I440FX_PCI_DEVICE(obj) \
> -    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> +#define I440FX_PCI_DEVICE(obj, type) \
> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>  
>  struct PCII440FXState {
>      /*< private >*/
> @@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
>  static void i440fx_write_config(PCIDevice *dev,
>                                  uint32_t address, uint32_t val, int len)
>  {
> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>  
>      /* XXX: implement SMRAM.D_LOCK */
>      pci_default_write_config(dev, address, val, len);
> @@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>  
>  static int i440fx_initfn(PCIDevice *dev)
>  {
> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>  
>      dev->config[I440FX_SMRAM] = 0x02;
>  
> @@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>      qdev_init_nofail(dev);
>  
>      d = pci_create_simple(b, 0, pci_type);
> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
> +    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
>      f = *pi440fx_state;
>      f->system_memory = address_space_mem;
>      f->pci_address_space = pci_address_space;
> -- 
> 1.9.1

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> We need to use this index to reuse this macro later
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>

Which index?
Most users don't need to change.
Just open-code OBJECT_CHECK where necessary, or add
a new wrapper.

> ---
>  hw/pci-host/piix.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> v4:
> 
> * New patch to extend I440FX_PCI_DEVICE
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 0cd82b8..4330599 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -90,8 +90,8 @@ typedef struct PIIX3State {
>      MemoryRegion rcr_mem;
>  } PIIX3State;
>  
> -#define I440FX_PCI_DEVICE(obj) \
> -    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> +#define I440FX_PCI_DEVICE(obj, type) \
> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>  
>  struct PCII440FXState {
>      /*< private >*/
> @@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
>  static void i440fx_write_config(PCIDevice *dev,
>                                  uint32_t address, uint32_t val, int len)
>  {
> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>  
>      /* XXX: implement SMRAM.D_LOCK */
>      pci_default_write_config(dev, address, val, len);
> @@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>  
>  static int i440fx_initfn(PCIDevice *dev)
>  {
> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>  
>      dev->config[I440FX_SMRAM] = 0x02;
>  
> @@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>      qdev_init_nofail(dev);
>  
>      d = pci_create_simple(b, 0, pci_type);
> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
> +    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
>      f = *pi440fx_state;
>      f->system_memory = address_space_mem;
>      f->pci_address_space = pci_address_space;
> -- 
> 1.9.1

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>> We need to use this index to reuse this macro later
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>
> Which index?
> Most users don't need to change.
> Just open-code OBJECT_CHECK where necessary, or add
> a new wrapper.

Okay so what about this?

     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE

     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
     object with type, then we can reuse i440fx_init() simply.

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

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0cd82b8..8c74653 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -93,6 +93,9 @@ typedef struct PIIX3State {
  #define I440FX_PCI_DEVICE(obj) \
      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)

+#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
+    OBJECT_CHECK(PCII440FXState, (obj), type)
+
  struct PCII440FXState {
      /*< private >*/
      PCIDevice parent_obj;
@@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const 
char *pci_type,
      qdev_init_nofail(dev);

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


Thanks
Tiejun

>
>> ---
>>   hw/pci-host/piix.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> v4:
>>
>> * New patch to extend I440FX_PCI_DEVICE
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 0cd82b8..4330599 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -90,8 +90,8 @@ typedef struct PIIX3State {
>>       MemoryRegion rcr_mem;
>>   } PIIX3State;
>>
>> -#define I440FX_PCI_DEVICE(obj) \
>> -    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>> +#define I440FX_PCI_DEVICE(obj, type) \
>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>
>>   struct PCII440FXState {
>>       /*< private >*/
>> @@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
>>   static void i440fx_write_config(PCIDevice *dev,
>>                                   uint32_t address, uint32_t val, int len)
>>   {
>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>
>>       /* XXX: implement SMRAM.D_LOCK */
>>       pci_default_write_config(dev, address, val, len);
>> @@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>>
>>   static int i440fx_initfn(PCIDevice *dev)
>>   {
>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>
>>       dev->config[I440FX_SMRAM] = 0x02;
>>
>> @@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>       qdev_init_nofail(dev);
>>
>>       d = pci_create_simple(b, 0, pci_type);
>> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
>> +    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
>>       f = *pi440fx_state;
>>       f->system_memory = address_space_mem;
>>       f->pci_address_space = pci_address_space;
>> --
>> 1.9.1
>
>

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>> We need to use this index to reuse this macro later
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>
> Which index?
> Most users don't need to change.
> Just open-code OBJECT_CHECK where necessary, or add
> a new wrapper.

Okay so what about this?

     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE

     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
     object with type, then we can reuse i440fx_init() simply.

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

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0cd82b8..8c74653 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -93,6 +93,9 @@ typedef struct PIIX3State {
  #define I440FX_PCI_DEVICE(obj) \
      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)

+#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
+    OBJECT_CHECK(PCII440FXState, (obj), type)
+
  struct PCII440FXState {
      /*< private >*/
      PCIDevice parent_obj;
@@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const 
char *pci_type,
      qdev_init_nofail(dev);

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


Thanks
Tiejun

>
>> ---
>>   hw/pci-host/piix.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> v4:
>>
>> * New patch to extend I440FX_PCI_DEVICE
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 0cd82b8..4330599 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -90,8 +90,8 @@ typedef struct PIIX3State {
>>       MemoryRegion rcr_mem;
>>   } PIIX3State;
>>
>> -#define I440FX_PCI_DEVICE(obj) \
>> -    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>> +#define I440FX_PCI_DEVICE(obj, type) \
>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>
>>   struct PCII440FXState {
>>       /*< private >*/
>> @@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
>>   static void i440fx_write_config(PCIDevice *dev,
>>                                   uint32_t address, uint32_t val, int len)
>>   {
>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>
>>       /* XXX: implement SMRAM.D_LOCK */
>>       pci_default_write_config(dev, address, val, len);
>> @@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>>
>>   static int i440fx_initfn(PCIDevice *dev)
>>   {
>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>
>>       dev->config[I440FX_SMRAM] = 0x02;
>>
>> @@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>       qdev_init_nofail(dev);
>>
>>       d = pci_create_simple(b, 0, pci_type);
>> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
>> +    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
>>       f = *pi440fx_state;
>>       f->system_memory = address_space_mem;
>>       f->pci_address_space = pci_address_space;
>> --
>> 1.9.1
>
>

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> >On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> >>We need to use this index to reuse this macro later
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >
> >Which index?
> >Most users don't need to change.
> >Just open-code OBJECT_CHECK where necessary, or add
> >a new wrapper.
> 
> Okay so what about this?
> 
>     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
> 
>     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>     object with type, then we can reuse i440fx_init() simply.
> 
>     Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 0cd82b8..8c74653 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>  #define I440FX_PCI_DEVICE(obj) \
>      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> 
> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
> +    OBJECT_CHECK(PCII440FXState, (obj), type)

This is just wrong. If you are casting to PCII440FXState,
there is no reason not to use TYPE_I440FX_PCI_DEVICE.

> +
>  struct PCII440FXState {
>      /*< private >*/
>      PCIDevice parent_obj;
> @@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char
> *pci_type,
>      qdev_init_nofail(dev);
> 
>      d = pci_create_simple(b, 0, pci_type);
> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
> +    *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type);
>      f = *pi440fx_state;
>      f->system_memory = address_space_mem;
>      f->pci_address_space = pci_address_space;
> 
> 
> Thanks
> Tiejun
> 
> >
> >>---
> >>  hw/pci-host/piix.c | 10 +++++-----
> >>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >>v4:
> >>
> >>* New patch to extend I440FX_PCI_DEVICE
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 0cd82b8..4330599 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -90,8 +90,8 @@ typedef struct PIIX3State {
> >>      MemoryRegion rcr_mem;
> >>  } PIIX3State;
> >>
> >>-#define I440FX_PCI_DEVICE(obj) \
> >>-    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>+#define I440FX_PCI_DEVICE(obj, type) \
> >>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >>
> >>  struct PCII440FXState {
> >>      /*< private >*/
> >>@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
> >>  static void i440fx_write_config(PCIDevice *dev,
> >>                                  uint32_t address, uint32_t val, int len)
> >>  {
> >>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>
> >>      /* XXX: implement SMRAM.D_LOCK */
> >>      pci_default_write_config(dev, address, val, len);
> >>@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
> >>
> >>  static int i440fx_initfn(PCIDevice *dev)
> >>  {
> >>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>
> >>      dev->config[I440FX_SMRAM] = 0x02;
> >>
> >>@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>      qdev_init_nofail(dev);
> >>
> >>      d = pci_create_simple(b, 0, pci_type);
> >>-    *pi440fx_state = I440FX_PCI_DEVICE(d);
> >>+    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
> >>      f = *pi440fx_state;
> >>      f->system_memory = address_space_mem;
> >>      f->pci_address_space = pci_address_space;
> >>--
> >>1.9.1
> >
> >

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> >On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> >>We need to use this index to reuse this macro later
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >
> >Which index?
> >Most users don't need to change.
> >Just open-code OBJECT_CHECK where necessary, or add
> >a new wrapper.
> 
> Okay so what about this?
> 
>     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
> 
>     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>     object with type, then we can reuse i440fx_init() simply.
> 
>     Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 0cd82b8..8c74653 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>  #define I440FX_PCI_DEVICE(obj) \
>      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> 
> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
> +    OBJECT_CHECK(PCII440FXState, (obj), type)

This is just wrong. If you are casting to PCII440FXState,
there is no reason not to use TYPE_I440FX_PCI_DEVICE.

> +
>  struct PCII440FXState {
>      /*< private >*/
>      PCIDevice parent_obj;
> @@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char
> *pci_type,
>      qdev_init_nofail(dev);
> 
>      d = pci_create_simple(b, 0, pci_type);
> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
> +    *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type);
>      f = *pi440fx_state;
>      f->system_memory = address_space_mem;
>      f->pci_address_space = pci_address_space;
> 
> 
> Thanks
> Tiejun
> 
> >
> >>---
> >>  hw/pci-host/piix.c | 10 +++++-----
> >>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >>v4:
> >>
> >>* New patch to extend I440FX_PCI_DEVICE
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 0cd82b8..4330599 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -90,8 +90,8 @@ typedef struct PIIX3State {
> >>      MemoryRegion rcr_mem;
> >>  } PIIX3State;
> >>
> >>-#define I440FX_PCI_DEVICE(obj) \
> >>-    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>+#define I440FX_PCI_DEVICE(obj, type) \
> >>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >>
> >>  struct PCII440FXState {
> >>      /*< private >*/
> >>@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
> >>  static void i440fx_write_config(PCIDevice *dev,
> >>                                  uint32_t address, uint32_t val, int len)
> >>  {
> >>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>
> >>      /* XXX: implement SMRAM.D_LOCK */
> >>      pci_default_write_config(dev, address, val, len);
> >>@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
> >>
> >>  static int i440fx_initfn(PCIDevice *dev)
> >>  {
> >>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>
> >>      dev->config[I440FX_SMRAM] = 0x02;
> >>
> >>@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>      qdev_init_nofail(dev);
> >>
> >>      d = pci_create_simple(b, 0, pci_type);
> >>-    *pi440fx_state = I440FX_PCI_DEVICE(d);
> >>+    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
> >>      f = *pi440fx_state;
> >>      f->system_memory = address_space_mem;
> >>      f->pci_address_space = pci_address_space;
> >>--
> >>1.9.1
> >
> >

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/7 5:07, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>> We need to use this index to reuse this macro later
>>>>
>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>
>>> Which index?
>>> Most users don't need to change.
>>> Just open-code OBJECT_CHECK where necessary, or add
>>> a new wrapper.
>>
>> Okay so what about this?
>>
>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>
>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>      object with type, then we can reuse i440fx_init() simply.
>>
>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 0cd82b8..8c74653 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>   #define I440FX_PCI_DEVICE(obj) \
>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>
>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>
> This is just wrong. If you are casting to PCII440FXState,

Why? We will have two different QOM typenames of PCII440FXStates.

> there is no reason not to use TYPE_I440FX_PCI_DEVICE.

As you know we already have this original,

static const TypeInfo i440fx_info = {
     .name          = TYPE_I440FX_PCI_DEVICE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCII440FXState),
     .class_init    = i440fx_class_init,
};

and in patch #4, we will register that new host bridge to IGD passthrough:

static const TypeInfo xen_igd_passthrough_i440fx_info = {
     .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCII440FXState),
     .class_init    = xen_igd_passthrough_i440fx_class_init,
};

After that, we have two different QOM typenames of PCII440FXStates, 
TYPE_I440FX_PCI_DEVICE and TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, 
right? So if you think we still should use this original,

#define I440FX_PCI_DEVICE(obj) \
	OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)

I really don't understand how to distinguish TYPE_I440FX_PCI_DEVICE and 
TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE just with I440FX_PCI_DEVICE(obj).

This should involve two places:

#1: static int i440fx_initfn(PCIDevice *dev)
{
     PCII440FXState *d = I440FX_PCI_DEVICE(dev);
     ...

#2: 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,
                     MemoryRegion *address_space_io,
                     ram_addr_t ram_size,
                     ram_addr_t below_4g_mem_size,
                     ram_addr_t above_4g_mem_size,
                     MemoryRegion *pci_address_space,
                     MemoryRegion *ram_memory)
{
     ...
     PCIDevice *d;
     ...
     *pi440fx_state = I440FX_PCI_DEVICE(d);
     ...

So if you think I'm wrong please show me how to do just with 
I440FX_PCI_DEVICE() to work all scenarios from your point of view, then 
I'd like to followup yours to validate.

Thanks
Tiejun

>
>> +
>>   struct PCII440FXState {
>>       /*< private >*/
>>       PCIDevice parent_obj;
>> @@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char
>> *pci_type,
>>       qdev_init_nofail(dev);
>>
>>       d = pci_create_simple(b, 0, pci_type);
>> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
>> +    *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type);
>>       f = *pi440fx_state;
>>       f->system_memory = address_space_mem;
>>       f->pci_address_space = pci_address_space;
>>
>>
>> Thanks
>> Tiejun
>>
>>>
>>>> ---
>>>>   hw/pci-host/piix.c | 10 +++++-----
>>>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> v4:
>>>>
>>>> * New patch to extend I440FX_PCI_DEVICE
>>>>
>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>> index 0cd82b8..4330599 100644
>>>> --- a/hw/pci-host/piix.c
>>>> +++ b/hw/pci-host/piix.c
>>>> @@ -90,8 +90,8 @@ typedef struct PIIX3State {
>>>>       MemoryRegion rcr_mem;
>>>>   } PIIX3State;
>>>>
>>>> -#define I440FX_PCI_DEVICE(obj) \
>>>> -    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>> +#define I440FX_PCI_DEVICE(obj, type) \
>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>>
>>>>   struct PCII440FXState {
>>>>       /*< private >*/
>>>> @@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
>>>>   static void i440fx_write_config(PCIDevice *dev,
>>>>                                   uint32_t address, uint32_t val, int len)
>>>>   {
>>>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>>>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>>>
>>>>       /* XXX: implement SMRAM.D_LOCK */
>>>>       pci_default_write_config(dev, address, val, len);
>>>> @@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>>>>
>>>>   static int i440fx_initfn(PCIDevice *dev)
>>>>   {
>>>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>>>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>>>
>>>>       dev->config[I440FX_SMRAM] = 0x02;
>>>>
>>>> @@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>>>       qdev_init_nofail(dev);
>>>>
>>>>       d = pci_create_simple(b, 0, pci_type);
>>>> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
>>>> +    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
>>>>       f = *pi440fx_state;
>>>>       f->system_memory = address_space_mem;
>>>>       f->pci_address_space = pci_address_space;
>>>> --
>>>> 1.9.1
>>>
>>>
>

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/7 5:07, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>> We need to use this index to reuse this macro later
>>>>
>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>
>>> Which index?
>>> Most users don't need to change.
>>> Just open-code OBJECT_CHECK where necessary, or add
>>> a new wrapper.
>>
>> Okay so what about this?
>>
>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>
>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>      object with type, then we can reuse i440fx_init() simply.
>>
>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 0cd82b8..8c74653 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>   #define I440FX_PCI_DEVICE(obj) \
>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>
>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>
> This is just wrong. If you are casting to PCII440FXState,

Why? We will have two different QOM typenames of PCII440FXStates.

> there is no reason not to use TYPE_I440FX_PCI_DEVICE.

As you know we already have this original,

static const TypeInfo i440fx_info = {
     .name          = TYPE_I440FX_PCI_DEVICE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCII440FXState),
     .class_init    = i440fx_class_init,
};

and in patch #4, we will register that new host bridge to IGD passthrough:

static const TypeInfo xen_igd_passthrough_i440fx_info = {
     .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCII440FXState),
     .class_init    = xen_igd_passthrough_i440fx_class_init,
};

After that, we have two different QOM typenames of PCII440FXStates, 
TYPE_I440FX_PCI_DEVICE and TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, 
right? So if you think we still should use this original,

#define I440FX_PCI_DEVICE(obj) \
	OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)

I really don't understand how to distinguish TYPE_I440FX_PCI_DEVICE and 
TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE just with I440FX_PCI_DEVICE(obj).

This should involve two places:

#1: static int i440fx_initfn(PCIDevice *dev)
{
     PCII440FXState *d = I440FX_PCI_DEVICE(dev);
     ...

#2: 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,
                     MemoryRegion *address_space_io,
                     ram_addr_t ram_size,
                     ram_addr_t below_4g_mem_size,
                     ram_addr_t above_4g_mem_size,
                     MemoryRegion *pci_address_space,
                     MemoryRegion *ram_memory)
{
     ...
     PCIDevice *d;
     ...
     *pi440fx_state = I440FX_PCI_DEVICE(d);
     ...

So if you think I'm wrong please show me how to do just with 
I440FX_PCI_DEVICE() to work all scenarios from your point of view, then 
I'd like to followup yours to validate.

Thanks
Tiejun

>
>> +
>>   struct PCII440FXState {
>>       /*< private >*/
>>       PCIDevice parent_obj;
>> @@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char
>> *pci_type,
>>       qdev_init_nofail(dev);
>>
>>       d = pci_create_simple(b, 0, pci_type);
>> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
>> +    *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type);
>>       f = *pi440fx_state;
>>       f->system_memory = address_space_mem;
>>       f->pci_address_space = pci_address_space;
>>
>>
>> Thanks
>> Tiejun
>>
>>>
>>>> ---
>>>>   hw/pci-host/piix.c | 10 +++++-----
>>>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> v4:
>>>>
>>>> * New patch to extend I440FX_PCI_DEVICE
>>>>
>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>> index 0cd82b8..4330599 100644
>>>> --- a/hw/pci-host/piix.c
>>>> +++ b/hw/pci-host/piix.c
>>>> @@ -90,8 +90,8 @@ typedef struct PIIX3State {
>>>>       MemoryRegion rcr_mem;
>>>>   } PIIX3State;
>>>>
>>>> -#define I440FX_PCI_DEVICE(obj) \
>>>> -    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>> +#define I440FX_PCI_DEVICE(obj, type) \
>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>>
>>>>   struct PCII440FXState {
>>>>       /*< private >*/
>>>> @@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
>>>>   static void i440fx_write_config(PCIDevice *dev,
>>>>                                   uint32_t address, uint32_t val, int len)
>>>>   {
>>>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>>>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>>>
>>>>       /* XXX: implement SMRAM.D_LOCK */
>>>>       pci_default_write_config(dev, address, val, len);
>>>> @@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
>>>>
>>>>   static int i440fx_initfn(PCIDevice *dev)
>>>>   {
>>>> -    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>>>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
>>>>
>>>>       dev->config[I440FX_SMRAM] = 0x02;
>>>>
>>>> @@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>>>       qdev_init_nofail(dev);
>>>>
>>>>       d = pci_create_simple(b, 0, pci_type);
>>>> -    *pi440fx_state = I440FX_PCI_DEVICE(d);
>>>> +    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
>>>>       f = *pi440fx_state;
>>>>       f->system_memory = address_space_mem;
>>>>       f->pci_address_space = pci_address_space;
>>>> --
>>>> 1.9.1
>>>
>>>
>

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
> 
> 
> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
> >On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
> >>On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> >>>On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> >>>>We need to use this index to reuse this macro later
> >>>>
> >>>>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>
> >>>Which index?
> >>>Most users don't need to change.
> >>>Just open-code OBJECT_CHECK where necessary, or add
> >>>a new wrapper.
> >>
> >>Okay so what about this?
> >>
> >>     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
> >>
> >>     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
> >>     object with type, then we can reuse i440fx_init() simply.
> >>
> >>     Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 0cd82b8..8c74653 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -93,6 +93,9 @@ typedef struct PIIX3State {
> >>  #define I440FX_PCI_DEVICE(obj) \
> >>      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>
> >>+#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
> >>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >
> >This is just wrong. If you are casting to PCII440FXState,
> 
> Why? We will have two different QOM typenames of PCII440FXStates.
> 
> >there is no reason not to use TYPE_I440FX_PCI_DEVICE.
> 
> As you know we already have this original,
> 
> static const TypeInfo i440fx_info = {
>     .name          = TYPE_I440FX_PCI_DEVICE,
>     .parent        = TYPE_PCI_DEVICE,
>     .instance_size = sizeof(PCII440FXState),
>     .class_init    = i440fx_class_init,
> };
> 
> and in patch #4, we will register that new host bridge to IGD passthrough:
> 
> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>     .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>     .parent        = TYPE_PCI_DEVICE,
>     .instance_size = sizeof(PCII440FXState),
>     .class_init    = xen_igd_passthrough_i440fx_class_init,
> };

My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
Then you can reuse regular piix code which casts to
TYPE_I440FX_PCI_DEVICE because
TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
be a subclass of TYPE_I440FX_PCI_DEVICE.

> After that, we have two different QOM typenames of PCII440FXStates,
> TYPE_I440FX_PCI_DEVICE and TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> right? So if you think we still should use this original,
> 
> #define I440FX_PCI_DEVICE(obj) \
> 	OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> 
> I really don't understand how to distinguish TYPE_I440FX_PCI_DEVICE and
> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE just with I440FX_PCI_DEVICE(obj).
> 
> This should involve two places:
> 
> #1: static int i440fx_initfn(PCIDevice *dev)
> {
>     PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>     ...
> 
> #2: 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,
>                     MemoryRegion *address_space_io,
>                     ram_addr_t ram_size,
>                     ram_addr_t below_4g_mem_size,
>                     ram_addr_t above_4g_mem_size,
>                     MemoryRegion *pci_address_space,
>                     MemoryRegion *ram_memory)
> {
>     ...
>     PCIDevice *d;
>     ...
>     *pi440fx_state = I440FX_PCI_DEVICE(d);
>     ...
> 
> So if you think I'm wrong please show me how to do just with
> I440FX_PCI_DEVICE() to work all scenarios from your point of view, then I'd
> like to followup yours to validate.
> 
> Thanks
> Tiejun


See above.

> >
> >>+
> >>  struct PCII440FXState {
> >>      /*< private >*/
> >>      PCIDevice parent_obj;
> >>@@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char
> >>*pci_type,
> >>      qdev_init_nofail(dev);
> >>
> >>      d = pci_create_simple(b, 0, pci_type);
> >>-    *pi440fx_state = I440FX_PCI_DEVICE(d);
> >>+    *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type);
> >>      f = *pi440fx_state;
> >>      f->system_memory = address_space_mem;
> >>      f->pci_address_space = pci_address_space;
> >>
> >>
> >>Thanks
> >>Tiejun
> >>
> >>>
> >>>>---
> >>>>  hw/pci-host/piix.c | 10 +++++-----
> >>>>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>>>
> >>>>v4:
> >>>>
> >>>>* New patch to extend I440FX_PCI_DEVICE
> >>>>
> >>>>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>>>index 0cd82b8..4330599 100644
> >>>>--- a/hw/pci-host/piix.c
> >>>>+++ b/hw/pci-host/piix.c
> >>>>@@ -90,8 +90,8 @@ typedef struct PIIX3State {
> >>>>      MemoryRegion rcr_mem;
> >>>>  } PIIX3State;
> >>>>
> >>>>-#define I440FX_PCI_DEVICE(obj) \
> >>>>-    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>>>+#define I440FX_PCI_DEVICE(obj, type) \
> >>>>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >>>>
> >>>>  struct PCII440FXState {
> >>>>      /*< private >*/
> >>>>@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
> >>>>  static void i440fx_write_config(PCIDevice *dev,
> >>>>                                  uint32_t address, uint32_t val, int len)
> >>>>  {
> >>>>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>>>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>>>
> >>>>      /* XXX: implement SMRAM.D_LOCK */
> >>>>      pci_default_write_config(dev, address, val, len);
> >>>>@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
> >>>>
> >>>>  static int i440fx_initfn(PCIDevice *dev)
> >>>>  {
> >>>>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>>>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>>>
> >>>>      dev->config[I440FX_SMRAM] = 0x02;
> >>>>
> >>>>@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>>>      qdev_init_nofail(dev);
> >>>>
> >>>>      d = pci_create_simple(b, 0, pci_type);
> >>>>-    *pi440fx_state = I440FX_PCI_DEVICE(d);
> >>>>+    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
> >>>>      f = *pi440fx_state;
> >>>>      f->system_memory = address_space_mem;
> >>>>      f->pci_address_space = pci_address_space;
> >>>>--
> >>>>1.9.1
> >>>
> >>>
> >

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
> 
> 
> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
> >On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
> >>On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> >>>On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> >>>>We need to use this index to reuse this macro later
> >>>>
> >>>>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>
> >>>Which index?
> >>>Most users don't need to change.
> >>>Just open-code OBJECT_CHECK where necessary, or add
> >>>a new wrapper.
> >>
> >>Okay so what about this?
> >>
> >>     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
> >>
> >>     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
> >>     object with type, then we can reuse i440fx_init() simply.
> >>
> >>     Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 0cd82b8..8c74653 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -93,6 +93,9 @@ typedef struct PIIX3State {
> >>  #define I440FX_PCI_DEVICE(obj) \
> >>      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>
> >>+#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
> >>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >
> >This is just wrong. If you are casting to PCII440FXState,
> 
> Why? We will have two different QOM typenames of PCII440FXStates.
> 
> >there is no reason not to use TYPE_I440FX_PCI_DEVICE.
> 
> As you know we already have this original,
> 
> static const TypeInfo i440fx_info = {
>     .name          = TYPE_I440FX_PCI_DEVICE,
>     .parent        = TYPE_PCI_DEVICE,
>     .instance_size = sizeof(PCII440FXState),
>     .class_init    = i440fx_class_init,
> };
> 
> and in patch #4, we will register that new host bridge to IGD passthrough:
> 
> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>     .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>     .parent        = TYPE_PCI_DEVICE,
>     .instance_size = sizeof(PCII440FXState),
>     .class_init    = xen_igd_passthrough_i440fx_class_init,
> };

My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
Then you can reuse regular piix code which casts to
TYPE_I440FX_PCI_DEVICE because
TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
be a subclass of TYPE_I440FX_PCI_DEVICE.

> After that, we have two different QOM typenames of PCII440FXStates,
> TYPE_I440FX_PCI_DEVICE and TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> right? So if you think we still should use this original,
> 
> #define I440FX_PCI_DEVICE(obj) \
> 	OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> 
> I really don't understand how to distinguish TYPE_I440FX_PCI_DEVICE and
> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE just with I440FX_PCI_DEVICE(obj).
> 
> This should involve two places:
> 
> #1: static int i440fx_initfn(PCIDevice *dev)
> {
>     PCII440FXState *d = I440FX_PCI_DEVICE(dev);
>     ...
> 
> #2: 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,
>                     MemoryRegion *address_space_io,
>                     ram_addr_t ram_size,
>                     ram_addr_t below_4g_mem_size,
>                     ram_addr_t above_4g_mem_size,
>                     MemoryRegion *pci_address_space,
>                     MemoryRegion *ram_memory)
> {
>     ...
>     PCIDevice *d;
>     ...
>     *pi440fx_state = I440FX_PCI_DEVICE(d);
>     ...
> 
> So if you think I'm wrong please show me how to do just with
> I440FX_PCI_DEVICE() to work all scenarios from your point of view, then I'd
> like to followup yours to validate.
> 
> Thanks
> Tiejun


See above.

> >
> >>+
> >>  struct PCII440FXState {
> >>      /*< private >*/
> >>      PCIDevice parent_obj;
> >>@@ -333,7 +336,7 @@ PCIBus *i440fx_init(const char *host_type, const char
> >>*pci_type,
> >>      qdev_init_nofail(dev);
> >>
> >>      d = pci_create_simple(b, 0, pci_type);
> >>-    *pi440fx_state = I440FX_PCI_DEVICE(d);
> >>+    *pi440fx_state = I440FX_PCI_DEVICE_FROM_TYPE(d, pci_type);
> >>      f = *pi440fx_state;
> >>      f->system_memory = address_space_mem;
> >>      f->pci_address_space = pci_address_space;
> >>
> >>
> >>Thanks
> >>Tiejun
> >>
> >>>
> >>>>---
> >>>>  hw/pci-host/piix.c | 10 +++++-----
> >>>>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>>>
> >>>>v4:
> >>>>
> >>>>* New patch to extend I440FX_PCI_DEVICE
> >>>>
> >>>>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>>>index 0cd82b8..4330599 100644
> >>>>--- a/hw/pci-host/piix.c
> >>>>+++ b/hw/pci-host/piix.c
> >>>>@@ -90,8 +90,8 @@ typedef struct PIIX3State {
> >>>>      MemoryRegion rcr_mem;
> >>>>  } PIIX3State;
> >>>>
> >>>>-#define I440FX_PCI_DEVICE(obj) \
> >>>>-    OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>>>+#define I440FX_PCI_DEVICE(obj, type) \
> >>>>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >>>>
> >>>>  struct PCII440FXState {
> >>>>      /*< private >*/
> >>>>@@ -155,7 +155,7 @@ static void i440fx_set_smm(int val, void *arg)
> >>>>  static void i440fx_write_config(PCIDevice *dev,
> >>>>                                  uint32_t address, uint32_t val, int len)
> >>>>  {
> >>>>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>>>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>>>
> >>>>      /* XXX: implement SMRAM.D_LOCK */
> >>>>      pci_default_write_config(dev, address, val, len);
> >>>>@@ -295,7 +295,7 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
> >>>>
> >>>>  static int i440fx_initfn(PCIDevice *dev)
> >>>>  {
> >>>>-    PCII440FXState *d = I440FX_PCI_DEVICE(dev);
> >>>>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev, TYPE_I440FX_PCI_DEVICE);
> >>>>
> >>>>      dev->config[I440FX_SMRAM] = 0x02;
> >>>>
> >>>>@@ -333,7 +333,7 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>>>      qdev_init_nofail(dev);
> >>>>
> >>>>      d = pci_create_simple(b, 0, pci_type);
> >>>>-    *pi440fx_state = I440FX_PCI_DEVICE(d);
> >>>>+    *pi440fx_state = I440FX_PCI_DEVICE(d, pci_type);
> >>>>      f = *pi440fx_state;
> >>>>      f->system_memory = address_space_mem;
> >>>>      f->pci_address_space = pci_address_space;
> >>>>--
> >>>>1.9.1
> >>>
> >>>
> >

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/11 4:27, Michael S. Tsirkin wrote:
> On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
>>
>>
>> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
>>> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>>>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>>>> We need to use this index to reuse this macro later
>>>>>>
>>>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>
>>>>> Which index?
>>>>> Most users don't need to change.
>>>>> Just open-code OBJECT_CHECK where necessary, or add
>>>>> a new wrapper.
>>>>
>>>> Okay so what about this?
>>>>
>>>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>>>
>>>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>>>      object with type, then we can reuse i440fx_init() simply.
>>>>
>>>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>
>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>> index 0cd82b8..8c74653 100644
>>>> --- a/hw/pci-host/piix.c
>>>> +++ b/hw/pci-host/piix.c
>>>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>>>   #define I440FX_PCI_DEVICE(obj) \
>>>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>>
>>>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>
>>> This is just wrong. If you are casting to PCII440FXState,
>>
>> Why? We will have two different QOM typenames of PCII440FXStates.
>>
>>> there is no reason not to use TYPE_I440FX_PCI_DEVICE.
>>
>> As you know we already have this original,
>>
>> static const TypeInfo i440fx_info = {
>>      .name          = TYPE_I440FX_PCI_DEVICE,
>>      .parent        = TYPE_PCI_DEVICE,
>>      .instance_size = sizeof(PCII440FXState),
>>      .class_init    = i440fx_class_init,
>> };
>>
>> and in patch #4, we will register that new host bridge to IGD passthrough:
>>
>> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>      .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>      .parent        = TYPE_PCI_DEVICE,
>>      .instance_size = sizeof(PCII440FXState),
>>      .class_init    = xen_igd_passthrough_i440fx_class_init,
>> };
>
> My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.

So here, this mean xen_igd_passthrough_i440fx_info's parent should be 
TYPE_I440FX_PCI_DEVICE, right?

> Then you can reuse regular piix code which casts to
> TYPE_I440FX_PCI_DEVICE because
> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
> be a subclass of TYPE_I440FX_PCI_DEVICE.
>

So since then, we can reuse i440fx_initfn.

As a summary, what we should do is like the following,

xen:hw:pci-host:piix: create host bridge to passthrough

Implement a pci host bridge specific to passthrough. Actually
this just inherits the standard one.

This is based on http://patchwork.ozlabs.org/patch/363810/.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
  hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
  include/hw/i386/pc.h |  2 ++
  2 files changed, 30 insertions(+)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0cd82b8..2ccd9ee 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
      .class_init    = i440fx_class_init,
  };

+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, 
void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = i440fx_initfn;
+    k->vendor_id = PCI_VENDOR_ID_INTEL;
+    k->device_id = PCI_DEVICE_ID_INTEL_82441;
+    k->revision = 0x02;
+    k->class_id = PCI_CLASS_BRIDGE_HOST;
+    dc->desc = "IGD PT XEN Host bridge";
+    dc->vmsd = &vmstate_i440fx;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->hotpluggable   = false;
+}
+
+static const TypeInfo xen_igd_passthrough_i440fx_info = {
+    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
+    .parent        = TYPE_I440FX_PCI_DEVICE,
+    .instance_size = sizeof(PCII440FXState),
+    .class_init    = xen_igd_passthrough_i440fx_class_init,
+};
+
  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                  PCIBus *rootbus)
  {
@@ -744,6 +771,7 @@ static const TypeInfo i440fx_pcihost_info = {
  static void i440fx_register_types(void)
  {
      type_register_static(&i440fx_info);
+    type_register_static(&xen_igd_passthrough_i440fx_info);
      type_register_static(&piix3_info);
      type_register_static(&piix3_xen_info);
      type_register_static(&i440fx_pcihost_info);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 11fb72f..de34aa6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
  #define TYPE_I440FX_PCI_DEVICE "i440FX"

+#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE 
"xen-igd-passthrough-i440FX"
+
  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                      PCII440FXState **pi440fx_state, int *piix_devfn,
                      ISABus **isa_bus, qemu_irq *pic,
-- 
1.9.1

Thanks
Tiejun

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/11 4:27, Michael S. Tsirkin wrote:
> On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
>>
>>
>> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
>>> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>>>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>>>> We need to use this index to reuse this macro later
>>>>>>
>>>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>
>>>>> Which index?
>>>>> Most users don't need to change.
>>>>> Just open-code OBJECT_CHECK where necessary, or add
>>>>> a new wrapper.
>>>>
>>>> Okay so what about this?
>>>>
>>>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>>>
>>>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>>>      object with type, then we can reuse i440fx_init() simply.
>>>>
>>>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>
>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>> index 0cd82b8..8c74653 100644
>>>> --- a/hw/pci-host/piix.c
>>>> +++ b/hw/pci-host/piix.c
>>>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>>>   #define I440FX_PCI_DEVICE(obj) \
>>>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>>
>>>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>
>>> This is just wrong. If you are casting to PCII440FXState,
>>
>> Why? We will have two different QOM typenames of PCII440FXStates.
>>
>>> there is no reason not to use TYPE_I440FX_PCI_DEVICE.
>>
>> As you know we already have this original,
>>
>> static const TypeInfo i440fx_info = {
>>      .name          = TYPE_I440FX_PCI_DEVICE,
>>      .parent        = TYPE_PCI_DEVICE,
>>      .instance_size = sizeof(PCII440FXState),
>>      .class_init    = i440fx_class_init,
>> };
>>
>> and in patch #4, we will register that new host bridge to IGD passthrough:
>>
>> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>      .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>      .parent        = TYPE_PCI_DEVICE,
>>      .instance_size = sizeof(PCII440FXState),
>>      .class_init    = xen_igd_passthrough_i440fx_class_init,
>> };
>
> My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.

So here, this mean xen_igd_passthrough_i440fx_info's parent should be 
TYPE_I440FX_PCI_DEVICE, right?

> Then you can reuse regular piix code which casts to
> TYPE_I440FX_PCI_DEVICE because
> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
> be a subclass of TYPE_I440FX_PCI_DEVICE.
>

So since then, we can reuse i440fx_initfn.

As a summary, what we should do is like the following,

xen:hw:pci-host:piix: create host bridge to passthrough

Implement a pci host bridge specific to passthrough. Actually
this just inherits the standard one.

This is based on http://patchwork.ozlabs.org/patch/363810/.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
  hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
  include/hw/i386/pc.h |  2 ++
  2 files changed, 30 insertions(+)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0cd82b8..2ccd9ee 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
      .class_init    = i440fx_class_init,
  };

+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, 
void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = i440fx_initfn;
+    k->vendor_id = PCI_VENDOR_ID_INTEL;
+    k->device_id = PCI_DEVICE_ID_INTEL_82441;
+    k->revision = 0x02;
+    k->class_id = PCI_CLASS_BRIDGE_HOST;
+    dc->desc = "IGD PT XEN Host bridge";
+    dc->vmsd = &vmstate_i440fx;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->hotpluggable   = false;
+}
+
+static const TypeInfo xen_igd_passthrough_i440fx_info = {
+    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
+    .parent        = TYPE_I440FX_PCI_DEVICE,
+    .instance_size = sizeof(PCII440FXState),
+    .class_init    = xen_igd_passthrough_i440fx_class_init,
+};
+
  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                  PCIBus *rootbus)
  {
@@ -744,6 +771,7 @@ static const TypeInfo i440fx_pcihost_info = {
  static void i440fx_register_types(void)
  {
      type_register_static(&i440fx_info);
+    type_register_static(&xen_igd_passthrough_i440fx_info);
      type_register_static(&piix3_info);
      type_register_static(&piix3_xen_info);
      type_register_static(&i440fx_pcihost_info);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 11fb72f..de34aa6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
  #define TYPE_I440FX_PCI_DEVICE "i440FX"

+#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE 
"xen-igd-passthrough-i440FX"
+
  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                      PCII440FXState **pi440fx_state, int *piix_devfn,
                      ISABus **isa_bus, qemu_irq *pic,
-- 
1.9.1

Thanks
Tiejun

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Mon, Aug 11, 2014 at 10:50:48AM +0800, Chen, Tiejun wrote:
> On 2014/8/11 4:27, Michael S. Tsirkin wrote:
> >On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
> >>
> >>
> >>On 2014/8/7 5:07, Michael S. Tsirkin wrote:
> >>>On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
> >>>>On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> >>>>>On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> >>>>>>We need to use this index to reuse this macro later
> >>>>>>
> >>>>>>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>>>
> >>>>>Which index?
> >>>>>Most users don't need to change.
> >>>>>Just open-code OBJECT_CHECK where necessary, or add
> >>>>>a new wrapper.
> >>>>
> >>>>Okay so what about this?
> >>>>
> >>>>     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
> >>>>
> >>>>     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
> >>>>     object with type, then we can reuse i440fx_init() simply.
> >>>>
> >>>>     Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>>
> >>>>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>>>index 0cd82b8..8c74653 100644
> >>>>--- a/hw/pci-host/piix.c
> >>>>+++ b/hw/pci-host/piix.c
> >>>>@@ -93,6 +93,9 @@ typedef struct PIIX3State {
> >>>>  #define I440FX_PCI_DEVICE(obj) \
> >>>>      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>>>
> >>>>+#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
> >>>>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >>>
> >>>This is just wrong. If you are casting to PCII440FXState,
> >>
> >>Why? We will have two different QOM typenames of PCII440FXStates.
> >>
> >>>there is no reason not to use TYPE_I440FX_PCI_DEVICE.
> >>
> >>As you know we already have this original,
> >>
> >>static const TypeInfo i440fx_info = {
> >>     .name          = TYPE_I440FX_PCI_DEVICE,
> >>     .parent        = TYPE_PCI_DEVICE,
> >>     .instance_size = sizeof(PCII440FXState),
> >>     .class_init    = i440fx_class_init,
> >>};
> >>
> >>and in patch #4, we will register that new host bridge to IGD passthrough:
> >>
> >>static const TypeInfo xen_igd_passthrough_i440fx_info = {
> >>     .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> >>     .parent        = TYPE_PCI_DEVICE,
> >>     .instance_size = sizeof(PCII440FXState),
> >>     .class_init    = xen_igd_passthrough_i440fx_class_init,
> >>};
> >
> >My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
> 
> So here, this mean xen_igd_passthrough_i440fx_info's parent should be
> TYPE_I440FX_PCI_DEVICE, right?
> 
> >Then you can reuse regular piix code which casts to
> >TYPE_I440FX_PCI_DEVICE because
> >TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
> >be a subclass of TYPE_I440FX_PCI_DEVICE.
> >
> 
> So since then, we can reuse i440fx_initfn.
> 
> As a summary, what we should do is like the following,
> 
> xen:hw:pci-host:piix: create host bridge to passthrough
> 
> Implement a pci host bridge specific to passthrough. Actually
> this just inherits the standard one.
> 
> This is based on http://patchwork.ozlabs.org/patch/363810/.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
>  include/hw/i386/pc.h |  2 ++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 0cd82b8..2ccd9ee 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
>      .class_init    = i440fx_class_init,
>  };
> 
> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void
> *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->init = i440fx_initfn;
> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
> +    k->revision = 0x02;
> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
> +    dc->desc = "IGD PT XEN Host bridge";
> +    dc->vmsd = &vmstate_i440fx;
> +    /*
> +     * PCI-facing part of the host bridge, not usable without the
> +     * host-facing part, which can't be device_add'ed, yet.
> +     */
> +    dc->cannot_instantiate_with_device_add_yet = true;
> +    dc->hotpluggable   = false;
> +}
> +

A bunch of code still duplicated here. Only override what you have to.


> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> +    .parent        = TYPE_I440FX_PCI_DEVICE,
> +    .instance_size = sizeof(PCII440FXState),
> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
> +};
> +
>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>                                                  PCIBus *rootbus)
>  {
> @@ -744,6 +771,7 @@ static const TypeInfo i440fx_pcihost_info = {
>  static void i440fx_register_types(void)
>  {
>      type_register_static(&i440fx_info);
> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>      type_register_static(&piix3_info);
>      type_register_static(&piix3_xen_info);
>      type_register_static(&i440fx_pcihost_info);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 11fb72f..de34aa6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>  #define TYPE_I440FX_PCI_DEVICE "i440FX"
> 
> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
> "xen-igd-passthrough-i440FX"
> +
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      PCII440FXState **pi440fx_state, int *piix_devfn,
>                      ISABus **isa_bus, qemu_irq *pic,
> -- 
> 1.9.1
> 
> Thanks
> Tiejun

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Michael S. Tsirkin]

On Mon, Aug 11, 2014 at 10:50:48AM +0800, Chen, Tiejun wrote:
> On 2014/8/11 4:27, Michael S. Tsirkin wrote:
> >On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
> >>
> >>
> >>On 2014/8/7 5:07, Michael S. Tsirkin wrote:
> >>>On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
> >>>>On 2014/8/6 17:45, Michael S. Tsirkin wrote:
> >>>>>On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
> >>>>>>We need to use this index to reuse this macro later
> >>>>>>
> >>>>>>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>>>
> >>>>>Which index?
> >>>>>Most users don't need to change.
> >>>>>Just open-code OBJECT_CHECK where necessary, or add
> >>>>>a new wrapper.
> >>>>
> >>>>Okay so what about this?
> >>>>
> >>>>     hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
> >>>>
> >>>>     We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
> >>>>     object with type, then we can reuse i440fx_init() simply.
> >>>>
> >>>>     Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>>>
> >>>>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>>>index 0cd82b8..8c74653 100644
> >>>>--- a/hw/pci-host/piix.c
> >>>>+++ b/hw/pci-host/piix.c
> >>>>@@ -93,6 +93,9 @@ typedef struct PIIX3State {
> >>>>  #define I440FX_PCI_DEVICE(obj) \
> >>>>      OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
> >>>>
> >>>>+#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
> >>>>+    OBJECT_CHECK(PCII440FXState, (obj), type)
> >>>
> >>>This is just wrong. If you are casting to PCII440FXState,
> >>
> >>Why? We will have two different QOM typenames of PCII440FXStates.
> >>
> >>>there is no reason not to use TYPE_I440FX_PCI_DEVICE.
> >>
> >>As you know we already have this original,
> >>
> >>static const TypeInfo i440fx_info = {
> >>     .name          = TYPE_I440FX_PCI_DEVICE,
> >>     .parent        = TYPE_PCI_DEVICE,
> >>     .instance_size = sizeof(PCII440FXState),
> >>     .class_init    = i440fx_class_init,
> >>};
> >>
> >>and in patch #4, we will register that new host bridge to IGD passthrough:
> >>
> >>static const TypeInfo xen_igd_passthrough_i440fx_info = {
> >>     .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> >>     .parent        = TYPE_PCI_DEVICE,
> >>     .instance_size = sizeof(PCII440FXState),
> >>     .class_init    = xen_igd_passthrough_i440fx_class_init,
> >>};
> >
> >My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
> 
> So here, this mean xen_igd_passthrough_i440fx_info's parent should be
> TYPE_I440FX_PCI_DEVICE, right?
> 
> >Then you can reuse regular piix code which casts to
> >TYPE_I440FX_PCI_DEVICE because
> >TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
> >be a subclass of TYPE_I440FX_PCI_DEVICE.
> >
> 
> So since then, we can reuse i440fx_initfn.
> 
> As a summary, what we should do is like the following,
> 
> xen:hw:pci-host:piix: create host bridge to passthrough
> 
> Implement a pci host bridge specific to passthrough. Actually
> this just inherits the standard one.
> 
> This is based on http://patchwork.ozlabs.org/patch/363810/.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
>  include/hw/i386/pc.h |  2 ++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 0cd82b8..2ccd9ee 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
>      .class_init    = i440fx_class_init,
>  };
> 
> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void
> *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->init = i440fx_initfn;
> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
> +    k->revision = 0x02;
> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
> +    dc->desc = "IGD PT XEN Host bridge";
> +    dc->vmsd = &vmstate_i440fx;
> +    /*
> +     * PCI-facing part of the host bridge, not usable without the
> +     * host-facing part, which can't be device_add'ed, yet.
> +     */
> +    dc->cannot_instantiate_with_device_add_yet = true;
> +    dc->hotpluggable   = false;
> +}
> +

A bunch of code still duplicated here. Only override what you have to.


> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> +    .parent        = TYPE_I440FX_PCI_DEVICE,
> +    .instance_size = sizeof(PCII440FXState),
> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
> +};
> +
>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>                                                  PCIBus *rootbus)
>  {
> @@ -744,6 +771,7 @@ static const TypeInfo i440fx_pcihost_info = {
>  static void i440fx_register_types(void)
>  {
>      type_register_static(&i440fx_info);
> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>      type_register_static(&piix3_info);
>      type_register_static(&piix3_xen_info);
>      type_register_static(&i440fx_pcihost_info);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 11fb72f..de34aa6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>  #define TYPE_I440FX_PCI_DEVICE "i440FX"
> 
> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
> "xen-igd-passthrough-i440FX"
> +
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      PCII440FXState **pi440fx_state, int *piix_devfn,
>                      ISABus **isa_bus, qemu_irq *pic,
> -- 
> 1.9.1
> 
> Thanks
> Tiejun

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/12 16:54, Michael S. Tsirkin wrote:
> On Mon, Aug 11, 2014 at 10:50:48AM +0800, Chen, Tiejun wrote:
>> On 2014/8/11 4:27, Michael S. Tsirkin wrote:
>>> On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
>>>>
>>>>
>>>> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
>>>>> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>>>>>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>>>>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>>>>>> We need to use this index to reuse this macro later
>>>>>>>>
>>>>>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>>
>>>>>>> Which index?
>>>>>>> Most users don't need to change.
>>>>>>> Just open-code OBJECT_CHECK where necessary, or add
>>>>>>> a new wrapper.
>>>>>>
>>>>>> Okay so what about this?
>>>>>>
>>>>>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>>>>>
>>>>>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>>>>>      object with type, then we can reuse i440fx_init() simply.
>>>>>>
>>>>>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>
>>>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>>>> index 0cd82b8..8c74653 100644
>>>>>> --- a/hw/pci-host/piix.c
>>>>>> +++ b/hw/pci-host/piix.c
>>>>>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>>>>>   #define I440FX_PCI_DEVICE(obj) \
>>>>>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>>>>
>>>>>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>>>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>>>
>>>>> This is just wrong. If you are casting to PCII440FXState,
>>>>
>>>> Why? We will have two different QOM typenames of PCII440FXStates.
>>>>
>>>>> there is no reason not to use TYPE_I440FX_PCI_DEVICE.
>>>>
>>>> As you know we already have this original,
>>>>
>>>> static const TypeInfo i440fx_info = {
>>>>      .name          = TYPE_I440FX_PCI_DEVICE,
>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>      .instance_size = sizeof(PCII440FXState),
>>>>      .class_init    = i440fx_class_init,
>>>> };
>>>>
>>>> and in patch #4, we will register that new host bridge to IGD passthrough:
>>>>
>>>> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>>>      .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>      .instance_size = sizeof(PCII440FXState),
>>>>      .class_init    = xen_igd_passthrough_i440fx_class_init,
>>>> };
>>>
>>> My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
>>
>> So here, this mean xen_igd_passthrough_i440fx_info's parent should be
>> TYPE_I440FX_PCI_DEVICE, right?
>>
>>> Then you can reuse regular piix code which casts to
>>> TYPE_I440FX_PCI_DEVICE because
>>> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
>>> be a subclass of TYPE_I440FX_PCI_DEVICE.
>>>
>>
>> So since then, we can reuse i440fx_initfn.
>>
>> As a summary, what we should do is like the following,
>>
>> xen:hw:pci-host:piix: create host bridge to passthrough
>>
>> Implement a pci host bridge specific to passthrough. Actually
>> this just inherits the standard one.
>>
>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
>>   include/hw/i386/pc.h |  2 ++
>>   2 files changed, 30 insertions(+)
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 0cd82b8..2ccd9ee 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
>>       .class_init    = i440fx_class_init,
>>   };
>>
>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void
>> *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +
>> +    k->init = i440fx_initfn;
>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>> +    k->revision = 0x02;
>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>> +    dc->desc = "IGD PT XEN Host bridge";
>> +    dc->vmsd = &vmstate_i440fx;
>> +    /*
>> +     * PCI-facing part of the host bridge, not usable without the
>> +     * host-facing part, which can't be device_add'ed, yet.
>> +     */
>> +    dc->cannot_instantiate_with_device_add_yet = true;
>> +    dc->hotpluggable   = false;
>> +}
>> +
>
> A bunch of code still duplicated here. Only override what you have to.

You mean all stuffs can be inherited from its own parent, even including 
vendor_id/device_id?

If yes, currently the follows should be enough,

static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, 
void *data)
{
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);

     dc->desc = "IGD PT XEN Host bridge";
}

We will add k->config_write/k->config_read here when we introduce IGD 
passthrough later.

Thanks
Tiejun

>
>
>> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
>> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>> +    .parent        = TYPE_I440FX_PCI_DEVICE,
>> +    .instance_size = sizeof(PCII440FXState),
>> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
>> +};
>> +
>>   static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>>                                                   PCIBus *rootbus)
>>   {
>> @@ -744,6 +771,7 @@ static const TypeInfo i440fx_pcihost_info = {
>>   static void i440fx_register_types(void)
>>   {
>>       type_register_static(&i440fx_info);
>> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>>       type_register_static(&piix3_info);
>>       type_register_static(&piix3_xen_info);
>>       type_register_static(&i440fx_pcihost_info);
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 11fb72f..de34aa6 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>>   #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>>   #define TYPE_I440FX_PCI_DEVICE "i440FX"
>>
>> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
>> "xen-igd-passthrough-i440FX"
>> +
>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                       PCII440FXState **pi440fx_state, int *piix_devfn,
>>                       ISABus **isa_bus, qemu_irq *pic,
>> --
>> 1.9.1
>>
>> Thanks
>> Tiejun
>
>

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/12 16:54, Michael S. Tsirkin wrote:
> On Mon, Aug 11, 2014 at 10:50:48AM +0800, Chen, Tiejun wrote:
>> On 2014/8/11 4:27, Michael S. Tsirkin wrote:
>>> On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
>>>>
>>>>
>>>> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
>>>>> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>>>>>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>>>>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>>>>>> We need to use this index to reuse this macro later
>>>>>>>>
>>>>>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>>
>>>>>>> Which index?
>>>>>>> Most users don't need to change.
>>>>>>> Just open-code OBJECT_CHECK where necessary, or add
>>>>>>> a new wrapper.
>>>>>>
>>>>>> Okay so what about this?
>>>>>>
>>>>>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>>>>>
>>>>>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>>>>>      object with type, then we can reuse i440fx_init() simply.
>>>>>>
>>>>>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>
>>>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>>>> index 0cd82b8..8c74653 100644
>>>>>> --- a/hw/pci-host/piix.c
>>>>>> +++ b/hw/pci-host/piix.c
>>>>>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>>>>>   #define I440FX_PCI_DEVICE(obj) \
>>>>>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>>>>
>>>>>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>>>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>>>
>>>>> This is just wrong. If you are casting to PCII440FXState,
>>>>
>>>> Why? We will have two different QOM typenames of PCII440FXStates.
>>>>
>>>>> there is no reason not to use TYPE_I440FX_PCI_DEVICE.
>>>>
>>>> As you know we already have this original,
>>>>
>>>> static const TypeInfo i440fx_info = {
>>>>      .name          = TYPE_I440FX_PCI_DEVICE,
>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>      .instance_size = sizeof(PCII440FXState),
>>>>      .class_init    = i440fx_class_init,
>>>> };
>>>>
>>>> and in patch #4, we will register that new host bridge to IGD passthrough:
>>>>
>>>> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>>>      .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>      .instance_size = sizeof(PCII440FXState),
>>>>      .class_init    = xen_igd_passthrough_i440fx_class_init,
>>>> };
>>>
>>> My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
>>
>> So here, this mean xen_igd_passthrough_i440fx_info's parent should be
>> TYPE_I440FX_PCI_DEVICE, right?
>>
>>> Then you can reuse regular piix code which casts to
>>> TYPE_I440FX_PCI_DEVICE because
>>> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
>>> be a subclass of TYPE_I440FX_PCI_DEVICE.
>>>
>>
>> So since then, we can reuse i440fx_initfn.
>>
>> As a summary, what we should do is like the following,
>>
>> xen:hw:pci-host:piix: create host bridge to passthrough
>>
>> Implement a pci host bridge specific to passthrough. Actually
>> this just inherits the standard one.
>>
>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
>>   include/hw/i386/pc.h |  2 ++
>>   2 files changed, 30 insertions(+)
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 0cd82b8..2ccd9ee 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
>>       .class_init    = i440fx_class_init,
>>   };
>>
>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void
>> *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +
>> +    k->init = i440fx_initfn;
>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>> +    k->revision = 0x02;
>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>> +    dc->desc = "IGD PT XEN Host bridge";
>> +    dc->vmsd = &vmstate_i440fx;
>> +    /*
>> +     * PCI-facing part of the host bridge, not usable without the
>> +     * host-facing part, which can't be device_add'ed, yet.
>> +     */
>> +    dc->cannot_instantiate_with_device_add_yet = true;
>> +    dc->hotpluggable   = false;
>> +}
>> +
>
> A bunch of code still duplicated here. Only override what you have to.

You mean all stuffs can be inherited from its own parent, even including 
vendor_id/device_id?

If yes, currently the follows should be enough,

static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, 
void *data)
{
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);

     dc->desc = "IGD PT XEN Host bridge";
}

We will add k->config_write/k->config_read here when we introduce IGD 
passthrough later.

Thanks
Tiejun

>
>
>> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
>> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>> +    .parent        = TYPE_I440FX_PCI_DEVICE,
>> +    .instance_size = sizeof(PCII440FXState),
>> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
>> +};
>> +
>>   static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>>                                                   PCIBus *rootbus)
>>   {
>> @@ -744,6 +771,7 @@ static const TypeInfo i440fx_pcihost_info = {
>>   static void i440fx_register_types(void)
>>   {
>>       type_register_static(&i440fx_info);
>> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>>       type_register_static(&piix3_info);
>>       type_register_static(&piix3_xen_info);
>>       type_register_static(&i440fx_pcihost_info);
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 11fb72f..de34aa6 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>>   #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>>   #define TYPE_I440FX_PCI_DEVICE "i440FX"
>>
>> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
>> "xen-igd-passthrough-i440FX"
>> +
>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                       PCII440FXState **pi440fx_state, int *piix_devfn,
>>                       ISABus **isa_bus, qemu_irq *pic,
>> --
>> 1.9.1
>>
>> Thanks
>> Tiejun
>
>

Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/12 17:25, Chen, Tiejun wrote:
> On 2014/8/12 16:54, Michael S. Tsirkin wrote:
>> On Mon, Aug 11, 2014 at 10:50:48AM +0800, Chen, Tiejun wrote:
>>> On 2014/8/11 4:27, Michael S. Tsirkin wrote:
>>>> On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
>>>>>
>>>>>
>>>>> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
>>>>>> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>>>>>>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>>>>>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>>>>>>> We need to use this index to reuse this macro later
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>>>
>>>>>>>> Which index?
>>>>>>>> Most users don't need to change.
>>>>>>>> Just open-code OBJECT_CHECK where necessary, or add
>>>>>>>> a new wrapper.
>>>>>>>
>>>>>>> Okay so what about this?
>>>>>>>
>>>>>>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>>>>>>
>>>>>>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>>>>>>      object with type, then we can reuse i440fx_init() simply.
>>>>>>>
>>>>>>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>>
>>>>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>>>>> index 0cd82b8..8c74653 100644
>>>>>>> --- a/hw/pci-host/piix.c
>>>>>>> +++ b/hw/pci-host/piix.c
>>>>>>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>>>>>>   #define I440FX_PCI_DEVICE(obj) \
>>>>>>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>>>>>
>>>>>>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>>>>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>>>>
>>>>>> This is just wrong. If you are casting to PCII440FXState,
>>>>>
>>>>> Why? We will have two different QOM typenames of PCII440FXStates.
>>>>>
>>>>>> there is no reason not to use TYPE_I440FX_PCI_DEVICE.
>>>>>
>>>>> As you know we already have this original,
>>>>>
>>>>> static const TypeInfo i440fx_info = {
>>>>>      .name          = TYPE_I440FX_PCI_DEVICE,
>>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>>      .instance_size = sizeof(PCII440FXState),
>>>>>      .class_init    = i440fx_class_init,
>>>>> };
>>>>>
>>>>> and in patch #4, we will register that new host bridge to IGD
>>>>> passthrough:
>>>>>
>>>>> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>>>>      .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>>      .instance_size = sizeof(PCII440FXState),
>>>>>      .class_init    = xen_igd_passthrough_i440fx_class_init,
>>>>> };
>>>>
>>>> My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
>>>
>>> So here, this mean xen_igd_passthrough_i440fx_info's parent should be
>>> TYPE_I440FX_PCI_DEVICE, right?
>>>
>>>> Then you can reuse regular piix code which casts to
>>>> TYPE_I440FX_PCI_DEVICE because
>>>> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
>>>> be a subclass of TYPE_I440FX_PCI_DEVICE.
>>>>
>>>
>>> So since then, we can reuse i440fx_initfn.
>>>
>>> As a summary, what we should do is like the following,
>>>
>>> xen:hw:pci-host:piix: create host bridge to passthrough
>>>
>>> Implement a pci host bridge specific to passthrough. Actually
>>> this just inherits the standard one.
>>>
>>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>>
>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>> ---
>>>   hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
>>>   include/hw/i386/pc.h |  2 ++
>>>   2 files changed, 30 insertions(+)
>>>
>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>> index 0cd82b8..2ccd9ee 100644
>>> --- a/hw/pci-host/piix.c
>>> +++ b/hw/pci-host/piix.c
>>> @@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
>>>       .class_init    = i440fx_class_init,
>>>   };
>>>
>>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass
>>> *klass, void
>>> *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>> +
>>> +    k->init = i440fx_initfn;
>>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>>> +    k->revision = 0x02;
>>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>>> +    dc->desc = "IGD PT XEN Host bridge";
>>> +    dc->vmsd = &vmstate_i440fx;
>>> +    /*
>>> +     * PCI-facing part of the host bridge, not usable without the
>>> +     * host-facing part, which can't be device_add'ed, yet.
>>> +     */
>>> +    dc->cannot_instantiate_with_device_add_yet = true;
>>> +    dc->hotpluggable   = false;
>>> +}
>>> +
>>
>> A bunch of code still duplicated here. Only override what you have to.
>
> You mean all stuffs can be inherited from its own parent, even including
> vendor_id/device_id?
>
> If yes, currently the follows should be enough,
>
> static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
> void *data)
> {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
>      dc->desc = "IGD PT XEN Host bridge";
> }
>
> We will add k->config_write/k->config_read here when we introduce IGD
> passthrough later.
>

Looks this is true after a quick test, so I will update this then send v5.

Thanks
Tiejun

Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index

[Chen, Tiejun]

On 2014/8/12 17:25, Chen, Tiejun wrote:
> On 2014/8/12 16:54, Michael S. Tsirkin wrote:
>> On Mon, Aug 11, 2014 at 10:50:48AM +0800, Chen, Tiejun wrote:
>>> On 2014/8/11 4:27, Michael S. Tsirkin wrote:
>>>> On Thu, Aug 07, 2014 at 09:40:49AM +0800, Chen, Tiejun wrote:
>>>>>
>>>>>
>>>>> On 2014/8/7 5:07, Michael S. Tsirkin wrote:
>>>>>> On Wed, Aug 06, 2014 at 06:17:02PM +0800, Chen, Tiejun wrote:
>>>>>>> On 2014/8/6 17:45, Michael S. Tsirkin wrote:
>>>>>>>> On Wed, Aug 06, 2014 at 02:50:33PM +0800, Tiejun Chen wrote:
>>>>>>>>> We need to use this index to reuse this macro later
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>>>
>>>>>>>> Which index?
>>>>>>>> Most users don't need to change.
>>>>>>>> Just open-code OBJECT_CHECK where necessary, or add
>>>>>>>> a new wrapper.
>>>>>>>
>>>>>>> Okay so what about this?
>>>>>>>
>>>>>>>      hw:pci-host:piix: define I440FX_PCI_DEVICE_FROM_TYPE
>>>>>>>
>>>>>>>      We need to introduce I440FX_PCI_DEVICE_FROM_TYPE to get
>>>>>>>      object with type, then we can reuse i440fx_init() simply.
>>>>>>>
>>>>>>>      Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>>>>>
>>>>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>>>>> index 0cd82b8..8c74653 100644
>>>>>>> --- a/hw/pci-host/piix.c
>>>>>>> +++ b/hw/pci-host/piix.c
>>>>>>> @@ -93,6 +93,9 @@ typedef struct PIIX3State {
>>>>>>>   #define I440FX_PCI_DEVICE(obj) \
>>>>>>>       OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
>>>>>>>
>>>>>>> +#define I440FX_PCI_DEVICE_FROM_TYPE(obj, type) \
>>>>>>> +    OBJECT_CHECK(PCII440FXState, (obj), type)
>>>>>>
>>>>>> This is just wrong. If you are casting to PCII440FXState,
>>>>>
>>>>> Why? We will have two different QOM typenames of PCII440FXStates.
>>>>>
>>>>>> there is no reason not to use TYPE_I440FX_PCI_DEVICE.
>>>>>
>>>>> As you know we already have this original,
>>>>>
>>>>> static const TypeInfo i440fx_info = {
>>>>>      .name          = TYPE_I440FX_PCI_DEVICE,
>>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>>      .instance_size = sizeof(PCII440FXState),
>>>>>      .class_init    = i440fx_class_init,
>>>>> };
>>>>>
>>>>> and in patch #4, we will register that new host bridge to IGD
>>>>> passthrough:
>>>>>
>>>>> static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>>>>      .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>>>>      .parent        = TYPE_PCI_DEVICE,
>>>>>      .instance_size = sizeof(PCII440FXState),
>>>>>      .class_init    = xen_igd_passthrough_i440fx_class_init,
>>>>> };
>>>>
>>>> My idea is to inherit TYPE_I440FX_PCI_DEVICE instead.
>>>
>>> So here, this mean xen_igd_passthrough_i440fx_info's parent should be
>>> TYPE_I440FX_PCI_DEVICE, right?
>>>
>>>> Then you can reuse regular piix code which casts to
>>>> TYPE_I440FX_PCI_DEVICE because
>>>> TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE would
>>>> be a subclass of TYPE_I440FX_PCI_DEVICE.
>>>>
>>>
>>> So since then, we can reuse i440fx_initfn.
>>>
>>> As a summary, what we should do is like the following,
>>>
>>> xen:hw:pci-host:piix: create host bridge to passthrough
>>>
>>> Implement a pci host bridge specific to passthrough. Actually
>>> this just inherits the standard one.
>>>
>>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>>
>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>> ---
>>>   hw/pci-host/piix.c   | 28 ++++++++++++++++++++++++++++
>>>   include/hw/i386/pc.h |  2 ++
>>>   2 files changed, 30 insertions(+)
>>>
>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>> index 0cd82b8..2ccd9ee 100644
>>> --- a/hw/pci-host/piix.c
>>> +++ b/hw/pci-host/piix.c
>>> @@ -703,6 +703,33 @@ static const TypeInfo i440fx_info = {
>>>       .class_init    = i440fx_class_init,
>>>   };
>>>
>>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass
>>> *klass, void
>>> *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>> +
>>> +    k->init = i440fx_initfn;
>>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>>> +    k->revision = 0x02;
>>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>>> +    dc->desc = "IGD PT XEN Host bridge";
>>> +    dc->vmsd = &vmstate_i440fx;
>>> +    /*
>>> +     * PCI-facing part of the host bridge, not usable without the
>>> +     * host-facing part, which can't be device_add'ed, yet.
>>> +     */
>>> +    dc->cannot_instantiate_with_device_add_yet = true;
>>> +    dc->hotpluggable   = false;
>>> +}
>>> +
>>
>> A bunch of code still duplicated here. Only override what you have to.
>
> You mean all stuffs can be inherited from its own parent, even including
> vendor_id/device_id?
>
> If yes, currently the follows should be enough,
>
> static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
> void *data)
> {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
>      dc->desc = "IGD PT XEN Host bridge";
> }
>
> We will add k->config_write/k->config_read here when we introduce IGD
> passthrough later.
>

Looks this is true after a quick test, so I will update this then send v5.

Thanks
Tiejun

[Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Tiejun Chen]

Implement a pci host bridge specific to passthrough. Actually
this just inherits the standard one.

This is based on http://patchwork.ozlabs.org/patch/363810/.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h |  2 ++
 2 files changed, 41 insertions(+)

v4:

* Rebase

v3:

* Rebase

v2:

* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 4330599..5cac398 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
     return 0;
 }
 
+static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
+{
+    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
+                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
+
+    dev->config[I440FX_SMRAM] = 0x02;
+
+    cpu_smm_register(&i440fx_set_smm, d);
+    return 0;
+}
+
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     PCII440FXState **pi440fx_state,
                     int *piix3_devfn,
@@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
     .class_init    = i440fx_class_init,
 };
 
+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = xen_igd_passthrough_i440fx_initfn;
+    k->vendor_id = PCI_VENDOR_ID_INTEL;
+    k->device_id = PCI_DEVICE_ID_INTEL_82441;
+    k->revision = 0x02;
+    k->class_id = PCI_CLASS_BRIDGE_HOST;
+    dc->desc = "IGD PT XEN Host bridge";
+    dc->vmsd = &vmstate_i440fx;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->hotpluggable   = false;
+}
+
+static const TypeInfo xen_igd_passthrough_i440fx_info = {
+    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCII440FXState),
+    .class_init    = xen_igd_passthrough_i440fx_class_init,
+};
+
 static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                 PCIBus *rootbus)
 {
@@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
 static void i440fx_register_types(void)
 {
     type_register_static(&i440fx_info);
+    type_register_static(&xen_igd_passthrough_i440fx_info);
     type_register_static(&piix3_info);
     type_register_static(&piix3_xen_info);
     type_register_static(&i440fx_pcihost_info);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 11fb72f..de34aa6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
 #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
 #define TYPE_I440FX_PCI_DEVICE "i440FX"
 
+#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
+
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     PCII440FXState **pi440fx_state, int *piix_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
-- 
1.9.1

[v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Tiejun Chen]

Implement a pci host bridge specific to passthrough. Actually
this just inherits the standard one.

This is based on http://patchwork.ozlabs.org/patch/363810/.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h |  2 ++
 2 files changed, 41 insertions(+)

v4:

* Rebase

v3:

* Rebase

v2:

* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 4330599..5cac398 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
     return 0;
 }
 
+static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
+{
+    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
+                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
+
+    dev->config[I440FX_SMRAM] = 0x02;
+
+    cpu_smm_register(&i440fx_set_smm, d);
+    return 0;
+}
+
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     PCII440FXState **pi440fx_state,
                     int *piix3_devfn,
@@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
     .class_init    = i440fx_class_init,
 };
 
+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = xen_igd_passthrough_i440fx_initfn;
+    k->vendor_id = PCI_VENDOR_ID_INTEL;
+    k->device_id = PCI_DEVICE_ID_INTEL_82441;
+    k->revision = 0x02;
+    k->class_id = PCI_CLASS_BRIDGE_HOST;
+    dc->desc = "IGD PT XEN Host bridge";
+    dc->vmsd = &vmstate_i440fx;
+    /*
+     * PCI-facing part of the host bridge, not usable without the
+     * host-facing part, which can't be device_add'ed, yet.
+     */
+    dc->cannot_instantiate_with_device_add_yet = true;
+    dc->hotpluggable   = false;
+}
+
+static const TypeInfo xen_igd_passthrough_i440fx_info = {
+    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCII440FXState),
+    .class_init    = xen_igd_passthrough_i440fx_class_init,
+};
+
 static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
                                                 PCIBus *rootbus)
 {
@@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
 static void i440fx_register_types(void)
 {
     type_register_static(&i440fx_info);
+    type_register_static(&xen_igd_passthrough_i440fx_info);
     type_register_static(&piix3_info);
     type_register_static(&piix3_xen_info);
     type_register_static(&i440fx_pcihost_info);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 11fb72f..de34aa6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
 #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
 #define TYPE_I440FX_PCI_DEVICE "i440FX"
 
+#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
+
 PCIBus *i440fx_init(const char *host_type, const char *pci_type,
                     PCII440FXState **pi440fx_state, int *piix_devfn,
                     ISABus **isa_bus, qemu_irq *pic,
-- 
1.9.1

Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
> Implement a pci host bridge specific to passthrough. Actually
> this just inherits the standard one.
> 
> This is based on http://patchwork.ozlabs.org/patch/363810/.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h |  2 ++
>  2 files changed, 41 insertions(+)
> 
> v4:
> 
> * Rebase
> 
> v3:
> 
> * Rebase
> 
> v2:
> 
> * Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 4330599..5cac398 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
>      return 0;
>  }
>  
> +static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
> +{
> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
> +                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
> +
> +    dev->config[I440FX_SMRAM] = 0x02;
> +
> +    cpu_smm_register(&i440fx_set_smm, d);
> +    return 0;
> +}
> +
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      PCII440FXState **pi440fx_state,
>                      int *piix3_devfn,

don't duplicate code.
Reuse the function from regular piix.

> @@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
>      .class_init    = i440fx_class_init,
>  };
>  
> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->init = xen_igd_passthrough_i440fx_initfn;
> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
> +    k->revision = 0x02;
> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
> +    dc->desc = "IGD PT XEN Host bridge";
> +    dc->vmsd = &vmstate_i440fx;
> +    /*
> +     * PCI-facing part of the host bridge, not usable without the
> +     * host-facing part, which can't be device_add'ed, yet.
> +     */
> +    dc->cannot_instantiate_with_device_add_yet = true;
> +    dc->hotpluggable   = false;
> +}
> +
> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCII440FXState),
> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
> +};
> +
>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>                                                  PCIBus *rootbus)
>  {
> @@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
>  static void i440fx_register_types(void)
>  {
>      type_register_static(&i440fx_info);
> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>      type_register_static(&piix3_info);
>      type_register_static(&piix3_xen_info);
>      type_register_static(&i440fx_pcihost_info);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 11fb72f..de34aa6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>  #define TYPE_I440FX_PCI_DEVICE "i440FX"
>  
> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
> +
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      PCII440FXState **pi440fx_state, int *piix_devfn,
>                      ISABus **isa_bus, qemu_irq *pic,
> -- 
> 1.9.1

Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
> Implement a pci host bridge specific to passthrough. Actually
> this just inherits the standard one.
> 
> This is based on http://patchwork.ozlabs.org/patch/363810/.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h |  2 ++
>  2 files changed, 41 insertions(+)
> 
> v4:
> 
> * Rebase
> 
> v3:
> 
> * Rebase
> 
> v2:
> 
> * Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 4330599..5cac398 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
>      return 0;
>  }
>  
> +static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
> +{
> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
> +                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
> +
> +    dev->config[I440FX_SMRAM] = 0x02;
> +
> +    cpu_smm_register(&i440fx_set_smm, d);
> +    return 0;
> +}
> +
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      PCII440FXState **pi440fx_state,
>                      int *piix3_devfn,

don't duplicate code.
Reuse the function from regular piix.

> @@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
>      .class_init    = i440fx_class_init,
>  };
>  
> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->init = xen_igd_passthrough_i440fx_initfn;
> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
> +    k->revision = 0x02;
> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
> +    dc->desc = "IGD PT XEN Host bridge";
> +    dc->vmsd = &vmstate_i440fx;
> +    /*
> +     * PCI-facing part of the host bridge, not usable without the
> +     * host-facing part, which can't be device_add'ed, yet.
> +     */
> +    dc->cannot_instantiate_with_device_add_yet = true;
> +    dc->hotpluggable   = false;
> +}
> +
> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(PCII440FXState),
> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
> +};
> +
>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>                                                  PCIBus *rootbus)
>  {
> @@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
>  static void i440fx_register_types(void)
>  {
>      type_register_static(&i440fx_info);
> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>      type_register_static(&piix3_info);
>      type_register_static(&piix3_xen_info);
>      type_register_static(&i440fx_pcihost_info);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 11fb72f..de34aa6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>  #define TYPE_I440FX_PCI_DEVICE "i440FX"
>  
> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
> +
>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>                      PCII440FXState **pi440fx_state, int *piix_devfn,
>                      ISABus **isa_bus, qemu_irq *pic,
> -- 
> 1.9.1

Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Chen, Tiejun]

On 2014/8/6 17:42, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
>> Implement a pci host bridge specific to passthrough. Actually
>> this just inherits the standard one.
>>
>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>   include/hw/i386/pc.h |  2 ++
>>   2 files changed, 41 insertions(+)
>>
>> v4:
>>
>> * Rebase
>>
>> v3:
>>
>> * Rebase
>>
>> v2:
>>
>> * Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 4330599..5cac398 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
>>       return 0;
>>   }
>>
>> +static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
>> +{
>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
>> +                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);

Sorry, I don't understand what you mean.

i440fx_initfn(PCIDevice *dev) just have one parameter, dev, how to 
multiplex that here?

Thanks
Tiejun

>> +
>> +    dev->config[I440FX_SMRAM] = 0x02;
>> +
>> +    cpu_smm_register(&i440fx_set_smm, d);
>> +    return 0;
>> +}
>> +
>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                       PCII440FXState **pi440fx_state,
>>                       int *piix3_devfn,
>
> don't duplicate code.
> Reuse the function from regular piix.



>
>> @@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
>>       .class_init    = i440fx_class_init,
>>   };
>>
>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +
>> +    k->init = xen_igd_passthrough_i440fx_initfn;
>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>> +    k->revision = 0x02;
>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>> +    dc->desc = "IGD PT XEN Host bridge";
>> +    dc->vmsd = &vmstate_i440fx;
>> +    /*
>> +     * PCI-facing part of the host bridge, not usable without the
>> +     * host-facing part, which can't be device_add'ed, yet.
>> +     */
>> +    dc->cannot_instantiate_with_device_add_yet = true;
>> +    dc->hotpluggable   = false;
>> +}
>> +
>> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
>> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>> +    .parent        = TYPE_PCI_DEVICE,
>> +    .instance_size = sizeof(PCII440FXState),
>> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
>> +};
>> +
>>   static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>>                                                   PCIBus *rootbus)
>>   {
>> @@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
>>   static void i440fx_register_types(void)
>>   {
>>       type_register_static(&i440fx_info);
>> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>>       type_register_static(&piix3_info);
>>       type_register_static(&piix3_xen_info);
>>       type_register_static(&i440fx_pcihost_info);
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 11fb72f..de34aa6 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>>   #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>>   #define TYPE_I440FX_PCI_DEVICE "i440FX"
>>
>> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
>> +
>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                       PCII440FXState **pi440fx_state, int *piix_devfn,
>>                       ISABus **isa_bus, qemu_irq *pic,
>> --
>> 1.9.1
>
>

Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Chen, Tiejun]

On 2014/8/6 17:42, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
>> Implement a pci host bridge specific to passthrough. Actually
>> this just inherits the standard one.
>>
>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>   include/hw/i386/pc.h |  2 ++
>>   2 files changed, 41 insertions(+)
>>
>> v4:
>>
>> * Rebase
>>
>> v3:
>>
>> * Rebase
>>
>> v2:
>>
>> * Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index 4330599..5cac398 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
>>       return 0;
>>   }
>>
>> +static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
>> +{
>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
>> +                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);

Sorry, I don't understand what you mean.

i440fx_initfn(PCIDevice *dev) just have one parameter, dev, how to 
multiplex that here?

Thanks
Tiejun

>> +
>> +    dev->config[I440FX_SMRAM] = 0x02;
>> +
>> +    cpu_smm_register(&i440fx_set_smm, d);
>> +    return 0;
>> +}
>> +
>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                       PCII440FXState **pi440fx_state,
>>                       int *piix3_devfn,
>
> don't duplicate code.
> Reuse the function from regular piix.



>
>> @@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
>>       .class_init    = i440fx_class_init,
>>   };
>>
>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +
>> +    k->init = xen_igd_passthrough_i440fx_initfn;
>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>> +    k->revision = 0x02;
>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>> +    dc->desc = "IGD PT XEN Host bridge";
>> +    dc->vmsd = &vmstate_i440fx;
>> +    /*
>> +     * PCI-facing part of the host bridge, not usable without the
>> +     * host-facing part, which can't be device_add'ed, yet.
>> +     */
>> +    dc->cannot_instantiate_with_device_add_yet = true;
>> +    dc->hotpluggable   = false;
>> +}
>> +
>> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
>> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>> +    .parent        = TYPE_PCI_DEVICE,
>> +    .instance_size = sizeof(PCII440FXState),
>> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
>> +};
>> +
>>   static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>>                                                   PCIBus *rootbus)
>>   {
>> @@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
>>   static void i440fx_register_types(void)
>>   {
>>       type_register_static(&i440fx_info);
>> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>>       type_register_static(&piix3_info);
>>       type_register_static(&piix3_xen_info);
>>       type_register_static(&i440fx_pcihost_info);
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 11fb72f..de34aa6 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>>   #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>>   #define TYPE_I440FX_PCI_DEVICE "i440FX"
>>
>> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
>> +
>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>                       PCII440FXState **pi440fx_state, int *piix_devfn,
>>                       ISABus **isa_bus, qemu_irq *pic,
>> --
>> 1.9.1
>
>

Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 05:47:12PM +0800, Chen, Tiejun wrote:
> On 2014/8/6 17:42, Michael S. Tsirkin wrote:
> >On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
> >>Implement a pci host bridge specific to passthrough. Actually
> >>this just inherits the standard one.
> >>
> >>This is based on http://patchwork.ozlabs.org/patch/363810/.
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>---
> >>  hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
> >>  include/hw/i386/pc.h |  2 ++
> >>  2 files changed, 41 insertions(+)
> >>
> >>v4:
> >>
> >>* Rebase
> >>
> >>v3:
> >>
> >>* Rebase
> >>
> >>v2:
> >>
> >>* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 4330599..5cac398 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
> >>      return 0;
> >>  }
> >>
> >>+static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
> >>+{
> >>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
> >>+                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
> 
> Sorry, I don't understand what you mean.
> 
> i440fx_initfn(PCIDevice *dev) just have one parameter, dev, how to multiplex
> that here?
> 
> Thanks
> Tiejun

You have exactly the same code (except you are doing the cast
differently, but that's just a bug in your code, i440fx_initfn
does it correctly). So nothing to multiplex.

> >>+
> >>+    dev->config[I440FX_SMRAM] = 0x02;
> >>+
> >>+    cpu_smm_register(&i440fx_set_smm, d);
> >>+    return 0;
> >>+}
> >>+
> >>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>                      PCII440FXState **pi440fx_state,
> >>                      int *piix3_devfn,
> >
> >don't duplicate code.
> >Reuse the function from regular piix.
> 
> 
> 
> >
> >>@@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
> >>      .class_init    = i440fx_class_init,
> >>  };
> >>
> >>+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> >>+{
> >>+    DeviceClass *dc = DEVICE_CLASS(klass);
> >>+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> >>+
> >>+    k->init = xen_igd_passthrough_i440fx_initfn;
> >>+    k->vendor_id = PCI_VENDOR_ID_INTEL;
> >>+    k->device_id = PCI_DEVICE_ID_INTEL_82441;
> >>+    k->revision = 0x02;
> >>+    k->class_id = PCI_CLASS_BRIDGE_HOST;
> >>+    dc->desc = "IGD PT XEN Host bridge";
> >>+    dc->vmsd = &vmstate_i440fx;
> >>+    /*
> >>+     * PCI-facing part of the host bridge, not usable without the
> >>+     * host-facing part, which can't be device_add'ed, yet.
> >>+     */
> >>+    dc->cannot_instantiate_with_device_add_yet = true;
> >>+    dc->hotpluggable   = false;
> >>+}
> >>+
> >>+static const TypeInfo xen_igd_passthrough_i440fx_info = {
> >>+    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> >>+    .parent        = TYPE_PCI_DEVICE,
> >>+    .instance_size = sizeof(PCII440FXState),
> >>+    .class_init    = xen_igd_passthrough_i440fx_class_init,
> >>+};
> >>+
> >>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
> >>                                                  PCIBus *rootbus)
> >>  {
> >>@@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
> >>  static void i440fx_register_types(void)
> >>  {
> >>      type_register_static(&i440fx_info);
> >>+    type_register_static(&xen_igd_passthrough_i440fx_info);
> >>      type_register_static(&piix3_info);
> >>      type_register_static(&piix3_xen_info);
> >>      type_register_static(&i440fx_pcihost_info);
> >>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>index 11fb72f..de34aa6 100644
> >>--- a/include/hw/i386/pc.h
> >>+++ b/include/hw/i386/pc.h
> >>@@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
> >>  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
> >>  #define TYPE_I440FX_PCI_DEVICE "i440FX"
> >>
> >>+#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
> >>+
> >>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>                      PCII440FXState **pi440fx_state, int *piix_devfn,
> >>                      ISABus **isa_bus, qemu_irq *pic,
> >>--
> >>1.9.1
> >
> >

Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Michael S. Tsirkin]

On Wed, Aug 06, 2014 at 05:47:12PM +0800, Chen, Tiejun wrote:
> On 2014/8/6 17:42, Michael S. Tsirkin wrote:
> >On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
> >>Implement a pci host bridge specific to passthrough. Actually
> >>this just inherits the standard one.
> >>
> >>This is based on http://patchwork.ozlabs.org/patch/363810/.
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>---
> >>  hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
> >>  include/hw/i386/pc.h |  2 ++
> >>  2 files changed, 41 insertions(+)
> >>
> >>v4:
> >>
> >>* Rebase
> >>
> >>v3:
> >>
> >>* Rebase
> >>
> >>v2:
> >>
> >>* Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 4330599..5cac398 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
> >>      return 0;
> >>  }
> >>
> >>+static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
> >>+{
> >>+    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
> >>+                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
> 
> Sorry, I don't understand what you mean.
> 
> i440fx_initfn(PCIDevice *dev) just have one parameter, dev, how to multiplex
> that here?
> 
> Thanks
> Tiejun

You have exactly the same code (except you are doing the cast
differently, but that's just a bug in your code, i440fx_initfn
does it correctly). So nothing to multiplex.

> >>+
> >>+    dev->config[I440FX_SMRAM] = 0x02;
> >>+
> >>+    cpu_smm_register(&i440fx_set_smm, d);
> >>+    return 0;
> >>+}
> >>+
> >>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>                      PCII440FXState **pi440fx_state,
> >>                      int *piix3_devfn,
> >
> >don't duplicate code.
> >Reuse the function from regular piix.
> 
> 
> 
> >
> >>@@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
> >>      .class_init    = i440fx_class_init,
> >>  };
> >>
> >>+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> >>+{
> >>+    DeviceClass *dc = DEVICE_CLASS(klass);
> >>+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> >>+
> >>+    k->init = xen_igd_passthrough_i440fx_initfn;
> >>+    k->vendor_id = PCI_VENDOR_ID_INTEL;
> >>+    k->device_id = PCI_DEVICE_ID_INTEL_82441;
> >>+    k->revision = 0x02;
> >>+    k->class_id = PCI_CLASS_BRIDGE_HOST;
> >>+    dc->desc = "IGD PT XEN Host bridge";
> >>+    dc->vmsd = &vmstate_i440fx;
> >>+    /*
> >>+     * PCI-facing part of the host bridge, not usable without the
> >>+     * host-facing part, which can't be device_add'ed, yet.
> >>+     */
> >>+    dc->cannot_instantiate_with_device_add_yet = true;
> >>+    dc->hotpluggable   = false;
> >>+}
> >>+
> >>+static const TypeInfo xen_igd_passthrough_i440fx_info = {
> >>+    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
> >>+    .parent        = TYPE_PCI_DEVICE,
> >>+    .instance_size = sizeof(PCII440FXState),
> >>+    .class_init    = xen_igd_passthrough_i440fx_class_init,
> >>+};
> >>+
> >>  static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
> >>                                                  PCIBus *rootbus)
> >>  {
> >>@@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
> >>  static void i440fx_register_types(void)
> >>  {
> >>      type_register_static(&i440fx_info);
> >>+    type_register_static(&xen_igd_passthrough_i440fx_info);
> >>      type_register_static(&piix3_info);
> >>      type_register_static(&piix3_xen_info);
> >>      type_register_static(&i440fx_pcihost_info);
> >>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>index 11fb72f..de34aa6 100644
> >>--- a/include/hw/i386/pc.h
> >>+++ b/include/hw/i386/pc.h
> >>@@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
> >>  #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
> >>  #define TYPE_I440FX_PCI_DEVICE "i440FX"
> >>
> >>+#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
> >>+
> >>  PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> >>                      PCII440FXState **pi440fx_state, int *piix_devfn,
> >>                      ISABus **isa_bus, qemu_irq *pic,
> >>--
> >>1.9.1
> >
> >

Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Chen, Tiejun]

On 2014/8/7 5:04, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 05:47:12PM +0800, Chen, Tiejun wrote:
>> On 2014/8/6 17:42, Michael S. Tsirkin wrote:
>>> On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
>>>> Implement a pci host bridge specific to passthrough. Actually
>>>> this just inherits the standard one.
>>>>
>>>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>>>
>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>> ---
>>>>   hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>>>   include/hw/i386/pc.h |  2 ++
>>>>   2 files changed, 41 insertions(+)
>>>>
>>>> v4:
>>>>
>>>> * Rebase
>>>>
>>>> v3:
>>>>
>>>> * Rebase
>>>>
>>>> v2:
>>>>
>>>> * Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
>>>>
>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>> index 4330599..5cac398 100644
>>>> --- a/hw/pci-host/piix.c
>>>> +++ b/hw/pci-host/piix.c
>>>> @@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
>>>>       return 0;
>>>>   }
>>>>
>>>> +static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
>>>> +{
>>>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
>>>> +                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
>>
>> Sorry, I don't understand what you mean.
>>
>> i440fx_initfn(PCIDevice *dev) just have one parameter, dev, how to multiplex
>> that here?
>>
>> Thanks
>> Tiejun
>
> You have exactly the same code (except you are doing the cast
> differently, but that's just a bug in your code, i440fx_initfn

I already explain this in anther email, please take a look at that then 
show me how to do exactly if you still think I'm wrong.

Thanks
Tiejun

> does it correctly). So nothing to multiplex.
>
>>>> +
>>>> +    dev->config[I440FX_SMRAM] = 0x02;
>>>> +
>>>> +    cpu_smm_register(&i440fx_set_smm, d);
>>>> +    return 0;
>>>> +}
>>>> +
>>>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>>>                       PCII440FXState **pi440fx_state,
>>>>                       int *piix3_devfn,
>>>
>>> don't duplicate code.
>>> Reuse the function from regular piix.
>>
>>
>>
>>>
>>>> @@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
>>>>       .class_init    = i440fx_class_init,
>>>>   };
>>>>
>>>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>>> +
>>>> +    k->init = xen_igd_passthrough_i440fx_initfn;
>>>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>>>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>>>> +    k->revision = 0x02;
>>>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>>>> +    dc->desc = "IGD PT XEN Host bridge";
>>>> +    dc->vmsd = &vmstate_i440fx;
>>>> +    /*
>>>> +     * PCI-facing part of the host bridge, not usable without the
>>>> +     * host-facing part, which can't be device_add'ed, yet.
>>>> +     */
>>>> +    dc->cannot_instantiate_with_device_add_yet = true;
>>>> +    dc->hotpluggable   = false;
>>>> +}
>>>> +
>>>> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>>> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>>> +    .parent        = TYPE_PCI_DEVICE,
>>>> +    .instance_size = sizeof(PCII440FXState),
>>>> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
>>>> +};
>>>> +
>>>>   static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>>>>                                                   PCIBus *rootbus)
>>>>   {
>>>> @@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
>>>>   static void i440fx_register_types(void)
>>>>   {
>>>>       type_register_static(&i440fx_info);
>>>> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>>>>       type_register_static(&piix3_info);
>>>>       type_register_static(&piix3_xen_info);
>>>>       type_register_static(&i440fx_pcihost_info);
>>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>>> index 11fb72f..de34aa6 100644
>>>> --- a/include/hw/i386/pc.h
>>>> +++ b/include/hw/i386/pc.h
>>>> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>>>>   #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>>>>   #define TYPE_I440FX_PCI_DEVICE "i440FX"
>>>>
>>>> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
>>>> +
>>>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>>>                       PCII440FXState **pi440fx_state, int *piix_devfn,
>>>>                       ISABus **isa_bus, qemu_irq *pic,
>>>> --
>>>> 1.9.1
>>>
>>>
>

Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough

[Chen, Tiejun]

On 2014/8/7 5:04, Michael S. Tsirkin wrote:
> On Wed, Aug 06, 2014 at 05:47:12PM +0800, Chen, Tiejun wrote:
>> On 2014/8/6 17:42, Michael S. Tsirkin wrote:
>>> On Wed, Aug 06, 2014 at 02:50:34PM +0800, Tiejun Chen wrote:
>>>> Implement a pci host bridge specific to passthrough. Actually
>>>> this just inherits the standard one.
>>>>
>>>> This is based on http://patchwork.ozlabs.org/patch/363810/.
>>>>
>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>> ---
>>>>   hw/pci-host/piix.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>>>   include/hw/i386/pc.h |  2 ++
>>>>   2 files changed, 41 insertions(+)
>>>>
>>>> v4:
>>>>
>>>> * Rebase
>>>>
>>>> v3:
>>>>
>>>> * Rebase
>>>>
>>>> v2:
>>>>
>>>> * Just add prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough
>>>>
>>>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>>>> index 4330599..5cac398 100644
>>>> --- a/hw/pci-host/piix.c
>>>> +++ b/hw/pci-host/piix.c
>>>> @@ -303,6 +303,17 @@ static int i440fx_initfn(PCIDevice *dev)
>>>>       return 0;
>>>>   }
>>>>
>>>> +static int xen_igd_passthrough_i440fx_initfn(PCIDevice *dev)
>>>> +{
>>>> +    PCII440FXState *d = I440FX_PCI_DEVICE(dev,
>>>> +                         TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
>>
>> Sorry, I don't understand what you mean.
>>
>> i440fx_initfn(PCIDevice *dev) just have one parameter, dev, how to multiplex
>> that here?
>>
>> Thanks
>> Tiejun
>
> You have exactly the same code (except you are doing the cast
> differently, but that's just a bug in your code, i440fx_initfn

I already explain this in anther email, please take a look at that then 
show me how to do exactly if you still think I'm wrong.

Thanks
Tiejun

> does it correctly). So nothing to multiplex.
>
>>>> +
>>>> +    dev->config[I440FX_SMRAM] = 0x02;
>>>> +
>>>> +    cpu_smm_register(&i440fx_set_smm, d);
>>>> +    return 0;
>>>> +}
>>>> +
>>>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>>>                       PCII440FXState **pi440fx_state,
>>>>                       int *piix3_devfn,
>>>
>>> don't duplicate code.
>>> Reuse the function from regular piix.
>>
>>
>>
>>>
>>>> @@ -703,6 +714,33 @@ static const TypeInfo i440fx_info = {
>>>>       .class_init    = i440fx_class_init,
>>>>   };
>>>>
>>>> +static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>>> +
>>>> +    k->init = xen_igd_passthrough_i440fx_initfn;
>>>> +    k->vendor_id = PCI_VENDOR_ID_INTEL;
>>>> +    k->device_id = PCI_DEVICE_ID_INTEL_82441;
>>>> +    k->revision = 0x02;
>>>> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
>>>> +    dc->desc = "IGD PT XEN Host bridge";
>>>> +    dc->vmsd = &vmstate_i440fx;
>>>> +    /*
>>>> +     * PCI-facing part of the host bridge, not usable without the
>>>> +     * host-facing part, which can't be device_add'ed, yet.
>>>> +     */
>>>> +    dc->cannot_instantiate_with_device_add_yet = true;
>>>> +    dc->hotpluggable   = false;
>>>> +}
>>>> +
>>>> +static const TypeInfo xen_igd_passthrough_i440fx_info = {
>>>> +    .name          = TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
>>>> +    .parent        = TYPE_PCI_DEVICE,
>>>> +    .instance_size = sizeof(PCII440FXState),
>>>> +    .class_init    = xen_igd_passthrough_i440fx_class_init,
>>>> +};
>>>> +
>>>>   static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge,
>>>>                                                   PCIBus *rootbus)
>>>>   {
>>>> @@ -744,6 +782,7 @@ static const TypeInfo i440fx_pcihost_info = {
>>>>   static void i440fx_register_types(void)
>>>>   {
>>>>       type_register_static(&i440fx_info);
>>>> +    type_register_static(&xen_igd_passthrough_i440fx_info);
>>>>       type_register_static(&piix3_info);
>>>>       type_register_static(&piix3_xen_info);
>>>>       type_register_static(&i440fx_pcihost_info);
>>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>>>> index 11fb72f..de34aa6 100644
>>>> --- a/include/hw/i386/pc.h
>>>> +++ b/include/hw/i386/pc.h
>>>> @@ -238,6 +238,8 @@ typedef struct PCII440FXState PCII440FXState;
>>>>   #define TYPE_I440FX_PCI_HOST_BRIDGE "i440FX-pcihost"
>>>>   #define TYPE_I440FX_PCI_DEVICE "i440FX"
>>>>
>>>> +#define TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX"
>>>> +
>>>>   PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>>>>                       PCII440FXState **pi440fx_state, int *piix_devfn,
>>>>                       ISABus **isa_bus, qemu_irq *pic,
>>>> --
>>>> 1.9.1
>>>
>>>
>

[Qemu-devel] [v4][PATCH 5/5] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough

[Tiejun Chen]

Now we can introduce a new machine, xenigd, specific to IGD
passthrough. This can avoid involving other common codes.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

v4:

* Rebase

v3:

* Rebase

v2:

* Unify prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough like patch #3

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2bf8046..9d37f18 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -309,6 +309,15 @@ static void pc_init_pci(MachineState *machine)
              TYPE_I440FX_PCI_DEVICE);
 }
 
+
+#ifdef CONFIG_XEN
+static void xen_igd_passthrough_pc_init_pci(MachineState *machine)
+{
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
+}
+#endif
+
 static void pc_compat_2_0(MachineState *machine)
 {
     /* This value depends on the actual DSDT and SSDT compiled into
@@ -457,6 +466,18 @@ static void pc_xen_hvm_init(MachineState *machine)
         pci_create_simple(bus, -1, "xen-platform");
     }
 }
+
+static void xen_igd_passthrough_pc_hvm_init(MachineState *machine)
+{
+    PCIBus *bus;
+
+    xen_igd_passthrough_pc_init_pci(machine);
+
+    bus = pci_find_primary_bus();
+    if (bus != NULL) {
+        pci_create_simple(bus, -1, "xen-platform");
+    }
+}
 #endif
 
 #define PC_I440FX_MACHINE_OPTIONS \
@@ -906,6 +927,27 @@ static QEMUMachine xenfv_machine = {
         { /* end of list */ }
     },
 };
+
+static QEMUMachine xenigd_machine = {
+    PC_COMMON_MACHINE_OPTIONS,
+    .name = "xenigd",
+    .desc = "Xen Fully-virtualized PC specific to IGD",
+    .init = xen_igd_passthrough_pc_hvm_init,
+    .max_cpus = HVM_MAX_VCPUS,
+    .default_machine_opts = "accel=xen",
+    .hot_add_cpu = pc_hot_add_cpu,
+    .compat_props = (GlobalProperty[]) {
+        /* xenfv has no fwcfg and so does not load acpi from QEMU.
+         * as such new acpi features don't work.
+         */
+        {
+            .driver   = "PIIX4_PM",
+            .property = "acpi-pci-hotplug-with-bridge-support",
+            .value    = "off",
+        },
+        { /* end of list */ }
+    },
+};
 #endif
 
 static void pc_machine_init(void)
@@ -929,6 +971,7 @@ static void pc_machine_init(void)
     qemu_register_pc_machine(&isapc_machine);
 #ifdef CONFIG_XEN
     qemu_register_pc_machine(&xenfv_machine);
+    qemu_register_pc_machine(&xenigd_machine);
 #endif
 }
 
-- 
1.9.1

[v4][PATCH 5/5] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough

[Tiejun Chen]

Now we can introduce a new machine, xenigd, specific to IGD
passthrough. This can avoid involving other common codes.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/pc_piix.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

v4:

* Rebase

v3:

* Rebase

v2:

* Unify prefix with XEN_IGD_PASSTHROUGH/xen_igd_passthrough like patch #3

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2bf8046..9d37f18 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -309,6 +309,15 @@ static void pc_init_pci(MachineState *machine)
              TYPE_I440FX_PCI_DEVICE);
 }
 
+
+#ifdef CONFIG_XEN
+static void xen_igd_passthrough_pc_init_pci(MachineState *machine)
+{
+    pc_init1(machine, 1, 1, TYPE_I440FX_PCI_HOST_BRIDGE,
+             TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE);
+}
+#endif
+
 static void pc_compat_2_0(MachineState *machine)
 {
     /* This value depends on the actual DSDT and SSDT compiled into
@@ -457,6 +466,18 @@ static void pc_xen_hvm_init(MachineState *machine)
         pci_create_simple(bus, -1, "xen-platform");
     }
 }
+
+static void xen_igd_passthrough_pc_hvm_init(MachineState *machine)
+{
+    PCIBus *bus;
+
+    xen_igd_passthrough_pc_init_pci(machine);
+
+    bus = pci_find_primary_bus();
+    if (bus != NULL) {
+        pci_create_simple(bus, -1, "xen-platform");
+    }
+}
 #endif
 
 #define PC_I440FX_MACHINE_OPTIONS \
@@ -906,6 +927,27 @@ static QEMUMachine xenfv_machine = {
         { /* end of list */ }
     },
 };
+
+static QEMUMachine xenigd_machine = {
+    PC_COMMON_MACHINE_OPTIONS,
+    .name = "xenigd",
+    .desc = "Xen Fully-virtualized PC specific to IGD",
+    .init = xen_igd_passthrough_pc_hvm_init,
+    .max_cpus = HVM_MAX_VCPUS,
+    .default_machine_opts = "accel=xen",
+    .hot_add_cpu = pc_hot_add_cpu,
+    .compat_props = (GlobalProperty[]) {
+        /* xenfv has no fwcfg and so does not load acpi from QEMU.
+         * as such new acpi features don't work.
+         */
+        {
+            .driver   = "PIIX4_PM",
+            .property = "acpi-pci-hotplug-with-bridge-support",
+            .value    = "off",
+        },
+        { /* end of list */ }
+    },
+};
 #endif
 
 static void pc_machine_init(void)
@@ -929,6 +971,7 @@ static void pc_machine_init(void)
     qemu_register_pc_machine(&isapc_machine);
 #ifdef CONFIG_XEN
     qemu_register_pc_machine(&xenfv_machine);
+    qemu_register_pc_machine(&xenigd_machine);
 #endif
 }
 
-- 
1.9.1