On Wed, Nov 27, 2013 at 11:00:48AM +0900, HATAYAMA Daisuke wrote:
> Add disable_cpu_apicid kernel parameter. To use this kernel parameter,
> specify an initial APIC ID of the corresponding CPU you want to
> disable.
>
> This is mostly used for the kdump 2nd kernel to disable BSP to wake up
> multiple CPUs without causing system reset or hang due to sending INIT
> from AP to BSP.
>
> Kdump users first figure out initial APIC ID of the BSP, CPU0 in the
> 1st kernel, for example from /proc/cpuinfo and then set up this kernel
> parameter for the 2nd kernel using the obtained APIC ID.
>
> However, doing this procedure at each boot time manually is awkward,
> which should be automatically done by user-land service scripts, for
> example, kexec-tools on fedora/RHEL distributions.
>
> This design is more flexible than disabling BSP in kernel boot time
> automatically in that in kernel boot time we have no choice but
> referring to ACPI/MP table to obtain initial APIC ID for BSP, meaning
> that the method is not applicable to the systems without such BIOS
> tables.
>
> One assumption behind this design is that users get initial APIC ID of
> the BSP in still healthy state and so BSP is uniquely kept in
> CPU0. Thus, through the kernel parameter, only one initial APIC ID can
> be specified.
>
> boot_cpu_physical_apicid is designed to have the apicid returned by
> read_apic_id(). However, on some platforms, it is temporarilly
> modified by the apicid reported as BSP through MP table. This function
> is called with the modified boot_cpu_physical_apicid.
>
> On the platforms, boot_cpu_physical_apicid is always forced to be the
> apicid of the BSP. Then, disabled_cpu_apicid kernel parameter doesn't
> work well for apicids for APs.
>
> Here we leave improvement of handling boot_cpu_physical_apicid as
> another work for now since the temporary modification is done on
> differnet platforms and tests on each platform are required. As a
> workaround, we directly update boot_cpu_physical_apicid by
> read_apic_id().
>
> Signed-off-by: HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com>
> ---
> arch/x86/kernel/apic/apic.c | 49 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index d278736..a94f618 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -75,6 +75,13 @@ unsigned int max_physical_apicid;
> physid_mask_t phys_cpu_present_map;
>
> /*
> + * Processor to be disabled specified by kernel parameter
> + * disable_cpu_apicid=<int>, mostly used for the kdump 2nd kernel to
> + * avoid undefined behaviour caused by sending INIT from AP to BSP.
> + */
> +unsigned int disabled_cpu_apicid = BAD_APICID;
> +
> +/*
> * Map cpu index to physical APIC ID
> */
> DEFINE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid, BAD_APICID);
> @@ -2111,9 +2118,42 @@ void disconnect_bsp_APIC(int virt_wire_setup)
> int generic_processor_info(int apicid, int version)
> {
> int cpu, max = nr_cpu_ids;
> + /*
> + * boot_cpu_physical_apicid is designed to have the apicid
> + * returned by read_apic_id(). However, on some platforms, it
> + * is temporarilly modified by the apicid reported as BSP
> + * through MP table. Concretely:
> + *
> + * - arch/x86/kernel/mpparse.c: MP_processor_info()
> + * - arch/x86/mm/amdtopology.c: amd_numa_init()
> + * - arch/x86/platform/visws/visws_quirks.c: MP_processor_info()
> + *
> + * This function is executed with the modified
> + * boot_cpu_physical_apicid. So, disabled_cpu_apicid kernel
> + * parameter doesn't work.
> + *
> + * Since fixing handling of boot_cpu_physical_apicid requires
> + * another discussion and tests on each platform, we leave it
> + * for now and here we use read_apic_id() directly by updating
> + * global scope of boot_cpu_physical_id with the local one.
> + */
> + unsigned int boot_cpu_physical_apicid = read_apic_id();
I think this is confusing. Why are you trying to define a local variable
with same name as global variable. There is no need. I think if we
simply put the comment there that why are we not making use of
boot_cpu_physical_apicid, that is good enough and directly read
the apic id.
if (disabled_cpu_apicid != BAD_APICID &&
disabled_cpu_apicid != read_apic_id() &&
disabled_cpu_apicid == apicid) {
}
Thanks
Vivek
> bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
> phys_cpu_present_map);
>
> + if (disabled_cpu_apicid != BAD_APICID &&
> + disabled_cpu_apicid != boot_cpu_physical_apicid &&
> + disabled_cpu_apicid == apicid) {
> + int thiscpu = num_processors + disabled_cpus;
> +
> + pr_warning("ACPI: Disabling requested cpu."
> + " Processor %d/0x%x ignored.\n",
> + thiscpu, apicid);
> +
> + disabled_cpus++;
> + return -ENODEV;
> + }
> +
> /*
> * If boot cpu has not been detected yet, then only allow upto
> * nr_cpu_ids - 1 processors and keep one slot free for boot cpu
> @@ -2592,3 +2632,12 @@ static int __init lapic_insert_resource(void)
> * that is using request_resource
> */
> late_initcall(lapic_insert_resource);
> +
> +static int __init apic_set_disabled_cpu_apicid(char *arg)
> +{
> + if (!arg || !get_option(&arg, &disabled_cpu_apicid))
> + return -EINVAL;
> +
> + return 0;
> +}
> +early_param("disable_cpu_apicid", apic_set_disabled_cpu_apicid);
