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: <77d51e2f-cd1c-9c30-5bd5-42b1d583db53@amd.com>
Date:   Thu, 22 Jun 2023 15:23:49 -0400
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Cc:     yazen.ghannam@amd.com,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH 1/2] x86/mce: Disable preemption for CPER decoding
Content-Language: en-US
To:     "Luck, Tony" <tony.luck@intel.com>,
        "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
References: <20230622131841.3153672-1-yazen.ghannam@amd.com>
 <20230622131841.3153672-2-yazen.ghannam@amd.com>
 <SJ1PR11MB6083664BCFC8047A5FE8F6A9FC22A@SJ1PR11MB6083.namprd11.prod.outlook.com>
 <f03b6c61-1669-c03e-310c-cc1364cf30a8@amd.com>
 <SJ1PR11MB6083961DFCA3D90922824189FC22A@SJ1PR11MB6083.namprd11.prod.outlook.com>
From:   Yazen Ghannam <yazen.ghannam@amd.com>
In-Reply-To: <SJ1PR11MB6083961DFCA3D90922824189FC22A@SJ1PR11MB6083.namprd11.prod.outlook.com>
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?NHptY1U5WnZ3UWMzRElNWXJqWWdkRmZvRHpyditRKzN0OHVjMGl0a0J6N2Ey?=
 =?utf-8?B?d3g5ekxTNFdVYytOdUtNYzVkdVBaWG5SNzlVbVd3dlBBcE10NkNGdDcrMG90?=
 =?utf-8?B?RlY1Y2kySHNoTGJERUpjQ09xNWo1c2JxTW5YYjM5NTRqbEQ5a3JraFFRNmkv?=
 =?utf-8?B?RUdBaUgxS2lkSm9VUDJXeXZzbFJQVUIvd2YzWTNpNzJ1TWZpSm5IaDRKcktQ?=
 =?utf-8?B?ZlJoTlB3a2JXb0FRdzhQNUdWMTA0ZXV4NWlQcUhaV3ErTGRHTE4ydUxBUUVN?=
 =?utf-8?B?MEVBd3BKQ1lWdHk3V1dtcFdNelpLcklCMS9QL1FHSXMxcEFFL0NHYlJaY0J4?=
 =?utf-8?B?aFV2ZTdGeXc2Qnlaek9GM29pRVp3QWI4QzlWM2pORE5XSUtWYk5nT0o3SGpJ?=
 =?utf-8?B?ckNkVjV5bFRHTkxGazh1VGRsdjFUcjB3bExYbStXTStNYmswcDdHVHdNd0dR?=
 =?utf-8?B?MDJyRWIvNjVINzIxWEltYURaK2F0YVlvVmRMQU1yS25PdWY5WEQyTStGZ3Bq?=
 =?utf-8?B?UnB0RWhrSFpZTkpvNnh3a0JCUFV0RFhqMmNvYktHbVFtcG92ay8wR1JtaEdq?=
 =?utf-8?B?TVMzWWVxQi9PZFk4TVUydGViNmZ1RDFjeWZFZTNLRVJ3WFBJc1M1eTRlbzQ5?=
 =?utf-8?B?elRRVzB5OTZmb0kxbFJ3ekRzMjVodVFFRGE2YWdwWEZyeFFnWGtXUjdneHlC?=
 =?utf-8?B?ckVaei9LdUN6ejIvUlVka0s4d0lMUGRjVU43ZCthNFk4TVlIRnA5NEtyOXFp?=
 =?utf-8?B?T1g1aGFsRjMrZXJkeDR1VWd0dDRNUG5FODVIeWUzeHN3eTJRVHlKa01XeHl0?=
 =?utf-8?B?TnlMVzdMVk51bVVLenlWeVBXRXRVNktmOEJ0b0lXK0ovbU1uM3NHNU15U0NZ?=
 =?utf-8?B?YnNTa1NNRUxldlRTUDlWcHN5S1NPdUluK01wbFlLa2lpbDVtTys5cFp5bTZn?=
 =?utf-8?B?d2FFYURNRXM3ankzKy8yZTFuTWVtekp6Y2ZsV1NhWTVWU3RQNXl0aGU0enJU?=
 =?utf-8?B?R2RoUXdDeTdKREN0SWhKSkdvMkQvcFliWGVOV0pQRTBRL25BaURnWkJPcVhk?=
 =?utf-8?B?RHQ0cTZESWQ3UERvTGZZQlZsRkpOMVgzaVdmbkFsVk8xMXJOWWRLVlhJR1BU?=
 =?utf-8?B?SHRHRHltT2RRbC82c3JmUXVzdEZLU0xBMjJuaHBOWDAxQkxmc0J3UmxkamV5?=
 =?utf-8?B?RkwwRysxNDdMY3ZNMlRVQlkwMTNKY29WUS84NGVDdlg1TzIrT292TmVmNTdv?=
 =?utf-8?B?cVVDQlczMGVjQ05MREVnQ3hCMFYvNWhyTHdDNnZYRTUzZUN1b3l2aWgwUUZZ?=
 =?utf-8?B?L2lsQlMzV0Y1VXkrUDVRd2xCZzJCaFhpTnRoNDN2Nk9pTWYwS1FiV0JhRkFr?=
 =?utf-8?B?V2RtdURUMFBCOGR2eGNUUTNBbDBXVkR2UmhkRDNVOGdpb09mTk1JbDVkOGh4?=
 =?utf-8?B?cTJjN2czaElXZGJOTDRwMDFmOVlYRUY4S0lFOEl3VDhrMmVYY2hxb0hNWXor?=
 =?utf-8?B?dWoxVlNTOUpoMmhicXlqN0JBeFAvV25MdXdnOHRvbVk3eThuWFZtdjhmYmNZ?=
 =?utf-8?B?NlVmQW1nVElhd2Q0cEhDTEdCY1VPWldVRzNvanF6cDROMVlIL25scHcwaUdw?=
 =?utf-8?B?UnExL3N2QXdSd2xPUjVna1N2Z25GdlUzSHB5ZDhadjVkeDRrMG1tbEt3VVFI?=
 =?utf-8?B?MU9VQzRhYVZVZlJrWXlEblhHTzRBekNDRk1yWGc1WUk2MWw1UllXY3N2OXcx?=
 =?utf-8?B?bE95bUROZVJ0dzkwb09YSHJEdzJyakQwNzNqRHMvdHN3VEEzZnBIYlNKOThz?=
 =?utf-8?B?RFBOMU05WmtRM29jNDE5L0dSNTJZR3ZwM0dreU93Y1l4dW1VcXk3akZZck0w?=
 =?utf-8?B?OE1XR2NRQno3S2lEcCtqbkxlRWFRaXo5OG9xOEppNFROTEpWKzV1aHZRaXE5?=
 =?utf-8?B?SkNhcURmTEJNaTlOTXBhT1NSSTJvQkNXRGltTGZTOGtobmdpSFg0Qk5HdlFB?=
 =?utf-8?B?bHU0RVJnbVVrSXBJemhMZWpjMUlpcDFpNjNqVW85WTFHRmRkRVRLdUFsQVk3?=
 =?utf-8?B?K211MWtrcFlybXRkZGNXdHphbjZGQjA3LzBRaGRFNGxIa0lXQk1DV0xIM3hu?=
 =?utf-8?Q?oxIF2wkxGsvFM7uxifh5V8S18?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 13e2187c-2aa7-4eeb-a914-08db735635f4
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jun 2023 19:23:51.4257
 (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: 5Gn1L8ltRjVB9gEk0zt3Y1DZ6+rhBwSKlNu8wrCXcWi7zaD0nXshKv91vm/SNE0BjtMaVLQ5nV4a8RDRwqMbfw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR12MB7781
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no 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 6/22/2023 1:05 PM, Luck, Tony wrote:
>> This is the reason we search for the logical CPU number using the Local
>> APIC ID provided in the CPER. And fill in relevant data using that CPU
>> number.
> 
> So you don't care which CPU number mce_setup() used because you are
> going to update it with the right one from CPER?
>

That's right.

> Then maybe the fix for part 1 is just to use raw_smp_processor_id() instead of
> smp_processor_id() to avoid the warning for calling with pre-emption enabled,
> instead of disabling premption with the get_cpu() ... put_cpu() wrap around the
> call to mce_setup()?

You mean use raw_smp_processor_id() in mce_setup()? I thought about 
that, but decided against it. I figure the preemption warning is helpful 
to catch issues when mce_setup() *is* supposed to run on the current CPU 
but doesn't.

This BERT decoding path is the only exception AFAIK. So I didn't want to 
change the common code for a single exception.

I just noticed a similar potential issue with mce_setup() in 
apei_mce_report_mem_error(). How is the CPU number decided there? Is it 
always "don't care", since the mce record is "fake"?

Here are another couple of solutions for the preemption issue.
1) Don't use mce_setup() at all. Instead, do the memset(), etc. in the 
local function. This would result in some code duplication.
2) Split mce_setup() into global and per_cpu parts. The memset(), cpuid, 
etc. would be global, and the cpu_data()* and rdmsr() would be per_cpu.

Option #2 can also be used in apei_mce_report_mem_error(), I think.

Thanks,
Yazen