From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a44Lx-00027B-29 for qemu-devel@nongnu.org; Wed, 02 Dec 2015 05:09:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a44Lr-00035M-Rj for qemu-devel@nongnu.org; Wed, 02 Dec 2015 05:09:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a44Lr-000358-KQ for qemu-devel@nongnu.org; Wed, 02 Dec 2015 05:09:31 -0500 References: <1449010688-19205-1-git-send-email-ehabkost@redhat.com> <1449010688-19205-4-git-send-email-ehabkost@redhat.com> From: Marcel Apfelbaum Message-ID: <565EC358.4030609@redhat.com> Date: Wed, 2 Dec 2015 12:09:28 +0200 MIME-Version: 1.0 In-Reply-To: <1449010688-19205-4-git-send-email-ehabkost@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 3/6] pc: Move acpi_data_size global 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 12:58 AM, Eduardo Habkost wrote: > This way we don't need code in pc_compat_*() functions to set the legacy > acpi_data_size value. > > Signed-off-by: Eduardo Habkost > --- > hw/i386/pc.c | 17 ++++++----------- > hw/i386/pc_piix.c | 2 +- > hw/i386/pc_q35.c | 2 +- > include/hw/i386/pc.h | 6 ++++-- > 4 files changed, 12 insertions(+), 15 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 129aa04..a79b8af 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -76,15 +76,6 @@ > #define DPRINTF(fmt, ...) > #endif > > -/* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables > - * (128K) and other BIOS datastructures (less than 4K reported to be used at > - * the moment, 32K should be enough for a while). */ > -static unsigned acpi_data_size = 0x20000 + 0x8000; > -void pc_set_legacy_acpi_data_size(void) > -{ > - acpi_data_size = 0x10000; > -} > - > #define BIOS_CFG_IOPORT 0x510 > #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0) > #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1) > @@ -840,6 +831,7 @@ static void load_linux(PCMachineState *pcms, > FILE *f; > char *vmode; > MachineState *machine = MACHINE(pcms); > + PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); > const char *kernel_filename = machine->kernel_filename; > const char *initrd_filename = machine->initrd_filename; > const char *kernel_cmdline = machine->kernel_cmdline; > @@ -907,8 +899,8 @@ static void load_linux(PCMachineState *pcms, > initrd_max = 0x37ffffff; > } > > - if (initrd_max >= pcms->below_4g_mem_size - acpi_data_size) { > - initrd_max = pcms->below_4g_mem_size - acpi_data_size - 1; > + if (initrd_max >= pcms->below_4g_mem_size - pcmc->acpi_data_size) { > + initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1; > } > > fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr); > @@ -1960,6 +1952,9 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > pcmc->gigabyte_align = true; > pcmc->has_reserved_memory = true; > pcmc->kvmclock_enabled = true; > + /* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K reported > + * to be used at the moment, 32K should be enough for a while. */ > + pcmc->acpi_data_size = 0x20000 + 0x8000; > 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 5f23e9c..3562888 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -317,7 +317,6 @@ static void pc_compat_2_1(MachineState *machine) > static void pc_compat_2_0(MachineState *machine) > { > pc_compat_2_1(machine); > - pc_set_legacy_acpi_data_size(); > } > > static void pc_compat_1_7(MachineState *machine) > @@ -516,6 +515,7 @@ static void pc_i440fx_2_0_machine_options(MachineClass *m) > * QEMU 1.7 it is 6414. For RHEL/CentOS 7.0 it is 6418. > */ > pcmc->legacy_acpi_table_size = 6652; > + pcmc->acpi_data_size = 0x10000; > } > > DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0, > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index f9afaa2..43e77f4 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -302,7 +302,6 @@ static void pc_compat_2_1(MachineState *machine) > static void pc_compat_2_0(MachineState *machine) > { > pc_compat_2_1(machine); > - pc_set_legacy_acpi_data_size(); > } > > static void pc_compat_1_7(MachineState *machine) > @@ -424,6 +423,7 @@ static void pc_q35_2_0_machine_options(MachineClass *m) > SET_MACHINE_COMPAT(m, PC_COMPAT_2_0); > pcmc->has_reserved_memory = false; > pcmc->smbios_legacy_mode = true; > + pcmc->acpi_data_size = 0x10000; > } > > DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0, > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 7c2cc9f..e604ca5 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -77,6 +77,10 @@ struct PCMachineClass { > bool has_reserved_memory; > bool kvmclock_enabled; > int legacy_acpi_table_size; > + /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables > + * and other BIOS datastructures. > + */ > + unsigned acpi_data_size; > }; > > #define TYPE_PC_MACHINE "generic-pc-machine" > @@ -188,8 +192,6 @@ void pc_acpi_init(const char *default_dsdt); > > PcGuestInfo *pc_guest_info_init(PCMachineState *pcms); > > -void pc_set_legacy_acpi_data_size(void); > - > #define PCI_HOST_PROP_PCI_HOLE_START "pci-hole-start" > #define PCI_HOST_PROP_PCI_HOLE_END "pci-hole-end" > #define PCI_HOST_PROP_PCI_HOLE64_START "pci-hole64-start" > Looks OK to me. Reviewed-by: Marcel Apfelbaum Thanks, Marcel