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;
Date:   Wed, 27 Jul 2022 13:01:31 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Jane Chu <jane.chu@oracle.com>,
        Dan Williams <dan.j.williams@intel.com>,
        "tony.luck@intel.com" <tony.luck@intel.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "hch@lst.de" <hch@lst.de>,
        "nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [PATCH v4] x86/mce: retrieve poison range from hardware
Message-ID: <62e1999b4121e_2d2079294ea@dwillia2-xfh.jf.intel.com.notmuch>
References: <20220727184644.2685405-1-jane.chu@oracle.com>
 <62e18a687aac2_2d20792944b@dwillia2-xfh.jf.intel.com.notmuch>
 <f22faecf-b4ea-cb39-bed3-3647842b814e@oracle.com>
 <382e9410-d964-5600-4481-ccad90dbc97b@oracle.com>
 <8e817134-bfa9-72bc-3601-eeb1a138fe37@oracle.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <8e817134-bfa9-72bc-3601-eeb1a138fe37@oracle.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?gD2KfjzN2qtBGIPcZ3O7hh/QWkpYWBlp4ok+1HHhRrvg9Aie0RCuDIoSm6?=
 =?iso-8859-1?Q?D00hdhHAK/N6PGtSpbh/NbQFvQo7JSQcaG7imhDMxSAUyra+paPv66M8d6?=
 =?iso-8859-1?Q?8sxhqAU9bdV0AI7YIBUz0dcPqpOpeeNGv2knGw3OgqDk1afJMQLt+2ZHiU?=
 =?iso-8859-1?Q?zhak8T8xfWzZh7sbSvBRhFp3q/inoU5oYnsBF3s3YmVzIOSf68M+sElFgT?=
 =?iso-8859-1?Q?MFEIsDXctDyH1kfSzHY1/yZDTC3R7Zyi+qXW6g1mzI5mSCk9cRt14sPXN0?=
 =?iso-8859-1?Q?eNSihA7+lK3TTpafem/4MgUkjQUZ9wnP5qrcW/jIscjmdFQ5+8zs48oosm?=
 =?iso-8859-1?Q?6nNls7YNGcTTVO0xNA7o+DFVp/9yY0lax2wBT1KiewY559QQ1F43xa++3t?=
 =?iso-8859-1?Q?0yEO69twQMzodRwd4sM5t9p3gQm4fWQzG/+Ur2LX9rEi42ef6N9LF2ZO9l?=
 =?iso-8859-1?Q?xYzyCwQYbXHX3amHjzHBGX+EbCK9QnWT6/HqY401LB1qiN/ND5oAKtUy14?=
 =?iso-8859-1?Q?6IqXQN328KyCmyWF+wNEOkOg3WPA7HSN1HM1K1h7ztjOVVet4PQ/7fKZfP?=
 =?iso-8859-1?Q?xPuoMCNVufYt+/pIpEY3+yJpo18VvvVRuktyZd+jDy79fsILdo/VnAx8lB?=
 =?iso-8859-1?Q?ftBS44dYoWCzKIe71SFxscW3biVcSithJ/HMsFLOHBr4fI+F1B3aRDjOZA?=
 =?iso-8859-1?Q?eEP+1usEkaUcoZi4uzFGQR7hYhyemol6D6yZmg+1VFxH8SKiqLJEUi2Wv8?=
 =?iso-8859-1?Q?2MPl7SeG9K/xP25P3Ge4rA2MZprSs0E7ehGdSdf1yMdlOp1E9Xe4StHmvi?=
 =?iso-8859-1?Q?GWfJ2mGeWaGEXL8i3dz1AJWeUtXqm0td4niYE+tR0vUJetyAJf2GQaO6Ks?=
 =?iso-8859-1?Q?wqrllLpEfu3lXTYArFjOKPZJ44bScqkGmOQtH8megssdSOuIu9bKxqa6dF?=
 =?iso-8859-1?Q?Wyh2JNqfKNBWbJMz91EtmmT3ZbIcaw/jSpwEoY6DSeNupejOYH3qpy66SL?=
 =?iso-8859-1?Q?d2zZ7KQZ1F2zJF8n+whjWwXwGHGIRezRhNi+hJGnour3GvYvIXCoDwZevF?=
 =?iso-8859-1?Q?l/zlyrIB/r6LogehR9yWEQ8v/SO1UpdhgaYnl0dy+ymW0oQT0aHiNeZhuR?=
 =?iso-8859-1?Q?uHulULOd7i64W4JbjDOeGqRsbT4sXk/9ZTiWblPe8J6cf7CvCouXtEY86d?=
 =?iso-8859-1?Q?oBl+3LMsY08DkI26eOP6lvrfzYEGWvd6pLmBYFqS24jeRHxmpgon1JluaX?=
 =?iso-8859-1?Q?xZE5zqKl55Z5puqYGBrBzepX4L6wtnwvDfgMzv5+Bwz/+0vzlg8XV5qo87?=
 =?iso-8859-1?Q?6UlET32PJsAUI4oGsX7Ph+XZI0Tip/2QpyfvZhd9pIF6JBPv110xPk9bEe?=
 =?iso-8859-1?Q?izUQpeELVSHzzMQAB53zWZdJrUm20b7mVx8p0/f2u2y3b1fC7OC/TMs8kO?=
 =?iso-8859-1?Q?iBMZwdGp6O/DowGCmf2l5p8N0FCWCKOtcE2hB1GLLGX76tqtbNbgHcfQsD?=
 =?iso-8859-1?Q?6W2FSXNC0MgQirF56J5h3o01yrYNvrtj4Mk88gKnmgGA2lXxEQUniepsOA?=
 =?iso-8859-1?Q?ZYyAvbpWpa6YmN8gYs/x0Xj8N642sRLA/dNZGbLE6+VP1J9bjqWN68/vQ0?=
 =?iso-8859-1?Q?Ztm112WvfGTNRnHgYeTzGdCC5PM/mkB+34vgFqoDb7cYKTtfPrFUEjbQ?=
 =?iso-8859-1?Q?=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6f81fa35-56ef-40db-bd8b-08da700ace6a
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jul 2022 20:01:34.3463
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: kSLluh61O7ag7tBKfgUx948SjvfFt+WZQh79wmP4mq4sd/a5E/juz6J2u1NULXby39Voa1W8zhMCaVzTUTZjVjVMuYtVcznCLEP+G6eW234=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR11MB1703
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        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

