[PATCH v9 4/6] ACPI: HMAT: Fix handling of changes from ACPI 6.2 to ACPI 6.3

Bjorn Helgaas helgaas at kernel.org
Fri Aug 21 08:13:56 EDT 2020


[+cc Keith, author of 3accf7ae37a9 ("acpi/hmat: Parse and report
heterogeneous memory")]

On Fri, Aug 21, 2020 at 09:42:58AM +0100, Jonathan Cameron wrote:
> On Thu, 20 Aug 2020 17:21:29 -0500
> Bjorn Helgaas <helgaas at kernel.org> wrote:
> 
> > On Wed, Aug 19, 2020 at 10:51:09PM +0800, Jonathan Cameron wrote:
> > > In ACPI 6.3, the Memory Proximity Domain Attributes Structure
> > > changed substantially.  One of those changes was that the flag
> > > for "Memory Proximity Domain field is valid" was deprecated.
> > > 
> > > This was because the field "Proximity Domain for the Memory"
> > > became a required field and hence having a validity flag makes
> > > no sense.
> > > 
> > > So the correct logic is to always assume the field is there.
> > > Current code assumes it never is.
> > > 
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron at huawei.com>
> > > ---
> > >  drivers/acpi/numa/hmat.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> > > index 2c32cfb72370..07cfe50136e0 100644
> > > --- a/drivers/acpi/numa/hmat.c
> > > +++ b/drivers/acpi/numa/hmat.c
> > > @@ -424,7 +424,7 @@ static int __init hmat_parse_proximity_domain(union acpi_subtable_headers *heade
> > >  		pr_info("HMAT: Memory Flags:%04x Processor Domain:%u Memory Domain:%u\n",
> > >  			p->flags, p->processor_PD, p->memory_PD);
> > >  
> > > -	if (p->flags & ACPI_HMAT_MEMORY_PD_VALID && hmat_revision == 1) {
> > > +	if ((p->flags & ACPI_HMAT_MEMORY_PD_VALID && hmat_revision == 1) || hmat_revision == 2) {  
> > 
> > I hope/assume the spec is written in such a way that p->memory_PD is
> > required for any revision > 1?  So maybe this should be:
> > 
> >   if ((p->flags & ACPI_HMAT_MEMORY_PD_VALID && hmat_revision == 1) ||
> >       hmat_revision > 1) {

I should have said simply:

  if (hmat_revision == 1 && p->flags & ACPI_HMAT_MEMORY_PD_VALID)

We shouldn't even test p->flags for ACPI_HMAT_MEMORY_PD_VALID unless
we already know it's revision 1.

And unless there was a revision 0 of HMAT, there's no need to look for
hmat_revison > 1.

> Good point.  We have existing protections elsewhere against
> hmat_revision being anything other than 1 or 2, so we should aim to
> keep that in only one place.

I think the "Ignoring HMAT: Unknown revision" test in hmat_init(),
added by 3accf7ae37a9 ("acpi/hmat: Parse and report heterogeneous
memory"), is a mistake.

And I think hmat_normalize() has a similar mistake in that it tests
explicitly for hmat_revision == 2 when it should accept 2 AND anything
later.

We should assume that future spec revisions will be backwards
compatible.  Otherwise we're forced to make kernel changes when we
otherwise would not have to.

Bjorn



More information about the linux-arm-kernel mailing list