ACPI: CPPC: Validate SystemIO register layouts

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. Accept 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. System I/O may use the same numeric port for
different read and write registers, but a direction-dependent alias is safe
only when the control is write-only. A readable control such as Minimum
Performance would otherwise return the read-side capability's value, which
cppc-cpufreq can then write back as its initial performance limit. Reject
such aliases and overlapping writers; retain read-only overlaps and aliases
between a read-only entry and a revision-4 write-only control.

Remove the EPP setter's duplicate SystemIO geometry preflight now that
probe validates every published writable control.

Fixes: a2c8f92bea5f ("ACPI: CPPC: Implement support for SystemIO registers")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
Tested-by: Sumit Gupta <sumitg@nvidia.com>
Link: https://patch.msgid.link/20260916162805.1039247-12-christian.loehle@arm.com
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

1 files changed, 66 insertions, 44 deletions

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index fe1d109afbd5..10cf96236dfb 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -244,7 +244,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)
 
@@ -297,23 +296,6 @@ show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time);
 	((((val) & GENMASK(((reg)->bit_width) - 1, 0)) << (reg)->bit_offset) |		\
 	((prev_val) & ~(GENMASK(((reg)->bit_width) - 1, 0) << (reg)->bit_offset)))	\
 
-static bool
-cpc_system_io_write_supported(const struct cpc_register_resource *reg)
-{
-	const struct cpc_reg *gas = &reg->cpc_entry.reg;
-	unsigned int access_width = GET_BIT_WIDTH(gas);
-	u64 access_size;
-
-	if (!IS_ENABLED(CONFIG_HAS_IOPORT) || !CPC_IN_SYSTEM_IO(reg) ||
-	    (access_width != 8 && access_width != 16 && access_width != 32))
-		return false;
-
-	access_size = access_width / 8;
-	return !gas->bit_offset && gas->bit_width == access_width &&
-	       gas->address <= U16_MAX - (access_size - 1) &&
-	       (IS_ENABLED(CONFIG_X86) || IS_ALIGNED(gas->address, access_size));
-}
-
 static unsigned int cpc_reg_access_width(const struct cpc_reg *reg)
 {
 	if (reg->access_width > 4)
@@ -610,6 +592,17 @@ static u64 cpc_non_mmio_access_size(const struct cpc_register_resource *reg)
 	return cpc_reg_access_width(gas) / 8;
 }
 
+static bool cpc_non_mmio_overlap_conflicts(u8 space_id, bool a_writable,
+					   bool b_writable, bool a_write_only,
+					   bool b_write_only)
+{
+	/* Only a write-only control can use a separate read-side port alias. */
+	if (space_id == ACPI_ADR_SPACE_SYSTEM_IO && a_writable != b_writable)
+		return a_writable ? !a_write_only : !b_write_only;
+
+	return a_writable || b_writable;
+}
+
 static int cpc_validate_non_mmio_overlaps(struct cpc_desc *cpc_desc,
 					  u8 space_id, const char *name)
 {
@@ -629,6 +622,7 @@ static int cpc_validate_non_mmio_overlaps(struct cpc_desc *cpc_desc,
 		for (j = i + 1; j < cpc_desc->num_entries - 2; j++) {
 			struct cpc_register_resource *b = &cpc_desc->cpc_regs[j];
 			const struct cpc_reg *b_gas;
+			bool a_writable, b_writable;
 			u64 b_size;
 
 			if (!CPC_SUPPORTED(b) || b->type != ACPI_TYPE_BUFFER ||
@@ -637,7 +631,12 @@ static int cpc_validate_non_mmio_overlaps(struct cpc_desc *cpc_desc,
 
 			b_gas = &b->cpc_entry.reg;
 			b_size = cpc_non_mmio_access_size(b);
-			if (!cpc_reg_is_writable(i) && !cpc_reg_is_writable(j))
+			a_writable = cpc_reg_is_writable(i);
+			b_writable = cpc_reg_is_writable(j);
+			if (!cpc_non_mmio_overlap_conflicts(space_id,
+							    a_writable, b_writable,
+							    cpc_reg_is_write_only(cpc_desc, i),
+							    cpc_reg_is_write_only(cpc_desc, j)))
 				continue;
 			if (a_gas->address < b_gas->address ?
 			    b_gas->address - a_gas->address >= a_size :
@@ -1769,21 +1768,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");
@@ -1871,6 +1890,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_bound_controls(cpc_ptr);
 	if (ret)
@@ -2016,11 +2040,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)) {
@@ -2029,7 +2055,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])
@@ -2109,10 +2135,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)) {
@@ -2701,12 +2729,6 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
 	auto_sel_pcc = cpc_is_writable(auto_sel_reg) &&
 		CPC_IN_PCC(auto_sel_reg);
 	epp_pcc = cpc_is_writable(epp_set_reg) && CPC_IN_PCC(epp_set_reg);
-	if ((cpc_is_writable(auto_sel_reg) &&
-	     CPC_IN_SYSTEM_IO(auto_sel_reg) &&
-	     !cpc_system_io_write_supported(auto_sel_reg)) ||
-	    (cpc_is_writable(epp_set_reg) && CPC_IN_SYSTEM_IO(epp_set_reg) &&
-	     !cpc_system_io_write_supported(epp_set_reg)))
-		return -EOPNOTSUPP;
 
 	auto_sel_non_pcc = cpc_is_writable(auto_sel_reg) && !auto_sel_pcc;
 	epp_non_pcc = cpc_is_writable(epp_set_reg) && !epp_pcc;