diff mbox

[8/9] pc: move apic_id_limit to PCMachineState

Message ID 1454586455-10202-8-git-send-email-imammedo@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Igor Mammedov Feb. 4, 2016, 11:47 a.m. UTC
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(-)

Comments

Igor Mammedov Feb. 4, 2016, 5:08 p.m. UTC | #1
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;
> >  
>
Michael S. Tsirkin Feb. 4, 2016, 6:18 p.m. UTC | #2
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;
> > >  
> >
Igor Mammedov Feb. 4, 2016, 6:24 p.m. UTC | #3
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 mbox

Patch

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;