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

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

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

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