diff mbox

[v4,05/16] ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug

Message ID 1409192561-19744-6-git-send-email-jiang.liu@linux.intel.com
State Not Applicable
Headers show

Commit Message

Jiang Liu Aug. 28, 2014, 2:22 a.m. UTC 
From: Yinghai Lu <yinghai@kernel.org>

We need to parse APIC ID for IOAPIC registration for IOAPIC hotplug.
ACPI _MAT method and MADT table are used to figure out IOAPIC ID, just
like parsing CPU APIC ID for CPU hotplug.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/apic_id.c |   96 +++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/acpi.h   |    4 ++
 2 files changed, 98 insertions(+), 2 deletions(-)

Comments

Thomas Gleixner Sept. 9, 2014, 10:54 a.m. UTC | #1 
On Thu, 28 Aug 2014, Jiang Liu wrote:
>  
> +static struct acpi_table_madt *madt;
> +static int read_madt;

Pretty lousy file visible variable names.

So we end up with two copies of the butt ugly 

   if (!read_madt) {
      .....
   }

stuff instead of creating a helper function which hides that and do

      madr = get_madt();
      if (!madt)
		return ...;

at the call sites.

>  static int map_lapic_id(struct acpi_subtable_header *entry,
>  		 u32 acpi_id, int *apic_id)
>  {
> @@ -64,8 +71,6 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
>  static int map_madt_entry(int type, u32 acpi_id)
>  {
>  	unsigned long madt_end, entry;
> -	static struct acpi_table_madt *madt;
> -	static int read_madt;
>  	int apic_id = -1;
>  
>  	if (!read_madt) {
> @@ -200,3 +205,90 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
>  	return acpi_map_cpuid(apic_id, acpi_id);
>  }
>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
> +
> +#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
> +static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
> +			 u64 *phys_addr, int *ioapic_id)
> +{
> +	struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
> +
> +	if (ioapic->global_irq_base != gsi_base)
> +		return 0;
> +
> +	*phys_addr = ioapic->address;
> +	*ioapic_id = ioapic->id;
> +	return 1;
> +}
> +
> +static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
> +{
> +	struct acpi_subtable_header *hdr;
> +	unsigned long madt_end, entry;
> +	int apic_id = -1;
> +
> +	if (!read_madt) {
> +		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
> +					(struct acpi_table_header **)&madt)))
> +			madt = NULL;
> +		read_madt++;
> +	}
> +
> +	if (!madt)
> +		return apic_id;
> +
> +	entry = (unsigned long)madt;
> +	madt_end = entry + madt->header.length;
> +
> +	/* Parse all entries looking for a match. */
> +	entry += sizeof(struct acpi_table_madt);
> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
> +		hdr = (struct acpi_subtable_header *)entry;
> +		if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
> +		    map_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
> +			break;
> +		else
> +			entry += hdr->length;
> +	}
> +
> +	return apic_id;
> +}
> +
> +static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
> +				u64 *phys_addr)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	struct acpi_subtable_header *header;
> +	int apic_id = -1;
> +
> +	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
> +		goto exit;
> +
> +	if (!buffer.length || !buffer.pointer)
> +		goto exit;
> +
> +	obj = buffer.pointer;
> +	if (obj->type != ACPI_TYPE_BUFFER ||
> +	    obj->buffer.length < sizeof(struct acpi_subtable_header))
> +		goto exit;
> +
> +	header = (struct acpi_subtable_header *)obj->buffer.pointer;
> +	if (header->type == ACPI_MADT_TYPE_IO_APIC)
> +		map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
> +
> +exit:
> +	kfree(buffer.pointer);
> +	return apic_id;
> +}


This stuff really wants proper documentation. The lack of comments is
just annoying.

> +int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
> +{
> +	int apic_id;
> +
> +	apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
> +	if (apic_id == -1)
> +		apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
> +
> +	return apic_id;
> +}

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu Sept. 10, 2014, 1:58 a.m. UTC | #2 
On 2014/9/9 18:54, Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Jiang Liu wrote:
>>  
>> +static struct acpi_table_madt *madt;
>> +static int read_madt;
> 
> Pretty lousy file visible variable names.
> 
> So we end up with two copies of the butt ugly 
> 
>    if (!read_madt) {
>       .....
>    }
> 
> stuff instead of creating a helper function which hides that and do
> 
>       madr = get_madt();
>       if (!madt)
> 		return ...;
> 
> at the call sites.
Thanks for suggestion, will improve in that way.

