Re: [Xen-devel] [PATCH v5 10/22] acpi/table: Introduce acpi_table_get_entry_madt to get specified entry

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.02.16 at 07:22, <zhaoshenglong@xxxxxxxxxx> wrote:
> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> 
> 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@xxxxxxxx>
> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> ---
> V5: address Jan's comments
> V4: Fix coding style and make the function only for MADT table
> ---
>  xen/drivers/acpi/tables.c | 62 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/xen/acpi.h    |  2 ++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
> index f81c369..fa4e666 100644
> --- a/xen/drivers/acpi/tables.c
> +++ b/xen/drivers/acpi/tables.c
> @@ -221,6 +221,68 @@ void __init acpi_table_print_madt_entry(struct 
> acpi_subtable_header *header)
>       }
>  }
>  
> +static struct acpi_subtable_header * __init
> +acpi_get_entry(const char *id, unsigned long table_size,
> +            const struct acpi_table_header *table_header,
> +            enum acpi_madt_type 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) <

Isn't this simply (unsigned long)(entry + 1)?

> +            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);

While a cast indeed is unavoidable here, this could also be simplified to

                entry = (void *)entry + entry->length;

afaict. Same, as I see only now, goes for the calculation right
ahead of the loop.

> +struct acpi_subtable_header * __init
> +acpi_table_get_entry_madt(enum acpi_madt_type entry_id,
> +                       unsigned int entry_index)
> +{
> +     struct acpi_table_header *table_header = NULL;

Afaict the initializer is not needed, since ...

> +     acpi_status status;
> +
> +     status = acpi_get_table(ACPI_SIG_MADT, acpi_apic_instance,
> +                             &table_header);
> +     if (ACPI_FAILURE(status)) {

... you properly check the status returned.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel