Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3bd962c3-7dc6-6dd0-6d0d-2e4667a69a8a@oracle.com>
Date:   Fri, 8 Jul 2022 16:07:20 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH v9 7/7] x86/crash: Add x86 crash hotplug support
Content-Language: en-US
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
        linux-kernel@vger.kernel.org, x86@kernel.org,
        kexec@lists.infradead.org, ebiederm@xmission.com,
        dyoung@redhat.com, bhe@redhat.com, vgoyal@redhat.com
Cc:     tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com,
        nramas@linux.microsoft.com, thomas.lendacky@amd.com,
        robh@kernel.org, efault@gmx.de, rppt@kernel.org, david@redhat.com,
        sourabhjain@linux.ibm.com, konrad.wilk@oracle.com,
        boris.ostrovsky@oracle.com
References: <20220613224240.79400-1-eric.devolder@oracle.com>
 <20220613224240.79400-8-eric.devolder@oracle.com>
 <2843978.e9J7NaK4W3@opensuse>
From:   Eric DeVolder <eric.devolder@oracle.com>
In-Reply-To: <2843978.e9J7NaK4W3@opensuse>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MHArVHVNaEpKSWJkMUNWWG1oNHNHUzFZZm5QZk5HK1puZW9RMFJ6L21DSUZ1?=
 =?utf-8?B?Rk1KQ3dpZjlkVi9iNDVXOFp6T1g0ampTV1BzQU90Nk8wbjg5N2ppOW5hakcw?=
 =?utf-8?B?TjNiK3RQV0pteFVBeHZzOElhRUVsWjJEMFljQXh4WDgyRE45emdEMkdrWnFB?=
 =?utf-8?B?dzIwN1RkTUxqem5qelFmQkNPNFpMZDcrcE9wbkdHM3haOUdHOXFFUDJ6U2p6?=
 =?utf-8?B?cE1oWG9HY2c4cXJkZ1VtT2dtZ3RuNHFMZ3hCWGVjdVhTeFJBR1kyS1RISDJz?=
 =?utf-8?B?WFlaVHV3dXJWODdFZG1BVmhKZi83ZUUrcVk0dkdpZjFvcTFDTnlQRy9XaVJZ?=
 =?utf-8?B?Nll0cFRoSDFQdVZEdUFvNGJKUExqWTJzQTdqU3o4UlAyT29GT3JSS01uWElN?=
 =?utf-8?B?bE1EZVYvdlN4eWwySVRKWWlXWmNMd21ESXVQcDdLWUFmazRmb2JQVjJEZmxu?=
 =?utf-8?B?UlUydXBxL2pGN3pYWU5Qc0xYRS84L3FsUXhCeFI1bHBlclE3eGc3eVNLYkpC?=
 =?utf-8?B?a09hSGN4QWN4TUNwbG9BeE05SWlsOElQam8wWVVrcEM2U1QzR3ZHSFFRNEwv?=
 =?utf-8?B?T0Ird0JNdnJVUkhodzNhaG1sQVdVQzFyK1Bpd2xSaWpyTzg5cy9WcUp0aTBW?=
 =?utf-8?B?dkI1S0dFKzlFdG0zNW1JMmh1OXRBQzUvaWEwT1llY2p0QkNXOXRCTE9Cd3Y2?=
 =?utf-8?B?MGlCT2FaQlRrMmRMNnQzMGdNN3E0Y2Z2UFU0YjI2SnlRUmRUQmg4Y2JOUWd3?=
 =?utf-8?B?emRRVG9QRk5KMkpFWGtoRk9OTTlqenVhN1MrOXdVMU5zVUpXRTU3ZEFmaE5J?=
 =?utf-8?B?UWluc1JmRFVWOFBjMHR2d0t2a2k0QkhqY3JWa3JJWlR4K0x0dEZXZEVUZ1NF?=
 =?utf-8?B?bFFWMUo4OEN3VUw2RXR4U1JLZVBWTzc0eDZpNzV2WitvaVNPcC9obWFwdCsv?=
 =?utf-8?B?K2o2Z0VlOE8xVk1WVUdyNW0rTGhtaHU4L2I3aXptTnBNV1BQcU95b3Q2SHBw?=
 =?utf-8?B?cWRFMjF0cmVaMUE3cVpvOGpFU2NOWjNZSE5GMHU5STlad2JJMmlmOG5JMCs0?=
 =?utf-8?B?TmtZMVB6UG9QZlZpdCtpTUpJZFJ2dGlCODkzeUZ6UDY4TlZVUVVnaFc3Tnp3?=
 =?utf-8?B?QkwzNkRzWSsxalNrUjlOMmptVXpSc0FyeHY4ZDJ4Tm1WdU9xSU8rSy8xNjd4?=
 =?utf-8?B?OWxKK3hJMU5pNnR1WnZaUW1JRGRyRlRvVExadmJFZFJuOWRndUx3YmxkWE9u?=
 =?utf-8?B?RVNmSWlCMFM5dWZpOEs0Mm0wR0M5eUhoVjNFbXZsdm9OVUtoaFQzOE5xZXpS?=
 =?utf-8?B?OUxCcWVlMHhHMVJOaVExckV6Rkh6QVhYYVRHRUZIVUNheXZKRFdTaVREOWtS?=
 =?utf-8?B?NDE2NW8yUHo2Q1RYQW5WaCtic0VLK2NUVGNWK3JpUjMvb2ZSTDZpcE1RbVln?=
 =?utf-8?B?Wk1kWTFObGlNQkMzc09LbXJnOG9PSU1pRzFkU1d5Q0JTQUN1SUlpb2hZK0Ft?=
 =?utf-8?B?aDFaaDVQbFVVK3RMc1VrWElUSnZTWjhacFN0SmtKWVpkZ1VOemNCVnlQQ21M?=
 =?utf-8?B?OXBCbW9PcEwxRWdsRTBnM0ZleU84cjJUOWNvZ3dkSmxTbmRsaTIvNHVMemFh?=
 =?utf-8?B?SXNQNkN4Um81Q2JYcG1VVmhOdmlnVmFDTW1vZ1QzNkl4UDhtNjdIMnlSSHJz?=
 =?utf-8?B?dEtxTXFVdXJCL0VWdHJFTVF2bXJJS0c1M2xiV1RoYVIxdHYxYXFqUmllano1?=
 =?utf-8?B?cnJPTW5DSzExSmdMMTNwOURIWFRBU3RROGdxSWNqbmhzdWZXR3NBRXptcEg0?=
 =?utf-8?B?SWhjck4rckxHWE1lUjd1RDdwU2ppZWtrTnkzUjlGVEtGL0NKK003VEJQcnEz?=
 =?utf-8?B?SzN4eUVMU2dBRWZ3MzZaZENKdTJjbEhubFhBRG1sdlpmZkVUQVloWm5Nd1B2?=
 =?utf-8?B?YzN0UlRmQVMzNFllaVlYQnZabC94L1VFdVd6YTJwNGY0OUxJbGZpOFUrNVJC?=
 =?utf-8?B?M1A3UkFlejN6eUdYNXlqZktmSlU4bDhoYlVKc2l1b3pyNGkzaG8wUWYxWi9F?=
 =?utf-8?B?RWdGUHdlc3VCY3FNU1l3MjV4dW92SEFpL01CVFViLy8zTW5OWmRPNzNIOHh2?=
 =?utf-8?B?alZRTndQMmttZ3hiTWJaaElWcW8zb04vVURKZXNZQnMyODRPK2xhWGNhNytX?=
 =?utf-8?Q?o0qQ2Wa1qRvu6x2ox2b8bvM=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0408f5b2-44d9-43e6-fe5a-08da6125dd70
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4531.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jul 2022 21:07:28.6793
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +VoTbSjfd+uGk6qAgcfSKXdZASHXRb5HFBPxSpGTnGkx/YZJDP20r7S4JVLYDOUhvNNaMbFpbb06mFOeKJWlS4TV1V0CRk8/awxJ+cqkZCA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR10MB5585
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-08_18:2022-07-08,2022-07-08 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 malwarescore=0
 mlxscore=0 spamscore=0 suspectscore=0 adultscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207080083
X-Proofpoint-GUID: cz6adxlhrxEbQl9Xbyz_sbwdR63Cleut
X-Proofpoint-ORIG-GUID: cz6adxlhrxEbQl9Xbyz_sbwdR63Cleut
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/8/22 14:38, Fabio M. De Francesco wrote:
> On martedì 14 giugno 2022 00:42:40 CEST Eric DeVolder wrote:
>> For x86_64, when CPU or memory is hot un/plugged, the crash
>> elfcorehdr, which describes the CPUs and memory in the system,
>> must also be updated.
>>
>> When loading the crash kernel via kexec_load or kexec_file_load,
>> the elfcorehdr is identified at run time in
>> crash_core:handle_hotplug_event().
>>
>> To update the elfcorehdr for x86_64, a new elfcorehdr must be
>> generated from the available CPUs and memory. The new elfcorehdr
>> is prepared into a buffer, and then installed over the top of
>> the existing elfcorehdr.
>>
>> In the patch 'kexec: exclude elfcorehdr from the segment digest'
>> the need to update purgatory due to the change in elfcorehdr was
>> eliminated.  As a result, no changes to purgatory or boot_params
>> (as the elfcorehdr= kernel command line parameter pointer
>> remains unchanged and correct) are needed, just elfcorehdr.
>>
>> To accommodate a growing number of resources via hotplug, the
>> elfcorehdr segment must be sufficiently large enough to accommodate
>> changes, see the CRASH_MAX_MEMORY_RANGES configure item.
>>
>> With this change, crash hotplug for kexec_file_load syscall
>> is supported. The kexec_load is also supported, but also
>> requires a corresponding change to userspace kexec-tools.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> ---
>>   arch/x86/Kconfig        |  11 ++++
>>   arch/x86/kernel/crash.c | 116 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 127 insertions(+)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 762a0b6ab8b6..e9eecff3b97d 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -2082,6 +2082,17 @@ config CRASH_DUMP
>>   	  (CONFIG_RELOCATABLE=y).
>>   	  For more details see Documentation/admin-guide/kdump/kdump.rst
>>   
>> +config CRASH_MAX_MEMORY_RANGES
>> +	depends on CRASH_DUMP && KEXEC_FILE && (HOTPLUG_CPU ||
> MEMORY_HOTPLUG)
>> +	int
>> +	default 32768
>> +	help
>> +	  For the kexec_file_load path, specify the maximum number of
>> +	  memory regions, eg. as represented by the 'System RAM' entries
>> +	  in /proc/iomem, that the elfcorehdr buffer/segment can
> accommodate.
>> +	  This value is combined with NR_CPUS and multiplied by
> Elf64_Phdr
>> +	  size to determine the final buffer size.
>> +
>>   config KEXEC_JUMP
>>   	bool "kexec jump"
>>   	depends on KEXEC && HIBERNATION
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 9db41cce8d97..b9cdf7a0d868 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -25,6 +25,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/memblock.h>
>> +#include <linux/highmem.h>
>>   
>>   #include <asm/processor.h>
>>   #include <asm/hardirq.h>
>> @@ -398,7 +399,17 @@ int crash_load_segments(struct kimage *image)
>>   	image->elf_headers = kbuf.buffer;
>>   	image->elf_headers_sz = kbuf.bufsz;
>>   
>> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_MEMORY_HOTPLUG)
>> +	/* Ensure elfcorehdr segment large enough for hotplug changes */
>> +	kbuf.memsz = (CONFIG_NR_CPUS_DEFAULT +
> CONFIG_CRASH_MAX_MEMORY_RANGES) * sizeof(Elf64_Phdr);
>> +	/* For marking as usable to crash kernel */
>> +	image->elf_headers_sz = kbuf.memsz;
>> +	/* Record the index of the elfcorehdr segment */
>> +	image->elfcorehdr_index = image->nr_segments;
>> +	image->elfcorehdr_index_valid = true;
>> +#else
>>   	kbuf.memsz = kbuf.bufsz;
>> +#endif
>>   	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
>>   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>>   	ret = kexec_add_buffer(&kbuf);
>> @@ -413,3 +424,108 @@ int crash_load_segments(struct kimage *image)
>>   	return ret;
>>   }
>>   #endif /* CONFIG_KEXEC_FILE */
>> +
>> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_MEMORY_HOTPLUG)
>> +int crash_hotplug_support(void) { return 1; }
>> +void *arch_map_crash_pages(unsigned long paddr, unsigned long size)
>> +{
>> +	/*
>> +	 * NOTE: The addresses and sizes passed to this routine have
>> +	 * already been fully aligned on page boundaries. There is no
>> +	 * need for massaging the address or size.
>> +	 */
>> +	void *ptr = NULL;
>> +
>> +	/* NOTE: requires arch_kexec_[un]protect_crashkres() for write
> access */
>> +	if (size > 0) {
>> +		struct page *page = pfn_to_page(paddr >> PAGE_SHIFT);
>> +
>> +		ptr = kmap(page);
>> +	}
> 
> kmap_local_page() is preferred over kmap() which should be avoided in new
> code (please take a look at highmem.rst which I recently updated).
> 
> Can you please check if you can easily avoid using kmap()? Otherwise, this
> code will be added to my (long) queue and changed one day or the other :-)
> 
> Thanks,
> 
> Fabio
Fabio,
I converted this to kmap_local_page()!
Thanks!
eric