> 
>>  static int map_lapic_id(struct acpi_subtable_header *entry,
>>  		 u32 acpi_id, int *apic_id)
>>  {
>> @@ -64,8 +71,6 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
>>  static int map_madt_entry(int type, u32 acpi_id)
>>  {
>>  	unsigned long madt_end, entry;
>> -	static struct acpi_table_madt *madt;
>> -	static int read_madt;
>>  	int apic_id = -1;
>>  
>>  	if (!read_madt) {
>> @@ -200,3 +205,90 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
>>  	return acpi_map_cpuid(apic_id, acpi_id);
>>  }
>>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>> +
>> +#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
>> +static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
>> +			 u64 *phys_addr, int *ioapic_id)
>> +{
>> +	struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
>> +
>> +	if (ioapic->global_irq_base != gsi_base)
>> +		return 0;
>> +
>> +	*phys_addr = ioapic->address;
>> +	*ioapic_id = ioapic->id;
>> +	return 1;
>> +}
>> +
>> +static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
>> +{
>> +	struct acpi_subtable_header *hdr;
>> +	unsigned long madt_end, entry;
>> +	int apic_id = -1;
>> +
>> +	if (!read_madt) {
>> +		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
>> +					(struct acpi_table_header **)&madt)))
>> +			madt = NULL;
>> +		read_madt++;
>> +	}
>> +
>> +	if (!madt)
>> +		return apic_id;
>> +
>> +	entry = (unsigned long)madt;
>> +	madt_end = entry + madt->header.length;
>> +
>> +	/* Parse all entries looking for a match. */
>> +	entry += sizeof(struct acpi_table_madt);
>> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
>> +		hdr = (struct acpi_subtable_header *)entry;
>> +		if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
>> +		    map_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
>> +			break;
>> +		else
>> +			entry += hdr->length;
>> +	}
>> +
>> +	return apic_id;
>> +}
>> +
>> +static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
>> +				u64 *phys_addr)
>> +{
>> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	union acpi_object *obj;
>> +	struct acpi_subtable_header *header;
>> +	int apic_id = -1;
>> +
>> +	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
>> +		goto exit;
>> +
>> +	if (!buffer.length || !buffer.pointer)
>> +		goto exit;
>> +
>> +	obj = buffer.pointer;
>> +	if (obj->type != ACPI_TYPE_BUFFER ||
>> +	    obj->buffer.length < sizeof(struct acpi_subtable_header))
>> +		goto exit;
>> +
>> +	header = (struct acpi_subtable_header *)obj->buffer.pointer;
>> +	if (header->type == ACPI_MADT_TYPE_IO_APIC)
>> +		map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
>> +
>> +exit:
>> +	kfree(buffer.pointer);
>> +	return apic_id;
>> +}
> 
> 
> This stuff really wants proper documentation. The lack of comments is
> just annoying.
Sure, will improve comments in next version.

Regards!
Gerry

> 
>> +int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
>> +{
>> +	int apic_id;
>> +
>> +	apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
>> +	if (apic_id == -1)
>> +		apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
>> +
>> +	return apic_id;
>> +}
> 
> Thanks,
> 
> 	tglx
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/apic_id.c b/drivers/acpi/apic_id.c
index ada5fd48bad4..ad2e2679c104 100644
--- a/drivers/acpi/apic_id.c
+++ b/drivers/acpi/apic_id.c
@@ -4,11 +4,18 @@ 
  *
  *	Alex Chiang <achiang@hp.com>
  *	- Unified x86/ia64 implementations
+ *
+ * I/O APIC hotplug support
+ *	Yinghai Lu <yinghai@kernel.org>
+ *	Jiang Liu <jiang.liu@intel.com>
  */
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include "internal.h"
 
+static struct acpi_table_madt *madt;
+static int read_madt;
+
 static int map_lapic_id(struct acpi_subtable_header *entry,
 		 u32 acpi_id, int *apic_id)
 {
@@ -64,8 +71,6 @@  static int map_lsapic_id(struct acpi_subtable_header *entry,
 static int map_madt_entry(int type, u32 acpi_id)
 {
 	unsigned long madt_end, entry;
-	static struct acpi_table_madt *madt;
-	static int read_madt;
 	int apic_id = -1;
 
 	if (!read_madt) {
@@ -200,3 +205,90 @@  int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
 	return acpi_map_cpuid(apic_id, acpi_id);
 }
 EXPORT_SYMBOL_GPL(acpi_get_cpuid);
+
+#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
+static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
+			 u64 *phys_addr, int *ioapic_id)
+{
+	struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
+
+	if (ioapic->global_irq_base != gsi_base)
+		return 0;
+
+	*phys_addr = ioapic->address;
+	*ioapic_id = ioapic->id;
+	return 1;
+}
+
+static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
+{
+	struct acpi_subtable_header *hdr;
+	unsigned long madt_end, entry;
+	int apic_id = -1;
+
+	if (!read_madt) {
+		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
+					(struct acpi_table_header **)&madt)))
+			madt = NULL;
+		read_madt++;
+	}
+
+	if (!madt)
+		return apic_id;
+
+	entry = (unsigned long)madt;
+	madt_end = entry + madt->header.length;
+
+	/* Parse all entries looking for a match. */
+	entry += sizeof(struct acpi_table_madt);
+	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
+		hdr = (struct acpi_subtable_header *)entry;
+		if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
+		    map_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
+			break;
+		else
+			entry += hdr->length;
+	}
+
+	return apic_id;
+}
+
+static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
+				u64 *phys_addr)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	struct acpi_subtable_header *header;
+	int apic_id = -1;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
+		goto exit;
+
+	if (!buffer.length || !buffer.pointer)
+		goto exit;
+
+	obj = buffer.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER ||
+	    obj->buffer.length < sizeof(struct acpi_subtable_header))
+		goto exit;
+
+	header = (struct acpi_subtable_header *)obj->buffer.pointer;
+	if (header->type == ACPI_MADT_TYPE_IO_APIC)
+		map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
+
+exit:
+	kfree(buffer.pointer);
+	return apic_id;
+}
+
+int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
+{
+	int apic_id;
+
+	apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
+	if (apic_id == -1)
+		apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
+
+	return apic_id;
+}
+#endif /* CONFIG_ACPI_HOTPLUG_IOAPIC */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 05ed6886f1f8..b41c5aef5336 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -149,6 +149,10 @@  int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu);
 int acpi_unmap_lsapic(int cpu);
 #endif /* CONFIG_ACPI_HOTPLUG_CPU */
 
+#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
+int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
+#endif
+
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base);
 int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base);
 void acpi_irq_stats_init(void);