Use access_width for _CPC package when provided by the platform firmware
cover Use access_width for _CPC package when provided by the platform firmware
This series backports the original patch[1] and fixes to it[2][3], all marked for stable, that fix a kernel panic when using the wrong access size when platform firmware provides the access size to use for the _CPC ACPI package.
This series was originally sent in response to an automated email notifying authors, reviewers, and maintainer of a failure to apply patch
[3] to linux-5.15.y at [4].
[1] 2f4a4d63a193 ("ACPI: CPPC: Use access_width over bit_width for system memory accesses")
[2] 92ee782ee ("ACPI: CPPC: Fix bit_offset shift in MASK_VAL() macro").
[3] f489c948028b ("ACPI: CPPC: Fix access width used for PCC registers")
[4] https://lore.kernel.org/all/2024042905-puppy-heritage-e422@gregkh/
Easwar Hariharan (1):
Revert "Revert "ACPI: CPPC: Use access_width over bit_width for system
memory accesses""
Jarred White (1):
ACPI: CPPC: Fix bit_offset shift in MASK_VAL() macro
Vanshidhar Konda (1):
ACPI: CPPC: Fix access width used for PCC registers
drivers/acpi/cppc_acpi.c | 67 +++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 11 deletions(-)
base-commit: 284087d4f7d57502b5a41b423b771f16cc6d157a
Signature · 1 line
2.34.1
PATCH RESEND 1/3 Revert "Revert "ACPI: CPPC: Use access_width over bit_width for system memory accesses""
This reverts commit b54c4632946ae42f2b39ed38abd909bbf78cbcc2 which was a revert of a backport of commit 2f4a4d63a193be6fd530d180bb13c3592052904c upstream to 5.15.y.
--- drivers/acpi/cppc_acpi.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)
drivers/acpi/cppc_acpi.c +24 -3
@@ -161,6 +161,13 @@ show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, nominal_freq);show_cppc_data (cppc_get_perf_ctrs, cppc_perf_fb_ctrs, reference_perf);show_cppc_data (cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time); +/* Check for valid access_width, otherwise, fallback to using bit_width */ +#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 & \ + GENMASK(((reg)->bit_width), 0))) +static ssize_t show_feedback_ctrs (struct kobject *kobj,struct kobj_attribute *attr,char *buf) {
@@ -762,8 +769,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) }else if (gas_t->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) {if (gas_t->address) {void __iomem *addr; +size_t access_width; - addr =ioremap (gas_t->address, gas_t->bit_width/8 ); + access_width =GET_BIT_WIDTH (gas_t) /8 ; + addr =ioremap (gas_t->address, access_width);if (!addr) goto out_free; cpc_ptr->cpc_regs[i-2 ].sys_mem_vaddr = addr;
@@ -936,6 +945,7 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val) {int ret_val =0 ;void __iomem *vaddr =NULL ; +int size;int pcc_ss_id =per_cpu (cpu_pcc_subspace_idx, cpu);struct cpc_reg *reg = ®_res->cpc_entry.reg;
@@ -955,7 +965,9 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)return acpi_os_read_memory ((acpi_physical_address )reg->address, val, reg->bit_width); -switch (reg->bit_width) { + size =GET_BIT_WIDTH (reg); + +switch (size) {case 8 : *val =readb_relaxed (vaddr);break ;
@@ -974,12 +986,16 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val) ret_val = -EFAULT ; } +if (reg->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) + *val =MASK_VAL (reg, *val); +return ret_val; }static int cpc_write (int cpu,struct cpc_register_resource *reg_res,u64 val) {int ret_val =0 ; +int size;void __iomem *vaddr =NULL ;int pcc_ss_id =per_cpu (cpu_pcc_subspace_idx, cpu);struct cpc_reg *reg = ®_res->cpc_entry.reg;
@@ -994,7 +1010,12 @@ 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, reg->bit_width); -switch (reg->bit_width) { + size =GET_BIT_WIDTH (reg); + +if (reg->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) + val =MASK_VAL (reg, val); + +switch (size) {case 8 :writeb_relaxed (val, vaddr);break ;
Signature · 1 line
2.34.1
PATCH RESEND 2/3 ACPI: CPPC: Fix bit_offset shift in MASK_VAL() macro
In reply to Easwar HariharanFrom: Jarred White <[email protected]>
commit 05d92ee782eeb7b939bdd0189e6efcab9195bf95 upstream
Commit 2f4a4d63a193 ("ACPI: CPPC: Use access_width over bit_width for system memory accesses") neglected to properly wrap the bit_offset shift when it comes to applying the mask. This may cause incorrect values to be read and may cause the cpufreq module not be loaded.
[ 11.059751] cpu_capacity: CPU0 missing/invalid highest performance. [ 11.066005] cpu_capacity: partial information: fallback to 1024 for all CPUs
Also, corrected the bitmask generation in GENMASK (extra bit being added).
--- drivers/acpi/cppc_acpi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/acpi/cppc_acpi.c +2 -2
@@ -165,8 +165,8 @@ 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 & \ - GENMASK(((reg)->bit_width), 0))) +#define MASK_VAL (reg, val) (((val) >> (reg)->bit_offset) & \ + GENMASK(((reg)->bit_width) - 1, 0))static ssize_t show_feedback_ctrs (struct kobject *kobj,struct kobj_attribute *attr,char *buf)
Signature · 1 line
2.34.1
PATCH RESEND 3/3 ACPI: CPPC: Fix access width used for PCC registers
In reply to Easwar HariharanFrom: Vanshidhar Konda <[email protected]>
commit f489c948028b69cea235d9c0de1cc10eeb26a172 upstream
commit 2f4a4d63a193 ("ACPI: CPPC: Use access_width over bit_width for system memory accesses") modified cpc_read()/cpc_write() to use access_width to read CPC registers.
However, for PCC registers the access width field in the ACPI register macro specifies the PCC subspace ID. For non-zero PCC subspace ID it is incorrectly treated as access width. This causes errors when reading from PCC registers in the CPPC driver.
For PCC registers, base the size of read/write on the bit width field. The debug message in cpc_read()/cpc_write() is updated to print relevant information for the address space type used to read the register.
[ eahariha: Backport to v5.15 by dropping SystemIO bits as commit
a2c8f92bea5f is not present ]
--- drivers/acpi/cppc_acpi.c | 48 ++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 12 deletions(-)
drivers/acpi/cppc_acpi.c +36 -12
@@ -955,17 +955,24 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val) } *val =0 ; -if (reg->space_id ==ACPI_ADR_SPACE_PLATFORM_COMM && pcc_ss_id >=0 ) + size =GET_BIT_WIDTH (reg); + +if (reg->space_id ==ACPI_ADR_SPACE_PLATFORM_COMM && pcc_ss_id >=0 ) { +/* +* For registers in PCC space, the register size is determined +* by the bit width field; the access size is used to indicate +* the PCC subspace id. +*/ + size = reg->bit_width; vaddr =GET_PCC_VADDR (reg->address, pcc_ss_id); + }else if (reg->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) vaddr = reg_res->sys_mem_vaddr;else if (reg->space_id ==ACPI_ADR_SPACE_FIXED_HARDWARE )return cpc_read_ffh (cpu, reg, val);else return acpi_os_read_memory ((acpi_physical_address )reg->address, - val, reg->bit_width); - - size =GET_BIT_WIDTH (reg); + val, size);switch (size) {case 8 :
@@ -981,8 +988,13 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val) *val =readq_relaxed (vaddr);break ;default : -pr_debug ("Error: Cannot read %u bit width from PCC for ss: %d\n" , - reg->bit_width, pcc_ss_id); +if (reg->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) { +pr_debug ("Error: Cannot read %u bit width from system memory: 0x%llx\n" , + size, reg->address); + }else if (reg->space_id ==ACPI_ADR_SPACE_PLATFORM_COMM ) { +pr_debug ("Error: Cannot read %u bit width from PCC for ss: %d\n" , + size, pcc_ss_id); + } ret_val = -EFAULT ; }
@@ -1000,17 +1012,24 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)int pcc_ss_id =per_cpu (cpu_pcc_subspace_idx, cpu);struct cpc_reg *reg = ®_res->cpc_entry.reg; -if (reg->space_id ==ACPI_ADR_SPACE_PLATFORM_COMM && pcc_ss_id >=0 ) + size =GET_BIT_WIDTH (reg); + +if (reg->space_id ==ACPI_ADR_SPACE_PLATFORM_COMM && pcc_ss_id >=0 ) { +/* +* For registers in PCC space, the register size is determined +* by the bit width field; the access size is used to indicate +* the PCC subspace id. +*/ + size = reg->bit_width; vaddr =GET_PCC_VADDR (reg->address, pcc_ss_id); + }else if (reg->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) vaddr = reg_res->sys_mem_vaddr;else if (reg->space_id ==ACPI_ADR_SPACE_FIXED_HARDWARE )return cpc_write_ffh (cpu, reg, val);else return acpi_os_write_memory ((acpi_physical_address )reg->address, - val, reg->bit_width); - - size =GET_BIT_WIDTH (reg); + val, size);if (reg->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) val =MASK_VAL (reg, val);
@@ -1029,8 +1048,13 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)writeq_relaxed (val, vaddr);break ;default : -pr_debug ("Error: Cannot write %u bit width to PCC for ss: %d\n" , - reg->bit_width, pcc_ss_id); +if (reg->space_id ==ACPI_ADR_SPACE_SYSTEM_MEMORY ) { +pr_debug ("Error: Cannot write %u bit width to system memory: 0x%llx\n" , + size, reg->address); + }else if (reg->space_id ==ACPI_ADR_SPACE_PLATFORM_COMM ) { +pr_debug ("Error: Cannot write %u bit width to PCC for ss: %d\n" , + size, pcc_ss_id); + } ret_val = -EFAULT ;break ; }
Signature · 1 line
2.34.1
Quoted 8 lines · Easwar Hariharan
This series backports the original patch[1] and fixes to it[2][3], all marked for stable, that fix a kernel panic when using the wrong access size when platform firmware provides the access size to use for the
_CPCACPI package.This series was originally sent in response to an automated email notifying authors, reviewers, and maintainer of a failure to apply patch
[3] to linux-5.15.y at [4].
Queued up, thanks!
Signature · 2 lines
Thanks, Sasha