Jane Chu wrote:
> On 7/27/2022 12:30 PM, Jane Chu wrote:
> > On 7/27/2022 12:24 PM, Jane Chu wrote:
> >> On 7/27/2022 11:56 AM, Dan Williams wrote:
> >>> Jane Chu wrote:
> >>>> With Commit 7917f9cdb503 ("acpi/nfit: rely on mce->misc to determine
> >>>> poison granularity") that changed nfit_handle_mce() callback to report
> >>>> badrange according to 1ULL << MCI_MISC_ADDR_LSB(mce->misc), it's been
> >>>> discovered that the mce->misc LSB field is 0x1000 bytes, hence 
> >>>> injecting
> >>>> 2 back-to-back poisons and the driver ends up logging 8 badblocks,
> >>>> because 0x1000 bytes is 8 512-byte.
> >>>>
> >>>> Dan Williams noticed that apei_mce_report_mem_error() hardcode
> >>>> the LSB field to PAGE_SHIFT instead of consulting the input
> >>>> struct cper_sec_mem_err record.? So change to rely on hardware whenever
> >>>> support is available.
> >>>>
> >>>> Link: 
> >>>> https://lore.kernel.org/r/7ed50fd8-521e-cade-77b1-738b8bfb8502@oracle.com 
> >>>>
> >>>>
> >>>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> >>>> Signed-off-by: Jane Chu <jane.chu@oracle.com>
> >>>> ---
> >>>> ? arch/x86/kernel/cpu/mce/apei.c | 14 +++++++++++++-
> >>>> ? 1 file changed, 13 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/x86/kernel/cpu/mce/apei.c 
> >>>> b/arch/x86/kernel/cpu/mce/apei.c
> >>>> index 717192915f28..26d63818b2de 100644
> >>>> --- a/arch/x86/kernel/cpu/mce/apei.c
> >>>> +++ b/arch/x86/kernel/cpu/mce/apei.c
> >>>> @@ -29,15 +29,27 @@
> >>>> ? void apei_mce_report_mem_error(int severity, struct 
> >>>> cper_sec_mem_err *mem_err)
> >>>> ? {
> >>>> ????? struct mce m;
> >>>> +??? int grain = PAGE_SHIFT;
> >>>> ????? if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
> >>>> ????????? return;
> >>>> +??? /*
> >>>> +???? * Even if the ->validation_bits are set for address mask,
> >>>> +???? * to be extra safe, check and reject an error radius '0',
> >>>> +???? * and fallback to the default page size.
> >>>> +???? */
> >>>> +??? if (mem_err->validation_bits & CPER_MEM_VALID_PA_MASK) {
> >>>> +??????? grain = ~mem_err->physical_addr_mask + 1;
> >>>> +??????? if (grain == 1)
> >>>> +??????????? grain = PAGE_SHIFT;
> >>>
> >>> Wait, if @grain is the number of bits to mask off the address, shouldn't
> >>> this be something like:
> >>>
> >>> ???? grain = min_not_zero(PAGE_SHIFT, 
> >>> hweight64(~mem_err->physical_addr_mask));
> >>
> >> I see. I guess what you meant is
> >> ??? grain = min(PAGE_SHIFT, (1 + 
> >> hweight64(~mem_err->physical_addr_mask)));
> > 
> > Sorry, take that back, it won't work either.
> 
> This will work,
>    grain = min_not_zero(PAGE_SHIFT - 1, 
> hweight64(~mem_err->physical_addr_mask));
>    grain++;
> but too sophisticated?  I guess I prefer the simple "if" expression.

An "if" is fine, I was more pointing out that:

    hweight64(~mem_err->physical_addr_mask) + 1

...and:

    ~mem_err->physical_addr_mask + 1;

...give different results.