| Message ID | 1423058539-26403-14-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Stefano, On 05/02/2015 18:29, Stefano Stabellini wrote: > On Wed, 4 Feb 2015, parth.dixit@linaro.org wrote: >> From: Naresh Bhat <naresh.bhat@linaro.org> >> >> Introduce acpi_parse_entries >> >> Signed-off-by: Naresh Bhat <Naresh.Bhat@linaro.org> >> --- >> xen/drivers/acpi/tables.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++ >> xen/include/xen/acpi.h | 4 +++ >> 2 files changed, 68 insertions(+) >> >> diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c >> index 6754933..5314f0b 100644 >> --- a/xen/drivers/acpi/tables.c >> +++ b/xen/drivers/acpi/tables.c >> @@ -239,6 +239,70 @@ void __init acpi_table_print_madt_entry(struct acpi_subtable_header *header) >> >> >> int __init >> +acpi_parse_entries(unsigned long table_size, >> + acpi_table_entry_handler handler, >> + struct acpi_table_header *table_header, >> + int entry_id, unsigned int max_entries) >> +{ >> + struct acpi_subtable_header *entry; >> + int count = 0; >> + unsigned long table_end; >> + >> + if (acpi_disabled) >> + return -ENODEV; >> + >> + if (!handler) >> + return -EINVAL; >> + >> + if (!table_size) >> + return -EINVAL; >> + >> + if (!table_header) { >> + printk("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)) { >> + count = -EINVAL; >> + goto err; >> + } >> + >> + /* >> + * If entry->length is 0, break from this loop to avoid >> + * infinite loop. >> + */ >> + if (entry->length == 0) { >> + printk("[0x%02x] Invalid zero length\n", entry_id); >> + count = -EINVAL; >> + goto err; >> + } >> + >> + entry = (struct acpi_subtable_header *) >> + ((unsigned long)entry + entry->length); >> + } >> + >> + if (max_entries && count > max_entries) { >> + printk("[0x%02x] ignored %i entries of %i found\n", >> + entry_id, count - max_entries, count); >> + } >> + >> +err: >> + return count; >> +} > > This function looks remarkably similar to acpi_table_parse_entries > below. > Could you use acpi_table_parse_entries? If not, why? I suspect that we don't even need to refactor the function acpi_table_parse_entries. The function acpi_parse_entries in only used for the GICv2 ACPI initialization. The current code in the GICv2 is manually doing the work of the acpi_table_parse_entries. All just for printing the Local ACPI address, we don't even store it... If we call acpi_parse_madt, it will make the code more readable by dropping nearly 30 lines of code, and also dropping this patch. Regards,
diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c index 6754933..5314f0b 100644 --- a/xen/drivers/acpi/tables.c +++ b/xen/drivers/acpi/tables.c @@ -239,6 +239,70 @@ void __init acpi_table_print_madt_entry(struct acpi_subtable_header *header) int __init +acpi_parse_entries(unsigned long table_size, + acpi_table_entry_handler handler, + struct acpi_table_header *table_header, + int entry_id, unsigned int max_entries) +{ + struct acpi_subtable_header *entry; + int count = 0; + unsigned long table_end; + + if (acpi_disabled) + return -ENODEV; + + if (!handler) + return -EINVAL; + + if (!table_size) + return -EINVAL; + + if (!table_header) { + printk("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)) { + count = -EINVAL; + goto err; + } + + /* + * If entry->length is 0, break from this loop to avoid + * infinite loop. + */ + if (entry->length == 0) { + printk("[0x%02x] Invalid zero length\n", entry_id); + count = -EINVAL; + goto err; + } + + entry = (struct acpi_subtable_header *) + ((unsigned long)entry + entry->length); + } + + if (max_entries && count > max_entries) { + printk("[0x%02x] ignored %i entries of %i found\n", + entry_id, count - max_entries, count); + } + +err: + return count; +} + + +int __init acpi_table_parse_entries(char *id, unsigned long table_size, int entry_id, diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h index 9387b36..bf60334 100644 --- a/xen/include/xen/acpi.h +++ b/xen/include/xen/acpi.h @@ -81,6 +81,10 @@ void acpi_hest_init(void); int acpi_table_init (void); int acpi_table_parse(char *id, acpi_table_handler handler); +int __init acpi_parse_entries(unsigned long table_size, + acpi_table_entry_handler handler, + struct acpi_table_header *table_header, + int entry_id, unsigned int max_entries); int acpi_table_parse_entries(char *id, unsigned long table_size, int entry_id, acpi_table_entry_handler handler, unsigned int max_entries); int acpi_table_parse_madt(enum acpi_madt_type id, acpi_table_entry_handler handler, unsigned int max_entries);