Add error checks to various methods and bail early if requisite tables are not found.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org --- drivers/acpi/plat/arm-core.c | 61 ++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 19 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 9704229..87e93b2 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -222,27 +222,40 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table) return 0; }
-static void __init acpi_process_madt(void) +static int __init acpi_process_madt(void) { - int error; - - if (!acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt)) { - - /* - * Parse MADT GIC cpu interface entries - */ - error = acpi_parse_madt_gic_entries(); - if (!error) { - /* - * Parse MADT GIC distributor entries - */ - acpi_parse_madt_gic_distributor_entries(); - } + int err = 0; + + err = acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt); + if (err) { + pr_err("Failed to parse MADT\n"); + goto out_err; + } + + /* + * Parse MADT GIC cpu interface entries + */ + err = acpi_parse_madt_gic_entries(); + if (err) { + pr_err("Failed to parse GIC entries from MADT\n"); + goto out_err; + } + + /* + * Parse MADT GIC distributor entries + */ + err = acpi_parse_madt_gic_distributor_entries(); + if (err) { + pr_err("Failed to find GIC Distributor entries from MADT\n"); + goto out_err; }
pr_info("Using ACPI for processor (GIC) configuration information\n"); + return 0;
- return; +out_err: + pr_err("Err processing GIC information\n"); + return err; }
/* @@ -273,20 +286,30 @@ void __init acpi_boot_table_init(void)
int __init acpi_boot_init(void) { + int err = 0; /* * If acpi_disabled, bail out */ if (acpi_disabled) return -ENODEV;
- acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt); + err = acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt); + if (err) { + pr_err("Failed to find FADT\n"); + goto out_err; + }
/* * Process the Multiple APIC Description Table (MADT), if present */ - acpi_process_madt(); + err = acpi_process_madt(); + if (err) { + pr_err("Failed to process MADT\n"); + goto out_err; + }
- return 0; +out_err: + return err; }
static int __init parse_acpi(char *arg)
Introduce a new function that takes a table_header pointer acquired from acpi_table_parse(). This eliminates the need to get the table pointer again by traversing the whole list of tables. e.g. as in acpi_table_parse_madt().
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org --- drivers/acpi/tables.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 4 ++++ 2 files changed, 59 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index be6a7d7..eaf805b 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -203,6 +203,61 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) } }
+int __init +acpi_parse_entries(unsigned long table_size, + acpi_tbl_entry_handler handler, + struct acpi_table_header *table_header, + int entry_id, unsigned int max_entries) +{ + struct acpi_subtable_header *entry; + unsigned int count = 0; + unsigned long table_end; + acpi_size tbl_size = table_header->length; + + if (acpi_disabled) + return -ENODEV; + + if (!table_header) { + pr_warn(PREFIX "Table header not present\n"); + return -ENODEV; + } + + table_end = (unsigned long)table_header + table_header->length; + + /* Parse all entries looking for a match. */ + entry = (struct acpi_subtable_header *) + ((unsigned long)table_header + table_size); + + while (((unsigned long)entry) + sizeof(struct acpi_subtable_header) < + table_end) { + if (entry->type == entry_id + && (!max_entries || count++ < max_entries)) + if (handler(entry, table_end)) + goto err; + + /* + * If entry->length is 0, break from this loop to avoid + * infinite loop. + */ + if (entry->length == 0) { + pr_err(PREFIX "[0x%02x] Invalid zero length\n", entry_id); + goto err; + } + + entry = (struct acpi_subtable_header *) + ((unsigned long)entry + entry->length); + } + if (max_entries && count > max_entries) { + pr_warn(PREFIX "[0x%02x] ignored %i entries of %i found\n", + entry_id, count - max_entries, count); + } + + early_acpi_os_unmap_memory((char *)table_header, tbl_size); + return count; +err: + early_acpi_os_unmap_memory((char *)table_header, tbl_size); + return -EINVAL; +}
int __init acpi_table_parse_entries(char *id, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 372038a..7d5dba2 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -122,6 +122,10 @@ int acpi_numa_init (void);
int acpi_table_init (void); int acpi_table_parse(char *id, acpi_tbl_table_handler handler); +int __init acpi_parse_entries(unsigned long table_size, + acpi_tbl_entry_handler handler, + struct acpi_table_header *table_header, + int entry_id, unsigned int max_entries); int __init acpi_table_parse_entries(char *id, unsigned long table_size, int entry_id, acpi_tbl_entry_handler handler,
Hi Ashwin,
I like this patch! Please see comments below.
On 19.03.2014 04:51, Ashwin Chaugule wrote:
Introduce a new function that takes a table_header pointer acquired from acpi_table_parse(). This eliminates the need to get the table pointer again by traversing the whole list of tables. e.g. as in acpi_table_parse_madt().
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/acpi/tables.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 4 ++++ 2 files changed, 59 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index be6a7d7..eaf805b 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -203,6 +203,61 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) } }
+int __init +acpi_parse_entries(unsigned long table_size,
acpi_tbl_entry_handler handler,struct acpi_table_header *table_header,int entry_id, unsigned int max_entries)+{
- struct acpi_subtable_header *entry;
- unsigned int count = 0;
- unsigned long table_end;
- acpi_size tbl_size = table_header->length;
- if (acpi_disabled)
return -ENODEV;- if (!table_header) {
pr_warn(PREFIX "Table header not present\n");return -ENODEV;- }
- table_end = (unsigned long)table_header + table_header->length;
- /* Parse all entries looking for a match. */
- entry = (struct acpi_subtable_header *)
((unsigned long)table_header + table_size);- while (((unsigned long)entry) + sizeof(struct acpi_subtable_header) <
table_end) {if (entry->type == entry_id&& (!max_entries || count++ < max_entries))if (handler(entry, table_end))goto err;/** If entry->length is 0, break from this loop to avoid* infinite loop.*/if (entry->length == 0) {pr_err(PREFIX "[0x%02x] Invalid zero length\n", entry_id);goto err;}entry = (struct acpi_subtable_header *)((unsigned long)entry + entry->length);- }
- if (max_entries && count > max_entries) {
pr_warn(PREFIX "[0x%02x] ignored %i entries of %i found\n",entry_id, count - max_entries, count);- }
- early_acpi_os_unmap_memory((char *)table_header, tbl_size);
- return count;
+err:
- early_acpi_os_unmap_memory((char *)table_header, tbl_size);
- return -EINVAL;
+}
Majority logic of acpi_parse_entries() overlaps acpi_table_parse_entries(). I think we can refactor acpi_table_parse_entries() function to take advantage of acpi_parse_entries(). Something like this:
int __init acpi_table_parse_entries(char *id, unsigned long table_size, int entry_id, acpi_tbl_entry_handler handler, unsigned int max_entries) { [...] if (strncmp(id, ACPI_SIG_MADT, 4) == 0) acpi_get_table_with_size(id, acpi_apic_instance, &table_header, &tbl_size); else acpi_get_table_with_size(id, 0, &table_header, &tbl_size);
[...]
return acpi_parse_entries(table_size, handler, table_header, entry_id, max_entries); }
Tomasz
int __init acpi_table_parse_entries(char *id, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 372038a..7d5dba2 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -122,6 +122,10 @@ int acpi_numa_init (void);
int acpi_table_init (void); int acpi_table_parse(char *id, acpi_tbl_table_handler handler); +int __init acpi_parse_entries(unsigned long table_size,
acpi_tbl_entry_handler handler,struct acpi_table_header *table_header,int entry_id, unsigned int max_entries);
Hi Tomasz,
On 19 March 2014 04:39, Tomasz Nowicki tomasz.nowicki@linaro.org wrote:
Majority logic of acpi_parse_entries() overlaps acpi_table_parse_entries(). I think we can refactor acpi_table_parse_entries() function to take advantage of acpi_parse_entries(). Something like this:
Gah. Right. Thanks for the review. Exactly what I needed for sending out late night patches.
Cheers, Ashwin
Hi Aswin,
This looks like a cleanup of upstream code, could you try and keep that seperate from patches to plat/arm-core.c
Its just that you should be attempting to push this to linux-acpi ASAP to avoid us building a backlog.
Also if they reject this then we don't want to internally build on top of it.
Thanks
Graeme
On Tue, Mar 18, 2014 at 11:51:33PM -0400, Ashwin Chaugule wrote:
Introduce a new function that takes a table_header pointer acquired from acpi_table_parse(). This eliminates the need to get the table pointer again by traversing the whole list of tables. e.g. as in acpi_table_parse_madt().
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/acpi/tables.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 4 ++++ 2 files changed, 59 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index be6a7d7..eaf805b 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -203,6 +203,61 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) } } +int __init +acpi_parse_entries(unsigned long table_size,
acpi_tbl_entry_handler handler,struct acpi_table_header *table_header,int entry_id, unsigned int max_entries)+{
- struct acpi_subtable_header *entry;
- unsigned int count = 0;
- unsigned long table_end;
- acpi_size tbl_size = table_header->length;
- if (acpi_disabled)
return -ENODEV;- if (!table_header) {
pr_warn(PREFIX "Table header not present\n");return -ENODEV;- }
- table_end = (unsigned long)table_header + table_header->length;
- /* Parse all entries looking for a match. */
- entry = (struct acpi_subtable_header *)
((unsigned long)table_header + table_size);- while (((unsigned long)entry) + sizeof(struct acpi_subtable_header) <
table_end) {if (entry->type == entry_id&& (!max_entries || count++ < max_entries))if (handler(entry, table_end))goto err;/** If entry->length is 0, break from this loop to avoid* infinite loop.*/if (entry->length == 0) {pr_err(PREFIX "[0x%02x] Invalid zero length\n", entry_id);goto err;}entry = (struct acpi_subtable_header *)((unsigned long)entry + entry->length);- }
- if (max_entries && count > max_entries) {
pr_warn(PREFIX "[0x%02x] ignored %i entries of %i found\n",entry_id, count - max_entries, count);- }
- early_acpi_os_unmap_memory((char *)table_header, tbl_size);
- return count;
+err:
- early_acpi_os_unmap_memory((char *)table_header, tbl_size);
- return -EINVAL;
+} int __init acpi_table_parse_entries(char *id, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 372038a..7d5dba2 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -122,6 +122,10 @@ int acpi_numa_init (void); int acpi_table_init (void); int acpi_table_parse(char *id, acpi_tbl_table_handler handler); +int __init acpi_parse_entries(unsigned long table_size,
acpi_tbl_entry_handler handler,struct acpi_table_header *table_header,int entry_id, unsigned int max_entries);int __init acpi_table_parse_entries(char *id, unsigned long table_size, int entry_id, acpi_tbl_entry_handler handler, -- 1.8.3.2
Linaro-acpi mailing list Linaro-acpi@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-acpi
Hi Graeme,
On 19 March 2014 05:13, Graeme Gregory graeme.gregory@linaro.org wrote:
Hi Aswin,
This looks like a cleanup of upstream code, could you try and keep that seperate from patches to plat/arm-core.c
Its just that you should be attempting to push this to linux-acpi ASAP to avoid us building a backlog.
Also if they reject this then we don't want to internally build on top of it.
Thanks
Makes sense. I'll address the feedback and resend to linux-acpi.
Cheers, Ashwin
Once we get the table header pointer to the MADT in the callback from acpi_parse_table(), we can use it to probe the GIC and its distributors.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org --- drivers/acpi/plat/arm-core.c | 54 ++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 37 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 87e93b2..7a93fef 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -92,19 +92,6 @@ void __init __acpi_unmap_table(char *map, unsigned long size) return; }
-static int __init acpi_parse_madt(struct acpi_table_header *table) -{ - struct acpi_table_madt *madt; - - madt = (struct acpi_table_madt *)table; - if (!madt) { - pr_warn(PREFIX "Unable to get MADT\n"); - return -ENODEV; - } - - return 0; -} - /* * GIC structures on ARM are somthing like Local APIC structures on x86, * which means GIC cpu interfaces for GICv2/v3. Every GIC structure in @@ -146,20 +133,21 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, return 0; }
-/* - * Parse GIC cpu interface related entries in MADT - * returns 0 on success, < 0 on error - */ -static int __init acpi_parse_madt_gic_entries(void) +static int __init acpi_parse_madt(struct acpi_table_header *header) { + struct acpi_table_madt *madt; int count;
- /* - * do a partial walk of MADT to determine how many CPUs - * we have including disabled CPUs - */ - count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, - acpi_parse_gic, MAX_GIC_CPU_INTERFACE); + madt = (struct acpi_table_madt *)header; + if (!madt) { + pr_warn(PREFIX "Unable to get MADT\n"); + return -ENODEV; + } + + /* Parse MADT for GIC entries. */ + count = acpi_parse_entries(sizeof(struct acpi_table_madt), + acpi_parse_gic, header, ACPI_MADT_TYPE_GENERIC_INTERRUPT, + MAX_GIC_CPU_INTERFACE);
if (!count) { pr_err(PREFIX "No GIC entries present\n"); @@ -169,19 +157,11 @@ static int __init acpi_parse_madt_gic_entries(void) return count; }
- return 0; -} - -/* - * Parse GIC distributor related entries in MADT - * returns 0 on success, < 0 on error - */ -static int __init acpi_parse_madt_gic_distributor_entries(void) -{ - int count; - - count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, - acpi_parse_gic_distributor, MAX_GIC_DISTRIBUTOR); + /* Parse MADT for GIC Distributors. */ + count = acpi_parse_entries(sizeof(struct acpi_table_madt), + acpi_parse_gic_distributor, header, + ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, + MAX_GIC_DISTRIBUTOR);
if (!count) { pr_err(PREFIX "No GIC distributor entries present\n");
On 19.03.2014 04:51, Ashwin Chaugule wrote:
Once we get the table header pointer to the MADT in the callback from acpi_parse_table(), we can use it to probe the GIC and its distributors.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/acpi/plat/arm-core.c | 54 ++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 37 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 87e93b2..7a93fef 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -92,19 +92,6 @@ void __init __acpi_unmap_table(char *map, unsigned long size) return; }
-static int __init acpi_parse_madt(struct acpi_table_header *table) -{
- struct acpi_table_madt *madt;
- madt = (struct acpi_table_madt *)table;
- if (!madt) {
pr_warn(PREFIX "Unable to get MADT\n");return -ENODEV;- }
- return 0;
-}
- /*
- GIC structures on ARM are somthing like Local APIC structures on x86,
- which means GIC cpu interfaces for GICv2/v3. Every GIC structure in
@@ -146,20 +133,21 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, return 0; }
-/*
- Parse GIC cpu interface related entries in MADT
- returns 0 on success, < 0 on error
- */
-static int __init acpi_parse_madt_gic_entries(void) +static int __init acpi_parse_madt(struct acpi_table_header *header) {
- struct acpi_table_madt *madt; int count;
- /*
* do a partial walk of MADT to determine how many CPUs* we have including disabled CPUs*/- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
acpi_parse_gic, MAX_GIC_CPU_INTERFACE);
- madt = (struct acpi_table_madt *)header;
- if (!madt) {
Hanjun, are we going to use some of "struct acpi_table_madt" fields here? If not, we could just get rid of this casting and use "header" pointer directly.
Tomasz
pr_warn(PREFIX "Unable to get MADT\n");return -ENODEV;}
/* Parse MADT for GIC entries. */
count = acpi_parse_entries(sizeof(struct acpi_table_madt),
acpi_parse_gic, header, ACPI_MADT_TYPE_GENERIC_INTERRUPT,MAX_GIC_CPU_INTERFACE);if (!count) { pr_err(PREFIX "No GIC entries present\n");
@@ -169,19 +157,11 @@ static int __init acpi_parse_madt_gic_entries(void) return count; }
- return 0;
-}
-/*
- Parse GIC distributor related entries in MADT
- returns 0 on success, < 0 on error
- */
-static int __init acpi_parse_madt_gic_distributor_entries(void) -{
- int count;
- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
acpi_parse_gic_distributor, MAX_GIC_DISTRIBUTOR);
/* Parse MADT for GIC Distributors. */
count = acpi_parse_entries(sizeof(struct acpi_table_madt),
acpi_parse_gic_distributor, header,ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,MAX_GIC_DISTRIBUTOR);if (!count) { pr_err(PREFIX "No GIC distributor entries present\n");
On 2014-3-19 16:51, Tomasz Nowicki wrote:
On 19.03.2014 04:51, Ashwin Chaugule wrote:
Once we get the table header pointer to the MADT in the callback from acpi_parse_table(), we can use it to probe the GIC and its distributors.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/acpi/plat/arm-core.c | 54 ++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 37 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 87e93b2..7a93fef 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -92,19 +92,6 @@ void __init __acpi_unmap_table(char *map, unsigned long size) return; }
-static int __init acpi_parse_madt(struct acpi_table_header *table) -{
- struct acpi_table_madt *madt;
- madt = (struct acpi_table_madt *)table;
- if (!madt) {
pr_warn(PREFIX "Unable to get MADT\n");return -ENODEV;- }
- return 0;
-}
- /*
- GIC structures on ARM are somthing like Local APIC structures on x86,
- which means GIC cpu interfaces for GICv2/v3. Every GIC structure in
@@ -146,20 +133,21 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, return 0; }
-/*
- Parse GIC cpu interface related entries in MADT
- returns 0 on success, < 0 on error
- */
-static int __init acpi_parse_madt_gic_entries(void) +static int __init acpi_parse_madt(struct acpi_table_header *header) {
- struct acpi_table_madt *madt; int count;
- /*
* do a partial walk of MADT to determine how many CPUs* we have including disabled CPUs*/- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
acpi_parse_gic, MAX_GIC_CPU_INTERFACE);
- madt = (struct acpi_table_madt *)header;
- if (!madt) {
Hanjun, are we going to use some of "struct acpi_table_madt" fields here? If not, we could just get rid of this casting and use "header" pointer directly.
Yes, we are going to get the "Local Interrupt Controller Address" field here for GIC initialization.
Thanks Hanjun
On 20.03.2014 08:52, Hanjun Guo wrote:
On 2014-3-19 16:51, Tomasz Nowicki wrote:
On 19.03.2014 04:51, Ashwin Chaugule wrote:
Once we get the table header pointer to the MADT in the callback from acpi_parse_table(), we can use it to probe the GIC and its distributors.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/acpi/plat/arm-core.c | 54 ++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 37 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 87e93b2..7a93fef 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -92,19 +92,6 @@ void __init __acpi_unmap_table(char *map, unsigned long size) return; }
-static int __init acpi_parse_madt(struct acpi_table_header *table) -{
- struct acpi_table_madt *madt;
- madt = (struct acpi_table_madt *)table;
- if (!madt) {
pr_warn(PREFIX "Unable to get MADT\n");return -ENODEV;- }
- return 0;
-}
- /*
- GIC structures on ARM are somthing like Local APIC structures on x86,
- which means GIC cpu interfaces for GICv2/v3. Every GIC structure in
@@ -146,20 +133,21 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, return 0; }
-/*
- Parse GIC cpu interface related entries in MADT
- returns 0 on success, < 0 on error
- */
-static int __init acpi_parse_madt_gic_entries(void) +static int __init acpi_parse_madt(struct acpi_table_header *header) {
- struct acpi_table_madt *madt; int count;
- /*
* do a partial walk of MADT to determine how many CPUs* we have including disabled CPUs*/- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
acpi_parse_gic, MAX_GIC_CPU_INTERFACE);
- madt = (struct acpi_table_madt *)header;
- if (!madt) {
Hanjun, are we going to use some of "struct acpi_table_madt" fields here? If not, we could just get rid of this casting and use "header" pointer directly.
Yes, we are going to get the "Local Interrupt Controller Address" field here for GIC initialization.
"Local Interrupt Controller Address" will be used in separate file, e.g. acpi-gic.c right before GIC initialization. At least, that's how I am implementing it right now.
Tomasz
On 2014-3-19 11:51, Ashwin Chaugule wrote:
Add error checks to various methods and bail early if requisite tables are not found.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/acpi/plat/arm-core.c | 61 ++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 19 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 9704229..87e93b2 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -222,27 +222,40 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table) return 0; } -static void __init acpi_process_madt(void) +static int __init acpi_process_madt(void) {
- int error;
- if (!acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt)) {
/** Parse MADT GIC cpu interface entries*/error = acpi_parse_madt_gic_entries();if (!error) {/** Parse MADT GIC distributor entries*/acpi_parse_madt_gic_distributor_entries();}
- int err = 0;
- err = acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt);
- if (err) {
pr_err("Failed to parse MADT\n");goto out_err;- }
- /*
* Parse MADT GIC cpu interface entries*/- err = acpi_parse_madt_gic_entries();
- if (err) {
pr_err("Failed to parse GIC entries from MADT\n");goto out_err;- }
- /*
* Parse MADT GIC distributor entries*/- err = acpi_parse_madt_gic_distributor_entries();
- if (err) {
pr_err("Failed to find GIC Distributor entries from MADT\n");goto out_err;pr_info("Using ACPI for processor (GIC) configuration information\n");
- return 0;
- return;
+out_err:
- pr_err("Err processing GIC information\n");
- return err;
} /* @@ -273,20 +286,30 @@ void __init acpi_boot_table_init(void) int __init acpi_boot_init(void) {
- int err = 0; /*
*/ if (acpi_disabled) return -ENODEV;
- If acpi_disabled, bail out
- acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt);
- err = acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt);
- if (err) {
pr_err("Failed to find FADT\n");goto out_err;
I think we can't jump out and ignore the MADT table parsing here, the system may still run ok if we meet some error when we parsing FADT (for example we may get wrong information from the FADT because of BIOS bug), we can go on and get MADT parsed for SMP.
Others look good to me.
Thanks Hanjun
- }
/* * Process the Multiple APIC Description Table (MADT), if present */
- acpi_process_madt();
- err = acpi_process_madt();
- if (err) {
pr_err("Failed to process MADT\n");goto out_err;- }
- return 0;
+out_err:
- return err;
} static int __init parse_acpi(char *arg)
-
Ashwin Chaugule -
Graeme Gregory -
Hanjun Guo -
Tomasz Nowicki