[PATCH v6 12/15] ACPI: CPPC: Validate SystemIO register layouts

Christian Loehle <[email protected]>
Newsgroups org.kernel.vger.linux-acpi,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pm
Message-ID <[email protected]>
cpc_read() and cpc_write() access SystemIO registers using the complete GAS
access width. cpc_read() does not extract a partial field, while the writer
does not preserve bits outside one. A partial register can therefore return
the wrong value or clobber adjacent fields on write.

Retain valid read-only fields in 8-, 16-, and 32-bit access units. Extract
them after reading the complete port. Continue to require writable controls
to cover the complete access unit at Bit Offset zero. A partial write would
require a serialized RMW. Keep accepting Access Size zero when Bit Width
supplies one of the supported sizes. Require natural alignment on
architectures which implement port I/O through MMIO and may fault on
unaligned Device-memory accesses. Preserve port layouts on x86; its native
port-I/O instructions support them.

When CONFIG_HAS_IOPORT is disabled, report unavailable port-I/O support and
mark SystemIO layouts inaccessible during probe. Also return -EOPNOTSUPP
explicitly in cpc_read() and cpc_write() so a SystemIO entry can never fall
through and treat its port number as a physical-memory address.

Resolve inaccessible entries using the control-specific policy established
for PCC: optional fields can be disabled, while mandatory or semantically
required controls fail probe. Reject overlapping logical port ranges when
either entry is writable; read-only overlaps remain allowed.

Partial writable forms are permitted by ACPI, but never worked with the
existing whole-width Linux writer. Implementing them would require
field-aware I/O and appropriate RMW serialization.

Fixes: a2c8f92bea5f ("ACPI: CPPC: Implement support for SystemIO registers")
Signed-off-by: Christian Loehle <[email protected]>
---
 drivers/acpi/cppc_acpi.c | 68 ++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index d81ac477d184..beeae0a983b8 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -237,7 +237,6 @@ static bool cpc_integer_entry_valid(unsigned int reg_idx, u64 value)
  */
 #define NUM_RETRIES 500ULL
 
-#define OVER_16BTS_MASK ~0xFFFFULL
 #define CPC_GENERIC_REGISTER_DESCRIPTOR 0x82
 #define CPC_GENERIC_REGISTER_LENGTH (sizeof(struct cpc_reg) - 3)
 
@@ -1734,21 +1733,41 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 					cpc_ptr->cpc_regs[i - 2].sys_mem_vaddr = addr;
 				}
 			} else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
-				if (gas_t->access_width < 1 || gas_t->access_width > 3) {
-					/*
-					 * 1 = 8-bit, 2 = 16-bit, and 3 = 32-bit.
-					 * SystemIO doesn't implement 64-bit
-					 * registers.
-					 */
-					pr_debug("Invalid access width %d for SystemIO register in _CPC\n",
-						 gas_t->access_width);
-					goto out_free;
+				u64 access_size;
+				const char *reason = "uses unsupported SystemIO geometry";
+				unsigned int access_width;
+				bool unsupported;
+
+				access_width = cpc_reg_access_width(gas_t);
+				unsupported = !IS_ENABLED(CONFIG_HAS_IOPORT);
+				if (unsupported)
+					reason = "requires unavailable SystemIO support";
+				else
+					unsupported = access_width != 8 &&
+					      access_width != 16 &&
+					      access_width != 32;
+				if (!unsupported) {
+					access_size = access_width / 8;
+					unsupported = !gas_t->bit_width ||
+						gas_t->bit_width > access_width ||
+						gas_t->bit_offset >= access_width ||
+						gas_t->bit_width > access_width -
+									   gas_t->bit_offset;
 				}
-				if (gas_t->address & OVER_16BTS_MASK) {
-					/* SystemIO registers use 16-bit integer addresses */
-					pr_debug("Invalid IO port %llu for SystemIO register in _CPC\n",
-						 gas_t->address);
-					goto out_free;
+				if (!unsupported) {
+					unsupported = (cpc_reg_is_writable(i - 2) &&
+						(gas_t->bit_offset ||
+						 gas_t->bit_width != access_width)) ||
+						!cpc_reg_access_aligned(gas_t,
+									access_size) ||
+						gas_t->address >
+						U16_MAX - (access_size - 1);
+				}
+				if (unsupported) {
+					pr_debug("CPU%d: _CPC register %u %s\n",
+						 pr->id, i - 2, reason);
+					unsupported_regs |= BIT(i - 2);
+					continue;
 				}
 				if (!osc_cpc_flexible_adr_space_confirmed) {
 					pr_debug("Flexible address space capability not supported\n");
@@ -1836,6 +1855,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 					     "PCC");
 	if (ret)
 		goto out_free;
+	ret = cpc_validate_non_mmio_overlaps(cpc_ptr,
+					     ACPI_ADR_SPACE_SYSTEM_IO,
+					     "SystemIO");
+	if (ret)
+		goto out_free;
 
 	ret = cpc_validate_required_controls(cpc_ptr);
 	if (ret)
@@ -1981,11 +2005,13 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 	*val = 0;
 	size = GET_BIT_WIDTH(reg);
 
-	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
-	    reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 		u32 val_u32;
 		acpi_status status;
 
+		if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+			return -EOPNOTSUPP;
+
 		status = acpi_os_read_port((acpi_io_address)reg->address,
 					   &val_u32, size);
 		if (ACPI_FAILURE(status)) {
@@ -1994,7 +2020,7 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 			return -EFAULT;
 		}
 
-		*val = val_u32;
+		*val = MASK_VAL_READ(reg, val_u32);
 		return 0;
 	} else if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
 		if (pcc_ss_id < 0 || !pcc_data[pcc_ss_id])
@@ -2081,10 +2107,12 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 
 	size = GET_BIT_WIDTH(reg);
 
-	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
-	    reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 		acpi_status status;
 
+		if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+			return -EOPNOTSUPP;
+
 		status = acpi_os_write_port((acpi_io_address)reg->address,
 					    (u32)val, size);
 		if (ACPI_FAILURE(status)) {
-- 
2.34.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.