Re: [Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
On Wed, Jan 20, 2016 at 10:10:11AM +0100, Gerd Hoffmann wrote: > Hi, > > > > > > +i440fx_realize = k->realize; > > > > > k->realize = igd_pt_i440fx_realize; > > > > > > ... because we are overriding it right here. > > > > Many device classes have a parent_realize field so they can keep > > a pointer to the original realize function. It's better than a > > static variable. > > How does the attached patch (incremental fix, not tested yet) look like? Looks good. Reviewed-by: Eduardo Habkost But, I have a similar question to the one I had about patch 04/11: how did this ever work before? Does that mean i440fx_realize() was never called when creating/initializing a TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE before? -- Eduardo ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
Hi, > > > > +i440fx_realize = k->realize; > > > > k->realize = igd_pt_i440fx_realize; > > > > ... because we are overriding it right here. > > Many device classes have a parent_realize field so they can keep > a pointer to the original realize function. It's better than a > static variable. How does the attached patch (incremental fix, not tested yet) look like? cheers, Gerd From 3d110e297b5182107e055db3ab69092affdef5bb Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 20 Jan 2016 10:08:19 +0100 Subject: [PATCH] [fixup] TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE realize_parent --- hw/pci-host/igd.c | 24 +++- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 160828f..2d51745 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -49,12 +49,24 @@ out_free: g_free(path); } -static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); +#define IGD_PT_I440FX_CLASS(class) \ +OBJECT_CLASS_CHECK(IGDPtI440fxClass, (class), \ + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE) +#define IGD_PT_I440FX_GET_CLASS(obj)\ +OBJECT_GET_CLASS(IGDPtI440fxClass, (obj), \ + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE) + +typedef struct IGDPtI440fxClass { +PCIDeviceClass parent_class; +void (*parent_realize)(PCIDevice *dev, Error **errp); +} IGDPtI440fxClass; + static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { +IGDPtI440fxClass *k = IGD_PT_I440FX_GET_CLASS(pci_dev); Error *err = NULL; -i440fx_realize(pci_dev, &err); +k->parent_realize(pci_dev, &err); if (err != NULL) { error_propagate(errp, err); return; @@ -72,11 +84,12 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) { +IGDPtI440fxClass *k = IGD_PT_I440FX_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); +PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass); -i440fx_realize = k->realize; -k->realize = igd_pt_i440fx_realize; +k->parent_realize = pc->realize; +pc->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge (i440fx)"; } @@ -84,6 +97,7 @@ static const TypeInfo igd_passthrough_i440fx_info = { .name = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, .parent= TYPE_I440FX_PCI_DEVICE, .class_init= igd_passthrough_i440fx_class_init, +.class_size= sizeof(IGDPtI440fxClass), }; static void (*q35_realize)(PCIDevice *pci_dev, Error **errp); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
On Wed, Jan 06, 2016 at 04:45:01PM +0100, Gerd Hoffmann wrote: > > > > > > +static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); > > > static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) > > > { > > > +Error *err = NULL; > > > uint32_t val = 0; > > > int rc, i, num; > > > int pos, len; > > > > Can't we get the parent PCIDeviceClass realize function from pci_dev? So > > that we don't have to introduce i440fx_realize? > > I don't think so ... > > > > > > > +i440fx_realize = k->realize; > > > k->realize = igd_pt_i440fx_realize; > > ... because we are overriding it right here. Many device classes have a parent_realize field so they can keep a pointer to the original realize function. It's better than a static variable. -- Eduardo ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
> > > > +static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); > > static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) > > { > > +Error *err = NULL; > > uint32_t val = 0; > > int rc, i, num; > > int pos, len; > > Can't we get the parent PCIDeviceClass realize function from pci_dev? So > that we don't have to introduce i440fx_realize? I don't think so ... > > > > +i440fx_realize = k->realize; > > k->realize = igd_pt_i440fx_realize; ... because we are overriding it right here. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
On Tue, 5 Jan 2016, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann > --- > hw/pci-host/igd.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c > index d1eeafb..6f52ab1 100644 > --- a/hw/pci-host/igd.c > +++ b/hw/pci-host/igd.c > @@ -53,12 +53,20 @@ out: > return ret; > } > > +static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); > static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) > { > +Error *err = NULL; > uint32_t val = 0; > int rc, i, num; > int pos, len; Can't we get the parent PCIDeviceClass realize function from pci_dev? So that we don't have to introduce i440fx_realize? > +i440fx_realize(pci_dev, &err); > +if (err != NULL) { > +error_propagate(errp, err); > +return; > +} > + > num = ARRAY_SIZE(igd_host_bridge_infos); > for (i = 0; i < num; i++) { > pos = igd_host_bridge_infos[i].offset; > @@ -77,6 +85,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass > *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > +i440fx_realize = k->realize; > k->realize = igd_pt_i440fx_realize; > dc->desc = "IGD Passthrough Host bridge"; > } > -- > 1.8.3.1 > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 9 + 1 file changed, 9 insertions(+) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index d1eeafb..6f52ab1 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -53,12 +53,20 @@ out: return ret; } +static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { +Error *err = NULL; uint32_t val = 0; int rc, i, num; int pos, len; +i440fx_realize(pci_dev, &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} + num = ARRAY_SIZE(igd_host_bridge_infos); for (i = 0; i < num; i++) { pos = igd_host_bridge_infos[i].offset; @@ -77,6 +85,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); +i440fx_realize = k->realize; k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel