Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <b399d57a-3773-a78e-94f7-aa6829568080@oracle.com>
Date:   Sat, 7 May 2022 12:30:33 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v24 3/6] arm64: kdump: Reimplement crashkernel=X
Content-Language: en-US
To:     Baoquan He <bhe@redhat.com>, Zhen Lei <thunder.leizhen@huawei.com>,
        Catalin Marinas <catalin.marinas@arm.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>,
        linux-kernel@vger.kernel.org, Dave Young <dyoung@redhat.com>,
        Vivek Goyal <vgoyal@redhat.com>,
        Eric Biederman <ebiederm@xmission.com>,
        kexec@lists.infradead.org, Will Deacon <will@kernel.org>,
        linux-arm-kernel@lists.infradead.org,
        Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        devicetree@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>,
        linux-doc@vger.kernel.org, Randy Dunlap <rdunlap@infradead.org>,
        Feng Zhou <zhoufeng.zf@bytedance.com>,
        Kefeng Wang <wangkefeng.wang@huawei.com>,
        Chen Zhou <dingguo.cz@antgroup.com>,
        Dave Kleikamp <dave.kleikamp@oracle.com>,
        John Donnelly <john.p.donnelly@oracle.com>
References: <20220506114402.365-1-thunder.leizhen@huawei.com>
 <20220506114402.365-4-thunder.leizhen@huawei.com>
 <20220506231032.GA122876@MiWiFi-R3L-srv>
From:   John Donnelly <John.p.donnelly@oracle.com>
In-Reply-To: <20220506231032.GA122876@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?REV3RkhIUVk0RWdTSVVFbGFaNlFPRVQ0YVY5YVh6NVJlYlcwQlQyYWJoblhH?=
 =?utf-8?B?VUN4U2szVHc5OXpSNllTVGt0RnFJcXNYNHVjbGR2d3JTa2xKZDN4K3hvc0Fq?=
 =?utf-8?B?cHY4RWpCSThKckZSbHpjV1lwZUwwUWJpbnU4WFl5WXdSTlMrMlAzckk5Zi96?=
 =?utf-8?B?QmtidjV2SFZKbW9hUWhOV3ErbEVTQUw1T2RWQXNUcCtLRk8vbDRlaUlNOERU?=
 =?utf-8?B?UmRXZ2xLdlRWUVFPUHhsQ3YybURtblN3OE1mWjMyamJqNEdxZXJLc1VSbzl6?=
 =?utf-8?B?Zitxa0hZQk5FYjFJdzgvWm5rSzN1U3JyaTFSNm1wZnpxNThELzRKYit0b0Jx?=
 =?utf-8?B?bzVXOHFLRlZDUnoxaXFPYUE3VmhRdXVIWFFlV1JoVlJqaUk1RVFPTUg1UjNE?=
 =?utf-8?B?SEVsMU1tcXZwc0pqTE9CQWhqVXlsUzhQK2ZaRDB1UTlKQzdqN2dXdFhoS2M2?=
 =?utf-8?B?RnJLSkhKdklHN242RlZVVDRaczBXQmFXcXdDWDVTR3RjZlNpelRXVFN5UXZW?=
 =?utf-8?B?eFJYVWRsSTJBdGZ1TysvMjR4Tm5TV1NFQ0hNbUQwVTZ2amwyTk1Da2RPaW4z?=
 =?utf-8?B?aVNDbWU3c3I2M2NFMkI4anpha2laWC84VXVNT0lyQ1MvdXU2ZGpUdWs3Y0hq?=
 =?utf-8?B?Rlo0MDg3bVdrS050THpZVUVqZTN1WmltbXRHOGxBL09kenZXTDEwNXF0eWdZ?=
 =?utf-8?B?RFdBazdMQXUvUzF4QW9jdTUzS0drTlFFV3R4dXp4Z2E4ZXB4VlFCODVqaHJu?=
 =?utf-8?B?TTltT1VmVkNQN1JtSE0vWkFWMHM5RnorTm1kZzFxSDNlOWdrdXREaUY4TEZF?=
 =?utf-8?B?VWxiK3NpRGJIL1R6Rk9nZFg3S0VweFJ2SThJa25tV1MybUQvY3c5S29oSEhw?=
 =?utf-8?B?OTEzV0NlZ1BrWlFkRWYxa29EZlpyd3hlYXVsb0JoT2RCR0dEMjJ1ekhMcU1B?=
 =?utf-8?B?VnZPVldRMnJJbUx0anZJdW8zK2t2eXQ2SUZ1QjNvNGJ0NlZlaUJad0pnNG50?=
 =?utf-8?B?NFhDQ1owR2dOY1V2bS90d3ZYNW11VHFneGlYVVdrMVZXejhrN0xKZEtJVDVO?=
 =?utf-8?B?SU0wUFpNbmVGSjlwcTRheURJUjFBc0JxSWFXSktIYUsyQjc1ZGhHUlNqK1Iy?=
 =?utf-8?B?SlFTZlBFdVlJcmZqcjR3QkVHa3haaWY4aHkrY1Q4VlNPY2thZ3IvWENKUUtv?=
 =?utf-8?B?MGplQ3YvVDdkbmplTGRJdG4vaFJXNmM4Z1V2SWxTVWViay9QbUt6WlpiZUJt?=
 =?utf-8?B?OGVKSkZjUGprMUp5N3hqNGw0bllOb1VzZ3dSNXNraU1id1d2WWVIV3laNHdE?=
 =?utf-8?B?VWd4N0RFenVvcmNhNk9Gd2ppaEx1VlQ3VXZIZkhKUXRGWWtCMzJXWXpHbXh1?=
 =?utf-8?B?U2FuSWZEWGQ4cE1FYlVSVEFaaWF0ZG9JeENYblpRb3oveU5VNG5LZ2xydmlF?=
 =?utf-8?B?M25GR09ibjEwSWIrcnRaOHg5UElJZk51SjRXdCtxSzJUU2pmUVlRTWI2YnpU?=
 =?utf-8?B?UDZqR0FDRG9LaG0xeUdmSjdYdmp2dEcraTdZVEdQK3JMOGZCVlVoaTUvZi9D?=
 =?utf-8?B?VnR2ampnSUczUGZmSHJNZGVMNzVEZjcxbGx2WDUwdGhrT3N4Q1VwWm9xUitr?=
 =?utf-8?B?bG1RekFhWk1pYmlkdHBuQ0lRcHFWNGRJMUdSWjluVFYwTlk5TkFPV3dTYUdK?=
 =?utf-8?B?YXhoWmVuNWw2bzFFNlo2Z0xjdUE0VElZY21TemdWamEwbFVsTG0yZ1FYejlK?=
 =?utf-8?B?MlJLbERiVjE0Z1lXeDRjTCt0YXdtbG9XNENVOE9tNzhvVFdydWhPODZXYld0?=
 =?utf-8?B?TGtRRVpUaVBXWkNLSzdGL295bUZ3bmhaelNBa0p1WldLM242elIwalZEVlZL?=
 =?utf-8?B?OUp3Z2Rwd0JyWVpOaVgvTkl0eDQ4S2h2bFVlQzVTUWRWYU9iVnJPZi9aN09U?=
 =?utf-8?B?azFSY3RuaTlKaXBnRytST293TFdOL1hkSWVtaGpZZUtZNDcxbGxReklsNDBX?=
 =?utf-8?B?VXlaWjc3WEl1YXVEWmNiQkdXRkJWcU9acDUzdms4eWhRY2FRTUw4SWNVSGcx?=
 =?utf-8?B?UTdiNzlZRjhrTWliL0xnOWFOTTRtZlhCOXJudWVjeFdDbktueDZDK3F1RFpI?=
 =?utf-8?B?VHpMNE1lbXNZRmtNWTRYQkdCdm1PY0puNDh2MFRYVHcwN2JGc3oyWGYwYk56?=
 =?utf-8?B?aCt5a2ZkTjdVMU5kMG1RTzlFWS9ZL0c1R0NIV2dNTXNJa3ZoWVJaNWNoSFIx?=
 =?utf-8?B?bDNVMnYwSUo5Q2UyM0dObHlRTVgwdExXVExCaU4yRVFsUXlrTVJXaDRmN0Vv?=
 =?utf-8?B?ZHdxYW4yTUtmZkxLMHhKVnc2VnBKcWpzSW5xU3pVUXU0R25Vck9XOTBRUGtF?=
 =?utf-8?Q?hhZk9Nt8H0Mlq1v8=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ecbe7baf-3eeb-4908-715c-08da304f7619
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4722.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2022 17:31:47.1086
 (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: XVOc4a9F05fBt8BPqaLod7yyGnfO5K0a8LlrG67p3dfzutEBMH9MU1R5SID03VBSyI8oKEDwgNWcMOA1CjJUHYoe1+RYU4S1CHVTVf0qJPA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5011
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-05-07_05:2022-05-05,2022-05-07 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0
 mlxlogscore=999 malwarescore=0 adultscore=0 suspectscore=0 spamscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2205070121
X-Proofpoint-ORIG-GUID: HZ2M05t3WgIk8YuD32CRZ9fz7cYltURN
X-Proofpoint-GUID: HZ2M05t3WgIk8YuD32CRZ9fz7cYltURN
X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable 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 5/6/22 6:10 PM, Baoquan He wrote:
> On 05/06/22 at 07:43pm, Zhen Lei wrote:
> ......
>> @@ -118,8 +162,7 @@ static void __init reserve_crashkernel(void)
>>   	if (crash_base)
>>   		crash_max = crash_base + crash_size;
>>   
>> -	/* Current arm64 boot protocol requires 2MB alignment */
>> -	crash_base = memblock_phys_alloc_range(crash_size, SZ_2M,
>> +	crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
>>   					       crash_base, crash_max);
>>   	if (!crash_base) {
>>   		pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
>> @@ -127,6 +170,11 @@ static void __init reserve_crashkernel(void)
>>   		return;
>>   	}
>>   
> 
> There's corner case missed, e.g
> 1) ,high and ,low are specified, CONFIG_ZONE_DMA|DMA32 is not enabled;
> 2) ,high and ,low are specified, the whole system memory is under 4G.
> 
> Below judgement can filter them away:
>          
> 	if (crash_base > arm64_dma_phys_limit && crash_low_size &&
> 	    reserve_crashkernel_low(crash_low_size)) {
> 
> What's your opinion? Leave it and add document to notice user, or fix it
> with code change >
> I would suggest merging this series, Lei can add this corner case
> handling on top. Since this is a newly added support, we don't have
> to make it one step. Doing step by step can make reviewing easier.

Lets get this added and tested with a broader audience. It has been in 
review since March, 4th 2019 - 3+ years. I applaud Zhen for his 
endurance and patience in carrying this for so long.


> 
>> +	if (crash_low_size && reserve_crashkernel_low(crash_low_size)) {
>> +		memblock_phys_free(crash_base, crash_size);
>> +		return;
>> +	}
>> +
>>   	pr_info("crashkernel reserved: 0x%016llx - 0x%016llx (%lld MB)\n",
>>   		crash_base, crash_base + crash_size, crash_size >> 20);
>>   
>> @@ -135,6 +183,9 @@ static void __init reserve_crashkernel(void)
>>   	 * map. Inform kmemleak so that it won't try to access it.
>>   	 */
>>   	kmemleak_ignore_phys(crash_base);
>> +	if (crashk_low_res.end)
>> +		kmemleak_ignore_phys(crashk_low_res.start);
>> +
>>   	crashk_res.start = crash_base;
>>   	crashk_res.end = crash_base + crash_size - 1;
>>   	insert_resource(&iomem_resource, &crashk_res);
>> -- 
>> 2.25.1
>>
>