Re: [PATCH 4/6] ACPICA: Add support for using logical addresses of GPE blocks

Matthieu Baerts
2020-10-16T14:31:00Z

Hi Rafael,

On 04/09/2020 19:24, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> The logical address of every GPE block in system memory must be
> known before passing it to acpi_ev_initialize_gpe_block(), because
> memory cannot be mapped on the fly from an interrupt handler.
> Accordingly, the host OS must map every GPE block in system
> memory upfront and it can store the logical addresses of GPE
> blocks for future use.

(...)

> diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c
> index a0e71f34c77a..37bb67ef3232 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -46,8 +46,13 @@ acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_gpe_address *reg)
> u32 value32;
>
> if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
> + *value = (u64)ACPI_GET8(reg->address);

Thank you for the patch!

When compiling net-next repo, recently sync with Linus repo, I got an
error when using i386 arch because of this line above.

Here are the commands I used:

================================================
$ make defconfig KBUILD_DEFCONFIG=i386_defconfig
*** Default configuration is based on 'i386_defconfig'
#
# configuration written to .config
#
$ scripts/config --disable DRM --disable PCCARD --disable ATA --disable
MD --disable PPS --disable SOUND --disable USB --disable IOMMU_SUPPORT
--disable INPUT_LEDS --disable AGP --disable VGA_ARB --disable EFI
--disable WLAN --disable WIRELESS --disable LOGO --disable NFS_FS
--disable XFRM_USER --disable INET6_AH --disable INET6_ESP --disable
NETDEVICES -e KUNIT -d KUNIT_DEBUGFS -d KUNIT_TEST -d KUNIT_EXAMPLE_TEST
-d EXT4_KUNIT_TESTS -d SYSCTL_KUNIT_TEST -d LIST_KUNIT_TEST -d
LINEAR_RANGES_TEST -d BITS_TEST -d KUNIT_ALL_TESTS -e INET_DIAG -d
INET_UDP_DIAG -d INET_RAW_DIAG -d INET_DIAG_DESTROY -e MPTCP -e
MPTCP_IPV6 -e MPTCP_KUNIT_TESTS
$ KCFLAGS=-Werror make -j8 -l8
scripts/kconfig/conf --syncconfig Kconfig
(...)
CC drivers/acpi/acpica/hwgpe.o
In file included from ./include/acpi/acpi.h:24,
from drivers/acpi/acpica/hwgpe.c:10:
drivers/acpi/acpica/hwgpe.c: In function 'acpi_hw_gpe_read':
./include/acpi/actypes.h:501:48: error: cast to pointer from integer of
different size [-Werror=int-to-pointer-cast]
501 | #define ACPI_CAST_PTR(t, p) ((t *) (acpi_uintptr_t)
(p))
| ^
drivers/acpi/acpica/acmacros.h:18:41: note: in expansion of macro
'ACPI_CAST_PTR'
18 | #define ACPI_CAST8(ptr) ACPI_CAST_PTR (u8, (ptr))
| ^~~~~~~~~~~~~
drivers/acpi/acpica/acmacros.h:22:43: note: in expansion of macro
'ACPI_CAST8'
22 | #define ACPI_GET8(ptr) (*ACPI_CAST8 (ptr))
| ^~~~~~~~~~
drivers/acpi/acpica/hwgpe.c:50:17: note: in expansion of macro 'ACPI_GET8'
50 | *value = (u64)ACPI_GET8(reg->address);
| ^~~~~~~~~
drivers/acpi/acpica/hwgpe.c: In function 'acpi_hw_gpe_write':
./include/acpi/actypes.h:501:48: error: cast to pointer from integer of
different size [-Werror=int-to-pointer-cast]
501 | #define ACPI_CAST_PTR(t, p) ((t *) (acpi_uintptr_t)
(p))
| ^
drivers/acpi/acpica/acmacros.h:18:41: note: in expansion of macro
'ACPI_CAST_PTR'
18 | #define ACPI_CAST8(ptr) ACPI_CAST_PTR (u8, (ptr))
| ^~~~~~~~~~~~~
drivers/acpi/acpica/acmacros.h:26:43: note: in expansion of macro
'ACPI_CAST8'
26 | #define ACPI_SET8(ptr, val) (*ACPI_CAST8 (ptr) =
(u8) (val))
| ^~~~~~~~~~
drivers/acpi/acpica/hwgpe.c:85:3: note: in expansion of macro 'ACPI_SET8'
85 | ACPI_SET8(reg->address, value);
| ^~~~~~~~~
cc1: all warnings being treated as errors
make[3]: *** [scripts/Makefile.build:283: drivers/acpi/acpica/hwgpe.o]
Error 1
make[2]: *** [scripts/Makefile.build:500: drivers/acpi/acpica] Error 2
make[1]: *** [scripts/Makefile.build:500: drivers/acpi] Error 2
make: *** [Makefile:1777: drivers] Error 2
================================================

> + return_ACPI_STATUS(AE_OK);
> +#else
> return acpi_os_read_memory((acpi_physical_address)reg->address,
> value, ACPI_GPE_REGISTER_WIDTH);
> +#endif
> }
>
> status = acpi_os_read_port((acpi_io_address)reg->address,
> @@ -76,8 +81,13 @@ acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_gpe_address *reg)
> acpi_status acpi_hw_gpe_write(u64 value, struct acpi_gpe_address *reg)
> {
> if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
> + ACPI_SET8(reg->address, value);

(and also because of this line)

By chance, do you already have a fix for that? I didn't see any other
email related to this issue, I am surprised no bot already reported the
problem but maybe I didn't look everywhere :)

Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions