From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <1658696.rIK19Js0WO@aspire.rjw.lan> References: <20171214021019.13579-1-ross.zwisler@linux.intel.com> <20171214021019.13579-2-ross.zwisler@linux.intel.com> <1658696.rIK19Js0WO@aspire.rjw.lan> From: Dan Williams Date: Fri, 15 Dec 2017 17:57:08 -0800 Message-ID: Subject: Re: [PATCH v3 1/3] acpi: HMAT support in acpi_parse_entries_array() Content-Type: text/plain; charset="UTF-8" Sender: owner-linux-mm@kvack.org To: "Rafael J. Wysocki" Cc: Linux ACPI , Ross Zwisler , "linux-kernel@vger.kernel.org" , "Anaczkowski, Lukasz" , "Box, David E" , "Kogut, Jaroslaw" , "Koss, Marcin" , "Koziej, Artur" , "Lahtinen, Joonas" , "Moore, Robert" , "Nachimuthu, Murugasamy" , "Odzioba, Lukasz" , "Rafael J. Wysocki" , "Schmauss, Erik" , "Verma, Vishal L" , "Zheng, Lv" , Andrew Morton , Balbir Singh , Brice Goglin , Dave Hansen , Jerome Glisse , John Hubbard , Len Brown , Tim Chen , Linux MM , "linux-nvdimm@lists.01.org" List-ID: On Fri, Dec 15, 2017 at 5:53 PM, Rafael J. Wysocki wrote: > On Friday, December 15, 2017 2:10:17 AM CET Dan Williams wrote: >> On Wed, Dec 13, 2017 at 6:10 PM, Ross Zwisler >> wrote: >> > The current implementation of acpi_parse_entries_array() assumes that each >> > subtable has a standard ACPI subtable entry of type struct >> > acpi_subtable_header. This standard subtable header has a one byte length >> > followed by a one byte type. >> > >> > The HMAT subtables have to allow for a longer length so they have subtable >> > headers of type struct acpi_hmat_structure which has a 2 byte type and a 4 >> > byte length. >> >> Hmm, NFIT has a 2 byte type and a 2 byte length, so its one more >> permutation. I happened to reinvent sub-table parsing in the NFIT >> driver, but it might be nice in the future to refactor that to use the >> common parsing. >> >> > >> > Enhance the subtable parsing in acpi_parse_entries_array() so that it can >> > handle these new HMAT subtables. >> > >> > Signed-off-by: Ross Zwisler >> > --- >> > drivers/acpi/tables.c | 52 ++++++++++++++++++++++++++++++++++++++++----------- >> > 1 file changed, 41 insertions(+), 11 deletions(-) >> > >> > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c >> > index 80ce2a7d224b..f777b94c234a 100644 >> > --- a/drivers/acpi/tables.c >> > +++ b/drivers/acpi/tables.c >> > @@ -218,6 +218,33 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) >> > } >> > } >> > >> > +static unsigned long __init >> > +acpi_get_entry_type(char *id, void *entry) >> > +{ >> > + if (strncmp(id, ACPI_SIG_HMAT, 4) == 0) >> > + return ((struct acpi_hmat_structure *)entry)->type; >> > + else >> > + return ((struct acpi_subtable_header *)entry)->type; >> > +} >> >> It seems inefficient to make all checks keep asking "is HMAT?". > > Well, ideally, the signature should be checked once. I guess that can be > arranged for here. > >> Especially if we want to extend this to other table types should we >> instead setup and pass a pair of function pointers to parse the >> sub-table format? > > Function pointers may be too much even. :-) True, how about an enum of acpi sub-table header types? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Williams Subject: Re: [PATCH v3 1/3] acpi: HMAT support in acpi_parse_entries_array() Date: Fri, 15 Dec 2017 17:57:08 -0800 Message-ID: References: <20171214021019.13579-1-ross.zwisler@linux.intel.com> <20171214021019.13579-2-ross.zwisler@linux.intel.com> <1658696.rIK19Js0WO@aspire.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <1658696.rIK19Js0WO@aspire.rjw.lan> Sender: owner-linux-mm@kvack.org To: "Rafael J. Wysocki" Cc: Linux ACPI , Ross Zwisler , "linux-kernel@vger.kernel.org" , "Anaczkowski, Lukasz" , "Box, David E" , "Kogut, Jaroslaw" , "Koss, Marcin" , "Koziej, Artur" , "Lahtinen, Joonas" , "Moore, Robert" , "Nachimuthu, Murugasamy" , "Odzioba, Lukasz" , "Rafael J. Wysocki" , "Schmauss, Erik" , "Verma, Vishal L" , "Zheng, Lv" , Andrew Morton , Balbir Singh , Brice Goglin List-Id: linux-acpi@vger.kernel.org On Fri, Dec 15, 2017 at 5:53 PM, Rafael J. Wysocki wrote: > On Friday, December 15, 2017 2:10:17 AM CET Dan Williams wrote: >> On Wed, Dec 13, 2017 at 6:10 PM, Ross Zwisler >> wrote: >> > The current implementation of acpi_parse_entries_array() assumes that each >> > subtable has a standard ACPI subtable entry of type struct >> > acpi_subtable_header. This standard subtable header has a one byte length >> > followed by a one byte type. >> > >> > The HMAT subtables have to allow for a longer length so they have subtable >> > headers of type struct acpi_hmat_structure which has a 2 byte type and a 4 >> > byte length. >> >> Hmm, NFIT has a 2 byte type and a 2 byte length, so its one more >> permutation. I happened to reinvent sub-table parsing in the NFIT >> driver, but it might be nice in the future to refactor that to use the >> common parsing. >> >> > >> > Enhance the subtable parsing in acpi_parse_entries_array() so that it can >> > handle these new HMAT subtables. >> > >> > Signed-off-by: Ross Zwisler >> > --- >> > drivers/acpi/tables.c | 52 ++++++++++++++++++++++++++++++++++++++++----------- >> > 1 file changed, 41 insertions(+), 11 deletions(-) >> > >> > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c >> > index 80ce2a7d224b..f777b94c234a 100644 >> > --- a/drivers/acpi/tables.c >> > +++ b/drivers/acpi/tables.c >> > @@ -218,6 +218,33 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) >> > } >> > } >> > >> > +static unsigned long __init >> > +acpi_get_entry_type(char *id, void *entry) >> > +{ >> > + if (strncmp(id, ACPI_SIG_HMAT, 4) == 0) >> > + return ((struct acpi_hmat_structure *)entry)->type; >> > + else >> > + return ((struct acpi_subtable_header *)entry)->type; >> > +} >> >> It seems inefficient to make all checks keep asking "is HMAT?". > > Well, ideally, the signature should be checked once. I guess that can be > arranged for here. > >> Especially if we want to extend this to other table types should we >> instead setup and pass a pair of function pointers to parse the >> sub-table format? > > Function pointers may be too much even. :-) True, how about an enum of acpi sub-table header types? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756659AbdLPB5P (ORCPT ); Fri, 15 Dec 2017 20:57:15 -0500 Received: from mail-ot0-f196.google.com ([74.125.82.196]:42284 "EHLO mail-ot0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755977AbdLPB5J (ORCPT ); Fri, 15 Dec 2017 20:57:09 -0500 X-Google-Smtp-Source: ACJfBotKxXla+Kxa4na4HKI+J4XYQuBBPVa00nI4B0IJu+1mcNmnv3A8h+e2aOYVtcehcJGnTN6Badx58Dt/z2NALw4= MIME-Version: 1.0 In-Reply-To: <1658696.rIK19Js0WO@aspire.rjw.lan> References: <20171214021019.13579-1-ross.zwisler@linux.intel.com> <20171214021019.13579-2-ross.zwisler@linux.intel.com> <1658696.rIK19Js0WO@aspire.rjw.lan> From: Dan Williams Date: Fri, 15 Dec 2017 17:57:08 -0800 Message-ID: Subject: Re: [PATCH v3 1/3] acpi: HMAT support in acpi_parse_entries_array() To: "Rafael J. Wysocki" Cc: Linux ACPI , Ross Zwisler , "linux-kernel@vger.kernel.org" , "Anaczkowski, Lukasz" , "Box, David E" , "Kogut, Jaroslaw" , "Koss, Marcin" , "Koziej, Artur" , "Lahtinen, Joonas" , "Moore, Robert" , "Nachimuthu, Murugasamy" , "Odzioba, Lukasz" , "Rafael J. Wysocki" , "Schmauss, Erik" , "Verma, Vishal L" , "Zheng, Lv" , Andrew Morton , Balbir Singh , Brice Goglin , Dave Hansen , Jerome Glisse , John Hubbard , Len Brown , Tim Chen , Linux MM , "linux-nvdimm@lists.01.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Dec 15, 2017 at 5:53 PM, Rafael J. Wysocki wrote: > On Friday, December 15, 2017 2:10:17 AM CET Dan Williams wrote: >> On Wed, Dec 13, 2017 at 6:10 PM, Ross Zwisler >> wrote: >> > The current implementation of acpi_parse_entries_array() assumes that each >> > subtable has a standard ACPI subtable entry of type struct >> > acpi_subtable_header. This standard subtable header has a one byte length >> > followed by a one byte type. >> > >> > The HMAT subtables have to allow for a longer length so they have subtable >> > headers of type struct acpi_hmat_structure which has a 2 byte type and a 4 >> > byte length. >> >> Hmm, NFIT has a 2 byte type and a 2 byte length, so its one more >> permutation. I happened to reinvent sub-table parsing in the NFIT >> driver, but it might be nice in the future to refactor that to use the >> common parsing. >> >> > >> > Enhance the subtable parsing in acpi_parse_entries_array() so that it can >> > handle these new HMAT subtables. >> > >> > Signed-off-by: Ross Zwisler >> > --- >> > drivers/acpi/tables.c | 52 ++++++++++++++++++++++++++++++++++++++++----------- >> > 1 file changed, 41 insertions(+), 11 deletions(-) >> > >> > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c >> > index 80ce2a7d224b..f777b94c234a 100644 >> > --- a/drivers/acpi/tables.c >> > +++ b/drivers/acpi/tables.c >> > @@ -218,6 +218,33 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) >> > } >> > } >> > >> > +static unsigned long __init >> > +acpi_get_entry_type(char *id, void *entry) >> > +{ >> > + if (strncmp(id, ACPI_SIG_HMAT, 4) == 0) >> > + return ((struct acpi_hmat_structure *)entry)->type; >> > + else >> > + return ((struct acpi_subtable_header *)entry)->type; >> > +} >> >> It seems inefficient to make all checks keep asking "is HMAT?". > > Well, ideally, the signature should be checked once. I guess that can be > arranged for here. > >> Especially if we want to extend this to other table types should we >> instead setup and pass a pair of function pointers to parse the >> sub-table format? > > Function pointers may be too much even. :-) True, how about an enum of acpi sub-table header types?