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

OcMainLib: Read and set serial PCD values from config #330

Merged
merged 40 commits into from Apr 4, 2022
Merged

Conversation

PMheart
Copy link
Member

@PMheart PMheart commented Mar 31, 2022

closes acidanthera/bugtracker#1954

TODO:

  • Drop legacy SerialInit
  • Update Configuration.tex
  • Update ocvalidate checklist

After merging:

  • Update Chagelog
  • Rebuild pdf

@PMheart PMheart marked this pull request as draft March 31, 2022 11:46
@PMheart PMheart marked this pull request as ready for review March 31, 2022 15:40
@PMheart
Copy link
Member Author

PMheart commented Apr 1, 2022

@joevt I think now everything is nearly complete. Could you please test it? Or, feel free to leave more comments here. :)

@joevt
Copy link

joevt commented Apr 2, 2022

I an working on testing.

@joevt
Copy link

joevt commented Apr 2, 2022

I haven't noticed a change in serial output yet. I'll add some debug statements to see if it's accessing the PCIe card.
serial output.zip
I'm still using my previous serial setup (redirection of EFI console and xnu patches) so a lot of stuff from OpenCore, boot.efi, and macOS is still being output. I'll eventually do a test without the redirection of EFI console.

@PMheart
Copy link
Member Author

PMheart commented Apr 2, 2022

Thanks for testing. I am away from keyboard right now, so could you please try adding some debug prints and see if the keys are successfully overridden? Or, will there be more things to handle, including booter/XNU patches?

Docs/Configuration.tex Outdated Show resolved Hide resolved
Docs/Configuration.tex Show resolved Hide resolved
Utilities/ocvalidate/ValidateMisc.c Outdated Show resolved Hide resolved
\texttt{PciDeviceInfo}\\
\textbf{Type}: \texttt{plist\ data}\\
\textbf{Failsafe}: \texttt{0xFF}\\
\textbf{Description}: Set PCI serial device information.
Copy link
Member Author

Choose a reason for hiding this comment

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

@joevt Would you please provide more information on how to convert a device path to the formatted PciDeviceInfo bytes? Thanks!

Copy link

Choose a reason for hiding this comment

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

My current theory is that I just need the device number and function number from each PCI device in the UEFI device path for the first two bytes of each node in the PciDeviceInfo path. The second two bytes I'll leave as 00 00. The last node is followed by an FF.

The following code find PCIe serial ports and outputs their paths.

