| Message ID | 1453540813-15764-11-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 23.01.16 at 10:20, <zhaoshenglong@huawei.com> wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > This function could get the specified index entry of MADT table. This > would be useful when it needs to get the contens of the entry. > > Cc: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> Again - if this is the counterpart to a Linux side change, please name that change. If it isn't, besides needing a better explanation of why you need it whether neither Linux nor x86 Xen does is needed, and a few code issues would need to be addressed too: Pointer type function parameters should be constified as much as possible, use of plain int should be avoided unless you truly need a signed type, and ... > +struct acpi_subtable_header * __init > +acpi_table_get_entry_madt(enum acpi_madt_type id, unsigned int entry_index) > +{ > + struct acpi_table_header *table_header = NULL; > + > + acpi_get_table(ACPI_SIG_MADT, acpi_apic_instance, &table_header); > + if (!table_header) { ... you should use the return value of the function instead of pre- setting table_header to NULL and then checking whether it changed. Jan
On 2016/1/25 23:02, Jan Beulich wrote: >>>> On 23.01.16 at 10:20, <zhaoshenglong@huawei.com> wrote: >> > From: Shannon Zhao <shannon.zhao@linaro.org> >> > >> > This function could get the specified index entry of MADT table. This >> > would be useful when it needs to get the contens of the entry. >> > >> > Cc: Jan Beulich <jbeulich@suse.com> >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > Again - if this is the counterpart to a Linux side change, please name > that change. If it isn't, besides needing a better explanation of why > you need it whether neither Linux nor x86 Xen does is needed, This is not a Linux commit. The reason is that on ARM Xen must create MADT table for Dom0 and it needs to get the contents of the physical MADT entry to make up that for Dom0. > a few code issues would need to be addressed too: Pointer type > function parameters should be constified as much as possible, use of > plain int should be avoided unless you truly need a signed type, and > ... > >> > +struct acpi_subtable_header * __init >> > +acpi_table_get_entry_madt(enum acpi_madt_type id, unsigned int entry_index) >> > +{ >> > + struct acpi_table_header *table_header = NULL; >> > + >> > + acpi_get_table(ACPI_SIG_MADT, acpi_apic_instance, &table_header); >> > + if (!table_header) { > ... you should use the return value of the function instead of pre- > setting table_header to NULL and then checking whether it changed. Ok, will update. But looking the codes in this file, there are some places using this way.
diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c index 84a290f..c50bae4 100644 --- a/xen/drivers/acpi/tables.c +++ b/xen/drivers/acpi/tables.c @@ -219,6 +219,65 @@ void __init acpi_table_print_madt_entry(struct acpi_subtable_header *header) } } +static struct acpi_subtable_header * __init +acpi_get_entry(char *id, unsigned long table_size, + struct acpi_table_header *table_header, int entry_id, + unsigned int entry_index) +{ + struct acpi_subtable_header *entry; + int count = 0; + unsigned long table_end; + + if (!table_size) + return NULL; + + if (!table_header) { + printk(KERN_WARNING PREFIX "%4.4s not present\n", id); + return NULL; + } + + 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->length < sizeof(*entry)) { + printk(KERN_ERR PREFIX "[%4.4s:%#x] Invalid length\n", + id, entry_id); + return NULL; + } + + if (entry->type == entry_id) { + if (count == entry_index) + return entry; + count++; + } + + entry = (struct acpi_subtable_header *) + ((unsigned long)entry + entry->length); + } + + return NULL; +} + +struct acpi_subtable_header * __init +acpi_table_get_entry_madt(enum acpi_madt_type id, unsigned int entry_index) +{ + struct acpi_table_header *table_header = NULL; + + acpi_get_table(ACPI_SIG_MADT, acpi_apic_instance, &table_header); + if (!table_header) { + printk(KERN_WARNING PREFIX "%4.4s not present\n", + ACPI_SIG_MADT); + return NULL; + } + + return acpi_get_entry(ACPI_SIG_MADT, sizeof(struct acpi_table_madt), + table_header, id, entry_index); +} int __init acpi_parse_entries(char *id, unsigned long table_size, diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h index bc73310..32d7a4f 100644 --- a/xen/include/xen/acpi.h +++ b/xen/include/xen/acpi.h @@ -74,6 +74,8 @@ int acpi_parse_entries(char *id, unsigned long table_size, 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); +struct acpi_subtable_header *acpi_table_get_entry_madt(enum acpi_madt_type id, + unsigned int entry_index); int acpi_table_parse_madt(enum acpi_madt_type id, acpi_table_entry_handler handler, unsigned int max_entries); int acpi_table_parse_srat(int id, acpi_madt_entry_handler handler, unsigned int max_entries);