> 
>> +
>> +	return ptr;
>> +}
>> +
>> +void arch_unmap_crash_pages(void **ptr)
>> +{
>> +	if (ptr) {
>> +		if (*ptr)
>> +			kunmap(*ptr);
>> +		*ptr = NULL;
>> +	}
>> +}
>> +
>> +/**
>> + * arch_crash_handle_hotplug_event() - Handle hotplug elfcorehdr changes
>> + * @image: the active struct kimage
>> + * @hp_action: the hot un/plug action being handled
>> + * @cpu: when KEXEC_CRASH_HP_ADD/REMOVE_CPU, the cpu affected
>> + *
>> + * To accurately reflect hot un/plug changes, the elfcorehdr (which
>> + * is passed to the crash kernel via the elfcorehdr= parameter)
>> + * must be updated with the new list of CPUs and memories. The new
>> + * elfcorehdr is prepared in a kernel buffer, and then it is
>> + * written on top of the existing/old elfcorehdr.
>> + *
>> + * For hotplug changes to elfcorehdr to work, two conditions are
>> + * needed:
>> + * First, the segment containing the elfcorehdr must be large enough
>> + * to permit a growing number of resources. See the
>> + * CONFIG_CRASH_MAX_MEMORY_RANGES description.
>> + * Second, purgatory must explicitly exclude the elfcorehdr from the
>> + * list of segments it checks (since the elfcorehdr changes and thus
>> + * would require an update to purgatory itself to update the digest).
>> + *
>> + */
>> +void arch_crash_handle_hotplug_event(struct kimage *image,
>> +	unsigned int hp_action, unsigned int cpu)
>> +{
>> +	struct kexec_segment *ksegment;
>> +	unsigned char *ptr = NULL;
>> +	unsigned long elfsz = 0;
>> +	void *elfbuf = NULL;
>> +	unsigned long mem, memsz;
>> +
>> +	/*
>> +	 * Elfcorehdr_index_valid checked in
> crash_core:handle_hotplug_event()
>> +	 */
>> +	ksegment = &image->segment[image->elfcorehdr_index];
>> +	mem = ksegment->mem;
>> +	memsz = ksegment->memsz;
>> +
>> +	/*
>> +	 * Create the new elfcorehdr reflecting the changes to CPU and/or
>> +	 * memory resources.
>> +	 */
>> +	if (prepare_elf_headers(image, &elfbuf, &elfsz)) {
>> +		pr_err("crash hp: unable to prepare elfcore headers");
>> +		goto out;
>> +	}
>> +	if (elfsz > memsz) {
>> +		pr_err("crash hp: update elfcorehdr elfsz %lu > memsz
> %lu",
>> +			elfsz, memsz);
>> +		goto out;
>> +	}
>> +
>> +	/*
>> +	 * At this point, we are all but assured of success.
>> +	 * Copy new elfcorehdr into destination.
>> +	 */
>> +	ptr = arch_map_crash_pages(mem, memsz);
>> +	if (ptr) {
>> +		/*
>> +		 * Temporarily invalidate the crash image while the
>> +		 * elfcorehdr is updated.
>> +		 */
>> +		xchg(&kexec_crash_image, NULL);
>> +		memcpy_flushcache((void *)ptr, elfbuf, elfsz);
>> +		xchg(&kexec_crash_image, image);
>> +	}
>> +	arch_unmap_crash_pages((void **)&ptr);
>> +	pr_debug("crash hp: re-loaded elfcorehdr at 0x%lx\n", mem);
>> +
>> +out:
>> +	if (elfbuf)
>> +		vfree(elfbuf);
>> +}
>> +#endif
>> -- 
>> 2.31.1
>>
>>
> 
> 
> 
>