[v4,10/21] acpi/table: Introduce acpi_table_get_entry_madt to get specified entry
diff mbox

Message ID 1453540813-15764-11-git-send-email-zhaoshenglong@huawei.com
State New, archived
Headers show

Commit Message

Shannon Zhao Jan. 23, 2016, 9:20 a.m. UTC
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>
---
V4: Fix coding style and make the function only for MADT table
---
 xen/drivers/acpi/tables.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/acpi.h    |  2 ++
 2 files changed, 61 insertions(+)

Comments

Jan Beulich Jan. 25, 2016, 3:02 p.m. UTC | #1
>>> 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
Shannon Zhao Jan. 29, 2016, 7:49 a.m. UTC | #2
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.

Patch
diff mbox

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