linux-kernel - RE: [PATCH v3 5/6] ACPI/SFI: Fix wrong <acpi/acpi.h> inclusion in SFI/ACPI wrapper

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:	Fri, 6 Dec 2013 14:53:00 +0000
From:	"Zheng, Lv" <lv.zheng@...el.com>
To:	"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
	"Brown, Len" <len.brown@...el.com>
CC:	Lv Zheng <zetalog@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Len Brown <lenb@...nel.org>,
	"Tang, Feng" <feng.tang@...el.com>,
	"sfi-devel@...plefirmware.org" <sfi-devel@...plefirmware.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Subject: RE: [PATCH v3 5/6] ACPI/SFI: Fix wrong <acpi/acpi.h> inclusion in
 SFI/ACPI wrapper - acpi_disabled linkage.

Hi,

> From: Zheng, Lv
> Sent: Friday, December 06, 2013 4:52 PM
> To: Wysocki, Rafael J; Brown, Len
> 
> In Linux kernel, ACPICA is wrapped and safely exported by CONFIG_ACPI.  So
> all external modules should depend on CONFIG_ACPI rather than using ACPICA
> header directly for stubbing.  But if we moves <acpi/acpi.h> inclusions
> into "#ifdef CONFIG_ACPI", build breakge can help to detect wrong ACPICA
> dependent modules.
> 
> One of the build breakage is:
> arch/x86/pci/mmconfig-shared.c:389:1: error: unknown type name 'acpi_status'
> arch/x86/pci/mmconfig-shared.c:389:47: warning: 'struct acpi_resource' declared inside parameter list [enabled by default]
> arch/x86/pci/mmconfig-shared.c:389:47: warning: its scope is only this definition or declaration, which is probably not what you want
> [enabled by default]
> arch/x86/pci/mmconfig-shared.c: In function 'check_mcfg_resource':
> arch/x86/pci/mmconfig-shared.c:392:33: error: storage size of 'address' isn't known
> arch/x86/pci/mmconfig-shared.c:393:2: error: unknown type name 'acpi_status'
> arch/x86/pci/mmconfig-shared.c:395:9: error: dereferencing pointer to incomplete type
> arch/x86/pci/mmconfig-shared.c:395:19: error: 'ACPI_RESOURCE_TYPE_FIXED_MEMORY32' undeclared (first use in this function)
> arch/x86/pci/mmconfig-shared.c:395:19: note: each undeclared identifier is reported only once for each function it appears in
> arch/x86/pci/mmconfig-shared.c:397:8: error: dereferencing pointer to incomplete type
> arch/x86/pci/mmconfig-shared.c:399:11: error: 'AE_OK' undeclared (first use in this function)
> arch/x86/pci/mmconfig-shared.c:400:35: error: dereferencing pointer to incomplete type
> arch/x86/pci/mmconfig-shared.c:401:33: error: dereferencing pointer to incomplete type
> arch/x86/pci/mmconfig-shared.c:402:19: error: dereferencing pointer to incomplete type
> arch/x86/pci/mmconfig-shared.c:404:11: error: 'AE_CTRL_TERMINATE' undeclared (first use in this function)
> arch/x86/pci/mmconfig-shared.c:407:10: error: dereferencing pointer to incomplete type
> arch/x86/pci/mmconfig-shared.c:407:20: error: 'ACPI_RESOURCE_TYPE_ADDRESS32' undeclared (first use in this function)
> arch/x86/pci/mmconfig-shared.c:408:10: error: dereferencing pointer to incomplete type
> arch/x86/pci/mmconfig-shared.c:408:20: error: 'ACPI_RESOURCE_TYPE_ADDRESS64' undeclared (first use in this function)
> arch/x86/pci/mmconfig-shared.c:411:2: error: implicit declaration of function 'acpi_resource_to_address64' [-Werror=implicit-
> function-declaration]
> arch/x86/pci/mmconfig-shared.c:412:2: error: implicit declaration of function 'ACPI_FAILURE' [-Werror=implicit-function-declaration]
> arch/x86/pci/mmconfig-shared.c:414:31: error: 'ACPI_MEMORY_RANGE' undeclared (first use in this function)
> arch/x86/pci/mmconfig-shared.c:392:33: warning: unused variable 'address' [-Wunused-variable]
> arch/x86/pci/mmconfig-shared.c: At top level:
> arch/x86/pci/mmconfig-shared.c:425:1: error: unknown type name 'acpi_status'
> arch/x86/pci/mmconfig-shared.c:425:41: error: unknown type name 'acpi_handle'
> arch/x86/pci/mmconfig-shared.c: In function 'is_acpi_reserved':
> arch/x86/pci/mmconfig-shared.c:447:2: error: implicit declaration of function 'acpi_get_devices' [-Werror=implicit-function-
> declaration]
> arch/x86/pci/mmconfig-shared.c:447:30: error: 'find_mboard_resource' undeclared (first use in this function)
> arch/x86/pci/mmconfig-shared.c: At top level:
> arch/x86/pci/mmconfig-shared.c:389:20: warning: 'check_mcfg_resource' defined but not used [-Wunused-function]

Let me say something about this error messages.  Someone may face difficulties to reproduce it.
Currently, this message cannot be seen in any kernel build.
This is because this patch series doesn't include a fix to remove <acpi/acpi.h> inclusion from linux/sfi_acpi.h.
To achieve this, I have offered 2 options:
1. Redefining ACPICA table stuff for CONFIG_ACPI=n environment, or
2. Implementing stubs for all ACPICA functions/globals/macros and introducing <linux/acpica.h> (where acpi/acpi.h is included) as a top level header.
3. Other solutions...

If we choose solution 1, then this issue is exposed by a build test where CONFIG_ACPI=n and CONFIG_SFI=y.
Since we haven't decided what to do for the table structures and it is really not a good approach to achieve the build safety for mmconfig-share.c by relying on link-stage optimization rather than compile-stage referencing.  IMO, we need to sort it out, thus I make it a part of this patchset.

Well, if we choose solution 2, this patch can only help to reduce the size of the kernel image.

Thanks and best regards
-Lv

> 
> The root cause of this breakage is:
> 1. The following commit doesn't protect ACPICA functions like
>    acpi_get_devices()/acpi_walk_resources()/acpi_resource_to_address64() in
>    CONFIG_SFI=y and CONFIG_ACPI=n environment:
>     Commit: 5f0db7a2fb78895a197f64e548333b3bbd433996
>     Author: Feng Tang <feng.tang@...el.com>
>     Subject: SFI: Hook PCI MMCONFIG
>     First check ACPI, and if that fails, ask SFI to find the MCFG.
>    So it actually depends on the logic that in !CONFIG_ACPI builds, code
>    blocks under "if (!acpi_disabled)" will not be linked in.
> 
> This patch fixes this issue by introducing a stub for MCFG entry checker
> is_acpi_reserved() in !CONFIG_ACPI builds.
> 
> Signed-off-by: Lv Zheng <lv.zheng@...el.com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: Len Brown <lenb@...nel.org>
> Cc: Feng Tang <feng.tang@...el.com>
> Cc: sfi-devel@...plefirmware.org
> Cc: linux-pci@...r.kernel.org
> ---
>  arch/x86/pci/mmconfig-shared.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index 248642f..2c04b22 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -369,6 +369,7 @@ static int __init pci_mmcfg_check_hostbridge(void)
>  	return !list_empty(&pci_mmcfg_list);
>  }
> 
> +#ifdef CONFIG_ACPI
>  static acpi_status check_mcfg_resource(struct acpi_resource *res, void *data)
>  {
>  	struct resource *mcfg_res = data;
> @@ -435,6 +436,12 @@ static int is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> 
>  	return mcfg_res.flags;
>  }
> +#else
> +static inline int is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> +{
> +	return 0;
> +}
> +#endif
> 
>  typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
> 
> --
> 1.7.10

--
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/