linux-kernel - Re: [Patch v4 05/16] ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 9 Sep 2014 12:54:55 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Jiang Liu <jiang.liu@...ux.intel.com>
cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Yinghai Lu <yinghai@...nel.org>,
	Borislav Petkov <bp@...en8.de>,
	Grant Likely <grant.likely@...aro.org>,
	Len Brown <lenb@...nel.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tony Luck <tony.luck@...el.com>,
	Joerg Roedel <joro@...tes.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	x86@...nel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: Re: [Patch v4 05/16] ACPI: Add interfaces to parse IOAPIC ID for
 IOAPIC hotplug

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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/