[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to inde
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index |
Date: |
Tue, 12 Aug 2014 10:54:35 +0200 |
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 <address@hidden>
> >>>>>
> >>>>>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 <address@hidden>
> >>>>
> >>>>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 <address@hidden>
> ---
> 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
- [Qemu-devel] [v4][PATCH 0/5] xen: introduce new machine for IGD passthrough, Tiejun Chen, 2014/08/06
- [Qemu-devel] [v4][PATCH 1/5] i440fx: make types configurable at run-time, Tiejun Chen, 2014/08/06
- [Qemu-devel] [v4][PATCH 2/5] pc_init1: pass parameters just with types, Tiejun Chen, 2014/08/06
- [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Tiejun Chen, 2014/08/06
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Michael S. Tsirkin, 2014/08/06
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Chen, Tiejun, 2014/08/06
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Michael S. Tsirkin, 2014/08/06
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Chen, Tiejun, 2014/08/06
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Michael S. Tsirkin, 2014/08/10
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Chen, Tiejun, 2014/08/10
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Chen, Tiejun, 2014/08/12
- Re: [Qemu-devel] [v4][PATCH 3/5] I440FX_PCI_DEVICE: add pci_type to index, Chen, Tiejun, 2014/08/12
[Qemu-devel] [v4][PATCH 4/5] xen:hw:pci-host:piix: create host bridge to passthrough, Tiejun Chen, 2014/08/06
[Qemu-devel] [v4][PATCH 5/5] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough, Tiejun Chen, 2014/08/06