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: <cd820d8a-7ce9-c32f-fc84-bbf20dd7d6e5@oracle.com>
Date:   Wed, 9 Nov 2022 09:48:33 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.1
Subject: Re: [PATCH v13 7/7] x86/crash: add x86 crash hotplug support
Content-Language: en-US
To:     Borislav Petkov <bp@alien8.de>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        kexec@lists.infradead.org, ebiederm@xmission.com,
        dyoung@redhat.com, bhe@redhat.com, vgoyal@redhat.com,
        tglx@linutronix.de, mingo@redhat.com, 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: <20221031193604.28779-1-eric.devolder@oracle.com>
 <20221031193604.28779-8-eric.devolder@oracle.com> <Y2A4TqyOaPYBu4Hk@zn.tnic>
 <1c11a429-b5a9-fb55-fbef-b49e760e2d1e@oracle.com> <Y2I3zQ9ZU+C9Kgf7@zn.tnic>
From:   Eric DeVolder <eric.devolder@oracle.com>
In-Reply-To: <Y2I3zQ9ZU+C9Kgf7@zn.tnic>
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?RHJrOHMzaWRuaWN1akRlUVBmR3drZDRmQ2dtMmNScFlhUkxmbnJ5SldENmFZ?=
 =?utf-8?B?UFVBd0lFbnlRd1JpdnA1QXg3Q05JejIxc3JqY2JJZVREMlhHWFplV2ozdG4w?=
 =?utf-8?B?YmluWlE2VEgvWnZUbHBqb01KQ213VmJQMFl6OTI3SDBLdHM2b21tNGhFZXha?=
 =?utf-8?B?QnJDZlZTdlZtcjJMNCswV3cvY05aNW1qOXBTUWtaUFZLdnE3MDM2Nm5qSFNN?=
 =?utf-8?B?bnlxT2ZIelJsU2ZZaWtPSktKT0k4UlhqMVFJVGFPN3lhMnRscHJRSEd0T0hm?=
 =?utf-8?B?bk5DTnMxMHdYbUFBVWY4YWlSVkl1N2UzQ25KQTNQdkUvRmcxSmtrdVF2ZlBO?=
 =?utf-8?B?dzZtekxIYnF1bVAzSE5jMGdodmh1VllyV0JGeHdTb0JHVmFqbFIyL1hoUjAx?=
 =?utf-8?B?b1RGM1BLWC9zNWE5RElVQ0pVQk40Ri9CZjF3RmlrWWRLMjdhbXhvMG56b3lQ?=
 =?utf-8?B?SENkTTluSzRpU2JNNmVlVTBhTnh6cktxTUZkUlp2NlRPcW9XNHlJMGRwTFBV?=
 =?utf-8?B?TVdXNjUwVkUxR3ExOGJaaVdmUzB6Y3hTa2ZCaHdXc1dwaFpWT2hXZ3VCaXBl?=
 =?utf-8?B?YmZqditXN0VhU2Zpck9PZzAwNC9XbWl1MS9IRkhqYmF1YTFLVnFGOEtUcUJK?=
 =?utf-8?B?RjVlQnZCZEJ6dU9WS2dFRlJJN2ZWQ1JKUkxNTHFRN2tTWjd4dXErQWxobzdI?=
 =?utf-8?B?RlNPdXgrQkxvQkJkRnpYeDZiY0FYV2VkMkdLUEhvbEFscXNRZ2MxN2Y0TTFX?=
 =?utf-8?B?eEVyTlpzUE9WY0N4dEhQaU45WVd5K3IyVjlVL3VZTit6bC8zeklFdVFIVjlm?=
 =?utf-8?B?c0RxMnBSalJKMUpWRnUya1RXQTVjNVRyWXRQa3g2bzVJanFjWkZ6Si9lVmgr?=
 =?utf-8?B?UXRmRVV0RHdVaHl5YzJEUHlJMU1KYXFuRmJoL3cxVUFlWEJWNTJaQjlyS3dq?=
 =?utf-8?B?ZmYxMnB3U25NVEo3dG83WFZRZ2dqMVE1elF5S0Q4WjZXUUVOVWV4eHA4SDJ1?=
 =?utf-8?B?L2FiQjNTNW1FaTJ4TzZIcmxldVJLV3YvdFduM3ROZUp1UkljWTE0VHMrbHd1?=
 =?utf-8?B?cVYzeFVjc0ZZQzFqOEV1L1ZhTG1aeE9wRmk5T1NlT09pdDIwL1dDbmVHT04y?=
 =?utf-8?B?TGp6cHJocnYrc1A0ZDA4VWcxdG5UeGRQOVV2K1ovdSsxdy9PTmliQ0pUa096?=
 =?utf-8?B?UnZsREppUTRYcDE3M1hpdkk1eERtOWpOdnBVS0J2QWJoWXNDZzZxSmYxZzlm?=
 =?utf-8?B?Rm9tSWYrV2ZMazAwUFN2ejBTRklRbklBbmxDZVB1a3NMV3pJcUh5VFQ1QzZu?=
 =?utf-8?B?Y2M4dnZlVnVidDdWMXlHOWNhWWhWUldJRkFOSURlWjJJZHowb0NBTGZIWUtx?=
 =?utf-8?B?ekVTajFUSTUxWmdiUTcvYmU3cDdDWXZtVCtTeFRhY2E1MXBQRjVkZHhqa01V?=
 =?utf-8?B?VXFRaDVQR1RZNmJrVmRqQWIyNXFISlI0UkpZNlc1WWpHREdKd1BDN1IyWUF1?=
 =?utf-8?B?aW9xWEdXNVR1QVZ5cUVteWRSOGlIMkozOGxlVXNlV0lDTDRPVVpzWUsxakI1?=
 =?utf-8?B?NU9QVXlhai9JZERhZVdvTHgwczgzNVRtVWhpbWM3cE1DQzl5SXBEdHVpZVoy?=
 =?utf-8?B?VGVWUFhCUFE3RE8xK0ovRXlsQ2lJckQyZis1a2ZYR2JLWjg1bVZoT29RTk9I?=
 =?utf-8?B?RlM5S0k0SWUxSmNGNGtSM0dUVTFYYzFVb0hJcEtoQU1ZVHA3am1HcHMrZ2Y4?=
 =?utf-8?B?bGh2bmlUSjNvTThjcnVacEFRbTRJQndpTkZaZzY1Q3pzMDBVdW9ncWwvYm4r?=
 =?utf-8?B?NUdqRytSOXpBVVl6MGk3QndLRy9vSUhkSWJFWnRtUW9zSHVMV01NOUFaNHZD?=
 =?utf-8?B?cU9BanhlVFR1YVZuendQRWNOWXRYT2x4dzcycW1SNC91NXJhMjdHcWowd0xX?=
 =?utf-8?B?ZmM0bDlQSVpsQkY2eTlTUWc4VmNXQ1d4Rk1BblJYdGNDeHpiM2FValR4aWN1?=
 =?utf-8?B?d211dThYdytqNHlnZk1lb09EQTJPYUZ1R3ZpZEJERERjVE1ONS9pZnNLbENn?=
 =?utf-8?B?WFhUWlllbEJEK1hsKzRNRndMaFVSTnFBMW5yVG04UXJwUnNiMFE5S2dJYi9u?=
 =?utf-8?B?cHYrN0xlRUNWcDhJU0N2Qk5ORm1wdzBTeDFtS1psL1YvcXhvWEkwYnZXb3pn?=
 =?utf-8?Q?tb8sPrKOhIKqFhwrOxS6nec=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0cd595f6-fa74-420c-02f3-08dac269dfc1
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4531.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Nov 2022 15:48:41.1419
 (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: xRop5+vlNhNG7edFoCA5qgSTny8utnPeSgIvDK9ZSnfw1YrZUii1aQYm/a6ml+vg+/pXRhNuM4XBhUBRq5xZgTo69FqoSCbBqJaADJ+ouJw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB6246
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-09_06,2022-11-09_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 spamscore=0 malwarescore=0
 adultscore=0 mlxscore=0 mlxlogscore=999 phishscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211090120
X-Proofpoint-GUID: Bo36abczxZ5O0rlpxj_9lBdKRYMVcUxu
X-Proofpoint-ORIG-GUID: Bo36abczxZ5O0rlpxj_9lBdKRYMVcUxu
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 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 11/2/22 04:26, Borislav Petkov wrote:
> On Tue, Nov 01, 2022 at 10:45:00AM -0500, Eric DeVolder wrote:
>> As I'm re-reading that message, I suspect now the preference is to just to
>> strike this ifdiffery line in this file and have the code always present?
>>
>> If the preference is actually for CRASH_HOTPLUG, then let me know.
> 
> Well, it is this part:
> 
> "But on a plain simple laptop or workstation which has CPU hotplug,
> would it make sense for the crash ranges to get updated too when CPUs
> are offlined?
> 
> If so, I think you want this code present there too, without a Kconfig
> item."
> 
> IOW, if this thing doesn't make sense to have on the majority of
> machines out there - and memory hotplug machines are not the majority -
> then it should be behind a Kconfig item which is default off and gets
> enabled only when the user selects crash and memory hotplug...
> 
> I'd say.
> 
Boris,
I apologize for the delay in responding, I've been away for the past week.

I'm re-reading the thread on this topic, and I apologize for backing up a bit,
but as I read the last paragraph again, your contention is that the bit of
code in this file *should* be behind a Kconfig item, and default to off, as
"memory hotplug machines are not the majority".

Does this mean then that I need to introduce CRASH_HOTPLUG again, so it can be
default off?

Examining arch/x86/configs/x86_64_defconfig, neither HOTPLUG_CPU or MEMORY_HOTPLUG are present, but 
CONFIG_SMP=y.

And in examining arch/x86/Kconfig, I see:

config HOTPLUG_CPU
     def_bool y
     depends on SMP

which then defaults HOTPLUG_CPU to on and thus this code/ifdef in question.

And as a reminder, the '#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_MEMORY_HOTPLUG)' shows up 
in only three locations: include/linux/kexec.h,  kernel/crash_core.c and arch/x86/kernel/crash.c.

And we resolved in this thread last week that this patch series is useful for cpu and/or memory hotplug.

So at this point, I'm still not sure if you want the ifdef line:
  - removed altogether
  - transitioned to CRASH_HOTPLUG
  - leave as is

If I could get clarity on that, that would be much appreciated!
Thanks!
eric