[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i44
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] xen:hw:pci-host:piix: introduce xen_igd_i440fx_init |
Date: |
Wed, 30 Jul 2014 19:46:49 +0200 |
On Wed, Jul 30, 2014 at 04:24:25PM +0800, Chen, Tiejun wrote:
> On 2014/7/29 19:19, Michael S. Tsirkin wrote:
> >On Thu, Jul 24, 2014 at 07:30:28PM +0800, Tiejun Chen wrote:
> >>This is almost same as an original i440fx_init but just
> >>work with that xen igd host bridge to passthrough.
> >>
> >>Signed-off-by: Tiejun Chen <address@hidden>
> >>---
> >> hw/pci-host/piix.c | 79
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> include/hw/i386/pc.h | 10 +++++++
> >> 2 files changed, 89 insertions(+)
> >>
> >>diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> >>index 9feddf5..7ef08d7 100644
> >>--- a/hw/pci-host/piix.c
> >>+++ b/hw/pci-host/piix.c
> >>@@ -407,6 +407,85 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> >> return b;
> >> }
> >>
> >>+PCIBus *xen_igd_i440fx_init(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)
> >>+{
> >>+ DeviceState *dev;
> >>+ PCIBus *b;
> >>+ PCIDevice *d;
> >>+ PCIHostState *s;
> >>+ PIIX3State *piix3;
> >>+ PCII440FXState *f;
> >>+ unsigned i;
> >>+ I440FXState *i440fx;
> >>+
> >>+ dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
> >>+ s = PCI_HOST_BRIDGE(dev);
> >>+ b = pci_bus_new(dev, NULL, pci_address_space,
> >>+ address_space_io, 0, TYPE_PCI_BUS);
> >>+ s->bus = b;
> >>+ object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev),
> >>NULL);
> >>+ qdev_init_nofail(dev);
> >>+
> >>+ d = pci_create_simple(b, 0, TYPE_I440FX_XEN_PCI_DEVICE);
> >>+ *pi440fx_state = I440FX_XEN_PCI_DEVICE(d);
> >>+ f = *pi440fx_state;
> >>+ f->system_memory = address_space_mem;
> >>+ f->pci_address_space = pci_address_space;
> >>+ f->ram_memory = ram_memory;
> >>+
> >>+ i440fx = I440FX_PCI_HOST_BRIDGE(dev);
> >>+ i440fx->pci_info.w32.begin = below_4g_mem_size;
> >>+
> >>+ /* setup pci memory mapping */
> >>+ pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
> >>+ f->pci_address_space);
> >>+
> >>+ memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
> >>+ f->pci_address_space, 0xa0000, 0x20000);
> >>+ memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
> >>+ &f->smram_region, 1);
> >>+ memory_region_set_enabled(&f->smram_region, false);
> >>+ init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
> >>+ &f->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
> >>+ for (i = 0; i < 12; ++i) {
> >>+ init_pam(dev, f->ram_memory, f->system_memory,
> >>f->pci_address_space,
> >>+ &f->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
> >>+ PAM_EXPAN_SIZE);
> >>+ }
> >>+
> >>+ /* Xen supports additional interrupt routes from the PCI devices to
> >>+ * the IOAPIC: the four pins of each PCI device on the bus are also
> >>+ * connected to the IOAPIC directly.
> >>+ * These additional routes can be discovered through ACPI. */
> >>+ piix3 = DO_UPCAST(PIIX3State, dev,
> >>+ pci_create_simple_multifunction(b, -1, true, "PIIX3-xen"));
> >>+ pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
> >>+ piix3, XEN_PIIX_NUM_PIRQS);
> >>+ piix3->pic = pic;
> >>+ *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
> >>+
> >>+ *piix3_devfn = piix3->dev.devfn;
> >>+
> >>+ ram_size = ram_size / 8 / 1024 / 1024;
> >>+ if (ram_size > 255) {
> >>+ ram_size = 255;
> >>+ }
> >>+ d->config[0x57] = ram_size;
> >>+
> >>+ i440fx_update_memory_mappings(f);
> >>+
> >>+ return b;
> >>+}
> >
> >Too much copy-paste. Please refactor to avoid code duplication.
>
> Okay.
>
> >Is the only difference here the use of TYPE_I440FX_XEN_PCI_DEVICE?
>
> Yes.
>
> >Then you can pass type in as a parameter to a common static sub-function.
>
> I'm fine but as I remember Paolo don't like we intervene a common function.
> And we'll introduce something specific to IGD, like that faked PCIe device.
>
> Thanks
> Tiejun
Exactly, so in this way, there is no "if (xen-igd-pt)" in common code.
> >
> >>+
> >> PCIBus *find_i440fx(void)
> >> {
> >> PCIHostState *s = OBJECT_CHECK(PCIHostState,
> >>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >>index 1c0c382..51656d9 100644
> >>--- a/include/hw/i386/pc.h
> >>+++ b/include/hw/i386/pc.h
> >>@@ -239,6 +239,16 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
> >>int *piix_devfn,
> >> MemoryRegion *pci_memory,
> >> MemoryRegion *ram_memory);
> >>
> >>+PCIBus *xen_igd_i440fx_init(PCII440FXState **pi440fx_state, int
> >>*piix_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_memory,
> >>+ MemoryRegion *ram_memory);
> >>+
> >> PCIBus *find_i440fx(void);
> >> /* piix4.c */
> >> extern PCIDevice *piix4_dev;
> >>--
> >>1.9.1
> >
> >
[Qemu-devel] [PATCH 4/4] xen:hw:i386:pc_piix: introduce new machine for IGD passthrough, Tiejun Chen, 2014/07/24
[Qemu-devel] [PATCH 2/4] xen:hw:pci-host:piix: create host bridge to passthrough, Tiejun Chen, 2014/07/24
Re: [Qemu-devel] [PATCH 0/4] xen:passthrough: introduce a separate machine to igd passthrough, Chen, Tiejun, 2014/07/29