Re: [Qemu-devel] [PATCH v3 5/7] pc: Refuse max_cpus if it results in too large APIC ID

[Laszlo Ersek]

On 03/14/14 19:52, Eduardo Habkost wrote:
> This changes the PC initialization code to reject max_cpus if it results
> in an APIC ID that's too large, instead of aborting or erroring out when
> it is already too late.
> 
> Currently there are two limits we need to check: the CPU hotplug APIC ID
> limit (due to the AcpiCpuHotplug.sts array length), and the
> MAX_CPUMASK_BITS limit (that's used for CPU bitmaps on NUMA code and
> hw/i386/acpi-build.c).
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> Changes v2 -> v3:
>  * No need to check against MAX_CPUMASK_BITS, as MAX_CPUMASK_BITS
>    is used only for CPU-index-based bitmaps on NUMA code, not for APIC
>    IDs.
> ---
>  hw/i386/pc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 74cb4f9..14f0d91 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -992,6 +992,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
> *icc_bridge)
>      int i;
>      X86CPU *cpu = NULL;
>      Error *error = NULL;
> +    unsigned long apic_id_limit;
>  
>      /* init CPUs */
>      if (cpu_model == NULL) {
> @@ -1003,6 +1004,13 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
> *icc_bridge)
>      }
>      current_cpu_model = cpu_model;
>  
> +    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);
> +        exit(1);
> +    }
> +
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
>                           icc_bridge, &error);
> 

This patch is now good, but you've forgotten to update the commit
message -- the v2->v3 change block (that you've put under the ---
separator) actually contradicts the commit message.

Please repost the patch with the updated commit message -- I think it
suffices to simply drop the second paragraph! --, and feel free to add
my R-b in that v4 posting.

Thanks,
Laszlo