From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47461) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a44Hw-0006Yg-9e for qemu-devel@nongnu.org; Wed, 02 Dec 2015 05:05:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a44Hr-0000wW-O3 for qemu-devel@nongnu.org; Wed, 02 Dec 2015 05:05:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32989) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a44Hr-0000wH-Gp for qemu-devel@nongnu.org; Wed, 02 Dec 2015 05:05:23 -0500 References: <1449010688-19205-1-git-send-email-ehabkost@redhat.com> <1449010688-19205-2-git-send-email-ehabkost@redhat.com> <565EC096.2090602@redhat.com> From: Marcel Apfelbaum Message-ID: <565EC260.5040306@redhat.com> Date: Wed, 2 Dec 2015 12:05:20 +0200 MIME-Version: 1.0 In-Reply-To: <565EC096.2090602@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 1/6] pc: Move compat boolean globals to PCMachineClass List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , qemu-devel@nongnu.org Cc: Marcel Apfelbaum , "Michael S. Tsirkin" , David Gibson On 12/02/2015 11:57 AM, Marcel Apfelbaum wrote: > On 12/02/2015 12:58 AM, Eduardo Habkost wrote: >> This way the compat flags can be initialized in the machine_options() >> function. This will help us to eventually eliminate the pc_compat_*() >> functions. > > Hi, I have only a minor comment here, > >> >> Signed-off-by: Eduardo Habkost >> --- >> hw/i386/pc.c | 8 +++++ >> hw/i386/pc_piix.c | 84 +++++++++++++++++++++++++--------------------------- >> hw/i386/pc_q35.c | 54 +++++++++++++++------------------ >> include/hw/i386/pc.h | 14 +++++++++ >> 4 files changed, 86 insertions(+), 74 deletions(-) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 5e20e07..129aa04 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -1952,6 +1952,14 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) >> HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); >> >> pcmc->get_hotplug_handler = mc->get_hotplug_handler; >> + pcmc->pci_enabled = true; >> + pcmc->has_acpi_build = true; >> + pcmc->rsdp_in_ram = true; >> + pcmc->smbios_defaults = true; >> + pcmc->smbios_uuid_encoded = true; >> + pcmc->gigabyte_align = true; >> + pcmc->has_reserved_memory = true; >> + pcmc->kvmclock_enabled = true; >> mc->get_hotplug_handler = pc_get_hotpug_handler; >> mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; >> mc->default_boot_order = "cad"; >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 2e41efe..7a7f748 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -60,26 +60,14 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; >> static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; >> static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; >> >> -static bool pci_enabled = true; >> -static bool has_acpi_build = true; >> -static bool rsdp_in_ram = true; >> static int legacy_acpi_table_size; >> -static bool smbios_defaults = true; >> -static bool smbios_legacy_mode; >> -static bool smbios_uuid_encoded = true; >> -/* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to >> - * host addresses aligned at 1Gbyte boundaries. This way we can use 1GByte >> - * pages in the host. >> - */ >> -static bool gigabyte_align = true; >> -static bool has_reserved_memory = true; >> -static bool kvmclock_enabled = true; >> >> /* PC hardware initialisation */ >> static void pc_init1(MachineState *machine, >> const char *host_type, const char *pci_type) >> { >> PCMachineState *pcms = PC_MACHINE(machine); >> + PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); >> MemoryRegion *system_memory = get_system_memory(); >> MemoryRegion *system_io = get_system_io(); >> int i; >> @@ -108,7 +96,7 @@ static void pc_init1(MachineState *machine, >> * breaking migration. >> */ >> if (machine->ram_size >= 0xe0000000) { >> - lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000; >> + lowmem = pcmc->gigabyte_align ? 0xc0000000 : 0xe0000000; >> } else { >> lowmem = 0xe0000000; >> } >> @@ -141,11 +129,11 @@ static void pc_init1(MachineState *machine, >> >> pc_cpus_init(pcms); >> >> - if (kvm_enabled() && kvmclock_enabled) { >> + if (kvm_enabled() && pcmc->kvmclock_enabled) { >> kvmclock_create(); >> } >> >> - if (pci_enabled) { >> + if (pcmc->pci_enabled) { >> pci_memory = g_new(MemoryRegion, 1); >> memory_region_init(pci_memory, NULL, "pci", UINT64_MAX); >> rom_memory = pci_memory; >> @@ -156,18 +144,19 @@ static void pc_init1(MachineState *machine, >> >> guest_info = pc_guest_info_init(pcms); >> >> - guest_info->has_acpi_build = has_acpi_build; >> + guest_info->has_acpi_build = pcmc->has_acpi_build; >> guest_info->legacy_acpi_table_size = legacy_acpi_table_size; > > Why is legacy_acpi_table_size left behind? Maybe it is a new field. Forget about it, the answer is the next patch :) Reviewed-by: Marcel Apfelbaum Thanks, Marcel