Re: acpi_nfit_query_poison() broken on non-ARS systems

From: Vishal Verma <vishal@kernel.org>
To: Linda Knippers <linda.knippers@hpe.com>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	Vishal L Verma <vishal.l.verma@intel.com>
Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: acpi_nfit_query_poison() broken on non-ARS systems
Date: Fri, 29 Apr 2016 18:03:19 -0600	[thread overview]
Message-ID: <1461974599.11211.6.camel@kernel.org> (raw)
In-Reply-To: <5723EFEE.9060200@hpe.com>

On Fri, 2016-04-29 at 19:36 -0400, Linda Knippers wrote:

[snip]
> 
> > static int acpi_nfit_query_poison(struct acpi_nfit_desc *acpi_desc,
> >                 struct nfit_spa *nfit_spa)
> > {
> >         struct acpi_nfit_system_address *spa = nfit_spa->spa;
> >         int rc;
> > 
> >         if (!nfit_spa->max_ars) {
> On a platform that doesn't support ARS, max_ars will always be zero
> so you'll keep executing this code when it seems like you're trying
> to avoid that.
> 
> > 
> >                 struct nd_cmd_ars_cap ars_cap;
> > 
> >                 memset(&ars_cap, 0, sizeof(ars_cap));
> >                 rc = ars_get_cap(acpi_desc, &ars_cap, nfit_spa);
> >                 if (rc < 0)
> >                         return rc;
> The call succeeds so this return isn't taken, but then the code
> assumes
> everything is good.  It should check ars_cap.status so see if the
> function
> is supported or if there was an error and return something
> appropriate.
> In previous version of the code, that's what acpi_nfit_find_poison()
> did.
> Instead, this function continues, using data that's not right and
> making
> more calls that also aren't supported.

Good point - I think we should add a check here and make sure ARS is
supported using the status field. I can work on a patch. Thanks for the
report!

	-Vishal

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