Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1428483 > unrolled thread

4.7 regression - ACPICA: Hardware: Enhance acpi_hw_validate_register() with access_width/bit_offset awareness

Back to article view | Back to linux.kernel

Contents

  4.7 regression - ACPICA: Hardware: Enhance  acpi_hw_validate_register() with access_width/bit_offset awareness Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-06-22 09:10 +0200
    RE: 4.7 regression - ACPICA: Hardware: Enhance  acpi_hw_validate_register() with access_width/bit_offset awareness "Zheng, Lv" <lv.zheng@intel.com> - 2016-06-23 02:40 +0200
      Re: 4.7 regression - ACPICA: Hardware: Enhance  acpi_hw_validate_register() with access_width/bit_offset awareness Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-06-23 06:40 +0200
    [patch] cpufreq/pcc-cpufreq: Fix doorbell.access_width Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-06-23 08:50 +0200
      Re: [patch] cpufreq/pcc-cpufreq: Fix doorbell.access_width "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> - 2016-06-23 23:20 +0200

#1428483 — 4.7 regression - ACPICA: Hardware: Enhance acpi_hw_validate_register() with access_width/bit_offset awareness

In my aging (ok old) HP DL980 G7 ->access_width may be either 0 or
max_bit_width, the later inspiring cpufreq to say go away.  The below
made box a happy camper again.

ACPI Error: Unsupported register access width: 0x40 (20160422/hwregs-165)

Dinged-up-by: my little hammer
---
 drivers/acpi/acpica/hwregs.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/acpi/acpica/hwregs.c
