Re: [PATCH 2/3] x86/mce: Define amd_mce_usable_address()

[William Roche <william.roche@oracle.com>]

On 6/16/23 03:59, Shuai Xue wrote:
> On 2023/6/15 23:15, Yazen Ghannam wrote:
>> On 6/14/2023 10:12 PM, Shuai Xue wrote:
>>>
>>> On 2023/6/14 23:09, Yazen Ghannam wrote:
>>>> On 6/13/2023 10:19 PM, Shuai Xue wrote:
>>>>>
>>>>> On 2023/6/13 22:11, Yazen Ghannam wrote:
>>>>>> Currently, all valid MCA_ADDR values are assumed to be usable on AMD
>>>>>> systems. However, this is not correct in most cases. Notifiers expecting
>>>>>> usable addresses may then operate on inappropriate values.
>>>>>>
>>>>>> Define a helper function to do AMD-specific checks for a usable memory
>>>>>> address. List out all known cases.
>>>>>>
>>>>>> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
>>>>>> ---
>>>>>>     arch/x86/kernel/cpu/mce/amd.c      | 38 ++++++++++++++++++++++++++++++
>>>>>>     arch/x86/kernel/cpu/mce/core.c     |  3 +++
>>>>>>     arch/x86/kernel/cpu/mce/internal.h |  2 ++
>>>>>>     3 files changed, 43 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
>>>>>> index 1ccfb0c9257f..ca79fa10b844 100644
>>>>>> --- a/arch/x86/kernel/cpu/mce/amd.c
>>>>>> +++ b/arch/x86/kernel/cpu/mce/amd.c
>>>>>> @@ -746,6 +746,44 @@ bool amd_mce_is_memory_error(struct mce *m)
>>>>>>         return legacy_mce_is_memory_error(m);
>>>>>>     }
>>>>>>     +/*
>>>>>> + * AMD systems do not have an explicit indicator that the value in MCA_ADDR is
>>>>>> + * a system physical address. Therefore individual cases need to be detected.
>>>>>> + * Future cases and checks will be added as needed.
>>>>>> + *
>>>>>> + * 1) General case
>>>>>> + *    a) Assume address is not usable.
>>>>>> + * 2) "Poison" errors
>>>>>> + *    a) Indicated by MCA_STATUS[43]: POISON. Defined for all banks except legacy
>>>>>> + *       Northbridge (bank 4).
>>>>>> + *    b) Refers to poison consumption in the Core. Does not include "no action",
>>>>>> + *       "action optional", or "deferred" error severities.
>>>>>> + *    c) Will include a usuable address so that immediate action can be taken.
>>>>>> + * 3) Northbridge DRAM ECC errors
>>>>>> + *    a) Reported in legacy bank 4 with XEC 8.
>>>>>> + *    b) MCA_STATUS[43] is *not* defined as POISON in legacy bank 4. Therefore,
>>>>>> + *       this bit should not be checked.
>>>>> [nit]
>>>>>
>>>>>> + *
>>>>>> + * NOTE: SMCA UMC memory errors fall into case #1.
>>>>> hi, Yazen
>>>>>
>>>>> The address for SMCA UMC memory error is not system physical address, it make sense
>>>>> to be not usable. But how we deal with the SMCA address? The MCE chain like
>>>>> uc_decode_notifier will do a sanity check with mce_usable_address and it will not
>>>>> handle SMCA address.
>>>>>
>>>> Hi Shuai,
>>>>
>>>> That's correct.
>>>>
>>>> There isn't a good solution today. This will be handled in future changes.
>>> Hi, Yazen,
>>>
>>> Do you have plan to address it? If not, I can help. We meet this problem in our products.
>>>
>> Yes, definitely. The first step is to update the address translation code; this is progress. Afterwards, we can find a way to leverage this in the MCE notifier flows.
> Look forward to it.
>
>> Just curious, how big is the benefit of this preemptive page offline in your use cases? That is, compared to page offline as part of poison data consumption.
> There are three aspects of benefits if SMCA address detected by scrubber is offlined
> in advance:
>
> - Free page: it should be isolated and not allocated by buddy so that the poison data
>    will never be consumed.
> - In-use page: the heath VMs could be migrated into other heath node if many UCE occurs.

I would also like to add that an application able to take action and 
re-create the impacted memory page (like a database for example), could 
do so before the data is requested by a user.

> - Mitigate the possibility of nested MCE which is a fatal error.
>
> Thank you.
>
> Best Regards,
> Shuai.

HTH,
William