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

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

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

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

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

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

static const TypeInfo xen_igd_passthrough_i440fx_info = {
     .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

Then you can reuse regular piix code which casts to
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@xxxxxxxxx>
  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
+    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

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.


Xen-devel mailing list



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