Hi All,
The underlying issue here is that in Linux calling
acpi_os_read_memory() or acpi_os_write_memory() from an interrupt
handler is generally invalid, because these functions may attempt
to map memory on the fly. It is only valid to call them from an
interrupt handler if it is known that there is a memory mapping
covering the physical address passed as the argument.
However, in that case using acpi_os_read_memory() or
acpi_os_write_memory() for accessing memory is inefficient, because
they need to look up the mapping in question every time in a global
list, and it would be much more straightforward to use the (known
already) logical address of the target memory region.
In ACPICA this problem affects GPE registers that are accessed
with the help of acpi_hw_read() and acpi_hw_write() which is
inefficient not just because they end up calling
acpi_os_read_memory() or acpi_os_write_memory() if the GPE
registers are located in system memory, but also because these
functions check things that need not be checked for GPE registers
in particular and they do that on every access.
This series of patches reworks the GPE register accesses in ACPICA
to be more efficient by omitting the unnecessary checks and making it
possible to use logical addresses directly if these registers are
located in system memory.
The first four patches modify ACPICA and the last two add the
requisite OS support to Linux on top of that.
Please refer to the changelogs of the patches for details.
Thanks,
Rafael
From: "Rafael J. Wysocki" <[email protected]>
Notice that the bit_width, bit_offset and access_width fields in
struct acpi_generic_address are not used during GPE register
accesses any more, so introduce a simplified address structure
type, struct acpi_gpe_address, to represent addresses of GPE
registers and use it instead of struct acpi_generic_address in
struct acpi_gpe_register_info.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/acpica/achware.h | 4 ++--
drivers/acpi/acpica/aclocal.h | 11 +++++++++--
drivers/acpi/acpica/evgpeblk.c | 6 ------
drivers/acpi/acpica/hwgpe.c | 8 ++++----
4 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/acpica/achware.h b/drivers/acpi/acpica/achware.h
index 4dba7229f9c1..6ab92e28330d 100644
--- a/drivers/acpi/acpica/achware.h
+++ b/drivers/acpi/acpica/achware.h
@@ -78,9 +78,9 @@ acpi_status acpi_hw_validate_io_block(u64 address, u32 bit_width, u32 count);
/*
* hwgpe - GPE support
*/
-acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_generic_address *reg);
+acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_gpe_address *reg);
-acpi_status acpi_hw_gpe_write(u64 value, struct acpi_generic_address *reg);
+acpi_status acpi_hw_gpe_write(u64 value, struct acpi_gpe_address *reg);
u32 acpi_hw_get_gpe_register_bit(struct acpi_gpe_event_info *gpe_event_info);
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h
index af58cd2dc9d3..f83b98fa13ac 100644
--- a/drivers/acpi/acpica/aclocal.h
+++ b/drivers/acpi/acpica/aclocal.h
@@ -454,11 +454,18 @@ struct acpi_gpe_event_info {
u8 disable_for_dispatch; /* Masked during dispatching */
};
+/* GPE register address */
+
+struct acpi_gpe_address {
+ u8 space_id; /* Address space where the register exists */
+ u64 address; /* 64-bit address of the register */
+};
+
/* Information about a GPE register pair, one per each status/enable pair in an array */
struct acpi_gpe_register_info {
- struct acpi_generic_address status_address; /* Address of status reg */
- struct acpi_generic_address enable_address; /* Address of enable reg */
+ struct acpi_gpe_address status_address; /* Address of status reg */
+ struct acpi_gpe_address enable_address; /* Address of enable reg */
u16 base_gpe_number; /* Base GPE number for this register */
u8 enable_for_wake; /* GPEs to keep enabled when sleeping */
u8 enable_for_run; /* GPEs to keep enabled when running */
diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c
index 150c916dca5e..f5298be4273a 100644
--- a/drivers/acpi/acpica/evgpeblk.c
+++ b/drivers/acpi/acpica/evgpeblk.c
@@ -233,12 +233,6 @@ acpi_ev_create_gpe_info_blocks(struct acpi_gpe_block_info *gpe_block)
this_register->status_address.space_id = gpe_block->space_id;
this_register->enable_address.space_id = gpe_block->space_id;
- this_register->status_address.bit_width =
- ACPI_GPE_REGISTER_WIDTH;
- this_register->enable_address.bit_width =
- ACPI_GPE_REGISTER_WIDTH;
- this_register->status_address.bit_offset = 0;
- this_register->enable_address.bit_offset = 0;
/* Init the event_info for each GPE within this register */
diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c
index 6cc88524839d..a0e71f34c77a 100644
--- a/drivers/acpi/acpica/hwgpe.c
+++ b/drivers/acpi/acpica/hwgpe.c
@@ -29,7 +29,7 @@ acpi_hw_gpe_enable_write(u8 enable_mask,
* FUNCTION: acpi_hw_gpe_read
*
* PARAMETERS: value - Where the value is returned
- * reg - GAS register structure
+ * reg - GPE register structure
*
* RETURN: Status
*
@@ -40,7 +40,7 @@ acpi_hw_gpe_enable_write(u8 enable_mask,
*
******************************************************************************/
-acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_generic_address *reg)
+acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_gpe_address *reg)
{
acpi_status status;
u32 value32;
@@ -65,7 +65,7 @@ acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_generic_address *reg)
* FUNCTION: acpi_hw_gpe_write
*
* PARAMETERS: value - Value to be written
- * reg - GAS register structure
+ * reg - GPE register structure
*
* RETURN: Status
*
@@ -73,7 +73,7 @@ acpi_status acpi_hw_gpe_read(u64 *value, struct acpi_generic_address *reg)
*
******************************************************************************/
-acpi_status acpi_hw_gpe_write(u64 value, struct acpi_generic_address *reg)
+acpi_status acpi_hw_gpe_write(u64 value, struct acpi_gpe_address *reg)
{
if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
return acpi_os_write_memory((acpi_physical_address)reg->address,
--
2.26.2
From: "Rafael J. Wysocki" <[email protected]>
Some of the checks done by acpi_hw_read() and acpi_hw_write(),
which are used for accessing GPE registers, are redundant in the
specific case of GPE registers and the ones that are not redundant
can be done upfront at the initialization time so as to fail the
initialization if they are not passed instead of failing every
access to the affected GPE registers going forward (including
accesses from the SCI interrupt handler).
Modify the GPE blocks initialization code accordingly.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/acpica/achware.h | 2 ++
drivers/acpi/acpica/evgpeblk.c | 17 +++++++++++++++++
drivers/acpi/acpica/hwvalid.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/drivers/acpi/acpica/achware.h b/drivers/acpi/acpica/achware.h
index ebf6453d0e21..f1f644b58b15 100644
--- a/drivers/acpi/acpica/achware.h
+++ b/drivers/acpi/acpica/achware.h
@@ -73,6 +73,8 @@ acpi_status acpi_hw_read_port(acpi_io_address address, u32 *value, u32 width);
acpi_status acpi_hw_write_port(acpi_io_address address, u32 value, u32 width);
+acpi_status acpi_hw_validate_io_block(u64 address, u32 bit_width, u32 count);
+
/*
* hwgpe - GPE support
*/
diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c
index 132adff1e131..eb5d98757fdc 100644
--- a/drivers/acpi/acpica/evgpeblk.c
+++ b/drivers/acpi/acpica/evgpeblk.c
@@ -317,6 +317,23 @@ acpi_ev_create_gpe_block(struct acpi_namespace_node *gpe_device,
return_ACPI_STATUS(AE_OK);
}
+ /* Validate the space_ID */
+
+ if ((space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY) &&
+ (space_id != ACPI_ADR_SPACE_SYSTEM_IO)) {
+ ACPI_ERROR((AE_INFO,
+ "Unsupported address space: 0x%X", space_id));
+ return_ACPI_STATUS(AE_SUPPORT);
+ }
+
+ if (space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+ status = acpi_hw_validate_io_block(address,
+ ACPI_GPE_REGISTER_WIDTH,
+ register_count);
+ if (ACPI_FAILURE(status))
+ return_ACPI_STATUS(status);
+ }
+
/* Allocate a new GPE block */
gpe_block = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_gpe_block_info));
diff --git a/drivers/acpi/acpica/hwvalid.c b/drivers/acpi/acpica/hwvalid.c
index 4d94861e6093..b2ca7dfd3fc9 100644
--- a/drivers/acpi/acpica/hwvalid.c
+++ b/drivers/acpi/acpica/hwvalid.c
@@ -292,3 +292,33 @@ acpi_status acpi_hw_write_port(acpi_io_address address, u32 value, u32 width)
return (AE_OK);
}
+
+/******************************************************************************
+ *
+ * FUNCTION: acpi_hw_validate_io_block
+ *
+ * PARAMETERS: Address Address of I/O port/register blobk
+ * bit_width Number of bits (8,16,32) in each register
+ * count Number of registers in the block
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Validates a block of I/O ports/registers.
+ *
+ ******************************************************************************/
+
+acpi_status acpi_hw_validate_io_block(u64 address, u32 bit_width, u32 count)
+{
+ acpi_status status;
+
+ while (count--) {
+ status = acpi_hw_validate_io_request((acpi_io_address)address,
+ bit_width);
+ if (ACPI_FAILURE(status))
+ return_ACPI_STATUS(status);
+
+ address += ACPI_DIV_8(bit_width);
+ }
+
+ return_ACPI_STATUS(AE_OK);
+}
--
2.26.2
From: "Rafael J. Wysocki" <[email protected]>
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.
If these logical addresses were known to ACPICA, it could use them
instead of the corresponding physical addresses of GPE block for
GPE register accesses and the memory mapping lookups carried out
by acpi_os_read_memory() and acpi_os_write_memory() on every
attempt to access a GPE register would not be necessary any more.
To allow that to happen, introduce the ACPI_GPE_USE_LOGICAL_ADDRESSES
symbol to indicate whether or not the host OS wants ACPICA to use the
logical addresses of GPE registers in system memory directly (which
is the case if this symbol is defined). Moreover, conditional on
whether ACPI_GPE_USE_LOGICAL_ADDRESSES is defined, introduce two new
global variables for storing the logical addresses of the FADT GPE
blocks 0 and 1, respectively, acpi_gbl_xgpe0_block_logical_address and
acpi_gbl_xgpe1_block_logical_address, make acpi_ev_gpe_initialize()
pass their values instead of the physical addresses of the GPE blocks
in question to acpi_ev_create_gpe_block() and modify
acpi_hw_gpe_read() and acpi_hw_gpe_write() to access memory directly
via the addresses stored in the struct acpi_gpe_address objects,
which are expected to be the logical addresses of GPE registers if
ACPI_GPE_USE_LOGICAL_ADDRESSES is defined.
With the above changes in place, a host OS wanting ACPICA to
access GPE registers directly through their logical addresses
needs to define the ACPI_GPE_USE_LOGICAL_ADDRESSES symbol and
make sure that the logical addresses of the FADT GPE blocks 0
and 1 are stored in acpi_gbl_xgpe0_block_logical_address and
acpi_gbl_xgpe1_block_logical_address, respectively, prior to
calling acpi_ev_gpe_initialize().
[If such a host OS also uses acpi_install_gpe_block() to add
non-FADT GPE register blocks located in system memory, it must
pass their logical addresses instead of their physical addresses
to this function.]
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/acpica/acglobal.h | 6 ++++++
drivers/acpi/acpica/evgpeinit.c | 23 +++++++++++++++++------
drivers/acpi/acpica/hwgpe.c | 10 ++++++++++
3 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index 1030a0ce1599..5215ff1cbbbe 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -42,6 +42,12 @@ ACPI_GLOBAL(struct acpi_generic_address, acpi_gbl_xpm1a_enable);
ACPI_GLOBAL(struct acpi_generic_address, acpi_gbl_xpm1b_status);
ACPI_GLOBAL(struct acpi_generic_address, acpi_gbl_xpm1b_enable);
+#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
+ACPI_GLOBAL(void *, acpi_gbl_xgpe0_block_logical_address);
+ACPI_GLOBAL(void *, acpi_gbl_xgpe1_block_logical_address);
+
+#endif /* ACPI_GPE_USE_LOGICAL_ADDRESSES */
+
/*
* Handle both ACPI 1.0 and ACPI 2.0+ Integer widths. The integer width is
* determined by the revision of the DSDT: If the DSDT revision is less than
diff --git a/drivers/acpi/acpica/evgpeinit.c b/drivers/acpi/acpica/evgpeinit.c
index 6effd8076dcc..6d82d30d8f7b 100644
--- a/drivers/acpi/acpica/evgpeinit.c
+++ b/drivers/acpi/acpica/evgpeinit.c
@@ -32,6 +32,16 @@ ACPI_MODULE_NAME("evgpeinit")
* kernel boot time as well.
*/
+#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
+#define ACPI_FADT_GPE_BLOCK_ADDRESS(N) \
+ acpi_gbl_FADT.xgpe##N##_block.space_id == \
+ ACPI_ADR_SPACE_SYSTEM_MEMORY ? \
+ (u64)acpi_gbl_xgpe##N##_block_logical_address : \
+ acpi_gbl_FADT.xgpe##N##_block.address
+#else
+#define ACPI_FADT_GPE_BLOCK_ADDRESS(N) acpi_gbl_FADT.xgpe##N##_block.address
+#endif /* ACPI_GPE_USE_LOGICAL_ADDRESSES */
+
/*******************************************************************************
*
* FUNCTION: acpi_ev_gpe_initialize
@@ -49,6 +59,7 @@ acpi_status acpi_ev_gpe_initialize(void)
u32 register_count1 = 0;
u32 gpe_number_max = 0;
acpi_status status;
+ u64 address;
ACPI_FUNCTION_TRACE(ev_gpe_initialize);
@@ -85,8 +96,9 @@ acpi_status acpi_ev_gpe_initialize(void)
* If EITHER the register length OR the block address are zero, then that
* particular block is not supported.
*/
- if (acpi_gbl_FADT.gpe0_block_length &&
- acpi_gbl_FADT.xgpe0_block.address) {
+ address = ACPI_FADT_GPE_BLOCK_ADDRESS(0);
+
+ if (acpi_gbl_FADT.gpe0_block_length && address) {
/* GPE block 0 exists (has both length and address > 0) */
@@ -97,7 +109,6 @@ acpi_status acpi_ev_gpe_initialize(void)
/* Install GPE Block 0 */
status = acpi_ev_create_gpe_block(acpi_gbl_fadt_gpe_device,
- acpi_gbl_FADT.xgpe0_block.
address,
acpi_gbl_FADT.xgpe0_block.
space_id, register_count0, 0,
@@ -110,8 +121,9 @@ acpi_status acpi_ev_gpe_initialize(void)
}
}
- if (acpi_gbl_FADT.gpe1_block_length &&
- acpi_gbl_FADT.xgpe1_block.address) {
+ address = ACPI_FADT_GPE_BLOCK_ADDRESS(1);
+
+ if (acpi_gbl_FADT.gpe1_block_length && address) {
/* GPE block 1 exists (has both length and address > 0) */
@@ -137,7 +149,6 @@ acpi_status acpi_ev_gpe_initialize(void)
status =
acpi_ev_create_gpe_block(acpi_gbl_fadt_gpe_device,
- acpi_gbl_FADT.xgpe1_block.
address,
acpi_gbl_FADT.xgpe1_block.
space_id, register_count1,
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);
+ 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);
+ return_ACPI_STATUS(AE_OK);
+#else
return acpi_os_write_memory((acpi_physical_address)reg->address,
value, ACPI_GPE_REGISTER_WIDTH);
+#endif
}
return acpi_os_write_port((acpi_io_address)reg->address, (u32)value,
--
2.26.2
From: "Rafael J. Wysocki" <[email protected]>
Modify acpi_os_map_generic_address() to return the pointer returned
by acpi_os_map_iomem() which represents the logical address
corresponding to the struct acpi_generic_address argument passed to
it or NULL if that address cannot be obtained (for example, the
argument does not represent an address in system memory or it could
not be mapped by the OS).
Among other things, that will allow the ACPI OS layer to pass the
logical addresses of the FADT GPE blocks 0 and 1 to ACPICA going
forward.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/apei/apei-base.c | 6 +++++-
drivers/acpi/osl.c | 18 +++++++-----------
include/acpi/acpi_io.h | 2 +-
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index e358d0046494..552fd9ffaca4 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -632,7 +632,11 @@ int apei_map_generic_address(struct acpi_generic_address *reg)
rc = apei_check_gar(reg, &address, &access_bit_width);
if (rc)
return rc;
- return acpi_os_map_generic_address(reg);
+
+ if (!acpi_os_map_generic_address(reg))
+ return -ENXIO;
+
+ return 0;
}
EXPORT_SYMBOL_GPL(apei_map_generic_address);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 4a0b07792233..3a50d8fa310b 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -447,24 +447,19 @@ void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
}
EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
-int acpi_os_map_generic_address(struct acpi_generic_address *gas)
+void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
{
u64 addr;
- void __iomem *virt;
if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
- return 0;
+ return NULL;
/* Handle possible alignment issues */
memcpy(&addr, &gas->address, sizeof(addr));
if (!addr || !gas->bit_width)
- return -EINVAL;
-
- virt = acpi_os_map_iomem(addr, gas->bit_width / 8);
- if (!virt)
- return -EIO;
+ return NULL;
- return 0;
+ return acpi_os_map_iomem(addr, gas->bit_width / 8);
}
EXPORT_SYMBOL(acpi_os_map_generic_address);
@@ -1756,10 +1751,11 @@ acpi_status __init acpi_os_initialize(void)
* Use acpi_os_map_generic_address to pre-map the reset
* register if it's in system memory.
*/
- int rv;
+ void *rv;
rv = acpi_os_map_generic_address(&acpi_gbl_FADT.reset_register);
- pr_debug(PREFIX "%s: map reset_reg status %d\n", __func__, rv);
+ pr_debug(PREFIX "%s: map reset_reg %s\n", __func__,
+ rv ? "successful" : "failed");
}
acpi_os_initialized = true;
diff --git a/include/acpi/acpi_io.h b/include/acpi/acpi_io.h
index 12d8bd333fe7..027faa8883aa 100644
--- a/include/acpi/acpi_io.h
+++ b/include/acpi/acpi_io.h
@@ -21,7 +21,7 @@ void __iomem __ref
void __ref acpi_os_unmap_iomem(void __iomem *virt, acpi_size size);
void __iomem *acpi_os_get_iomem(acpi_physical_address phys, unsigned int size);
-int acpi_os_map_generic_address(struct acpi_generic_address *addr);
+void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *addr);
void acpi_os_unmap_generic_address(struct acpi_generic_address *addr);
#endif
--
2.26.2
Hi Rafael,
On 04/09/2020 19:24, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <[email protected]>
>
> 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
http://www.tessares.net
Hi Rafael,
On 16/10/2020 19:15, Rafael J. Wysocki wrote:
> On Friday, October 16, 2020 4:30:55 PM CEST Matthieu Baerts wrote:
>>
>> By chance, do you already have a fix for that?
>
> Can you please try the appended patch?
Thank you for the patch, this fixes the compilation warning I got.
Reported-and-tested-by: Matthieu Baerts <[email protected]>
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net
On Friday, October 16, 2020 4:30:55 PM CEST Matthieu Baerts wrote:
> Hi Rafael,
Hi,
> On 04/09/2020 19:24, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <[email protected]>
> >
> > 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
This is what causes the build to terminate.
> 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?
Can you please try the appended patch?
> 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 :)
No, they didn't AFAICS.
---
drivers/acpi/acpica/hwgpe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/acpi/acpica/hwgpe.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/hwgpe.c
+++ linux-pm/drivers/acpi/acpica/hwgpe.c
@@ -47,7 +47,7 @@ acpi_status acpi_hw_gpe_read(u64 *value,
if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
- *value = (u64)ACPI_GET8(reg->address);
+ *value = (u64)ACPI_GET8((unsigned long)reg->address);
return_ACPI_STATUS(AE_OK);
#else
return acpi_os_read_memory((acpi_physical_address)reg->address,
@@ -82,7 +82,7 @@ acpi_status acpi_hw_gpe_write(u64 value,
{
if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
#ifdef ACPI_GPE_USE_LOGICAL_ADDRESSES
- ACPI_SET8(reg->address, value);
+ ACPI_SET8((unsigned long)reg->address, value);
return_ACPI_STATUS(AE_OK);
#else
return acpi_os_write_memory((acpi_physical_address)reg->address,