All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kani, Toshimitsu" <toshi.kani@hpe.com>
To: "dan.j.williams@intel.com" <dan.j.williams@intel.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"vishal.l.verma@intel.com" <vishal.l.verma@intel.com>
Subject: Re: [PATCH] Add support of NVDIMM memory error notification in ACPI 6.2
Date: Wed, 7 Jun 2017 21:33:31 +0000	[thread overview]
Message-ID: <1496871186.9288.3.camel@hpe.com> (raw)
In-Reply-To: <CAPcyv4hwhRc6+JW8m3zVn_tKz__wE9BHOfpjy69RS6OwiqV-PA@mail.gmail.com>

On Wed, 2017-06-07 at 14:06 -0700, Dan Williams wrote:
> On Wed, Jun 7, 2017 at 1:57 PM, Kani, Toshimitsu <toshi.kani@hpe.com>
> wrote:
> > On Wed, 2017-06-07 at 12:09 -0700, Dan Williams wrote:
> > > On Wed, Jun 7, 2017 at 11:49 AM, Toshi Kani <toshi.kani@hpe.com>
> > > wrote:
> > 
> >  :
> > > > +
> > > > +static void acpi_nfit_uc_error_notify(struct device *dev,
> > > > acpi_handle handle)
> > > > +{
> > > > +       struct acpi_nfit_desc *acpi_desc =
> > > > dev_get_drvdata(dev);
> > > > +
> > > > +       acpi_nfit_ars_rescan(acpi_desc);
> > > 
> > > I wonder if we should gate re-scanning with a similar:
> > > 
> > >     if (acpi_desc->scrub_mode == HW_ERROR_SCRUB_ON)
> > > 
> > > ...check that we do in the mce notification case? Maybe not since
> > > we
> > > don't get an indication of where the error is without a rescan.
> > 
> > I think this mce case is different since the MCE handler already
> > knows where the new poison location is and can update badblocks
> > information for it.  Starting ARS is an optional precaution.
> > 
> > > However, at a minimum I think we need support for the new Start
> > > ARS flag ("If set to 1 the firmware shall return data from a
> > > previous scrub, if any, without starting a new scrub") and use
> > > that for this case.
> > 
> > That's an interesting idea.  But I wonder how users know if it is
> > OK to set this flag as it relies on BIOS implementation that is not
> > described in ACPI...
> 
> Ugh, you're right. We might need a revision-id=2 version of Start ARS
> so software knows that the BIOS is aware of the new flag.

My bad.  Looking at ACPI 6.2, it actually defines what you described. 
Start ARS now defines bit[1] in Flags which can be set to avoid
scanning for this notification.  I will update the patch to set this
flag when HW_ERROR_SCRUB_ON is not set.

Thanks,
-Toshi

WARNING: multiple messages have this Message-ID (diff)
From: "Kani, Toshimitsu" <toshi.kani@hpe.com>
To: "dan.j.williams@intel.com" <dan.j.williams@intel.com>
Cc: "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: [PATCH] Add support of NVDIMM memory error notification in ACPI 6.2
Date: Wed, 7 Jun 2017 21:33:31 +0000	[thread overview]
Message-ID: <1496871186.9288.3.camel@hpe.com> (raw)
In-Reply-To: <CAPcyv4hwhRc6+JW8m3zVn_tKz__wE9BHOfpjy69RS6OwiqV-PA@mail.gmail.com>

On Wed, 2017-06-07 at 14:06 -0700, Dan Williams wrote:
> On Wed, Jun 7, 2017 at 1:57 PM, Kani, Toshimitsu <toshi.kani@hpe.com>
> wrote:
> > On Wed, 2017-06-07 at 12:09 -0700, Dan Williams wrote:
> > > On Wed, Jun 7, 2017 at 11:49 AM, Toshi Kani <toshi.kani@hpe.com>
> > > wrote:
> > 
> >  :
> > > > +
> > > > +static void acpi_nfit_uc_error_notify(struct device *dev,
> > > > acpi_handle handle)
> > > > +{
> > > > +       struct acpi_nfit_desc *acpi_desc =
> > > > dev_get_drvdata(dev);
> > > > +
> > > > +       acpi_nfit_ars_rescan(acpi_desc);
> > > 
> > > I wonder if we should gate re-scanning with a similar:
> > > 
> > >     if (acpi_desc->scrub_mode == HW_ERROR_SCRUB_ON)
> > > 
> > > ...check that we do in the mce notification case? Maybe not since
> > > we
> > > don't get an indication of where the error is without a rescan.
> > 
> > I think this mce case is different since the MCE handler already
> > knows where the new poison location is and can update badblocks
> > information for it.  Starting ARS is an optional precaution.
> > 
> > > However, at a minimum I think we need support for the new Start
> > > ARS flag ("If set to 1 the firmware shall return data from a
> > > previous scrub, if any, without starting a new scrub") and use
> > > that for this case.
> > 
> > That's an interesting idea.  But I wonder how users know if it is
> > OK to set this flag as it relies on BIOS implementation that is not
> > described in ACPI...
> 
> Ugh, you're right. We might need a revision-id=2 version of Start ARS
> so software knows that the BIOS is aware of the new flag.

My bad.  Looking at ACPI 6.2, it actually defines what you described. 
Start ARS now defines bit[1] in Flags which can be set to avoid
scanning for this notification.  I will update the patch to set this
flag when HW_ERROR_SCRUB_ON is not set.

Thanks,
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

  reply	other threads:[~2017-06-07 21:34 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-07 18:49 [PATCH] Add support of NVDIMM memory error notification in ACPI 6.2 Toshi Kani
2017-06-07 18:49 ` Toshi Kani
2017-06-07 19:09 ` Dan Williams
2017-06-07 19:09   ` Dan Williams
     [not found]   ` <CAPcyv4i7bLU17QEmdUBQrtWP3AZxPRyKK0NN105XrTj8K3nAAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-07 20:57     ` Kani, Toshimitsu
2017-06-07 20:57       ` Kani, Toshimitsu
2017-06-07 20:57       ` Kani, Toshimitsu
2017-06-07 21:06       ` Dan Williams
2017-06-07 21:06         ` Dan Williams
2017-06-07 21:33         ` Kani, Toshimitsu [this message]
2017-06-07 21:33           ` Kani, Toshimitsu
     [not found]           ` <1496871186.9288.3.camel-ZPxbGqLxI0U@public.gmane.org>
2017-06-08 17:30             ` Linda Knippers
2017-06-08 17:30               ` Linda Knippers
2017-06-08 17:30               ` Linda Knippers
     [not found]               ` <b91c4a7b-3e19-df5d-12e7-61cdf1bc2c84-ZPxbGqLxI0U@public.gmane.org>
2017-06-08 17:34                 ` Dan Williams
2017-06-08 17:34                   ` Dan Williams
2017-06-08 17:34                   ` Dan Williams
     [not found]                   ` <CAPcyv4hocL=1Mzqa+nJCdmRz7YgxUc3P1w-nn7F=uQG8wu94KA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-08 17:38                     ` Kani, Toshimitsu
2017-06-08 17:38                       ` Kani, Toshimitsu
2017-06-08 17:38                       ` Kani, Toshimitsu
2017-06-08 18:26                       ` Kani, Toshimitsu
2017-06-08 18:26                         ` Kani, Toshimitsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1496871186.9288.3.camel@hpe.com \
    --to=toshi.kani@hpe.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=rjw@rjwysocki.net \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.