Bug 151501
| Summary: | FADT address favor - 32-bit FADT address favor is not working correctly - Dell Vostro 1500 | ||
|---|---|---|---|
| Product: | ACPI | Reporter: | Lv Zheng (lv.zheng) |
| Component: | ACPICA-Core | Assignee: | Lv Zheng (lv.zheng) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | Andrej.Skvortzov, rui.zhang |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 4.7 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: |
'acpidump -c off' output
rsdt xsdt [PATCH] ACPICA: Tables: Override all 64-bit GAS fields when acpi_gbl_use32_bit_fadt_addresses is TRUE [PATCH] ACPICA: Tables: Make sure GAS code can fall back to use old logic [PATCH] ACPICA: Utilities: Add power of two rounding support [PATCH] ACPICA: Hardware: Sort access bit width algorithm [PATCH] ACPICA: Hardware: Add access_width/bit_offset support in acpi_hw_write() |
||
Hi, Andrey (Andrey Skvortsov <andrej.skvortzov@gmail.com>) Could you upload "acpidump -c off" output here? Thanks Lv, can you please also add the URL for the discussion in mailing list? Created attachment 228221 [details]
'acpidump -c off' output
Hi, Andrey Thanks for the information. It is said that for compatibility reasons, Windows requires vendors to provide 2 FADTs. In order to clarify the issue, I want to obtain all FADTs of your system. Let's figure them out. RSDP: [000h 0000 8] Signature : "RSD PTR " [008h 0008 1] Checksum : 26 [009h 0009 6] Oem ID : "DELL " [00Fh 0015 1] Revision : 02 [010h 0016 4] RSDT Address : DF66D93A [014h 0020 4] Length : 00000024 [018h 0024 8] XSDT Address : 00000000DF66F200 [020h 0032 1] Extended Checksum : A5 [021h 0033 3] Reserved : 000000 RSDT: [000h 0000 4] Signature : "RSDT" [Root System Description Table] [004h 0004 4] Table Length : 00000040 [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : 78 [00Ah 0010 6] Oem ID : "DELL " [010h 0016 8] Oem Table ID : "M08 " [018h 0024 4] Oem Revision : 27D80415 [01Ch 0028 4] Asl Compiler ID : "ASL " [020h 0032 4] Asl Compiler Revision : 00000061 [024h 0036 4] ACPI Table Address 0 : DF66EC00 <- 1st FADT [028h 0040 4] ACPI Table Address 1 : DF66F300 [02Ch 0044 4] ACPI Table Address 2 : DF66F400 [030h 0048 4] ACPI Table Address 3 : DF66F3C0 [034h 0052 4] ACPI Table Address 4 : DF66F49C [038h 0056 4] ACPI Table Address 5 : DF66EFC0 [03Ch 0060 4] ACPI Table Address 6 : DF66D97A XSDT: [000h 0000 4] Signature : "XSDT" [Extended System Description Table] [004h 0004 4] Table Length : 0000005C [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : B6 [00Ah 0010 6] Oem ID : "DELL " [010h 0016 8] Oem Table ID : "M08 " [018h 0024 4] Oem Revision : 27D80415 [01Ch 0028 4] Asl Compiler ID : "ASL " [020h 0032 4] Asl Compiler Revision : 00000061 [024h 0036 8] ACPI Table Address 0 : 00000000DF66F09C <- 2nd FADt [02Ch 0044 8] ACPI Table Address 1 : 00000000DF66F300 [034h 0052 8] ACPI Table Address 2 : 00000000DF66F400 [03Ch 0060 8] ACPI Table Address 3 : 00000000DF66F3C0 [044h 0068 8] ACPI Table Address 4 : 00000000DF66F49C [04Ch 0076 8] ACPI Table Address 5 : 00000000DF66EFC0 [054h 0084 8] ACPI Table Address 6 : 00000000DF66D97A So we have different FADT: [024h 0036 4] ACPI Table Address 0 : DF66EC00 [024h 0036 8] ACPI Table Address 0 : 00000000DF66F09C Please help to obtain the 2 FADT tables via: # acpidump -a 0xDF66EC00 > facp-rsdt.dat # acpidump -a 0xDF66F09C > facp-xsdt.dat And upload the 2 files here. Thanks Lv Created attachment 228311 [details]
rsdt
Created attachment 228321 [details]
xsdt
Here they are.
The FADT extracted from RSDT (let me call it RSDT-FADP later) is a revision 1 table:
[000h 0000 4] Signature : "FACP" [Fixed ACPI Description Table (FADT)]
[004h 0004 4] Table Length : 00000074
[008h 0008 1] Revision : 01
...
[038h 0056 4] PM1A Event Block Address : 00001000
[03Ch 0060 4] PM1B Event Block Address : 00000000
[040h 0064 4] PM1A Control Block Address : 00001004
[044h 0068 4] PM1B Control Block Address : 00000000
[048h 0072 4] PM2 Control Block Address : 00001020
[04Ch 0076 4] PM Timer Block Address : 00001008
[050h 0080 4] GPE0 Block Address : 00001028
[054h 0084 4] GPE1 Block Address : 00000000
[058h 0088 1] PM1 Event Block Length : 04
[059h 0089 1] PM1 Control Block Length : 02
[05Ah 0090 1] PM2 Control Block Length : 01
[05Bh 0091 1] PM Timer Block Length : 04
[05Ch 0092 1] GPE0 Block Length : 08
[05Dh 0093 1] GPE1 Block Length : 00
[05Eh 0094 1] GPE1 Base Offset : 00
...
Thus there is no GAS defined in this table.
My validation result shows if the RSDT-FADT is revision 5, Windows doesn't care GAS fields.
This seems to be a compatible mode for old Windows clones where only old ACPI spec (ACPI 1.0) is supported (no GAS support).
This may also explain why acpi=rsdt works for your platform.
The FADT extracted from XSDT (let me call it XSDT-FADP later) is a revision 4 table:
[000h 0000 4] Signature : "FACP" [Fixed ACPI Description Table (FADT)]
[004h 0004 4] Table Length : 000000F4
[008h 0008 1] Revision : 04
...
[038h 0056 4] PM1A Event Block Address : 00001000
[03Ch 0060 4] PM1B Event Block Address : 00000000
[040h 0064 4] PM1A Control Block Address : 00001004
[044h 0068 4] PM1B Control Block Address : 00000000
[048h 0072 4] PM2 Control Block Address : 00001020
[04Ch 0076 4] PM Timer Block Address : 00001008
[050h 0080 4] GPE0 Block Address : 00001028
[054h 0084 4] GPE1 Block Address : 00000000
[058h 0088 1] PM1 Event Block Length : 04
[059h 0089 1] PM1 Control Block Length : 02
[05Ah 0090 1] PM2 Control Block Length : 01
[05Bh 0091 1] PM Timer Block Length : 04
[05Ch 0092 1] GPE0 Block Length : 08
[05Dh 0093 1] GPE1 Block Length : 00
[05Eh 0094 1] GPE1 Base Offset : 00
...
[094h 0148 12] PM1A Event Block : [Generic Address Structure]
[094h 0148 1] Space ID : 01 [SystemIO]
[095h 0149 1] Bit Width : 20
[096h 0150 1] Bit Offset : 00
[097h 0151 1] Encoded Access Width : 01 [Byte Access:8]
[098h 0152 8] Address : 0000000000001000
[0A0h 0160 12] PM1B Event Block : [Generic Address Structure]
[0A0h 0160 1] Space ID : 01 [SystemIO]
[0A1h 0161 1] Bit Width : 00
[0A2h 0162 1] Bit Offset : 00
[0A3h 0163 1] Encoded Access Width : 00 [Undefined/Legacy]
[0A4h 0164 8] Address : 0000000000000000
[0ACh 0172 12] PM1A Control Block : [Generic Address Structure]
[0ACh 0172 1] Space ID : 01 [SystemIO]
[0ADh 0173 1] Bit Width : 10
[0AEh 0174 1] Bit Offset : 00
[0AFh 0175 1] Encoded Access Width : 01 [Byte Access:8]
[0B0h 0176 8] Address : 0000000000001004
[0B8h 0184 12] PM1B Control Block : [Generic Address Structure]
[0B8h 0184 1] Space ID : 01 [SystemIO]
[0B9h 0185 1] Bit Width : 00
[0BAh 0186 1] Bit Offset : 00
[0BBh 0187 1] Encoded Access Width : 00 [Undefined/Legacy]
[0BCh 0188 8] Address : 0000000000000000
[0C4h 0196 12] PM2 Control Block : [Generic Address Structure]
[0C4h 0196 1] Space ID : 01 [SystemIO]
[0C5h 0197 1] Bit Width : 08
[0C6h 0198 1] Bit Offset : 00
[0C7h 0199 1] Encoded Access Width : 01 [Byte Access:8]
[0C8h 0200 8] Address : 0000000000001020
[0D0h 0208 12] PM Timer Block : [Generic Address Structure]
[0D0h 0208 1] Space ID : 01 [SystemIO]
[0D1h 0209 1] Bit Width : 20
[0D2h 0210 1] Bit Offset : 00
[0D3h 0211 1] Encoded Access Width : 01 [Byte Access:8]
[0D4h 0212 8] Address : 0000000000001008
[0DCh 0220 12] GPE0 Block : [Generic Address Structure]
[0DCh 0220 1] Space ID : 01 [SystemIO]
[0DDh 0221 1] Bit Width : 40
[0DEh 0222 1] Bit Offset : 00
[0DFh 0223 1] Encoded Access Width : 01 [Byte Access:8]
[0E0h 0224 8] Address : 0000000000001028
[0E8h 0232 12] GPE1 Block : [Generic Address Structure]
[0E8h 0232 1] Space ID : 01 [SystemIO]
[0E9h 0233 1] Bit Width : 00
[0EAh 0234 1] Bit Offset : 00
[0EBh 0235 1] Encoded Access Width : 00 [Undefined/Legacy]
[0ECh 0236 8] Address : 0000000000000000
...
This table contains GAS fields.
I compared the valid register descriptors:
PM1AEvent PM1AControl PM2Control PMTimer GPE0
RSDT-FADT/32 1000/4 1004/2 1020/1 1008/4 1028/8
XSDT-FADT/32 1000/4 1004/2 1020/1 1008/4 1028/8
XSDT-FADT/64 1000/4 1004/2 1020/1 1008/4 1028/8
They are same, however, all of the AccessWidth is "Byte Access" in XSDT=FADT.
For now, I don't have mean to probe Windows behavior around the XSDT-FADT. So I don't know if Windows actually uses GAS fields in the XSDT-FADT.
However, I'm interested in the test result that:
Can the latest Windows (Windows 7+) be booted on this platform?
Thanks
Lv
Created attachment 228701 [details] [PATCH] ACPICA: Tables: Override all 64-bit GAS fields when acpi_gbl_use32_bit_fadt_addresses is TRUE Please apply this patch (based on this repo: https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/log/?h=linux-next) And help to confirm if the following quirk can work now: acpi_force_32bit_fadt_addr Thanks in advance. Best regards Lv Hi Lv, The patch looks good to me. I tried it on top of v4.7-rc3, where original problem exists. With your patch applied and acpi_force_32bit_fadt_addr command line parameter suspend to RAM is working correctly. I understand acpi_force_32bit_fadt_addr option as temporary solution for the issue and for the final solution it's necessary to implement Windows ACPI handling. Right? I'll try Windows 7 and report you how does it work with this FADT table. (In reply to Andrey Skvortsov from comment #10) > Hi Lv, > > The patch looks good to me. I tried it on top of v4.7-rc3, where original > problem exists. With your patch applied and acpi_force_32bit_fadt_addr > command line parameter suspend to RAM is working correctly. Sounds good. > > I understand acpi_force_32bit_fadt_addr option as temporary solution for the > issue and for the final solution it's necessary to implement Windows ACPI > handling. Right? Yes. We are putting efforts to reveal this. I'll also post an improved apci_hw_write() here, hope you can help to validate it. So let's open it for a while. > I'll try Windows 7 and report you how does it work with this FADT table. Appreciated. I actually do not know which Windows version starts to support GAS/XSDT. Probably: 1. Windows Vista starts to support ACPI 2.0. 2. Windows 8 starts to support ACPI 5.0. But this information may not be correct and Vista seems to be an unstable version as there was a huge publication delay. As a version between them, using Windows 7 to do the validation may be a safer choice. Thanks and best regards Lv IMO, there might be 2 kinds of GAS definitions:
1. register format: BitWidth is the register width, BitOffset is 0.
2. register field format: BitWidth/BitOffset is within the reigster, AccessWidth is the register width.
Our code need to ensure that for "register" format GAS, no additional reads should happen in acpi_hw_write().
FADT registers are all defined as "register" format.
I checked the acpi_hw_write() support. It contains code to avoid unexpected read():
if (BitOffset || BitWidth < AccessWidth)
Thus on your platform, the 16-bit PM1A control register writes will only be split into 2 8-bit system IO port writes.
In <<Intel® 64 and IA-32 Architectures Software Developer’s Manual>>:
The processor’s I/O address space is separate and distinct from the physical-memory address space. The I/O address space consists of 216 (64K) individually addressable 8-bit I/O ports, numbered 0 through FFFFH.
In <<Intel 80386 Reference Programmer's Manual>>:
The I/O address space consists of 2^(16) (64K) individually addressable 8-bit ports; any two consecutive 8-bit ports can be treated as a 16-bit port; and four consecutive 8-bit ports can be treated as a 32-bit port. Thus, the I/O address space can accommodate up to 64K 8-bit ports, up to 32K 16-bit ports, or up to 16K 32-bit ports.
As all FADT registers are IO ports, I really cannot see why we cannot do 2 8-bit split writes to one of its registers unless there are fields not bounded within 8-bit ports.
This is the definition of PM1A control register:
0 SCI_EN
1 BM_RLD
2 GBL_RLS
8:3 Reserved
9 Ignore
12:10 SLP_TYPx
13 SLP_EN
15:14 Reserved
Then I couldn't see such kind of fields in it. All fields are within 8-bit port boundary.
I then tried to customize my FADT, running it with all "Encoded Access Width" setting to 1 (Byte Access), no suspend/resume issue can be seen.
So I began to doubt if the problem on your platform is an isolated incident, caused by un-root-caused problems.
IMO, we can use the 32-bit fadt quirk to work it around.
Created attachment 229251 [details]
[PATCH] ACPICA: Tables: Make sure GAS code can fall back to use old logic
This patch can help to make GAS code to fall back to use the old logics - always use "Bit Width" for "register" format GAS registers.
Hi, Andrey
Please apply attachment 229251 [details], do not use any quirk mechanism, and try again.
Thanks in advance.
Created attachment 229291 [details]
[PATCH] ACPICA: Utilities: Add power of two rounding support
Created attachment 229301 [details]
[PATCH] ACPICA: Hardware: Sort access bit width algorithm
Created attachment 229311 [details]
[PATCH] ACPICA: Hardware: Add access_width/bit_offset support in acpi_hw_write()
The upstream reverted commit.
(In reply to Lv Zheng from comment #14) > Hi, Andrey > > Please apply attachment 229251 [details], do not use any quirk mechanism, > and try again. > > Thanks in advance. Hi, Andrey I updated the fix, so sorry for the noise and please ignore comment #14. Please apply the attachment 229291 [details] and attachment 229301 [details] on top of v4.7-rc3 and try again. Or if you are using the latest code, you can apply attachment 229311 [details] before applying the 2 patches and try again. Thanks in advance. Hi, Lv. Sorry for the delay. I took some time to find extra hdd for Windows installations. On Windows 7 suspend to RAM works like a charm. On Windows 8 after installation there is no suspend to RAM option. I think this is because after installation eight system devices and even video adapter doesn't have drivers according to device manager. Windows couldn't find drivers on net. Honestly I haven't spent much time to resolve the issue. If you really need information whether suspend to RAM is working on Windows 8 on this laptop, I could try again to find all necessary drivers. I applied your patches on top of v4.7-rc3 and the issue is gone. Suspend to RAM is working again. If you want, I could try to force "Encoded Access Width" setting to 1 (Byte Access) on other laptop and look whether the suspend issue is present there? (In reply to Andrey Skvortsov from comment #19) > Hi, Lv. > > Sorry for the delay. I took some time to find extra hdd for Windows > installations. > On Windows 7 suspend to RAM works like a charm. Windows seems to be always using 32-bit register addresses on x86 machines. That's why I asked your help to make the quirk working. > > On Windows 8 after installation there is no suspend to RAM option. I think > this is because after installation eight system devices and even video > adapter doesn't have drivers according to device manager. Windows couldn't > find drivers on net. Honestly I haven't spent much time to resolve the > issue. If you really need information whether suspend to RAM is working on > Windows 8 on this laptop, I could try again to find all necessary drivers. > No necessary. > > I applied your patches on top of v4.7-rc3 and the issue is gone. Suspend to > RAM is working again. I'm considering to provide a "unified ACPI register access" solution to use GAS support code to deal with both 32-bit and 64-bit registers. As some tables requires to use GAS (APEI registers) while some doesn't (FADT). This patch seems to be working for your platform. We actually have this solution done with previous 32-bit favor quirk improvement. We can do this in Linux: 1. Setting 32-bit favor for x86 by default 2. Enable GAS support by default It should work. If we want to facilitate BIOS developers to use 64-bit on x86 platforms, then this is a kind of "enabling for the future" work. With these solution improvement patches, it may work for most of the platforms. If some platforms still broke, we then could improve the solution or just use 32-bit favor quirk for them. > > If you want, I could try to force "Encoded Access Width" setting to 1 (Byte > Access) on other laptop and look whether the suspend issue is present there? I did this on several platforms. No breakage can be seen. So we actually have completed 2 things here: 1. Find a "unified ACPI register access" solution - attachment 228701 [details]. 2. Try to enable 64-bit support for all platforms - attachment 229291 [details] and attachment 229301 [details]. If you have different platforms broken for 2, you can provide dmidecode here. Let me first mark the bug as resolved. Thanks Lv Good. Lv, thank you for the patches. Closing as all code has been accepted by ACPICA uptream. Thanks for the support. It really helps a lot. https://github.com/acpica/acpica/pull/161 |
> > IMO, for the time being, you can use quirks. > > Booting your kernel with the following parameters: > > > > acpi=rsdt > > Or > > acpi_force_32bit_fadt_addr > > Or > > Both > > Rafael reverted commit, so I'm ok now. > > Actually acpi_force_32bit_fadt_addr will not help here, because it's take > effect only if address64 != address32. But here these addresses are > the same, therefore access_width is taken from extended address.