Message ID | 1406201429-21700-4-git-send-email-tiejun.chen@intel.com |
---|---|
State | New |
Headers | show |
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 <tiejun.chen@intel.com> > --- > 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. Is the only difference here the use of TYPE_I440FX_XEN_PCI_DEVICE? Then you can pass type in as a parameter to a common static sub-function. > + > 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
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 <tiejun.chen@intel.com> >> --- >> 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 > >> + >> 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 > >
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 <tiejun.chen@intel.com> > >>--- > >> 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 > > > >
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; +} + 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;
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 <tiejun.chen@intel.com> --- hw/pci-host/piix.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/hw/i386/pc.h | 10 +++++++ 2 files changed, 89 insertions(+)