ioreg -p IODeviceTree -lw0 | perl -e '
	$ioregpath=""; $pcipath=""; while (<>) {
		if ( /^([ |]*)\+\-o (.+)  <class (\w+)/ ) {
			$indent = (length $1) / 2; $name = $2; $class = $3;
			$ioregpath =~ s|^((/[^/]*){$indent}).*|$1/$name|;
			$pcipath =~ s|^((/[^/]*){$indent}).*|$1/|;
			$name = ""; $uid = 0; $classcode = "";
		}
		   if ( $class eq "IOACPIPlatformDevice" ) { if ( /^[ |]*"name" = <"([^\"]*)">/ ) { $name = $1; }; if ( /^[ |]*"_UID" = "(\d+)"/ ) { $uid = $1; } }
		elsif ( $class eq "IOPCIDevice" ) { if ( /^[ |]*"pcidebug" = "\d+:(\d+):(\d+).*"/ ) { $name = sprintf("Pci(0x%x,0x%x)", $1, $2);  } if ( /^[ |]*"class-code" = <(\w+)>/ ) { $classcode = $1; } }
		if ( /^[ |]*}/ && $name ) {
			if ( $class eq "IOACPIPlatformDevice" ) {
				   if ($name eq "PNP0A03") { $name = sprintf ("PciRoot(0x%x)", $uid); }
				elsif ($name eq "PNP0A08") { $name = sprintf ("PciRoot(0x%x)", $uid); }
				elsif ($name =~ /PNP..../) { $name = sprintf ("Acpi(%s,0x%x)", $name, $uid); }
				# not translating all ACPI types since we only care about PCI devices
			}
			$pcipath .= $name;
			if ( $classcode eq "02000700" ) {
				$serialdevicepath = $pcipath =~ s/.*PciRoot\(0x0\)//r =~ s|/Pci\(||gr =~ s|\)| 00 00 |gr =~ s|0x||gr =~ s|,| |gr =~ s|\b(\w)\b|0\1|gr =~ s|$|FF|r;
				print $ioregpath =~ s|/Root/||r . "\n";
				print $pcipath =~ s|/*||r  . "\n"; 
				print "xxd -p -r <<< \"" . $serialdevicepath . "\" | base64\n";
				print "\n";
			}
		}
	}
'

This is the result:

/PCI0@0/P0P9@9/P9P2@0/P2P4@1/PXS3@0
PciRoot(0x0)/Pci(0x9,0x0)/Pci(0x0,0x0)/Pci(0x1,0x0)/Pci(0x0,0x0)
xxd -p -r <<< "09 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 FF" | base64

/PCI0@0/P0P9@9/P9P2@0/P2P4@1/PXS3@0,2
PciRoot(0x0)/Pci(0x9,0x0)/Pci(0x0,0x0)/Pci(0x1,0x0)/Pci(0x0,0x2)
xxd -p -r <<< "09 00 00 00 00 00 00 00 01 00 00 00 00 02 00 00 FF" | base64

The following can take a UEFI device path and produce the serial PciDevicePath:

serialpath=PciRoot(0x0)/Pci(0x9,0x0)/Pci(0x0,0x0)/Pci(0x1,0x0)/Pci(0x0,0x0)
perl -pe 's/PciRoot(0x0)//;s|/Pci\(||g;s|\)| 00 00 |g;s|0x||g;s|,| |g;s|\b(\w)\b|0\1|g; s|$|FF|' <<< "$serialpath" | xxd -p -r | base64

I'll let you know when I find out if this works.

@@ -407,11 +406,39 @@ typedef enum {
OC_ARRAY (OC_MISC_TOOLS_ENTRY, _, __)
OC_DECLARE (OC_MISC_TOOLS_ARRAY)

///
/// Reference:
/// https://github.com/acidanthera/bugtracker/issues/1954#issuecomment-1084220743
Copy link
Member Author

Choose a reason for hiding this comment

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

Probably it is better to mention this 41-byte thing in docs, then remove these links in the comment. Same below.

@PMheart PMheart requested review from mhaeuser and vit9696 April 2, 2022 21:34
Utilities/ocvalidate/ValidateMisc.c Outdated Show resolved Hide resolved
Utilities/ocvalidate/ValidateMisc.c Show resolved Hide resolved
Docs/Configuration.tex Show resolved Hide resolved
Utilities/ocvalidate/ValidateMisc.c Show resolved Hide resolved
Docs/Configuration.tex Outdated Show resolved Hide resolved
Library/OcMainLib/OpenCoreMisc.c Show resolved Hide resolved
@joevt
Copy link

joevt commented Apr 3, 2022

I found an issue. Variable length pcd's need to be initialized with the max size that you want to support. I found this while looking at the *.i files generated by -save-temps.

diff --git a/OpenCorePkg.dsc b/OpenCorePkg.dsc
index 3632ebaa484e98b7bf4ddf7444856e82c8c8e310..1bf72260adc220238edae652bb7646fb43ee9326 100755
--- a/OpenCorePkg.dsc
+++ b/OpenCorePkg.dsc
@@ -336,7 +336,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialLineControl|0x03
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x07
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo|{0xFF}
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo|{0xFF,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|64
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|1
 
@@ -345,21 +345,21 @@
   DEFINE OCPKG_BUILD_OPTIONS_GEN = -D DISABLE_NEW_DEPRECATED_INTERFACES $(OCPKG_BUILD_OPTIONS) -D OC_TARGET_$(TARGET)=1
   DEFINE OCPKG_ANAL_OPTIONS_GEN = "-DANALYZER_UNREACHABLE=__builtin_unreachable" "-DANALYZER_NORETURN=__attribute__((noreturn))"
 
-  GCC:DEBUG_*_*_CC_FLAGS        = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -Wuninitialized
-  GCC:NOOPT_*_*_CC_FLAGS        = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -Wuninitialized
-  GCC:RELEASE_*_*_CC_FLAGS      = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -Wuninitialized
-  CLANGPDB:DEBUG_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern
-  CLANGPDB:NOOPT_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern
-  CLANGPDB:RELEASE_*_*_CC_FLAGS = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern
-  CLANGGCC:DEBUG_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern
-  CLANGGCC:NOOPT_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern
-  CLANGGCC:RELEASE_*_*_CC_FLAGS = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern
+  GCC:DEBUG_*_*_CC_FLAGS        = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -Wuninitialized -save-temps -Wno-parentheses-equality
+  GCC:NOOPT_*_*_CC_FLAGS        = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -Wuninitialized -save-temps -Wno-parentheses-equality
+  GCC:RELEASE_*_*_CC_FLAGS      = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -Wuninitialized -save-temps -Wno-parentheses-equality
+  CLANGPDB:DEBUG_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
+  CLANGPDB:NOOPT_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
+  CLANGPDB:RELEASE_*_*_CC_FLAGS = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
+  CLANGGCC:DEBUG_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
+  CLANGGCC:NOOPT_*_*_CC_FLAGS   = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
+  CLANGGCC:RELEASE_*_*_CC_FLAGS = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -mstack-protector-guard=global -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
   MSFT:DEBUG_*_*_CC_FLAGS       = $(OCPKG_BUILD_OPTIONS_GEN) /wd4723 /GS /kernel
   MSFT:NOOPT_*_*_CC_FLAGS       = $(OCPKG_BUILD_OPTIONS_GEN) /wd4723 /GS /kernel
   MSFT:RELEASE_*_*_CC_FLAGS     = $(OCPKG_BUILD_OPTIONS_GEN) /wd4723 /GS /kernel
-  XCODE:DEBUG_*_*_CC_FLAGS      = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -ftrivial-auto-var-init=pattern
-  XCODE:NOOPT_*_*_CC_FLAGS      = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -ftrivial-auto-var-init=pattern
-  XCODE:RELEASE_*_*_CC_FLAGS    = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -Oz -flto -fstack-protector-strong -ftrivial-auto-var-init=pattern
+  XCODE:DEBUG_*_*_CC_FLAGS      = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
+  XCODE:NOOPT_*_*_CC_FLAGS      = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -fstack-protector-strong -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
+  XCODE:RELEASE_*_*_CC_FLAGS    = $(OCPKG_BUILD_OPTIONS_GEN) $(OCPKG_ANAL_OPTIONS_GEN) -Oz -flto -fstack-protector-strong -ftrivial-auto-var-init=pattern -save-temps -Wno-parentheses-equality
 
   # Force page alignment for all files allowing for page protection.
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000

In OpenCoreMisc.c, the line

PatchPcdSetPtr (PcdSerialPciDeviceInfo, &PciDeviceInfoSize, OC_BLOB_GET (&Config->Misc.Serial.PciDeviceInfo));

is translated to

LibPatchPcdSetPtrAndSize ( (void *)_gPcd_BinaryPatch_PcdSerialPciDeviceInfo, &_gPcd_BinaryPatch_Size_PcdSerialPciDeviceInfo, (UINTN)_gPcd_BinaryPatch_MaxSize_PcdSerialPciDeviceInfo, (&PciDeviceInfoSize), ((((&Config->Misc.Serial.PciDeviceInfo)->DynValue) != ((void *) 0) ? ((&Config->Misc.Serial.PciDeviceInfo)->DynValue) : ((&Config->Misc.Serial.PciDeviceInfo)->Value))) );

according to OpenCoreMisc.i.

The Pcds are defined in AutoGen.c.

#define _PCD_TOKEN_PcdSerialPciDeviceInfo  0U
#define _PCD_PATCHABLE_VALUE_PcdSerialPciDeviceInfo  (VOID *)_gPcd_BinaryPatch_PcdSerialPciDeviceInfo
GLOBAL_REMOVE_IF_UNREFERENCED  UINT8 _gPcd_BinaryPatch_PcdSerialPciDeviceInfo[41] = {0xFF,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00};
extern  UINT8 _gPcd_BinaryPatch_PcdSerialPciDeviceInfo[41];
#define _PCD_GET_MODE_PTR_PcdSerialPciDeviceInfo  (VOID *)_gPcd_BinaryPatch_PcdSerialPciDeviceInfo
#define _PCD_PATCHABLE_PcdSerialPciDeviceInfo_SIZE 41
#define _PCD_GET_MODE_SIZE_PcdSerialPciDeviceInfo  _gPcd_BinaryPatch_Size_PcdSerialPciDeviceInfo 
extern UINTN _gPcd_BinaryPatch_Size_PcdSerialPciDeviceInfo; 
GLOBAL_REMOVE_IF_UNREFERENCED UINTN _gPcd_BinaryPatch_Size_PcdSerialPciDeviceInfo = 41;
GLOBAL_REMOVE_IF_UNREFERENCED const UINTN _gPcd_BinaryPatch_MaxSize_PcdSerialPciDeviceInfo = 41;
#define _PCD_SET_MODE_PTR_PcdSerialPciDeviceInfo(SizeOfBuffer, Buffer)  LibPatchPcdSetPtrAndSize((VOID *)_gPcd_BinaryPatch_PcdSerialPciDeviceInfo, &_gPcd_BinaryPatch_Size_PcdSerialPciDeviceInfo, (UINTN)_PCD_PATCHABLE_PcdSerialPciDeviceInfo_SIZE, (SizeOfBuffer), (Buffer))
#define _PCD_SET_MODE_PTR_S_PcdSerialPciDeviceInfo(SizeOfBuffer, Buffer)  LibPatchPcdSetPtrAndSizeS((VOID *)_gPcd_BinaryPatch_PcdSerialPciDeviceInfo, &_gPcd_BinaryPatch_Size_PcdSerialPciDeviceInfo, (UINTN)_PCD_PATCHABLE_PcdSerialPciDeviceInfo_SIZE, (SizeOfBuffer), (Buffer))

I couldn't find a macro in PcdLib.h that returns _gPcd_BinaryPatch_MaxSize_PcdSerialPciDeviceInfo or _PCD_PATCHABLE_PcdSerialPciDeviceInfo_SIZE in order to replace OC_SERIAL_PCI_DEVICE_INFO_MAX_SIZE.

Anyway, with the change to gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo in the OpenCorePkg.dsc file, the code is able to find the correct SerialRegisterBase for my PCIe serial port card, but I'm getting a hang after that even when Serial->Init is false so I'm trying to get past that.

@PMheart
Copy link
Member Author

PMheart commented Apr 3, 2022

Thanks. I committed the proper initialization at 22a852d. As for the -save-temps, I am afraid that it will significantly slow down the building process, thus not added.

Is it useful to change the failsafe value in OcConfigurationLib.h as well? In my opinion, no.

Also, if we specify OC_SERIAL_PCI_DEVICE_INFO_MAX_SIZE to be 41, then does PciDeviceInfo received from config always have to be this long? Or, we can fill the rest with zero code-wise too.

@joevt
Copy link

joevt commented Apr 4, 2022

I don't think -save-temps significantly slows the building process. You can compare with and without to be sure.

It is a useful tool for debugging but also for development. For example, you can open any *.i file and find any type definition very quickly and it shows which .h file that the type was defined in so you can go to it easily - especially useful if the type is defined in many similarly named *.h files.

I don't think the failsafe needs to be changed. The *.dsc defines the max size, not this failsafe.

PciDeviceInfo size just needs to be ≤ the max size and end with a 0xff. No need to fill with zeros. The zero fill in the *.dsc file is just to define the max size.

I am continuing testing. I added some DEBUG((DEBUG_INFO, "...")) to BaseSerialPortLib16550.c but maybe I need to remove some of it for OpenCore to load successfully... I'll let you know when I have it working.

FYI, This is what I'm using for Serial for my PCIe Serial port:

		<key>Serial</key>
		<dict>
			<key>BaudRate</key> <integer>115200</integer>
			<key>ClockRate</key> <integer>1843200</integer>
			<key>DetectCable</key> <false/>
			<key>ExtendedTxFifoSize</key> <integer>64</integer>
			<key>FifoControl</key> <integer>7</integer>
			<key>Init</key> <false/>
			<key>LineControl</key> <integer>3</integer>
			<key>PciDeviceInfo</key> <data>CQAAAAAAAAABAAAAAAAAAP8=</data>
			<key>RegisterAccessWidth</key> <integer>8</integer>
			<key>RegisterBase</key> <integer>0</integer>
			<key>RegisterStride</key> <integer>1</integer>
			<key>UseHardwareFlowControl</key> <false/>
			<key>UseMmio</key> <false/>
		</dict>

@joevt
Copy link

joevt commented Apr 4, 2022

I removed my UDK folder to get a clean checkout and recompiled. It appears to be working now.

In the attached serial output capture OC serial enable true working.txt.zip, you can see that all the messages sent to console are now also sent to serial with a time stamp.

I wonder if there's any messages sent to serial that are not also sent to console or vice versa? In other words, which method is best? With console redirection, it can capture output from EFI Shell and boot.efi. What benefits does serial have over console redirection? I'll do another test without console redirection to make sure it works with a more normal setup.

I found some lines that only exist from console.

  • The lines from PciSioSerialDxe listed below are console only.
  • The last message OCUI: UseDiskLabel: 0, UseGenericLabel: 0 appears to be console only but all previous occurrences of that line exist from serial also.
  • booter messages starting from #[EB|LOG:VERBOSE] 2022-04-04T05:46:09 are output 3 times. Once from console directly (from boot.efi?), once from console (OpenCore), and once from serial (OpenCore).
  • Serial output stops when xnu starts outputting (after ExitbootServices?). I think that's expected. BaseSerialPortLib16550.c looks like it does too much to work beyond ExitbootServices? The only thing I would think to use it for is from inside the nvram read and write runtime service functions.

When OpenCore connects drivers, it makes PciSioSerialDxe.efi reinit the serial port.

ClockRate = 1843200
Divisor   = 1
BaudRate/Actual (115200/115200) = 100%
ClockRate = 1843200
Divisor   = 1
BaudRate/Actual (115200/115200) = 100%
PciSioSerial: Create PCI child serial device (single) - Success
ClockRate = 1843200
Divisor   = 1
BaudRate/Actual (115200/115200) = 100%
ClockRate = 1843200
Divisor   = 1
BaudRate/Actual (115200/115200) = 100%

So I'll disable PciSioSerialDxe.efi (which I load using Driver#### instead of OpenCore) to make sure it's not required for serial output (it's used for console redirection since my MacPro3,1 doesn't come with a serial driver).

In my previous comment posted earlier today, you see that I didn't have standard whitespace in my plist (the value follows </key> on the same line). Does OpenCore handle arbitrary white space in plist? Does it handle having no white space? I ask because I got some panics in a previous test. Maybe I'll test that more later.

@joevt
Copy link

joevt commented Apr 4, 2022

I think this is my final test. This time I'm not using PciSioSerialDxe.efi and console redirection to serial port. Console is set to display only.
OC serial enable true working without console redirection.txt.zip

One good thing is that the console output to my 2560x1600 display is no longer reduced to 80x25 characters (as I described in acidanthera/bugtracker#1954 )

As you can see in the attached output, my xnu patch to have serial output go to the PCIe serial port still works (xnu output starts at kprintf initialized ) It works because the serial port is initialized using OpenCore Serial->Init (previously I was initializing it with PciSioSerialDxe.efi)

I guess the only thing missing from this serial setup would be output from EFI Shell or any drivers that might output to console.

@PMheart
Copy link
Member Author

PMheart commented Apr 4, 2022

Thanks again for testing. Shall we integrate the IOPCIFamily (sorry, should be XNU) patch as well? Or is there anything else?

UPDATE - #331

@joevt
Copy link

joevt commented Apr 4, 2022

Shall we integrate the IOPCIFamily patch as well? Or is there anything else?

Which IOPCIFamily patch? We've done everything we need to do for Serial in OpenCore. The rest of the issues/questions in acidanthera/bugtracker#1954 are mostly related to console redirection in EFI or serial output from macOS.

@vit9696
Copy link
Contributor

vit9696 commented Apr 4, 2022

@joevt looks good to me, shall we merge this?

@joevt
Copy link

joevt commented Apr 4, 2022

Yes.

@PMheart PMheart merged commit 6825e9a into master Apr 4, 2022
@PMheart PMheart deleted the pcd-test branch April 4, 2022 21:43
@PMheart
Copy link
Member Author

PMheart commented Apr 4, 2022

Done.

PMheart added a commit that referenced this pull request Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants