Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ResizeUsePciRbIo to workaround broken PciIo on some UEFI firmwares #418

Closed
wants to merge 7 commits into from

Conversation

xCuri0
Copy link
Contributor

@xCuri0 xCuri0 commented Jan 17, 2023

I created this patch because I would get Capability I/O Error when using ResizeGpuBars or ResizeAppleGpuBars. It turns out my system had a buggy PciIo implementation that would not allow access to the PCIe extended configuration space (above 256 bytes). So I created this patch to add an option to use PciRootBridgeIo instead which works without any issues on my system. OpenCore has no issues resizing my RX 580's BAR from 8GB to 256MB and booting macOS after this patch.

Before

03:002 00:065 OC: Increasing GPU BARs to 8
03:034 00:031 OCDM: Setting RBAR to 8 inc 1 on 3/19
03:100 00:065 OCDM: Capability I/O error - Unsupported
03:132 00:031 OCDM: RBAR is unsupported by device - Device Error
03:197 00:065 OCDM: Setting RBAR to 8 inc 1 on 5/19
03:229 00:031 OCDM: Capability I/O error - Unsupported
03:261 00:031 OCDM: RBAR is unsupported by device - Device Error

After (using patch with ResizeUsePciRbIo enabled)

03:000 00:065 OC: Setting GPU BARs to 8
03:032 00:031 OCDM: RBAR using PciRootBridgeIo
03:064 00:032 OCDM: Setting RBAR to 8 inc 1 on 0/2/0
03:130 00:066 OCDM: Read from disabled device
03:162 00:031 OCDM: RBAR is unsupported by device - Invalid Parameter
03:228 00:066 OCDM: Setting RBAR to 8 inc 1 on 1/0/0
03:260 00:031 OCDM: RBAR control is D20, total 1 - Success
03:327 00:066 OCDM: Old BAR 0000000C 0000000C 0000000C 0000000E 0000E001 F7E00000 - Success
03:358 00:031 OCDM: RBAR 1/1 supports 0x3F00, sizing 8 inc 1 results setting from 13 to 8
03:390 00:032 OCDM: New BAR 0000000C 00000000 0000000C 0000000E 0000E001 F7E00000 - Success
03:456 00:065 OCDM: Reprogrammed BARs to original - Success

Newer UEFI firmwares supporting Resizable BAR officially likely don't need this workaround. My motherboard is a Gigabyte B75M-D3H running BIOS F16 (December 2018) with ReBarUEFI module/patches I developed.

There is one issue which is Windows giving Code 43 on the GPU when using ResizeGpuBars however Linux works without any issues even when using pci=nocrs to stop the AMDGPU driver from resizing BAR to 8GB. ResizeUsePciRbIo should be tested on other systems where PciIo ResizeGpuBars works in Windows to see if using PciRootBridgeIo is causing that (IMO it's not, it's probably PCI Bridge or something related?).

Copy link
Member

@mhaeuser mhaeuser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I only have general remarks, because PCIe is not my strong suit.

Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Copy link
Contributor

@vit9696 vit9696 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good to me. Added a few comments to @mhaeuser's.

Library/OcDeviceMiscLib/SetResizableBar.c Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Show resolved Hide resolved
@xCuri0
Copy link
Contributor Author

xCuri0 commented Jan 23, 2023

Applied the requested changes and confirmed it works (atleast for my hardware configuration). Do I need to squash the commits before they can be merged ? And maybe build documentation too

@vit9696
Copy link
Contributor

vit9696 commented Jan 23, 2023

We can squash the commits at merge time, thankfully GH has a button for this. Looks good on my side, @mhaeuser could you merge?

Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
Library/OcDeviceMiscLib/SetResizableBar.c Outdated Show resolved Hide resolved
@xCuri0
Copy link
Contributor Author

xCuri0 commented Jan 23, 2023

Requested changes all done, lmk if I missed something. I also compiled with -Wall -Wextra -Werror (GCC) and there were no errors

@xCuri0
Copy link
Contributor Author

xCuri0 commented Jan 29, 2023

Any updates on this ?

@vit9696
Copy link
Contributor

vit9696 commented Jan 29, 2023

We have CI issues at the moment (GH Actions fail to start all over the org), and need to wait till GitHub support fixes it for us. We will merge this soon ^__^

@mikebeaton
Copy link
Contributor

Squashed and merged in d928b90 -- thank you

@xCuri0
Copy link
Contributor Author

xCuri0 commented May 5, 2023

I found another bug relating to this, PciIo->Mem.Read / PciIo->Mem.Read doesn't work on devices allocated above 4G.
On my system AudioDxe doesn't work because of it though it does identify the GPU audio which is allocated below 4G. I don't have a monitor with speakers to test it though.

00:210 00:007 HDA: Connecting controller - PciRoot(0x0)/Pci(0x1,0x0)/Pci(0x0,0x1)
00:211 00:001 HDA: Controller version 1.0
00:212 00:001 HDA: Capabilities:
00:213 00:001 HDA:  | 64-bit: Yes  Serial Data Out Signals: 0
00:214 00:001 HDA:  | Bidir streams: 0  Input streams: 0  Output streams: 6
00:215 00:001 HDA: Controller is AMD Ellesmere HD Audio Controller
00:316 00:101 HDA: Controller protocols installed
00:327 00:011 HDA: Controller initialized
00:328 00:001 HDA: Connecting codec 0x0
00:334 00:006 HDA:  | Codec ID: 0x1002:0xAA01
00:340 00:006 HDA:  | Codec name: AMD (Unknown)
00:346 00:006 HDA:  | Codec contains 1 function groups, start @ 0x1, end @ 0x1
00:352 00:006 HDA:  | Function group @ 0x1 is of type 0x1
00:378 00:026 HDA:  | Function group @ 0x1 output amp capabilities: 0x0
00:389 00:011 HDA:  | Function group @ 0x1 GPIO capabilities: 0x0
00:395 00:006 HDA:  | Function group @ 0x1 contains 14 widgets, start @ 0x2, end @ 0xF
00:822 00:426 HDA:  |  Widget @ 0x2 (type 0x0)
00:823 00:001 HDA:  | Port widget @ 0x3 is an output (pin defaults 0x185600F0) (bitmask 1)
00:824 00:001 HDA:  |  Widget @ 0x4 (type 0x0)
00:825 00:001 HDA:  | Port widget @ 0x5 is an output (pin defaults 0x185600F0) (bitmask 2)
00:826 00:001 HDA:  |  Widget @ 0x6 (type 0x0)
00:827 00:001 HDA:  | Port widget @ 0x7 is an output (pin defaults 0x185600F0) (bitmask 4)
00:828 00:001 HDA:  |  Widget @ 0x8 (type 0x0)
00:829 00:001 HDA:  | Port widget @ 0x9 is an output (pin defaults 0x185600F0) (bitmask 8)
00:830 00:001 HDA:  |  Widget @ 0xA (type 0x0)
00:831 00:001 HDA:  | Port widget @ 0xB is an output (pin defaults 0x185600F0) (bitmask 16)
00:832 00:001 HDA:  |  Widget @ 0xC (type 0x0)
00:833 00:001 HDA:  | Port widget @ 0xD is an output (pin defaults 0x185600F0) (bitmask 32)
00:834 00:001 HDA: Codec protocols installed
00:835 00:001 HDA: Codec initialized
00:836 00:001 HDA: Connecting controller - PciRoot(0x0)/Pci(0x1B,0x0)
00:837 00:001 HDA: Controller disable no snoop
**00:838 00:001 HDA: Init PCI HW - Invalid Parameter**
00:889 00:050 OC: Connecting drivers done...

I traced it to this line https://github.com/acidanthera/OpenCorePkg/blob/master/Staging/AudioDxe/HdaController/HdaController.c#L300

My AMD Ellesmere HD Audio Controller is allocated at 0xF7EFC000 (below 4G) and High Definition Audio Controller at 0xFFFFFC000 (above 4G).

