[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/4] hw:i386:pc_piix: split pc_init1()
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 <tiejun.chen@xxxxxxxxx> > > > >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 > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |