Received-SPF: pass (google.com: domain of linux-kernel+bounces-62224-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <b5904910-ed58-405f-9425-566383b48068@amd.com>
Date: Mon, 12 Feb 2024 13:47:14 -0500
User-Agent: Mozilla Thunderbird
Cc: yazen.ghannam@amd.com, Sohil Mehta <sohil.mehta@intel.com>,
 x86@kernel.org, linux-edac@vger.kernel.org, tony.luck@intel.com,
 linux-kernel@vger.kernel.org, Avadhut Naik <avadhut.naik@amd.com>
Subject: Re: [PATCH 2/2] x86/MCE: Add command line option to extend MCE
 Records pool
To: Borislav Petkov <bp@alien8.de>, "Naik, Avadhut" <avadnaik@amd.com>
References: <20240207225632.159276-1-avadhut.naik@amd.com>
 <20240207225632.159276-3-avadhut.naik@amd.com>
 <8b4f8ec2-7534-4f77-b44f-6728c699ff64@intel.com>
 <51255499-0b5d-45c6-9c72-f353bae83c0d@amd.com>
 <20240209200920.GFZcaGcOr757W9O3IG@fat_crate.local>
 <7a4945b0-322a-444e-a0ca-860a062a49c3@amd.com>
 <20240209205111.GGZcaQP1gb6C9m0WZB@fat_crate.local>
 <5DB0FF8D-C6DA-45DC-B287-201A9BF48BDA@alien8.de>
 <75ddf61d-8dda-47fa-9da0-24221feb22a2@amd.com>
 <20240211111455.GAZcisL09LeFPWa2EI@fat_crate.local>
Content-Language: en-US
From: Yazen Ghannam <yazen.ghannam@amd.com>
In-Reply-To: <20240211111455.GAZcisL09LeFPWa2EI@fat_crate.local>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?S1VMSnF0RXlhOXRlKzZRNWJrSGZ6R1FHZEdrMTBBeUpDSVBjdVRuRCtkeTdV?=
 =?utf-8?B?QndpSHhVaVdHNmVTVTRmdVpmWkEyR2U5U0hVVEFFWnZ0K2h4ZDZ1VzkxY1k2?=
 =?utf-8?B?S0hTM1pFUGZPZDRRcXVSeFFCODhZVUx2alEzdkhSd2tmRW9yamZHU0hhd0xz?=
 =?utf-8?B?V21KZmJ0M1RYOVpBUFJrUjZsUkVVZGxYNWF3dmJkdzUxNUhodk5UbWZQVkNN?=
 =?utf-8?B?N3VwMUZHd29qR1A4RjN2d1NDeEt0bXltZjUyVy9Bbzl2emZpZlVBNW5Rc1JX?=
 =?utf-8?B?WlRDNitPVHpEcmZ0YzR0VDFWdEsxaXgzdE4rRnF4OVlDdG8zemgreCtWYWRI?=
 =?utf-8?B?NjJXdkc3Ry96Z2syN014U01hUDhtbWl3U3p2VUt0aTFVeHFYUm1sWW5tQkdw?=
 =?utf-8?B?aW9GTUtVUVI2Q2Q3UVVYeWRGVlVla2YwVURjMnI0WTZyL21hZmF2MUU3U2lU?=
 =?utf-8?B?eGt5andxeHRROFRPZHdKN1E4YVlBRnp1eXd1NXA1cTFIUlErY21PaEFvcVJI?=
 =?utf-8?B?c2hvbTZwOXoxbkRFSXFwUGl6eGlSdlpuTk5pSGcyWDJjL1EyWnh5SitOS0RE?=
 =?utf-8?B?WnhIeThRSXFNSGxNT1UzUzl4eUFvQ2RNRzVEdGgwVzA1SEs2ZkNoZkRYNTh2?=
 =?utf-8?B?akp3bFlDL1RhVFR0cEpNUllMTWlDUWRraFpBQ0xFME44QkE4NFVScXdzUVZC?=
 =?utf-8?B?SG5hM0J0YzdNRUt0Q211UjZRM0R6cmxaOHdzWk95NHFkOE1WOGhqV2hBbTVi?=
 =?utf-8?B?MUx0dzZPK1lQQ1BMQjZmR3dwVjdrYzYyTnZwdnB3cU1CSHk1NnpaVDRHMjBU?=
 =?utf-8?B?LzNaOFFtVEJNK2RIRW5hcXVWY09xczErUnRhaWVteGZsSFM2UUlidXhHZUZx?=
 =?utf-8?B?dDNoR1NmZ0ZhUHo0dTBYRllhWElqQzRORi9MelpmQlAwREJSNVdzRktmR1Nr?=
 =?utf-8?B?VEVzNWxMUTZ4N3UxMmhTVFZ4Y1l4RjVFL3pXajdGTE1Tb1NoK25EaWJOOWVr?=
 =?utf-8?B?WTdWaU5QTTE5TlkveGNnTUlOR2lQQmJPbGd5UmhBNjJxMmJIL0RDVmtFY3pJ?=
 =?utf-8?B?NkRGMktOYlp0Q3h4U0VwM0hxVjNRZWRReHZwZEhoTWhPUkhMZ0t0dFExSVIw?=
 =?utf-8?B?UDRHTFdIZ1JUOUlGMit4RVhCcllRVTB2UUQwZkQ2UGVKRjJMeDUra3I3WFlX?=
 =?utf-8?B?RkRJTHJtMjdoamc4Q2oxclpLek1TditMYmJLNG1neXIrZGlnUnJaNFp6U01D?=
 =?utf-8?B?M0lTc2UwQzZRRDJXelNaRXphSTRvdVdLRTVrVU5PZVdzclpoZ1lpTGk0OGRZ?=
 =?utf-8?B?dGhZbjBXT0RNK3FKRzRYay9zclJxR0krV2Y0bnNXeU9PU2RDQUNOUWpyeGdK?=
 =?utf-8?B?di9EVmtBREhkU2RabTM2OWdlOFpGSDVjRXZIbGFydjRSWHJMMk9MUmM5WURF?=
 =?utf-8?B?NjVLY1dzTGNoN0pPMzdoT3M5RzJBM2MrRFFjeW04L3FWQ3dTdUdGUnJndWkr?=
 =?utf-8?B?eWYrWkkwczlTbUJKa0ZuYVhzc0lVN0xwRUlJM0I3VWNqOHMzWkNDL0RVOHFu?=
 =?utf-8?B?L0pDcXZvazFqaXp6MHY0aFR4VklyMmVpL2lIMkpkd1huVFM1RkVxaUZqWm1R?=
 =?utf-8?B?aUFwU0EvRnBGb05ZQW1zeldhNTFCR2U2Q0M4Q203YklQQ1EyRzQxSUF3Sjlm?=
 =?utf-8?B?cHQrT0ErWTdWVXlFZXlMUjhLeWMxdElnMDVUSjd1ZXZiYlJrV3NIazkvSWVF?=
 =?utf-8?B?ZGEwbUE1bHg4VUtaU2tidSt3M3ViUjZQOG5MVnhIMmZmL0p1R1NodFFwNGJk?=
 =?utf-8?B?OVFhMkZwWFdNTUFDQ0ZnQm1FMS9CT3h2VjJLWHlXdGo0OTFoa3ZvY0ZLWUhi?=
 =?utf-8?B?ZURvb1lOV25KUUoya0k1U0hxUzZWKzAyRFNVMXhDODR1WFBqTUZWQ2RMdDhW?=
 =?utf-8?B?cjM1YWcwWlk2YWU1WkgxZGc1cXZsaFFRL1c3Tjdmb3hmQzVoRDhiU08zTnIw?=
 =?utf-8?B?aDltdmtYY2tYZTJlYlhEUU5POExNRnM1WXkzNHRjR0o3NytrSGwvc0F4MHdq?=
 =?utf-8?B?MldxRkRpd0xRYTExQnZvSDRMS2F3VWxIZG02LzdsS0tvcjRiS1UydkVDK1Fw?=
 =?utf-8?Q?K2oakPy4RyIS08BwS0uVLNDI1?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 64c8159e-9e0b-418e-9169-08dc2bfb0918
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Feb 2024 18:47:17.0642
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: B5KORadjW0EiEcVNh8LltNxp+Bxi3wG+d0pswnKl41CTkIpiJBjvT8thvHWGUxGJP14q4g6u9QXDp0mrbYWG8g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB8920

On 2/11/2024 6:14 AM, Borislav Petkov wrote:
> On Sat, Feb 10, 2024 at 03:15:26PM -0600, Naik, Avadhut wrote:
>> IIUC, you wouldn't want to extend the pool through late_initcall().
>> Instead, you would want for memory to be allocated (on the heap) and
>> size of the pool to be set at the very beginning i.e. when the pool
>> is created (~2 seconds, according to dmesg timestamps).
>>
>> Please correct me if I have understood wrong.
> 
> Nah, you got it right. I went, looked and realized that we have to do
> this early dance because we have no allocator yet. And we can't move
> this gen_pool allocation to later, when we *do* have an allocator
> because MCA is up and logging already.
> 
> But your extending approach doesn't fly in all cases either:
> 
> gen_pool_add->gen_pool_add_virt->gen_pool_add_owner
> 
> it grabs the pool->lock spinlock and adds to &pool->chunks while, at the
> exact same time, gen_pool_alloc(), in *NMI* context iterates over that
> same &pool->chunks in the case we're logging an MCE at exact that same
> time when you're extending the buffer.
> 
> And Tony already said that in the thread you're quoting:
> 
> https://lore.kernel.org/linux-edac/SJ1PR11MB60832922E4D036138FF390FAFCD7A@SJ1PR11MB6083.namprd11.prod.outlook.com/
> 
> So no, that doesn't work either.
> 

I'm confused why it won't work.

X86 has ARCH_HAVE_NMI_SAFE_CMPXCHG. I expect atomics/caches will still
work in interrupt or #MC context. If not, then we'd have a fatal error
that causes a hardware reset or a kernel panic before we get to logging,
I think.

Or is the issue when running on the same CPU? In this case, either
&pool->chunks was updated before taking the #MC, so the extra memory
is there and can be used. Or it wasn't updated, so the extra memory is
not available during the #MC which is the same behavior as now.

I need to look more at the genpool code, but I thought I'd ask too.

Thanks,
Yazen