This is a note to let you know that I've just added the patch titled
ACPI: CPPC: Fix MASK_VAL() usage
to the 6.6-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
acpi-cppc-fix-mask_val-usage.patch
and it can be found in the queue-6.6 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
commit 584bd4b2b307029affb673783cf753fc2599dbfd
Author: Clément Léger <cleger@xxxxxxxxxxxx>
Date: Mon Aug 26 12:16:44 2024 +0200
ACPI: CPPC: Fix MASK_VAL() usage
[ Upstream commit 60949b7b805424f21326b450ca4f1806c06d982e ]
MASK_VAL() was added as a way to handle bit_offset and bit_width for
registers located in system memory address space. However, while suited
for reading, it does not work for writing and result in corrupted
registers when writing values with bit_offset > 0. Moreover, when a
register is collocated with another one at the same address but with a
different mask, the current code results in the other registers being
overwritten with 0s. The write procedure for SYSTEM_MEMORY registers
should actually read the value, mask it, update it and write it with the
updated value. Moreover, since registers can be located in the same
word, we must take care of locking the access before doing it. We should
potentially use a global lock since we don't know in if register
addresses aren't shared with another _CPC package but better not
encourage vendors to do so. Assume that registers can use the same word
inside a _CPC package and thus, use a per _CPC package lock.
Fixes: 2f4a4d63a193 ("ACPI: CPPC: Use access_width over bit_width for system memory accesses")
Signed-off-by: Clément Léger <cleger@xxxxxxxxxxxx>
Link: https://patch.msgid.link/20240826101648.95654-1-cleger@xxxxxxxxxxxx
[ rjw: Dropped redundant semicolon ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index d6934ba7a3154..28217a995f795 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -167,8 +167,11 @@ show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time);
#define GET_BIT_WIDTH(reg) ((reg)->access_width ? (8 << ((reg)->access_width - 1)) : (reg)->bit_width)
/* Shift and apply the mask for CPC reads/writes */
-#define MASK_VAL(reg, val) (((val) >> (reg)->bit_offset) & \
+#define MASK_VAL_READ(reg, val) (((val) >> (reg)->bit_offset) & \
GENMASK(((reg)->bit_width) - 1, 0))
+#define MASK_VAL_WRITE(reg, prev_val, val) \
+ ((((val) & GENMASK(((reg)->bit_width) - 1, 0)) << (reg)->bit_offset) | \
+ ((prev_val) & ~(GENMASK(((reg)->bit_width) - 1, 0) << (reg)->bit_offset))) \
static ssize_t show_feedback_ctrs(struct kobject *kobj,
struct kobj_attribute *attr, char *buf)
@@ -852,6 +855,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
/* Store CPU Logical ID */
cpc_ptr->cpu_id = pr->id;
+ spin_lock_init(&cpc_ptr->rmw_lock);
/* Parse PSD data for this CPU */
ret = acpi_get_psd(cpc_ptr, handle);
@@ -1057,7 +1061,7 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
}
if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
- *val = MASK_VAL(reg, *val);
+ *val = MASK_VAL_READ(reg, *val);
return 0;
}
@@ -1066,9 +1070,11 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
{
int ret_val = 0;
int size;
+ u64 prev_val;
void __iomem *vaddr = NULL;
int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
struct cpc_reg *reg = ®_res->cpc_entry.reg;
+ struct cpc_desc *cpc_desc;
size = GET_BIT_WIDTH(reg);
@@ -1101,8 +1107,34 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
return acpi_os_write_memory((acpi_physical_address)reg->address,
val, size);
- if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
- val = MASK_VAL(reg, val);
+ if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+ cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+ if (!cpc_desc) {
+ pr_debug("No CPC descriptor for CPU:%d\n", cpu);
+ return -ENODEV;
+ }
+
+ spin_lock(&cpc_desc->rmw_lock);
+ switch (size) {
+ case 8:
+ prev_val = readb_relaxed(vaddr);
+ break;
+ case 16:
+ prev_val = readw_relaxed(vaddr);
+ break;
+ case 32:
+ prev_val = readl_relaxed(vaddr);
+ break;
+ case 64:
+ prev_val = readq_relaxed(vaddr);
+ break;
+ default:
+ spin_unlock(&cpc_desc->rmw_lock);
+ return -EFAULT;
+ }
+ val = MASK_VAL_WRITE(reg, prev_val, val);
+ val |= prev_val;
+ }
switch (size) {
case 8:
@@ -1129,6 +1161,9 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
break;
}
+ if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
+ spin_unlock(&cpc_desc->rmw_lock);
+
return ret_val;
}
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index c0b69ffe7bdb4..ec425d2834f86 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -64,6 +64,8 @@ struct cpc_desc {
int cpu_id;
int write_cmd_status;
int write_cmd_id;
+ /* Lock used for RMW operations in cpc_write() */
+ spinlock_t rmw_lock;
struct cpc_register_resource cpc_regs[MAX_CPC_REG_ENT];
struct acpi_psd_package domain_info;
struct kobject kobj;
