| Message ID | 1454586455-10202-8-git-send-email-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 4 Feb 2016 14:48:58 +0200 Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote: > On 02/04/2016 01:47 PM, Igor Mammedov wrote: > > yet another cleanup that replaces multiple apic_id_limit > > variables that were initialized at differnt places with > > a one initialization at pc_cpus_init() time and letting > > other code to just read it. That also allows to reduce > > number of a function arguments where it takes > > MachineState as an argument. > > > > This patch can collide with Eduardo's PcGuestInfo elimination series. > I am not sure what is the status of those patches. I guess it's Eduardo can take this patch or use his. Eduardo, I don't care much about this patch, just let me know what would you prefer to do with it. > > Thanks, > Marcel > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > hw/i386/acpi-build.c | 14 +++++++------- > > hw/i386/pc.c | 49 +++++++++++++++++++++---------------------------- > > include/hw/i386/pc.h | 4 +++- > > 3 files changed, 31 insertions(+), 36 deletions(-) > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 921830e..d6cd06a 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -937,7 +937,7 @@ static Aml *build_crs(PCIHostState *host, > > return crs; > > } > > > > -static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > +static void build_processor_devices(Aml *sb_scope, > > AcpiPmInfo *pm, MachineState *machine) > > { > > int i; > > @@ -948,12 +948,13 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > Aml *ifctx; > > Aml *method; > > MachineClass *mc = MACHINE_GET_CLASS(machine); > > + PCMachineState *pcms = PC_MACHINE(machine); > > GArray *apic_id_list = mc->possible_cpu_arch_ids(); > > > > /* The current AML generator can cover the APIC ID range [0..255], > > * inclusive, for VCPU hotplug. */ > > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); > > - g_assert(acpi_cpus <= ACPI_CPU_HOTPLUG_ID_LIMIT); > > + g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT); > > > > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */ > > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE)); > > @@ -1029,10 +1030,10 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > * ith up to 255 elements. Windows guests up to win2k8 fail when > > * VarPackageOp is used. > > */ > > - pkg = acpi_cpus <= 255 ? aml_package(acpi_cpus) : > > - aml_varpackage(acpi_cpus); > > + pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) : > > + aml_varpackage(pcms->apic_id_limit); > > > > - for (i = 0; i < acpi_cpus; i++) { > > + for (i = 0; i < pcms->apic_id_limit; i++) { > > uint8_t b = qemu_get_cpu_by_arch_id(i) ? 0x01 : 0x00; > > aml_append(pkg, aml_int(b)); > > } > > @@ -2233,8 +2234,7 @@ build_dsdt(GArray *table_data, GArray *linker, > > > > sb_scope = aml_scope("\\_SB"); > > { > > - build_processor_devices(sb_scope, guest_info->apic_id_limit, pm, > > - machine); > > + build_processor_devices(sb_scope, pm, machine); > > > > build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base, > > pm->mem_hp_io_len); > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 2fd8fc8..61fbb11 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index) > > } > > } > > > > -/* Calculates the limit to CPU APIC ID values > > - * > > - * This function returns the limit for the APIC ID value, so that all > > - * CPU APIC IDs are < pc_apic_id_limit(). > > - * > > - * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). > > - */ > > -static unsigned int pc_apic_id_limit(unsigned int max_cpus) > > -{ > > - return x86_cpu_apic_id_from_index(max_cpus - 1) + 1; > > -} > > - > > static void pc_build_smbios(FWCfgState *fw_cfg) > > { > > uint8_t *smbios_tables, *smbios_anchor; > > @@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg) > > } > > } > > > > -static FWCfgState *bochs_bios_init(AddressSpace *as) > > +static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms) > > { > > FWCfgState *fw_cfg; > > uint64_t *numa_fw_cfg; > > int i, j; > > - unsigned int apic_id_limit = pc_apic_id_limit(max_cpus); > > > > fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as); > > > > @@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is > > * the APIC ID, not the "CPU index" > > */ > > - fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit); > > + fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit); > > fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size); > > fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, > > acpi_tables, acpi_tables_len); > > @@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > * of nodes, one word for each VCPU->node and one word for each node to > > * hold the amount of memory. > > */ > > - numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes); > > + numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes); > > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes); > > for (i = 0; i < max_cpus; i++) { > > unsigned int apic_id = x86_cpu_apic_id_from_index(i); > > - assert(apic_id < apic_id_limit); > > + assert(apic_id < pcms->apic_id_limit); > > for (j = 0; j < nb_numa_nodes; j++) { > > if (test_bit(i, numa_info[j].node_cpu)) { > > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j); > > @@ -803,10 +790,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > } > > } > > for (i = 0; i < nb_numa_nodes; i++) { > > - numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem); > > + numa_fw_cfg[pcms->apic_id_limit + 1 + i] = > > + cpu_to_le64(numa_info[i].node_mem); > > } > > fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg, > > - (1 + apic_id_limit + nb_numa_nodes) * > > + (1 + pcms->apic_id_limit + nb_numa_nodes) * > > sizeof(*numa_fw_cfg)); > > > > return fw_cfg; > > @@ -1120,7 +1108,14 @@ void pc_cpus_init(PCMachineState *pcms) > > int i; > > X86CPU *cpu = NULL; > > MachineState *machine = MACHINE(pcms); > > - unsigned long apic_id_limit; > > + > > + /* Calculates the limit to CPU APIC ID values so that all > > + * CPU APIC IDs are < pcms->apic_id_limit. > > + * > > + * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). > > + * and bitmap based ACPI CPU hotplug interface > > + */ > > + pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1; > > > > /* init CPUs */ > > if (machine->cpu_model == NULL) { > > @@ -1131,10 +1126,9 @@ void pc_cpus_init(PCMachineState *pcms) > > #endif > > } > > > > - apic_id_limit = pc_apic_id_limit(max_cpus); > > - if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > > - error_report("max_cpus is too large. APIC ID of last CPU is %lu", > > - apic_id_limit - 1); > > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > > + error_report("max_cpus is too large. APIC ID of last CPU is %u", > > + pcms->apic_id_limit - 1); > > exit(1); > > } > > > > @@ -1197,7 +1191,6 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) > > > > guest_info->ram_size_below_4g = pcms->below_4g_mem_size; > > guest_info->ram_size = pcms->below_4g_mem_size + pcms->above_4g_mem_size; > > - guest_info->apic_id_limit = pc_apic_id_limit(max_cpus); > > guest_info->apic_xrupt_override = kvm_allows_irq0_override(); > > guest_info->numa_nodes = nb_numa_nodes; > > guest_info->node_mem = g_malloc0(guest_info->numa_nodes * > > @@ -1206,12 +1199,12 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) > > guest_info->node_mem[i] = numa_info[i].node_mem; > > } > > > > - guest_info->node_cpu = g_malloc0(guest_info->apic_id_limit * > > + guest_info->node_cpu = g_malloc0(pcms->apic_id_limit * > > sizeof *guest_info->node_cpu); > > > > for (i = 0; i < max_cpus; i++) { > > unsigned int apic_id = x86_cpu_apic_id_from_index(i); > > - assert(apic_id < guest_info->apic_id_limit); > > + assert(apic_id < pcms->apic_id_limit); > > for (j = 0; j < nb_numa_nodes; j++) { > > if (test_bit(i, numa_info[j].node_cpu)) { > > guest_info->node_cpu[apic_id] = j; > > @@ -1386,7 +1379,7 @@ FWCfgState *pc_memory_init(PCMachineState *pcms, > > option_rom_mr, > > 1); > > > > - fw_cfg = bochs_bios_init(&address_space_memory); > > + fw_cfg = bochs_bios_init(&address_space_memory, pcms); > > > > rom_set_fw(fw_cfg); > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index 65e8f24..315fed2 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -59,6 +59,9 @@ struct PCMachineState { > > > > /* RAM information (sizes, addresses, configuration): */ > > ram_addr_t below_4g_mem_size, above_4g_mem_size; > > + > > + /* APIC ID limit for current max_cpus */ > > + unsigned apic_id_limit; > > }; > > > > #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" > > @@ -154,7 +157,6 @@ typedef struct PcPciInfo { > > struct PcGuestInfo { > > bool isapc_ram_fw; > > hwaddr ram_size, ram_size_below_4g; > > - unsigned apic_id_limit; > > bool apic_xrupt_override; > > uint64_t numa_nodes; > > uint64_t *node_mem; > > >
On Thu, Feb 04, 2016 at 06:08:38PM +0100, Igor Mammedov wrote: > On Thu, 4 Feb 2016 14:48:58 +0200 > Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote: > > > On 02/04/2016 01:47 PM, Igor Mammedov wrote: > > > yet another cleanup that replaces multiple apic_id_limit > > > variables that were initialized at differnt places with > > > a one initialization at pc_cpus_init() time and letting > > > other code to just read it. That also allows to reduce > > > number of a function arguments where it takes > > > MachineState as an argument. > > > > > > > This patch can collide with Eduardo's PcGuestInfo elimination series. > > I am not sure what is the status of those patches. > I guess it's Eduardo can take this patch or use his. > > Eduardo, > I don't care much about this patch, just let me know > what would you prefer to do with it. Eduardo's work is in my tree, you can rebase on top of it. > > > > Thanks, > > Marcel > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > hw/i386/acpi-build.c | 14 +++++++------- > > > hw/i386/pc.c | 49 +++++++++++++++++++++---------------------------- > > > include/hw/i386/pc.h | 4 +++- > > > 3 files changed, 31 insertions(+), 36 deletions(-) > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > index 921830e..d6cd06a 100644 > > > --- a/hw/i386/acpi-build.c > > > +++ b/hw/i386/acpi-build.c > > > @@ -937,7 +937,7 @@ static Aml *build_crs(PCIHostState *host, > > > return crs; > > > } > > > > > > -static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > > +static void build_processor_devices(Aml *sb_scope, > > > AcpiPmInfo *pm, MachineState *machine) > > > { > > > int i; > > > @@ -948,12 +948,13 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > > Aml *ifctx; > > > Aml *method; > > > MachineClass *mc = MACHINE_GET_CLASS(machine); > > > + PCMachineState *pcms = PC_MACHINE(machine); > > > GArray *apic_id_list = mc->possible_cpu_arch_ids(); > > > > > > /* The current AML generator can cover the APIC ID range [0..255], > > > * inclusive, for VCPU hotplug. */ > > > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); > > > - g_assert(acpi_cpus <= ACPI_CPU_HOTPLUG_ID_LIMIT); > > > + g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT); > > > > > > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */ > > > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE)); > > > @@ -1029,10 +1030,10 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > > * ith up to 255 elements. Windows guests up to win2k8 fail when > > > * VarPackageOp is used. > > > */ > > > - pkg = acpi_cpus <= 255 ? aml_package(acpi_cpus) : > > > - aml_varpackage(acpi_cpus); > > > + pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) : > > > + aml_varpackage(pcms->apic_id_limit); > > > > > > - for (i = 0; i < acpi_cpus; i++) { > > > + for (i = 0; i < pcms->apic_id_limit; i++) { > > > uint8_t b = qemu_get_cpu_by_arch_id(i) ? 0x01 : 0x00; > > > aml_append(pkg, aml_int(b)); > > > } > > > @@ -2233,8 +2234,7 @@ build_dsdt(GArray *table_data, GArray *linker, > > > > > > sb_scope = aml_scope("\\_SB"); > > > { > > > - build_processor_devices(sb_scope, guest_info->apic_id_limit, pm, > > > - machine); > > > + build_processor_devices(sb_scope, pm, machine); > > > > > > build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base, > > > pm->mem_hp_io_len); > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > index 2fd8fc8..61fbb11 100644 > > > --- a/hw/i386/pc.c > > > +++ b/hw/i386/pc.c > > > @@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index) > > > } > > > } > > > > > > -/* Calculates the limit to CPU APIC ID values > > > - * > > > - * This function returns the limit for the APIC ID value, so that all > > > - * CPU APIC IDs are < pc_apic_id_limit(). > > > - * > > > - * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). > > > - */ > > > -static unsigned int pc_apic_id_limit(unsigned int max_cpus) > > > -{ > > > - return x86_cpu_apic_id_from_index(max_cpus - 1) + 1; > > > -} > > > - > > > static void pc_build_smbios(FWCfgState *fw_cfg) > > > { > > > uint8_t *smbios_tables, *smbios_anchor; > > > @@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg) > > > } > > > } > > > > > > -static FWCfgState *bochs_bios_init(AddressSpace *as) > > > +static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms) > > > { > > > FWCfgState *fw_cfg; > > > uint64_t *numa_fw_cfg; > > > int i, j; > > > - unsigned int apic_id_limit = pc_apic_id_limit(max_cpus); > > > > > > fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as); > > > > > > @@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > > * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is > > > * the APIC ID, not the "CPU index" > > > */ > > > - fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit); > > > + fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit); > > > fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size); > > > fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, > > > acpi_tables, acpi_tables_len); > > > @@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > > * of nodes, one word for each VCPU->node and one word for each node to > > > * hold the amount of memory. > > > */ > > > - numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes); > > > + numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes); > > > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes); > > > for (i = 0; i < max_cpus; i++) { > > > unsigned int apic_id = x86_cpu_apic_id_from_index(i); > > > - assert(apic_id < apic_id_limit); > > > + assert(apic_id < pcms->apic_id_limit); > > > for (j = 0; j < nb_numa_nodes; j++) { > > > if (test_bit(i, numa_info[j].node_cpu)) { > > > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j); > > > @@ -803,10 +790,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > > } > > > } > > > for (i = 0; i < nb_numa_nodes; i++) { > > > - numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem); > > > + numa_fw_cfg[pcms->apic_id_limit + 1 + i] = > > > + cpu_to_le64(numa_info[i].node_mem); > > > } > > > fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg, > > > - (1 + apic_id_limit + nb_numa_nodes) * > > > + (1 + pcms->apic_id_limit + nb_numa_nodes) * > > > sizeof(*numa_fw_cfg)); > > > > > > return fw_cfg; > > > @@ -1120,7 +1108,14 @@ void pc_cpus_init(PCMachineState *pcms) > > > int i; > > > X86CPU *cpu = NULL; > > > MachineState *machine = MACHINE(pcms); > > > - unsigned long apic_id_limit; > > > + > > > + /* Calculates the limit to CPU APIC ID values so that all > > > + * CPU APIC IDs are < pcms->apic_id_limit. > > > + * > > > + * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). > > > + * and bitmap based ACPI CPU hotplug interface > > > + */ > > > + pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1; > > > > > > /* init CPUs */ > > > if (machine->cpu_model == NULL) { > > > @@ -1131,10 +1126,9 @@ void pc_cpus_init(PCMachineState *pcms) > > > #endif > > > } > > > > > > - apic_id_limit = pc_apic_id_limit(max_cpus); > > > - if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > > > - error_report("max_cpus is too large. APIC ID of last CPU is %lu", > > > - apic_id_limit - 1); > > > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > > > + error_report("max_cpus is too large. APIC ID of last CPU is %u", > > > + pcms->apic_id_limit - 1); > > > exit(1); > > > } > > > > > > @@ -1197,7 +1191,6 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) > > > > > > guest_info->ram_size_below_4g = pcms->below_4g_mem_size; > > > guest_info->ram_size = pcms->below_4g_mem_size + pcms->above_4g_mem_size; > > > - guest_info->apic_id_limit = pc_apic_id_limit(max_cpus); > > > guest_info->apic_xrupt_override = kvm_allows_irq0_override(); > > > guest_info->numa_nodes = nb_numa_nodes; > > > guest_info->node_mem = g_malloc0(guest_info->numa_nodes * > > > @@ -1206,12 +1199,12 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) > > > guest_info->node_mem[i] = numa_info[i].node_mem; > > > } > > > > > > - guest_info->node_cpu = g_malloc0(guest_info->apic_id_limit * > > > + guest_info->node_cpu = g_malloc0(pcms->apic_id_limit * > > > sizeof *guest_info->node_cpu); > > > > > > for (i = 0; i < max_cpus; i++) { > > > unsigned int apic_id = x86_cpu_apic_id_from_index(i); > > > - assert(apic_id < guest_info->apic_id_limit); > > > + assert(apic_id < pcms->apic_id_limit); > > > for (j = 0; j < nb_numa_nodes; j++) { > > > if (test_bit(i, numa_info[j].node_cpu)) { > > > guest_info->node_cpu[apic_id] = j; > > > @@ -1386,7 +1379,7 @@ FWCfgState *pc_memory_init(PCMachineState *pcms, > > > option_rom_mr, > > > 1); > > > > > > - fw_cfg = bochs_bios_init(&address_space_memory); > > > + fw_cfg = bochs_bios_init(&address_space_memory, pcms); > > > > > > rom_set_fw(fw_cfg); > > > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > > index 65e8f24..315fed2 100644 > > > --- a/include/hw/i386/pc.h > > > +++ b/include/hw/i386/pc.h > > > @@ -59,6 +59,9 @@ struct PCMachineState { > > > > > > /* RAM information (sizes, addresses, configuration): */ > > > ram_addr_t below_4g_mem_size, above_4g_mem_size; > > > + > > > + /* APIC ID limit for current max_cpus */ > > > + unsigned apic_id_limit; > > > }; > > > > > > #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" > > > @@ -154,7 +157,6 @@ typedef struct PcPciInfo { > > > struct PcGuestInfo { > > > bool isapc_ram_fw; > > > hwaddr ram_size, ram_size_below_4g; > > > - unsigned apic_id_limit; > > > bool apic_xrupt_override; > > > uint64_t numa_nodes; > > > uint64_t *node_mem; > > > > >
On Thu, 4 Feb 2016 20:18:04 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Feb 04, 2016 at 06:08:38PM +0100, Igor Mammedov wrote: > > On Thu, 4 Feb 2016 14:48:58 +0200 > > Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote: > > > > > On 02/04/2016 01:47 PM, Igor Mammedov wrote: > > > > yet another cleanup that replaces multiple apic_id_limit > > > > variables that were initialized at differnt places with > > > > a one initialization at pc_cpus_init() time and letting > > > > other code to just read it. That also allows to reduce > > > > number of a function arguments where it takes > > > > MachineState as an argument. > > > > > > > > > > This patch can collide with Eduardo's PcGuestInfo elimination series. > > > I am not sure what is the status of those patches. > > I guess it's Eduardo can take this patch or use his. > > > > Eduardo, > > I don't care much about this patch, just let me know > > what would you prefer to do with it. > > Eduardo's work is in my tree, you can rebase on top of it. Ok thanks, I'll wait for comments for a couple days and then rebase. > > > > > > > Thanks, > > > Marcel > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > --- > > > > hw/i386/acpi-build.c | 14 +++++++------- > > > > hw/i386/pc.c | 49 +++++++++++++++++++++---------------------------- > > > > include/hw/i386/pc.h | 4 +++- > > > > 3 files changed, 31 insertions(+), 36 deletions(-) > > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > index 921830e..d6cd06a 100644 > > > > --- a/hw/i386/acpi-build.c > > > > +++ b/hw/i386/acpi-build.c > > > > @@ -937,7 +937,7 @@ static Aml *build_crs(PCIHostState *host, > > > > return crs; > > > > } > > > > > > > > -static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > > > +static void build_processor_devices(Aml *sb_scope, > > > > AcpiPmInfo *pm, MachineState *machine) > > > > { > > > > int i; > > > > @@ -948,12 +948,13 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > > > Aml *ifctx; > > > > Aml *method; > > > > MachineClass *mc = MACHINE_GET_CLASS(machine); > > > > + PCMachineState *pcms = PC_MACHINE(machine); > > > > GArray *apic_id_list = mc->possible_cpu_arch_ids(); > > > > > > > > /* The current AML generator can cover the APIC ID range [0..255], > > > > * inclusive, for VCPU hotplug. */ > > > > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); > > > > - g_assert(acpi_cpus <= ACPI_CPU_HOTPLUG_ID_LIMIT); > > > > + g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT); > > > > > > > > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */ > > > > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE)); > > > > @@ -1029,10 +1030,10 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, > > > > * ith up to 255 elements. Windows guests up to win2k8 fail when > > > > * VarPackageOp is used. > > > > */ > > > > - pkg = acpi_cpus <= 255 ? aml_package(acpi_cpus) : > > > > - aml_varpackage(acpi_cpus); > > > > + pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) : > > > > + aml_varpackage(pcms->apic_id_limit); > > > > > > > > - for (i = 0; i < acpi_cpus; i++) { > > > > + for (i = 0; i < pcms->apic_id_limit; i++) { > > > > uint8_t b = qemu_get_cpu_by_arch_id(i) ? 0x01 : 0x00; > > > > aml_append(pkg, aml_int(b)); > > > > } > > > > @@ -2233,8 +2234,7 @@ build_dsdt(GArray *table_data, GArray *linker, > > > > > > > > sb_scope = aml_scope("\\_SB"); > > > > { > > > > - build_processor_devices(sb_scope, guest_info->apic_id_limit, pm, > > > > - machine); > > > > + build_processor_devices(sb_scope, pm, machine); > > > > > > > > build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base, > > > > pm->mem_hp_io_len); > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > > index 2fd8fc8..61fbb11 100644 > > > > --- a/hw/i386/pc.c > > > > +++ b/hw/i386/pc.c > > > > @@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index) > > > > } > > > > } > > > > > > > > -/* Calculates the limit to CPU APIC ID values > > > > - * > > > > - * This function returns the limit for the APIC ID value, so that all > > > > - * CPU APIC IDs are < pc_apic_id_limit(). > > > > - * > > > > - * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). > > > > - */ > > > > -static unsigned int pc_apic_id_limit(unsigned int max_cpus) > > > > -{ > > > > - return x86_cpu_apic_id_from_index(max_cpus - 1) + 1; > > > > -} > > > > - > > > > static void pc_build_smbios(FWCfgState *fw_cfg) > > > > { > > > > uint8_t *smbios_tables, *smbios_anchor; > > > > @@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg) > > > > } > > > > } > > > > > > > > -static FWCfgState *bochs_bios_init(AddressSpace *as) > > > > +static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms) > > > > { > > > > FWCfgState *fw_cfg; > > > > uint64_t *numa_fw_cfg; > > > > int i, j; > > > > - unsigned int apic_id_limit = pc_apic_id_limit(max_cpus); > > > > > > > > fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as); > > > > > > > > @@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > > > * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is > > > > * the APIC ID, not the "CPU index" > > > > */ > > > > - fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit); > > > > + fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit); > > > > fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size); > > > > fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, > > > > acpi_tables, acpi_tables_len); > > > > @@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > > > * of nodes, one word for each VCPU->node and one word for each node to > > > > * hold the amount of memory. > > > > */ > > > > - numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes); > > > > + numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes); > > > > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes); > > > > for (i = 0; i < max_cpus; i++) { > > > > unsigned int apic_id = x86_cpu_apic_id_from_index(i); > > > > - assert(apic_id < apic_id_limit); > > > > + assert(apic_id < pcms->apic_id_limit); > > > > for (j = 0; j < nb_numa_nodes; j++) { > > > > if (test_bit(i, numa_info[j].node_cpu)) { > > > > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j); > > > > @@ -803,10 +790,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) > > > > } > > > > } > > > > for (i = 0; i < nb_numa_nodes; i++) { > > > > - numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem); > > > > + numa_fw_cfg[pcms->apic_id_limit + 1 + i] = > > > > + cpu_to_le64(numa_info[i].node_mem); > > > > } > > > > fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg, > > > > - (1 + apic_id_limit + nb_numa_nodes) * > > > > + (1 + pcms->apic_id_limit + nb_numa_nodes) * > > > > sizeof(*numa_fw_cfg)); > > > > > > > > return fw_cfg; > > > > @@ -1120,7 +1108,14 @@ void pc_cpus_init(PCMachineState *pcms) > > > > int i; > > > > X86CPU *cpu = NULL; > > > > MachineState *machine = MACHINE(pcms); > > > > - unsigned long apic_id_limit; > > > > + > > > > + /* Calculates the limit to CPU APIC ID values so that all > > > > + * CPU APIC IDs are < pcms->apic_id_limit. > > > > + * > > > > + * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). > > > > + * and bitmap based ACPI CPU hotplug interface > > > > + */ > > > > + pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1; > > > > > > > > /* init CPUs */ > > > > if (machine->cpu_model == NULL) { > > > > @@ -1131,10 +1126,9 @@ void pc_cpus_init(PCMachineState *pcms) > > > > #endif > > > > } > > > > > > > > - apic_id_limit = pc_apic_id_limit(max_cpus); > > > > - if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > > > > - error_report("max_cpus is too large. APIC ID of last CPU is %lu", > > > > - apic_id_limit - 1); > > > > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > > > > + error_report("max_cpus is too large. APIC ID of last CPU is %u", > > > > + pcms->apic_id_limit - 1); > > > > exit(1); > > > > } > > > > > > > > @@ -1197,7 +1191,6 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) > > > > > > > > guest_info->ram_size_below_4g = pcms->below_4g_mem_size; > > > > guest_info->ram_size = pcms->below_4g_mem_size + pcms->above_4g_mem_size; > > > > - guest_info->apic_id_limit = pc_apic_id_limit(max_cpus); > > > > guest_info->apic_xrupt_override = kvm_allows_irq0_override(); > > > > guest_info->numa_nodes = nb_numa_nodes; > > > > guest_info->node_mem = g_malloc0(guest_info->numa_nodes * > > > > @@ -1206,12 +1199,12 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) > > > > guest_info->node_mem[i] = numa_info[i].node_mem; > > > > } > > > > > > > > - guest_info->node_cpu = g_malloc0(guest_info->apic_id_limit * > > > > + guest_info->node_cpu = g_malloc0(pcms->apic_id_limit * > > > > sizeof *guest_info->node_cpu); > > > > > > > > for (i = 0; i < max_cpus; i++) { > > > > unsigned int apic_id = x86_cpu_apic_id_from_index(i); > > > > - assert(apic_id < guest_info->apic_id_limit); > > > > + assert(apic_id < pcms->apic_id_limit); > > > > for (j = 0; j < nb_numa_nodes; j++) { > > > > if (test_bit(i, numa_info[j].node_cpu)) { > > > > guest_info->node_cpu[apic_id] = j; > > > > @@ -1386,7 +1379,7 @@ FWCfgState *pc_memory_init(PCMachineState *pcms, > > > > option_rom_mr, > > > > 1); > > > > > > > > - fw_cfg = bochs_bios_init(&address_space_memory); > > > > + fw_cfg = bochs_bios_init(&address_space_memory, pcms); > > > > > > > > rom_set_fw(fw_cfg); > > > > > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > > > index 65e8f24..315fed2 100644 > > > > --- a/include/hw/i386/pc.h > > > > +++ b/include/hw/i386/pc.h > > > > @@ -59,6 +59,9 @@ struct PCMachineState { > > > > > > > > /* RAM information (sizes, addresses, configuration): */ > > > > ram_addr_t below_4g_mem_size, above_4g_mem_size; > > > > + > > > > + /* APIC ID limit for current max_cpus */ > > > > + unsigned apic_id_limit; > > > > }; > > > > > > > > #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" > > > > @@ -154,7 +157,6 @@ typedef struct PcPciInfo { > > > > struct PcGuestInfo { > > > > bool isapc_ram_fw; > > > > hwaddr ram_size, ram_size_below_4g; > > > > - unsigned apic_id_limit; > > > > bool apic_xrupt_override; > > > > uint64_t numa_nodes; > > > > uint64_t *node_mem; > > > > > > > >
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 921830e..d6cd06a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -937,7 +937,7 @@ static Aml *build_crs(PCIHostState *host, return crs; } -static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, +static void build_processor_devices(Aml *sb_scope, AcpiPmInfo *pm, MachineState *machine) { int i; @@ -948,12 +948,13 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, Aml *ifctx; Aml *method; MachineClass *mc = MACHINE_GET_CLASS(machine); + PCMachineState *pcms = PC_MACHINE(machine); GArray *apic_id_list = mc->possible_cpu_arch_ids(); /* The current AML generator can cover the APIC ID range [0..255], * inclusive, for VCPU hotplug. */ QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); - g_assert(acpi_cpus <= ACPI_CPU_HOTPLUG_ID_LIMIT); + g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT); /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */ dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE)); @@ -1029,10 +1030,10 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus, * ith up to 255 elements. Windows guests up to win2k8 fail when * VarPackageOp is used. */ - pkg = acpi_cpus <= 255 ? aml_package(acpi_cpus) : - aml_varpackage(acpi_cpus); + pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) : + aml_varpackage(pcms->apic_id_limit); - for (i = 0; i < acpi_cpus; i++) { + for (i = 0; i < pcms->apic_id_limit; i++) { uint8_t b = qemu_get_cpu_by_arch_id(i) ? 0x01 : 0x00; aml_append(pkg, aml_int(b)); } @@ -2233,8 +2234,7 @@ build_dsdt(GArray *table_data, GArray *linker, sb_scope = aml_scope("\\_SB"); { - build_processor_devices(sb_scope, guest_info->apic_id_limit, pm, - machine); + build_processor_devices(sb_scope, pm, machine); build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base, pm->mem_hp_io_len); diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 2fd8fc8..61fbb11 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index) } } -/* Calculates the limit to CPU APIC ID values - * - * This function returns the limit for the APIC ID value, so that all - * CPU APIC IDs are < pc_apic_id_limit(). - * - * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). - */ -static unsigned int pc_apic_id_limit(unsigned int max_cpus) -{ - return x86_cpu_apic_id_from_index(max_cpus - 1) + 1; -} - static void pc_build_smbios(FWCfgState *fw_cfg) { uint8_t *smbios_tables, *smbios_anchor; @@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg) } } -static FWCfgState *bochs_bios_init(AddressSpace *as) +static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms) { FWCfgState *fw_cfg; uint64_t *numa_fw_cfg; int i, j; - unsigned int apic_id_limit = pc_apic_id_limit(max_cpus); fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as); @@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is * the APIC ID, not the "CPU index" */ - fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit); + fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit); fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size); fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, acpi_tables, acpi_tables_len); @@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) * of nodes, one word for each VCPU->node and one word for each node to * hold the amount of memory. */ - numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes); + numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes); numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes); for (i = 0; i < max_cpus; i++) { unsigned int apic_id = x86_cpu_apic_id_from_index(i); - assert(apic_id < apic_id_limit); + assert(apic_id < pcms->apic_id_limit); for (j = 0; j < nb_numa_nodes; j++) { if (test_bit(i, numa_info[j].node_cpu)) { numa_fw_cfg[apic_id + 1] = cpu_to_le64(j); @@ -803,10 +790,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as) } } for (i = 0; i < nb_numa_nodes; i++) { - numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem); + numa_fw_cfg[pcms->apic_id_limit + 1 + i] = + cpu_to_le64(numa_info[i].node_mem); } fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg, - (1 + apic_id_limit + nb_numa_nodes) * + (1 + pcms->apic_id_limit + nb_numa_nodes) * sizeof(*numa_fw_cfg)); return fw_cfg; @@ -1120,7 +1108,14 @@ void pc_cpus_init(PCMachineState *pcms) int i; X86CPU *cpu = NULL; MachineState *machine = MACHINE(pcms); - unsigned long apic_id_limit; + + /* Calculates the limit to CPU APIC ID values so that all + * CPU APIC IDs are < pcms->apic_id_limit. + * + * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init(). + * and bitmap based ACPI CPU hotplug interface + */ + pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1; /* init CPUs */ if (machine->cpu_model == NULL) { @@ -1131,10 +1126,9 @@ void pc_cpus_init(PCMachineState *pcms) #endif } - apic_id_limit = pc_apic_id_limit(max_cpus); - if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { - error_report("max_cpus is too large. APIC ID of last CPU is %lu", - apic_id_limit - 1); + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { + error_report("max_cpus is too large. APIC ID of last CPU is %u", + pcms->apic_id_limit - 1); exit(1); } @@ -1197,7 +1191,6 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) guest_info->ram_size_below_4g = pcms->below_4g_mem_size; guest_info->ram_size = pcms->below_4g_mem_size + pcms->above_4g_mem_size; - guest_info->apic_id_limit = pc_apic_id_limit(max_cpus); guest_info->apic_xrupt_override = kvm_allows_irq0_override(); guest_info->numa_nodes = nb_numa_nodes; guest_info->node_mem = g_malloc0(guest_info->numa_nodes * @@ -1206,12 +1199,12 @@ PcGuestInfo *pc_guest_info_init(PCMachineState *pcms) guest_info->node_mem[i] = numa_info[i].node_mem; } - guest_info->node_cpu = g_malloc0(guest_info->apic_id_limit * + guest_info->node_cpu = g_malloc0(pcms->apic_id_limit * sizeof *guest_info->node_cpu); for (i = 0; i < max_cpus; i++) { unsigned int apic_id = x86_cpu_apic_id_from_index(i); - assert(apic_id < guest_info->apic_id_limit); + assert(apic_id < pcms->apic_id_limit); for (j = 0; j < nb_numa_nodes; j++) { if (test_bit(i, numa_info[j].node_cpu)) { guest_info->node_cpu[apic_id] = j; @@ -1386,7 +1379,7 @@ FWCfgState *pc_memory_init(PCMachineState *pcms, option_rom_mr, 1); - fw_cfg = bochs_bios_init(&address_space_memory); + fw_cfg = bochs_bios_init(&address_space_memory, pcms); rom_set_fw(fw_cfg); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 65e8f24..315fed2 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -59,6 +59,9 @@ struct PCMachineState { /* RAM information (sizes, addresses, configuration): */ ram_addr_t below_4g_mem_size, above_4g_mem_size; + + /* APIC ID limit for current max_cpus */ + unsigned apic_id_limit; }; #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" @@ -154,7 +157,6 @@ typedef struct PcPciInfo { struct PcGuestInfo { bool isapc_ram_fw; hwaddr ram_size, ram_size_below_4g; - unsigned apic_id_limit; bool apic_xrupt_override; uint64_t numa_nodes; uint64_t *node_mem;
yet another cleanup that replaces multiple apic_id_limit variables that were initialized at differnt places with a one initialization at pc_cpus_init() time and letting other code to just read it. That also allows to reduce number of a function arguments where it takes MachineState as an argument. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/i386/acpi-build.c | 14 +++++++------- hw/i386/pc.c | 49 +++++++++++++++++++++---------------------------- include/hw/i386/pc.h | 4 +++- 3 files changed, 31 insertions(+), 36 deletions(-)