I could just make it possible for AudioDxe to use PciRootBridgeIo too but it does seem like ReleaseUsbOwnership is affected too (some related to Bds too but I don't think that's used outside legacy BIOS which doesn't have 4G decode).

Is there a better way we can implement a workaround for this ? Maybe a universal option for using PciRootBridgeIo ?

@xCuri0
Copy link
Contributor Author

xCuri0 commented May 5, 2023

I think we could replace PciIo->Mem.Read / PciIo->Mem.Read with 2 wrapped functions that use PciRootBridgeIo if requested (say UseRbIoForPciIo) ?

Or could even replace PciIo completely with our own that uses PciRootBridgeIo if requested ?

These are the 2 bugs I'm aware of in these Aptio 4 firmware PciIo. There's probably more which I could do additional testing for.

@xCuri0
Copy link
Contributor Author

xCuri0 commented May 7, 2023

I've decided to go with replacing the faulty Mem.Read/Write functions with custom ones. Will provide an update

Probably will leave the IO functions as is for now

@vit9696
Copy link
Contributor

vit9696 commented May 7, 2023

Yeah, that makes good sense to me. Please provide a PR on that ^_^

@xCuri0
Copy link
Contributor Author

xCuri0 commented May 8, 2023

Turns out the issue is actually PciRootBridgeIo here, I rewrote PciIo->Mem.Read to use PciRootBridgeIo and it works exactly the same as before failing on the 64-bit device.

32-bit AMD GPU audio

00:652 00:004 OCPIO: Reading from PciRootBridgeIo F7E60003 0 1
00:658 00:005 OCPIO: Reading from PciRootBridgeIo F7E60002 0 1
00:663 00:005 HDA: Controller version 1.0
00:668 00:004 OCPIO: Reading from PciRootBridgeIo F7E60000 1 1
00:674 00:005 HDA: Capabilities:
00:677 00:003 HDA:  | 64-bit: Yes  Serial Data Out Signals: 0
00:683 00:005 HDA:  | Bidir streams: 0  Input streams: 0  Output streams: 6
00:690 00:006 HDA: Controller is AMD Ellesmere HD Audio Controller
00:696 00:006 OCPIO: Reading from PciRootBridgeIo F7E60008 2 1

64-bit Motherboard audio

02:234 00:004 HDA: Controller disable no snoop
02:239 00:004 OCPIO: Reading from PciRootBridgeIo FFFC10003 0 1
02:245 00:005 HDA: Init PCI HW - Invalid Parameter
02:250 00:005 OCPIO: Reading from PciRootBridgeIo FFFC10008 2 1

Will still try and investigate this further. Need to learn more about MMIO because I thought CopyMem using MMIO as source would work but it didn't. Seems like I need to use CpuIo for that

EDK2 source shows most likely cause is the Mem64 limit being configured wrong
image

iirc i was also able to confirm this limit while reverse engineering pcihostbridge a few months ago, it was 4gb.

@xCuri0
Copy link
Contributor Author

xCuri0 commented May 8, 2023

@vit9696 Finally managed to get it working (haven't implemented write yet)

03:171 00:004 HDA: Controller disable no snoop
03:176 00:004 OCPIO: Reading from custom MMIO FFFC10003 0 1
03:182 00:005 OCPIO: CpuMemoryServiceRead
03:186 00:004 OCPIO: Reading from custom MMIO FFFC10002 0 1
03:192 00:005 OCPIO: CpuMemoryServiceRead
03:196 00:004 HDA: Controller version 1.0
03:201 00:004 OCPIO: Reading from custom MMIO FFFC10000 1 1
03:206 00:005 OCPIO: CpuMemoryServiceRead
03:211 00:004 HDA: Capabilities:
03:215 00:003 HDA:  | 64-bit: Yes  Serial Data Out Signals: 0
03:221 00:005 HDA:  | Bidir streams: 0  Input streams: 4  Output streams: 4
03:227 00:006 HDA: Controller is Intel 7 Series HD Audio Controller
03:234 00:006 OCPIO: Reading from custom MMIO FFFC10008 2 1
03:239 00:005 OCPIO: CpuMemoryServiceRead
03:244 00:004 HDA: Controller reset - Invalid Parameter

It turns out both PciRootBridgeIo and CpuIo2 have the invalid MMIO limitation. Using IoLib MMIO got it working

So CpuIo2, PciRootBridgeIo and PciIo all need overrides which I'll submit in a PR. PciIo additionally needs fix for IO space 256 byte limit worked around in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants