* [Qemu-devel] [v4][PATCH 0/5] xen: introduce new machine for IGD passthrough
@ 2014-08-06 6:50 ` Tiejun Chen
0 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [v4][PATCH 0/5] xen: introduce new machine for IGD passthrough
@ 2014-08-06 6:50 ` Tiejun Chen
0 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [v4][PATCH 1/5] i440fx: make types configurable at run-time
2014-08-06 6:50 ` Tiejun Chen
@ 2014-08-06 6:50 ` Tiejun Chen
-1 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [v4][PATCH 1/5] i440fx: make types configurable at run-time
@ 2014-08-06 6:50 ` Tiejun Chen
0 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [v4][PATCH 2/5] pc_init1: pass parameters just with types
2014-08-06 6:50 ` Tiejun Chen
@ 2014-08-06 6:50 ` Tiejun Chen
-1 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [v4][PATCH 2/5] pc_init1: pass parameters just with types
@ 2014-08-06 6:50 ` Tiejun Chen
0 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-06 6:50 ` Tiejun Chen
@ 2014-08-06 6:50 ` Tiejun Chen
-1 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-06 6:50 ` Tiejun Chen
0 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
2014-08-06 6:50 ` Tiejun Chen
@ 2014-08-06 6:50 ` Tiejun Chen
-1 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
@ 2014-08-06 6:50 ` Tiejun Chen
0 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [v4][PATCH 5/5] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough
2014-08-06 6:50 ` Tiejun Chen
@ 2014-08-06 6:50 ` Tiejun Chen
-1 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [v4][PATCH 5/5] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough
@ 2014-08-06 6:50 ` Tiejun Chen
0 siblings, 0 replies; 38+ messages in thread
From: Tiejun Chen @ 2014-08-06 6:50 UTC (permalink / raw)
To: mst; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
2014-08-06 6:50 ` Tiejun Chen
@ 2014-08-06 9:42 ` Michael S. Tsirkin
-1 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 9:42 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
@ 2014-08-06 9:42 ` Michael S. Tsirkin
0 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 9:42 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-06 6:50 ` Tiejun Chen
@ 2014-08-06 9:45 ` Michael S. Tsirkin
-1 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 9:45 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-06 9:45 ` Michael S. Tsirkin
0 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 9:45 UTC (permalink / raw)
To: Tiejun Chen; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
2014-08-06 9:42 ` Michael S. Tsirkin
@ 2014-08-06 9:47 ` Chen, Tiejun
-1 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-06 9:47 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
@ 2014-08-06 9:47 ` Chen, Tiejun
0 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-06 9:47 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-06 9:45 ` Michael S. Tsirkin
@ 2014-08-06 10:17 ` Chen, Tiejun
-1 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-06 10:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>
>
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-06 10:17 ` Chen, Tiejun
0 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-06 10:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>
>
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
2014-08-06 9:47 ` Chen, Tiejun
@ 2014-08-06 21:04 ` Michael S. Tsirkin
-1 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 21:04 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
> >
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
@ 2014-08-06 21:04 ` Michael S. Tsirkin
0 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 21:04 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
> >
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-06 10:17 ` Chen, Tiejun
@ 2014-08-06 21:07 ` Michael S. Tsirkin
-1 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 21:07 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
> >
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-06 21:07 ` Michael S. Tsirkin
0 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-06 21:07 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
> >
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-06 21:07 ` Michael S. Tsirkin
@ 2014-08-07 1:40 ` Chen, Tiejun
-1 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-07 1:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>>>
>>>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-07 1:40 ` Chen, Tiejun
0 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-07 1:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>>>
>>>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
2014-08-06 21:04 ` Michael S. Tsirkin
@ 2014-08-07 1:44 ` Chen, Tiejun
-1 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-07 1:44 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>>>
>>>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough
@ 2014-08-07 1:44 ` Chen, Tiejun
0 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-07 1:44 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>>>
>>>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-07 1:40 ` Chen, Tiejun
@ 2014-08-10 20:27 ` Michael S. Tsirkin
-1 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-10 20:27 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
> >>>
> >>>
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-10 20:27 ` Michael S. Tsirkin
0 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-10 20:27 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
> >>>
> >>>
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-10 20:27 ` Michael S. Tsirkin
@ 2014-08-11 2:50 ` Chen, Tiejun
-1 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-11 2:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-11 2:50 ` Chen, Tiejun
0 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-11 2:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-11 2:50 ` Chen, Tiejun
@ 2014-08-12 8:54 ` Michael S. Tsirkin
-1 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-12 8:54 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-12 8:54 ` Michael S. Tsirkin
0 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2014-08-12 8:54 UTC (permalink / raw)
To: Chen, Tiejun; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-12 8:54 ` Michael S. Tsirkin
@ 2014-08-12 9:25 ` Chen, Tiejun
-1 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-12 9:25 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-12 9:25 ` Chen, Tiejun
0 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-12 9:25 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
2014-08-12 9:25 ` Chen, Tiejun
@ 2014-08-12 9:39 ` Chen, Tiejun
-1 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-12 9:39 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index
@ 2014-08-12 9:39 ` Chen, Tiejun
0 siblings, 0 replies; 38+ messages in thread
From: Chen, Tiejun @ 2014-08-12 9:39 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: pbonzini, xen-devel, qemu-devel, stefano.stabellini
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
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2014-08-12 9:39 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-06 6:50 [Qemu-devel] [v4][PATCH 0/5] xen: introduce new machine for IGD passthrough Tiejun Chen
2014-08-06 6:50 ` Tiejun Chen
2014-08-06 6:50 ` [Qemu-devel] [v4][PATCH 1/5] i440fx: make types configurable at run-time Tiejun Chen
2014-08-06 6:50 ` Tiejun Chen
2014-08-06 6:50 ` [Qemu-devel] [v4][PATCH 2/5] pc_init1: pass parameters just with types Tiejun Chen
2014-08-06 6:50 ` Tiejun Chen
2014-08-06 6:50 ` [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index Tiejun Chen
2014-08-06 6:50 ` Tiejun Chen
2014-08-06 9:45 ` [Qemu-devel] " Michael S. Tsirkin
2014-08-06 9:45 ` Michael S. Tsirkin
2014-08-06 10:17 ` [Qemu-devel] " Chen, Tiejun
2014-08-06 10:17 ` Chen, Tiejun
2014-08-06 21:07 ` [Qemu-devel] " Michael S. Tsirkin
2014-08-06 21:07 ` Michael S. Tsirkin
2014-08-07 1:40 ` [Qemu-devel] " Chen, Tiejun
2014-08-07 1:40 ` Chen, Tiejun
2014-08-10 20:27 ` [Qemu-devel] " Michael S. Tsirkin
2014-08-10 20:27 ` Michael S. Tsirkin
2014-08-11 2:50 ` [Qemu-devel] " Chen, Tiejun
2014-08-11 2:50 ` Chen, Tiejun
2014-08-12 8:54 ` [Qemu-devel] " Michael S. Tsirkin
2014-08-12 8:54 ` Michael S. Tsirkin
2014-08-12 9:25 ` [Qemu-devel] " Chen, Tiejun
2014-08-12 9:25 ` Chen, Tiejun
2014-08-12 9:39 ` [Qemu-devel] " Chen, Tiejun
2014-08-12 9:39 ` Chen, Tiejun
2014-08-06 6:50 ` [Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough Tiejun Chen
2014-08-06 6:50 ` Tiejun Chen
2014-08-06 9:42 ` [Qemu-devel] " Michael S. Tsirkin
2014-08-06 9:42 ` Michael S. Tsirkin
2014-08-06 9:47 ` [Qemu-devel] " Chen, Tiejun
2014-08-06 9:47 ` Chen, Tiejun
2014-08-06 21:04 ` [Qemu-devel] " Michael S. Tsirkin
2014-08-06 21:04 ` Michael S. Tsirkin
2014-08-07 1:44 ` [Qemu-devel] " Chen, Tiejun
2014-08-07 1:44 ` Chen, Tiejun
2014-08-06 6:50 ` [Qemu-devel] [v4][PATCH 5/5] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough Tiejun Chen
2014-08-06 6:50 ` Tiejun Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.