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: <943d7b14-4278-0259-cc20-c9c72b7ab085@oracle.com>
Date:   Wed, 31 Aug 2022 12:20:09 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH v11 1/7] crash: move crash_prepare_elf64_headers
Content-Language: en-US
To:     Baoquan He <bhe@redhat.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        kexec@lists.infradead.org, ebiederm@xmission.com,
        dyoung@redhat.com, vgoyal@redhat.com, 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: <20220826173704.1895-1-eric.devolder@oracle.com>
 <20220826173704.1895-2-eric.devolder@oracle.com>
 <Yw7MhU957+cPQ50/@MiWiFi-R3L-srv>
From:   Eric DeVolder <eric.devolder@oracle.com>
In-Reply-To: <Yw7MhU957+cPQ50/@MiWiFi-R3L-srv>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?THhsTXV2aEgwVHdONzhRK2ZkaGRxclZ3Q2N6VWxzMEM0L1dCZzM2ZVlFUnpu?=
 =?utf-8?B?a1k0MmdLVEZ0UUF5U3l4eG9CVTI5L3gwNjR0Mlc1TGxpNDEzMGk1MGFmN3du?=
 =?utf-8?B?NjFCMlAzSEFpaGNUZUVoZTh1NmJkUldNcXpkalc0WTNHRENaZlhWK1duUXV1?=
 =?utf-8?B?ZmNId0pSUUYzQlh4VThpL3d1ZDBhNlRFeGJ0alJvanhCR3ZJdC9HOWpwMW9s?=
 =?utf-8?B?QlZnOUNJTVhYNHNNTGkxY01PVWk4UUF5YjYzVW9WcjhiS0ZuR3ZhMHdFOXNj?=
 =?utf-8?B?NytpSE84bFN4akJ4WVdDUFNqV2p3VVVmQzFHOTJMeldPM3VTVFIzRWtndmd4?=
 =?utf-8?B?QmZaQW0vaFdFbkpsUkN5OVdNczgwTFJQMkJRMGF3Z0Q4bTlTL0w1VWNIM0ZD?=
 =?utf-8?B?L00yWmUvMllPaC8zTU1uM3ZCdnY3QWFCbjUyMWU0aUdKVXVSNllvMHc2bWJO?=
 =?utf-8?B?OWRtTjJCRHJianlRZXJsL3RBSDdHRFoyUy9vaFM3SlV4M1NCWlQ5ejV5L2dh?=
 =?utf-8?B?YVhBOXMxdHdyTnMxVlErUFhQNnMwV2xwcXp2RCtFczNhUzk3dmdzNTVuQllG?=
 =?utf-8?B?d1NINkRtdHlLclZ5NlM4WjNGNXlFSkVzUTBCVzJtdTNmbDhNUk54alN6V3J0?=
 =?utf-8?B?MzNYYkFnNXVzUlJjbzFsdjlNanFPc2l1TlhLMTJ2ekdiS2xjY2t1bTdVK3Vh?=
 =?utf-8?B?QnhZa0ppWlpyV3JIMVcyM29XUGhLS1Vmak9TZVhqTGgzL3ZDR2RvQytCNmpv?=
 =?utf-8?B?aHRVSUFjaC9ZZG1weUsyZ1dDQ2thQ0dYT0h6aDlSbVY1MXQ3UVBKeUFxT0No?=
 =?utf-8?B?RlZhVjFZVTRBTmtsQkU1Rlpxd214T013Mmgya0ZKNkh1RTlCYitDWG90U0Js?=
 =?utf-8?B?NWh4ZmF4bGRyV0F4RUdJQ0YzUmh4KzREZzlSdXd5WW1sbmJDZDJyakpFT2pU?=
 =?utf-8?B?STlSOVBIaDd0TFBVd1MzSWxaV0NBL3IrcG1vUi9KUTFKYTA0RDZ0VEE0MGdT?=
 =?utf-8?B?RmlueXU5MWhiaDhvQ0V6UDlmcVN5UHFUSkZpQzdGM2I2MGZPOU5tck1KUjla?=
 =?utf-8?B?WVcrSXUyM2F0Y3o1bElWdWxDZTE1amJVTkF6RENvd3o0NXRHNkRxZTBaS0V4?=
 =?utf-8?B?VFZPeFl5Q0huWjdFWUs5amJ3MlZuQ1dEMHNxN2trTlBzRjBpQndIOWo3OUhK?=
 =?utf-8?B?THNqd3UzZm9mUnc0K0dHbXJHUzlJMlBENWNkYlFUSS9zZFlKUEY5VTc2ZHVt?=
 =?utf-8?B?NC9FM09uVHZDU1ZYUVIwT09PU3dNUk9lc05uTnVydElWcUhxV2VVNzJPYUlV?=
 =?utf-8?B?a2s3Uk9kKzE3TDU3SlQwamlNV0ZZUDBTbHNEbGZhL2s1Y251WnJVclBCNU51?=
 =?utf-8?B?K1M5eWlnai9wUldrb1U0dkQ0azlEbStqSisrbVIxQ0dSM2l3cTZxR0ZZR2c4?=
 =?utf-8?B?M0Z5dlowaEZQSnlCSUc3Q3E0NllqbXY3RnYwd2UwcFJCUGN2VVgzd1ZVdVZO?=
 =?utf-8?B?czBqSkZuSFZqTTMyck15blFPRkkxUVh1ZkpUbzIwQ2FpZHNKZGwxUTZLQmti?=
 =?utf-8?B?MkxQMjlvajhoVy9SNXpoMmJVZFlnOU5jMDNRWXE5Tk1UdmQ2SlVrdUFCWTFi?=
 =?utf-8?B?NExUaDRkQlpiVjZIU0ZyL25jTTdibElkTE9Wa1g0VmlMbVdVS2lpWjBxRjRi?=
 =?utf-8?B?cUNVbVkydGtkM1ZlWitMcWRLelFVTmdzRXJYVHZra2JWNVRTWHZkS3laSndG?=
 =?utf-8?B?dUpTeVBId0o3ZzYydWxUY01OdXRqdkw4UjJyd2xKQTN1cmNtVHNCSnNJd0l5?=
 =?utf-8?B?Uy9jbWpZS3d3Z2p2bXFlZDBUU3JsQ3V5dVhVYXYvdzB5SVlpR0xBMUVwaFhL?=
 =?utf-8?B?aFNwYW95Ym83WUEwM0MrR1VWOHdMQWpRWTRTR0N6c0dxeHNxZjhWOEZuL3Yy?=
 =?utf-8?B?WmJLNm90dGpkeFQ1MGF3WWo4a0hSb2ZOU0dkL0hIcE8wSFlER0tWZ3ZTTGhC?=
 =?utf-8?B?b2NIU2lqSk9hUjYvSjd3K0h2QnFQUEI3UFIzK3hUYk96M09zRXVMdFJEUFZh?=
 =?utf-8?B?Sk9UcUYrM3EyWFZjc1BDNDRmT29wSjdLQzhZQi9tUEpIckVNWUc5dnl3ZUVG?=
 =?utf-8?B?T3Rubkk2MjBQdmRqVTJreTdzWGIxWmFxSDlBckM3bDJhdjJpak1WQkllT2do?=
 =?utf-8?Q?yf51rKWfSqfJe1yaXXvzjMM=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 20df6c18-77a8-4f33-5609-08da8b75139c
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4531.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Aug 2022 17:20:18.6074
 (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: HFXn5p7CwdpQj50zRfxuTVCmiGy3zw3BEdtHfXsATIf3w4R9OEvbkKxdDeO7uqvA/jUlv2VKBH/5NJ1tas2gUBPdj02iN153k5eBleylm9Y=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB5099
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-31_10,2022-08-31_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=0
 mlxlogscore=999 adultscore=0 phishscore=0 spamscore=0 bulkscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2207270000 definitions=main-2208310083
X-Proofpoint-GUID: cbBfyG4F-_O7OxVQ_MN4UVo_R1ffNxKW
X-Proofpoint-ORIG-GUID: cbBfyG4F-_O7OxVQ_MN4UVo_R1ffNxKW
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 8/30/22 21:50, Baoquan He wrote:
> On 08/26/22 at 01:36pm, Eric DeVolder wrote:
>> At the outcome of this patch set, the crash_prepare_elf64_headers()
>> is utilized on both the kexec_file_load and kexec_load paths. As
>> such, need to move this function out of kexec_file.c and into a
>> common location crash_core.c.
>>
>> No functionality change.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> 
> LGTM,
> 
> Baoquan He <bhe@redhat.com>

Thank you! I'll record that as Acked-by, unless you tell me otherwise.
eric

> 
>> ---
>>   kernel/crash_core.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
>>   kernel/kexec_file.c |  99 -------------------------------------------
>>   2 files changed, 100 insertions(+), 99 deletions(-)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 07b26df453a9..8c8c82386e8d 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/utsname.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/sizes.h>
>> +#include <linux/kexec.h>
>>   
>>   #include <asm/page.h>
>>   #include <asm/sections.h>
>> @@ -314,6 +315,105 @@ static int __init parse_crashkernel_dummy(char *arg)
>>   }
>>   early_param("crashkernel", parse_crashkernel_dummy);
>>   
>> +int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>> +			  void **addr, unsigned long *sz)
>> +{
>> +	Elf64_Ehdr *ehdr;
>> +	Elf64_Phdr *phdr;
>> +	unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
>> +	unsigned char *buf;
>> +	unsigned int cpu, i;
>> +	unsigned long long notes_addr;
>> +	unsigned long mstart, mend;
>> +
>> +	/* extra phdr for vmcoreinfo ELF note */
>> +	nr_phdr = nr_cpus + 1;
>> +	nr_phdr += mem->nr_ranges;
>> +
>> +	/*
>> +	 * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
>> +	 * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
>> +	 * I think this is required by tools like gdb. So same physical
>> +	 * memory will be mapped in two ELF headers. One will contain kernel
>> +	 * text virtual addresses and other will have __va(physical) addresses.
>> +	 */
>> +
>> +	nr_phdr++;
>> +	elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
>> +	elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
>> +
>> +	buf = vzalloc(elf_sz);
>> +	if (!buf)
>> +		return -ENOMEM;
>> +
>> +	ehdr = (Elf64_Ehdr *)buf;
>> +	phdr = (Elf64_Phdr *)(ehdr + 1);
>> +	memcpy(ehdr->e_ident, ELFMAG, SELFMAG);
>> +	ehdr->e_ident[EI_CLASS] = ELFCLASS64;
>> +	ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
>> +	ehdr->e_ident[EI_VERSION] = EV_CURRENT;
>> +	ehdr->e_ident[EI_OSABI] = ELF_OSABI;
>> +	memset(ehdr->e_ident + EI_PAD, 0, EI_NIDENT - EI_PAD);
>> +	ehdr->e_type = ET_CORE;
>> +	ehdr->e_machine = ELF_ARCH;
>> +	ehdr->e_version = EV_CURRENT;
>> +	ehdr->e_phoff = sizeof(Elf64_Ehdr);
>> +	ehdr->e_ehsize = sizeof(Elf64_Ehdr);
>> +	ehdr->e_phentsize = sizeof(Elf64_Phdr);
>> +
>> +	/* Prepare one phdr of type PT_NOTE for each present CPU */
>> +	for_each_present_cpu(cpu) {
>> +		phdr->p_type = PT_NOTE;
>> +		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
>> +		phdr->p_offset = phdr->p_paddr = notes_addr;
>> +		phdr->p_filesz = phdr->p_memsz = sizeof(note_buf_t);
>> +		(ehdr->e_phnum)++;
>> +		phdr++;
>> +	}
>> +
>> +	/* Prepare one PT_NOTE header for vmcoreinfo */
>> +	phdr->p_type = PT_NOTE;
>> +	phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
>> +	phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>> +	(ehdr->e_phnum)++;
>> +	phdr++;
>> +
>> +	/* Prepare PT_LOAD type program header for kernel text region */
>> +	if (need_kernel_map) {
>> +		phdr->p_type = PT_LOAD;
>> +		phdr->p_flags = PF_R|PF_W|PF_X;
>> +		phdr->p_vaddr = (unsigned long) _text;
>> +		phdr->p_filesz = phdr->p_memsz = _end - _text;
>> +		phdr->p_offset = phdr->p_paddr = __pa_symbol(_text);
>> +		ehdr->e_phnum++;
>> +		phdr++;
>> +	}
>> +
>> +	/* Go through all the ranges in mem->ranges[] and prepare phdr */
>> +	for (i = 0; i < mem->nr_ranges; i++) {
>> +		mstart = mem->ranges[i].start;
>> +		mend = mem->ranges[i].end;
>> +
>> +		phdr->p_type = PT_LOAD;
>> +		phdr->p_flags = PF_R|PF_W|PF_X;
>> +		phdr->p_offset  = mstart;
>> +
>> +		phdr->p_paddr = mstart;
>> +		phdr->p_vaddr = (unsigned long) __va(mstart);
>> +		phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
>> +		phdr->p_align = 0;
>> +		ehdr->e_phnum++;
>> +		pr_debug("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx, paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
>> +			phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
>> +			ehdr->e_phnum, phdr->p_offset);
>> +		phdr++;
>> +	}
>> +
>> +	*addr = buf;
>> +	*sz = elf_sz;
>> +	return 0;
>> +}
>> +
>>   Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
>>   			  void *data, size_t data_len)
>>   {
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 1d546dc97c50..8017eeb43036 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -1217,102 +1217,3 @@ int crash_exclude_mem_range(struct crash_mem *mem,
>>   	mem->nr_ranges++;
>>   	return 0;
>>   }
>> -
>> -int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>> -			  void **addr, unsigned long *sz)
>> -{
>> -	Elf64_Ehdr *ehdr;
>> -	Elf64_Phdr *phdr;
>> -	unsigned long nr_cpus = num_possible_cpus(), nr_phdr, elf_sz;
>> -	unsigned char *buf;
>> -	unsigned int cpu, i;
>> -	unsigned long long notes_addr;
>> -	unsigned long mstart, mend;
>> -
>> -	/* extra phdr for vmcoreinfo ELF note */
>> -	nr_phdr = nr_cpus + 1;
>> -	nr_phdr += mem->nr_ranges;
>> -
>> -	/*
>> -	 * kexec-tools creates an extra PT_LOAD phdr for kernel text mapping
>> -	 * area (for example, ffffffff80000000 - ffffffffa0000000 on x86_64).
>> -	 * I think this is required by tools like gdb. So same physical
>> -	 * memory will be mapped in two ELF headers. One will contain kernel
>> -	 * text virtual addresses and other will have __va(physical) addresses.
>> -	 */
>> -
>> -	nr_phdr++;
>> -	elf_sz = sizeof(Elf64_Ehdr) + nr_phdr * sizeof(Elf64_Phdr);
>> -	elf_sz = ALIGN(elf_sz, ELF_CORE_HEADER_ALIGN);
>> -
>> -	buf = vzalloc(elf_sz);
>> -	if (!buf)
>> -		return -ENOMEM;
>> -
>> -	ehdr = (Elf64_Ehdr *)buf;
>> -	phdr = (Elf64_Phdr *)(ehdr + 1);
>> -	memcpy(ehdr->e_ident, ELFMAG, SELFMAG);
>> -	ehdr->e_ident[EI_CLASS] = ELFCLASS64;
>> -	ehdr->e_ident[EI_DATA] = ELFDATA2LSB;
>> -	ehdr->e_ident[EI_VERSION] = EV_CURRENT;
>> -	ehdr->e_ident[EI_OSABI] = ELF_OSABI;
>> -	memset(ehdr->e_ident + EI_PAD, 0, EI_NIDENT - EI_PAD);
>> -	ehdr->e_type = ET_CORE;
>> -	ehdr->e_machine = ELF_ARCH;
>> -	ehdr->e_version = EV_CURRENT;
>> -	ehdr->e_phoff = sizeof(Elf64_Ehdr);
>> -	ehdr->e_ehsize = sizeof(Elf64_Ehdr);
>> -	ehdr->e_phentsize = sizeof(Elf64_Phdr);
>> -
>> -	/* Prepare one phdr of type PT_NOTE for each present CPU */
>> -	for_each_present_cpu(cpu) {
>> -		phdr->p_type = PT_NOTE;
>> -		notes_addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpu));
>> -		phdr->p_offset = phdr->p_paddr = notes_addr;
>> -		phdr->p_filesz = phdr->p_memsz = sizeof(note_buf_t);
>> -		(ehdr->e_phnum)++;
>> -		phdr++;
>> -	}
>> -
>> -	/* Prepare one PT_NOTE header for vmcoreinfo */
>> -	phdr->p_type = PT_NOTE;
>> -	phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
>> -	phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>> -	(ehdr->e_phnum)++;
>> -	phdr++;
>> -
>> -	/* Prepare PT_LOAD type program header for kernel text region */
>> -	if (need_kernel_map) {
>> -		phdr->p_type = PT_LOAD;
>> -		phdr->p_flags = PF_R|PF_W|PF_X;
>> -		phdr->p_vaddr = (unsigned long) _text;
>> -		phdr->p_filesz = phdr->p_memsz = _end - _text;
>> -		phdr->p_offset = phdr->p_paddr = __pa_symbol(_text);
>> -		ehdr->e_phnum++;
>> -		phdr++;
>> -	}
>> -
>> -	/* Go through all the ranges in mem->ranges[] and prepare phdr */
>> -	for (i = 0; i < mem->nr_ranges; i++) {
>> -		mstart = mem->ranges[i].start;
>> -		mend = mem->ranges[i].end;
>> -
>> -		phdr->p_type = PT_LOAD;
>> -		phdr->p_flags = PF_R|PF_W|PF_X;
>> -		phdr->p_offset  = mstart;
>> -
>> -		phdr->p_paddr = mstart;
>> -		phdr->p_vaddr = (unsigned long) __va(mstart);
>> -		phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
>> -		phdr->p_align = 0;
>> -		ehdr->e_phnum++;
>> -		pr_debug("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx, paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
>> -			phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
>> -			ehdr->e_phnum, phdr->p_offset);
>> -		phdr++;
>> -	}
>> -
>> -	*addr = buf;
>> -	*sz = elf_sz;
>> -	return 0;
>> -}
>> -- 
>> 2.31.1
>>
>