[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1()
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1() |
Date: |
Wed, 30 Jul 2014 19:48:27 +0200 |
On Wed, Jul 30, 2014 at 04:19:58PM +0800, Chen, Tiejun wrote:
> On 2014/7/29 19:26, Michael S. Tsirkin wrote:
> >On Thu, Jul 24, 2014 at 07:30:26PM +0800, Tiejun Chen wrote:
> >>We'd like to split pc_init1 and then we can share something
> >>with other stuff.
> >>
> >>Signed-off-by: Tiejun Chen <address@hidden>
> >
> >Did you test this patch? It does not look like it can work.
>
> Just compile but I think you're right. Here something is really wrong here,
> so this is my fault.
>
> I will regenerate this patch and do test to double check.
>
> Thanks
> Tiejun
Yea, and pls tell us what you tested exactly in the cover letter.
> >
> >>---
> >> hw/i386/pc_piix.c | 93
> >> +++++++++++++++++++++++++++++++++++++++++--------------
> >> 1 file changed, 70 insertions(+), 23 deletions(-)
> >>
> >>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >>index 7081c08..2391fda 100644
> >>--- a/hw/i386/pc_piix.c
> >>+++ b/hw/i386/pc_piix.c
> >>@@ -70,34 +70,21 @@ static bool smbios_legacy_mode;
> >> static bool gigabyte_align = true;
> >> static bool has_reserved_memory = true;
> >>
> >>-/* PC hardware initialisation */
> >>-static void pc_init1(MachineState *machine,
> >>+static ram_addr_t below_4g_mem_size;
> >>+static ram_addr_t above_4g_mem_size;
> >>+static void pc_machine_base_init(MachineState *machine,
> >> int pci_enabled,
> >>- int kvmclock_enabled)
> >>+ int kvmclock_enabled,
> >>+ DeviceState *icc_bridge,
> >>+ MemoryRegion *ram_memory,
> >>+ MemoryRegion *pci_memory,
> >>+ qemu_irq *gsi,
> >>+ GSIState *gsi_state,
> >>+ FWCfgState *fw_cfg)
> >> {
> >> PCMachineState *pc_machine = PC_MACHINE(machine);
> >> MemoryRegion *system_memory = get_system_memory();
> >>- MemoryRegion *system_io = get_system_io();
> >>- int i;
> >>- ram_addr_t below_4g_mem_size, above_4g_mem_size;
> >>- PCIBus *pci_bus;
> >>- ISABus *isa_bus;
> >>- PCII440FXState *i440fx_state;
> >>- int piix3_devfn = -1;
> >>- qemu_irq *cpu_irq;
> >>- qemu_irq *gsi;
> >>- qemu_irq *i8259;
> >>- qemu_irq *smi_irq;
> >>- GSIState *gsi_state;
> >>- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> >>- BusState *idebus[MAX_IDE_BUS];
> >>- ISADevice *rtc_state;
> >>- ISADevice *floppy;
> >>- MemoryRegion *ram_memory;
> >>- MemoryRegion *pci_memory;
> >> MemoryRegion *rom_memory;
> >>- DeviceState *icc_bridge;
> >>- FWCfgState *fw_cfg = NULL;
> >> PcGuestInfo *guest_info;
> >> ram_addr_t lowmem;
> >>
> >>@@ -190,6 +177,20 @@ static void pc_init1(MachineState *machine,
> >> } else {
> >> gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> >> }
> >>+}
> >>+
> >>+static void pc_machine_pci_bus_init(MachineState *machine,
> >>+ int pci_enabled,
> >>+ PCII440FXState *i440fx_state,
> >>+ int piix3_devfn,
> >>+ PCIBus *pci_bus,
> >>+ ISABus *isa_bus,
> >>+ qemu_irq *gsi,
> >>+ MemoryRegion *pci_memory,
> >>+ MemoryRegion *ram_memory)
> >>+{
> >>+ MemoryRegion *system_memory = get_system_memory();
> >>+ MemoryRegion *system_io = get_system_io();
> >>
> >> if (pci_enabled) {
> >> pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
> >>@@ -203,6 +204,28 @@ static void pc_init1(MachineState *machine,
> >> isa_bus = isa_bus_new(NULL, system_io);
> >> no_hpet = 1;
> >> }
> >>+}
> >>+
> >>+static void pc_machine_device_init(MachineState *machine,
> >>+ int pci_enabled,
> >>+ GSIState *gsi_state,
> >>+ DeviceState *icc_bridge,
> >>+ int piix3_devfn,
> >>+ FWCfgState *fw_cfg,
> >>+ PCIBus *pci_bus,
> >>+ ISABus *isa_bus,
> >>+ qemu_irq *gsi)
> >>+{
> >>+ int i;
> >>+ DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> >>+ BusState *idebus[MAX_IDE_BUS];
> >>+ qemu_irq *smi_irq;
> >>+ PCMachineState *pc_machine = PC_MACHINE(machine);
> >>+ qemu_irq *cpu_irq;
> >>+ qemu_irq *i8259;
> >>+ ISADevice *rtc_state;
> >>+ ISADevice *floppy;
> >>+
> >> isa_bus_irqs(isa_bus, gsi);
> >>
> >> if (kvm_irqchip_in_kernel()) {
> >>@@ -290,6 +313,30 @@ static void pc_init1(MachineState *machine,
> >> }
> >> }
> >>
> >>+/* PC hardware initialisation */
> >>+static void pc_init1(MachineState *machine,
> >>+ int pci_enabled,
> >>+ int kvmclock_enabled)
> >>+{
> >>+ PCIBus *pci_bus = NULL;
> >>+ ISABus *isa_bus = NULL;
> >>+ PCII440FXState *i440fx_state = NULL;
> >>+ int piix3_devfn = -1;
> >>+ qemu_irq *gsi = NULL;
> >>+ GSIState *gsi_state = NULL;
> >>+ MemoryRegion *ram_memory = NULL;
> >>+ MemoryRegion *pci_memory = NULL;
> >>+ DeviceState *icc_bridge = NULL;
> >>+ FWCfgState *fw_cfg = NULL;
> >
> >These are set to NULL here and never modified below.
> >Why does it make sense?
> >
> >
> >>+
> >>+ pc_machine_base_init(machine, pci_enabled, kvmclock_enabled,
> >>icc_bridge,
> >>+ ram_memory, pci_memory, gsi, gsi_state, fw_cfg);
> >>+ pc_machine_pci_bus_init(machine, pci_enabled, i440fx_state,
> >>piix3_devfn,
> >>+ pci_bus, isa_bus, gsi, pci_memory, ram_memory);
> >>+ pc_machine_device_init(machine, pci_enabled, gsi_state, icc_bridge,
> >>+ piix3_devfn, fw_cfg, pci_bus, isa_bus, gsi);
> >>+}
> >>+
> >> static void pc_init_pci(MachineState *machine)
> >> {
> >> pc_init1(machine, 1, 1);
> >>--
> >>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