+++ b/drivers/acpi/acpica/hwregs.c
@@ -83,7 +83,7 @@ acpi_hw_write_multiple(u32 value,
 static u8
 acpi_hw_get_access_bit_width(struct acpi_generic_address *reg, u8 max_bit_width)
 {
-	if (!reg->access_width) {
+	if (!reg->access_width || reg->access_width == max_bit_width) {
 		if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 			max_bit_width = 32;
 		}
@@ -152,9 +152,16 @@ acpi_hw_validate_register(struct acpi_ge
 		return (AE_SUPPORT);
 	}
 
-	/* Validate the access_width */
+	/* Validate the access_width, or bit_width for old register descriptors */
 
-	if (reg->access_width > 4) {
+	if ((!reg->access_width || reg->access_width == max_bit_width)) {
+		if (reg->bit_width != 8 && reg->bit_width != 16 &&
+		    reg->bit_width != 32 && reg->bit_width != max_bit_width) {
+			ACPI_ERROR((AE_INFO, "Unsupported register bit width: 0x%X",
+				   reg->bit_width));
+			return (AE_SUPPORT);
+		}
+	} else if (reg->access_width > 4) {
 		ACPI_ERROR((AE_INFO,
 			    "Unsupported register access width: 0x%X",
 			    reg->access_width));

[toc] | [next] | [standalone]

#1429365

Hi, Mike

> From: Mike Galbraith [mailto:umgwanakikbuti@gmail.com]
> Subject: 4.7 regression - ACPICA: Hardware: Enhance
> acpi_hw_validate_register() with access_width/bit_offset awareness
> 
> In my aging (ok old) HP DL980 G7
[Lv Zheng] 

Which may mean Windows Vista cannot run on this machine, and you need to use quirks to run recent Linux.
Could you try to boot it with one of the following parameters or both of the parameters:
acpi=rsdt
Or
acpi_force_32bit_fadt_addr

TBH, Windows preference of RSDT/XSDT and the preference of old 32-bit register/new GAS register descriptor are unknown to us.
But we prepared quirk for the users that my require an explicit preference to use the old platforms.

Please also send us the acpidump/dmidecode outputs for confirmation.

> ->access_width may be either 0 or
> max_bit_width, the later inspiring cpufreq to say go away.  The below
> made box a happy camper again.
> 
> ACPI Error: Unsupported register access width: 0x40 (20160422/hwregs-
> 165)
[Lv Zheng] 
According to the ACPI specification.
The valid access size is:
Specifies access size.
0 Undefined (legacy reasons)
1 Byte access
2 Word access
3 Dword access
4 QWord access

0x40 is a too big number.
So you surely need a quirk because the value filled by the BIOS is not spec compliant.

Thanks and best regards
-Lv

> 
> Dinged-up-by: my little hammer
> ---
>  drivers/acpi/acpica/hwregs.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> --- a/drivers/acpi/acpica/hwregs.c
> +++ b/drivers/acpi/acpica/hwregs.c
> @@ -83,7 +83,7 @@ acpi_hw_write_multiple(u32 value,
>  static u8
>  acpi_hw_get_access_bit_width(struct acpi_generic_address *reg, u8
> max_bit_width)
>  {
> -	if (!reg->access_width) {
> +	if (!reg->access_width || reg->access_width == max_bit_width) {
>  		if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
>  			max_bit_width = 32;
>  		}
> @@ -152,9 +152,16 @@ acpi_hw_validate_register(struct acpi_ge
>  		return (AE_SUPPORT);
>  	}
> 
> -	/* Validate the access_width */
> +	/* Validate the access_width, or bit_width for old register
> descriptors */
> 
> -	if (reg->access_width > 4) {
> +	if ((!reg->access_width || reg->access_width == max_bit_width)) {
> +		if (reg->bit_width != 8 && reg->bit_width != 16 &&
> +		    reg->bit_width != 32 && reg->bit_width !=
> max_bit_width) {
> +			ACPI_ERROR((AE_INFO, "Unsupported register bit
> width: 0x%X",
> +				   reg->bit_width));
> +			return (AE_SUPPORT);
> +		}
> +	} else if (reg->access_width > 4) {
>  		ACPI_ERROR((AE_INFO,
>  			    "Unsupported register access width: 0x%X",
>  			    reg->access_width));

[toc] | [prev] | [next] | [standalone]

#1429431

On Thu, 2016-06-23 at 00:32 +0000, Zheng, Lv wrote:

> Could you try to boot it with one of the following parameters or both of the parameters:
> acpi=rsdt
> Or
> acpi_force_32bit_fadt_addr

Box was unimpressed by either or both.
  
> Please also send us the acpidump/dmidecode outputs for confirmation.

(offlist)

	-Mike

[toc] | [prev] | [next] | [standalone]

#1429506 — [patch] cpufreq/pcc-cpufreq: Fix doorbell.access_width

Commit 920de6ebfab8 apparently exposed a latent bug, doorbell.access_width
is initialized to 64, but per Lv Zheng, it should be 4, and indeed, making
that change does bring pcc-cpufreq back to life.

Suggested-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
---
 drivers/cpufreq/pcc-cpufreq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -487,7 +487,7 @@ static int __init pcc_cpufreq_probe(void
 	doorbell.space_id = reg_resource->space_id;
 	doorbell.bit_width = reg_resource->bit_width;
 	doorbell.bit_offset = reg_resource->bit_offset;
-	doorbell.access_width = 64;
+	doorbell.access_width = 4;
 	doorbell.address = reg_resource->address;
 
 	pr_debug("probe: doorbell: space_id is %d, bit_width is %d, "

[toc] | [prev] | [next] | [standalone]

#1430164 — Re: [patch] cpufreq/pcc-cpufreq: Fix doorbell.access_width

On 6/23/2016 8:45 AM, Mike Galbraith wrote:
> Commit 920de6ebfab8 apparently exposed a latent bug, doorbell.access_width
> is initialized to 64, but per Lv Zheng, it should be 4, and indeed, making
> that change does bring pcc-cpufreq back to life.
>
> Suggested-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
> ---
>   drivers/cpufreq/pcc-cpufreq.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/drivers/cpufreq/pcc-cpufreq.c
> +++ b/drivers/cpufreq/pcc-cpufreq.c
> @@ -487,7 +487,7 @@ static int __init pcc_cpufreq_probe(void
>   	doorbell.space_id = reg_resource->space_id;
>   	doorbell.bit_width = reg_resource->bit_width;
>   	doorbell.bit_offset = reg_resource->bit_offset;
> -	doorbell.access_width = 64;
> +	doorbell.access_width = 4;
>   	doorbell.address = reg_resource->address;
>   
>   	pr_debug("probe: doorbell: space_id is %d, bit_width is %d, "


OK, I picked this up from the LKML patchwork (will push to Linus 
tomorrow), but next time please CC patches related to ACPI to linux-acpi 
and PM patches to linux-pm (so this one should have gone to both, 
ideally).  They are much easier to handle/review then and more people 
are likely to look at them too.


Thanks,

Rafael

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web