Jump to content

Clover Problems and Solutions

ErmaC

By ErmaC,
in Clover Development

 Share
3,206 posts in this topic

Recommended Posts

Sherlocks
Sherlocks
Posted
Posted

I will look into the interactions between HWPEnable and Generate/PluginType=true to make sure they make sense. (Generate/PluginType=false should override any CpuPm/PluginType=1 inject forced by HWPEnable).

 

in current code, except HWPEnable case, looks good. i checked it before on sandy system and skylake system according to each options.

  • Like 1
Link to comment
Share on other sites
RehabMan
RehabMan
Posted
Posted

in current code, except HWPEnable case, looks good. i checked it before on sandy system and skylake system according to each options.

Even though it is a potential backward compatibility "break", I'm tempted to remove the automatic PluginType=1 inject when HWPEnable is set. If the user wants PluginType=1 inject, they can just use Generate/PluginType=true (along with HWPEnable, if they are using it).

Are you thinking the same?

Link to comment
Share on other sites
Sherlocks
Sherlocks
Posted
Posted

Even though it is a potential backward compatibility "break", I'm tempted to remove the automatic PluginType=1 inject when HWPEnable is set. If the user wants PluginType=1 inject, they can just use Generate/PluginType=true (along with HWPEnable, if they are using it).

Are you thinking the same?

 

current code is good except HWPEnable. Clover always use P/C States options for ivy+ since long time ago. default enabled P/C states with plugintype1 for only ivy+.

if user want to use only plugin-type1, have to use this in config.plist

PStates false(in this case diasbled APLF/APSF. no need consider)

CStates false

PluginType true

 

you can see clover log that i added to check it for debug on sandy and skylake system.

https://sourceforge.net/p/cloverefiboot/code/4324/tree/rEFIt_UEFI/Platform/StateGenerator.c#l430

 

EDIT1.

i think that need to check stategenerator.c again.

for this case

PStates false(in this case diasbled APLF/APSF. no need consider)

CStates false

PluginType true

 

log shown

2:781  0:000  CPUBase=0 and ApicCPUBase=1 ApicCPUNum=8
2:781  0:000  Maximum control=0x17
2:781  0:000  Turbo control=0x17
2:781  0:000  P-States: min 0x4, max 0x17
2:781  0:000  SSDT with plugin-type without P-States is generated
 
whether this part actually added in ssdt 
2:781  0:000  Maximum control=0x17
2:781  0:000  Turbo control=0x17
2:781  0:000  P-States: min 0x4, max 0x17
 

or only plugin-type1

2:781  0:000  SSDT with plugin-type without P-States is generated

 

EDIT2.

seems to add P-States info.

i will debug

 

EDIT3.

i checked it with ssdt table. clover is still good working. logic is no problem.

CLOVER plugintype.zip

Link to comment
Share on other sites
RehabMan
RehabMan
Posted
Posted

@Slice

 

Another update for AcpiPatcher.c (and a few other files)

 

This one corrects a problem with config.plist/ACPI/DSDT/Patches being applied to non-merged SSDTs from ACPI/patched (eg. "add-on" SSDTs) when there are one or more NULL pointers in the XSDT.

As part of that, I also did a bit of cleanup on AcpiPatcher.c and some related files.

The commit is here:

https://github.com/RehabMan/Clover/commit/02ad74860b4632f902a540b0804af9a43cdddbe4

 

Affected files:

AcpiPatcher.c

FixBiosDsdt.c

Platform.h

StateGenerator.c

 

It is probably easier to just import each file into the svn repo, especially for the complex set of changes in AcpiPatcher.c.

 

Patching the SSDTs and loading ACPI/patched content is now in these distinct steps:

- "pre-cleanup" PreCleanupRSDT and PreCleanupXSDT. These functions deal with the strange XSDT overlap with RSDT, and the double-NULL terminate (would be nice to see an XSDT or RSDT that has the double NULL followed by other entries!).

- load ACPI/patched that are merged into XSDT (eg. ACPI/patched content that matches existing entries, AutoMerge=true only)

- drop any tables matching config.plist/ACPI/DropTables by replacing the pointer in XSDT/RDST with NULL

(this method of dropping simplifies later steps... no need for XsdtOriginalEntryCount, and no need to adjust XsdtReplaceSizes, for example). Note: I think I should add code that prevents dropping a merged table (a bit of user error)... will add as a separate commit later.

- load ACPI/patched content that are add-on tables (eg. ACPI/patched content not matching earlier)

- "post-cleanup" PostCleanupRSDT and PostCleanupXSDT. These deal with any NULL entries in RSDT and XSDT, including NULL entries created by DropTables, or NULL entries that existed in native. NULL entries are handled correctly, even when not at the end of the table (contrary to the original code).

 

The next step will be to make a new feature that will dump a complete set of files (ACPI/to_inject), but post-patch (eg. like ACPI/origin, but after going through the entire Clover patching process). That will require a bit more code cleanup (globals!).

  • Like 6
Link to comment
Share on other sites
RehabMan
RehabMan
Posted
Posted

Note: I think I should add code that prevents dropping a merged table (a bit of user error)... will add as a separate commit later.

Fixed this issue (attempt to drop a merged table):

https://github.com/RehabMan/Clover/commit/53c638f060f8b853161b5cc98e9e01651fffb274

 

One file affected: AcpiPatcher.c

  • Like 1
Link to comment
Share on other sites
RehabMan
RehabMan
Posted
Posted

Don't thank me.  Thank GCC 7.2.1.  Do you seriously think I'd have found that myself?  I'm not that bored.

Ah... I see.

I suspect there is some xcode/clang compiler warnings we could turn on that might help catch that sort of thing.

 

BTW, how difficult to get setup with newer GCC? (I think current script assumes/setups/builds GCC53).

Link to comment
Share on other sites
Zenith432
Zenith432
Posted
Posted

Where did you find the 7.2.1, I thought 7.2.0 was latest.

 

Using built-in specs.

COLLECT_GCC=gcc

COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/7/lto-wrapper

OFFLOAD_TARGET_NAMES=nvptx-none

OFFLOAD_TARGET_DEFAULT=1

Target: x86_64-redhat-linux

Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla--enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl --enable-libmpx --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux

Thread model: posix

gcc version 7.2.1 20170915 (Red Hat 7.2.1-2) (GCC)

Link to comment
Share on other sites
Zenith432
Zenith432
Posted
Posted

Ok, the problem is actually in svn commit 25815 that adds some code to GenSec.c that doesn't compile with Xcode.  The tool wasn't built, so I didn't have it which is why the build broke.

 

Here's a patch to fix GenSec.c

diff a/BaseTools/Source/C/GenSec/GenSec.c b/BaseTools/Source/C/GenSec/GenSec.c
--- a/BaseTools/Source/C/GenSec/GenSec.c
+++ b/BaseTools/Source/C/GenSec/GenSec.c
@@ -1034,9 +1034,9 @@ Returns:
   CHAR8                     *DummyFileName;
   FILE                      *DummyFile;
   UINTN                     DummyFileSize;
-  UINT8                     *DummyFileBuffer;
+  CHAR8                     *DummyFileBuffer;
   FILE                      *InFile;
-  UINT8                     *InFileBuffer;
+  CHAR8                     *InFileBuffer;
   UINTN                     InFileSize;
 
   InputFileAlign        = NULL;
@@ -1326,13 +1326,13 @@ Returns:
       DummyFile = fopen (LongFilePath (DummyFileName), "rb");
       if (DummyFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", DummyFileName);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (DummyFile, 0, SEEK_END);
       DummyFileSize = ftell (DummyFile);
       fseek (DummyFile, 0, SEEK_SET);
-      DummyFileBuffer = (UINT8 *) malloc (DummyFileSize);
+      DummyFileBuffer = (CHAR8 *) malloc (DummyFileSize);
       fread(DummyFileBuffer, 1, DummyFileSize, DummyFile);
       fclose(DummyFile);
       DebugMsg (NULL, 0, 9, "Dummy files", "the dummy file name is %s and the size is %u bytes", DummyFileName, (unsigned) DummyFileSize);
@@ -1340,13 +1340,13 @@ Returns:
       InFile = fopen(LongFilePath(InputFileName[0]), "rb");
       if (InFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", InputFileName[0]);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (InFile, 0, SEEK_END);
       InFileSize = ftell (InFile);
       fseek (InFile, 0, SEEK_SET);
-      InFileBuffer = (UINT8 *) malloc (InFileSize);
+      InFileBuffer = (CHAR8 *) malloc (InFileSize);
       fread(InFileBuffer, 1, InFileSize, InFile);
       fclose(InFile);
       DebugMsg (NULL, 0, 9, "Input files", "the input file name is %s and the size is %u bytes", InputFileName[0], (unsigned) InFileSize);
  • Like 4
Link to comment
Share on other sites
RehabMan
RehabMan
Posted
Posted

Ok, the problem is actually in svn commit 25815 that adds some code to GenSec.c that doesn't compile with Xcode.  The tool wasn't built, so I didn't have it which is why the build broke.

 

Here's a patch to fix GenSec.c

diff a/BaseTools/Source/C/GenSec/GenSec.c b/BaseTools/Source/C/GenSec/GenSec.c
--- a/BaseTools/Source/C/GenSec/GenSec.c
+++ b/BaseTools/Source/C/GenSec/GenSec.c
@@ -1034,9 +1034,9 @@ Returns:
   CHAR8                     *DummyFileName;
   FILE                      *DummyFile;
   UINTN                     DummyFileSize;
-  UINT8                     *DummyFileBuffer;
+  CHAR8                     *DummyFileBuffer;
   FILE                      *InFile;
-  UINT8                     *InFileBuffer;
+  CHAR8                     *InFileBuffer;
   UINTN                     InFileSize;
 
   InputFileAlign        = NULL;
@@ -1326,13 +1326,13 @@ Returns:
       DummyFile = fopen (LongFilePath (DummyFileName), "rb");
       if (DummyFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", DummyFileName);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (DummyFile, 0, SEEK_END);
       DummyFileSize = ftell (DummyFile);
       fseek (DummyFile, 0, SEEK_SET);
-      DummyFileBuffer = (UINT8 *) malloc (DummyFileSize);
+      DummyFileBuffer = (CHAR8 *) malloc (DummyFileSize);
       fread(DummyFileBuffer, 1, DummyFileSize, DummyFile);
       fclose(DummyFile);
       DebugMsg (NULL, 0, 9, "Dummy files", "the dummy file name is %s and the size is %u bytes", DummyFileName, (unsigned) DummyFileSize);
@@ -1340,13 +1340,13 @@ Returns:
       InFile = fopen(LongFilePath(InputFileName[0]), "rb");
       if (InFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", InputFileName[0]);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (InFile, 0, SEEK_END);
       InFileSize = ftell (InFile);
       fseek (InFile, 0, SEEK_SET);
-      InFileBuffer = (UINT8 *) malloc (InFileSize);
+      InFileBuffer = (CHAR8 *) malloc (InFileSize);
       fread(InFileBuffer, 1, InFileSize, InFile);
       fclose(InFile);
       DebugMsg (NULL, 0, 9, "Input files", "the input file name is %s and the size is %u bytes", InputFileName[0], (unsigned) InFileSize);

 

I didn't run into any issue.

Building with Xcode 9.2.

Link to comment
Share on other sites
Sherlocks
Sherlocks
Posted
Posted

@Zenith432

does clover have boot0xg now?
i saw your work
Zenith432 : Replace boot0 with boot0xg. Now boot0xg has all features of previous boot0.
 
long time ago, i tested clover legacy on Phoenix Tiano Secure Core Bios in DELL XPS L502X.
always stop
Boot1 : /boot
6 and sometimes 7
never pass this and can't get CLOVER GUI
recently, user reported same issue
 
chameleon bootloader is no problem on Phoenix Tiano Secure Core Bios. 
in clover, must be use custom bios with patched UEFI. without this, user never used clover.
it also mentioned in here
 
can we use boot0xg as option in clover pkg or implement?
Link to comment
Share on other sites
Slice
Slice
Posted
Posted

 

 
long time ago, i tested clover legacy on Phoenix Tiano Secure Core Bios in DELL XPS L502X.
always stop
Boot1 : /boot
6 and sometimes 7
never pass this and can't get CLOVER GUI
recently, user reported same issue
 
 

See explanation of commit 4331. Legacy clover stops at 6_ before this revision.

The bug in EDK2 BaseLib is from 2014 year.

 

Ok, the problem is actually in svn commit 25815 that adds some code to GenSec.c that doesn't compile with Xcode.  The tool wasn't built, so I didn't have it which is why the build broke.

 

Here's a patch to fix GenSec.c

diff a/BaseTools/Source/C/GenSec/GenSec.c b/BaseTools/Source/C/GenSec/GenSec.c
--- a/BaseTools/Source/C/GenSec/GenSec.c
+++ b/BaseTools/Source/C/GenSec/GenSec.c
@@ -1034,9 +1034,9 @@ Returns:
   CHAR8                     *DummyFileName;
   FILE                      *DummyFile;
   UINTN                     DummyFileSize;
-  UINT8                     *DummyFileBuffer;
+  CHAR8                     *DummyFileBuffer;
   FILE                      *InFile;
-  UINT8                     *InFileBuffer;
+  CHAR8                     *InFileBuffer;
   UINTN                     InFileSize;
 
   InputFileAlign        = NULL;
@@ -1326,13 +1326,13 @@ Returns:
       DummyFile = fopen (LongFilePath (DummyFileName), "rb");
       if (DummyFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", DummyFileName);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (DummyFile, 0, SEEK_END);
       DummyFileSize = ftell (DummyFile);
       fseek (DummyFile, 0, SEEK_SET);
-      DummyFileBuffer = (UINT8 *) malloc (DummyFileSize);
+      DummyFileBuffer = (CHAR8 *) malloc (DummyFileSize);
       fread(DummyFileBuffer, 1, DummyFileSize, DummyFile);
       fclose(DummyFile);
       DebugMsg (NULL, 0, 9, "Dummy files", "the dummy file name is %s and the size is %u bytes", DummyFileName, (unsigned) DummyFileSize);
@@ -1340,13 +1340,13 @@ Returns:
       InFile = fopen(LongFilePath(InputFileName[0]), "rb");
       if (InFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", InputFileName[0]);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (InFile, 0, SEEK_END);
       InFileSize = ftell (InFile);
       fseek (InFile, 0, SEEK_SET);
-      InFileBuffer = (UINT8 *) malloc (InFileSize);
+      InFileBuffer = (CHAR8 *) malloc (InFileSize);
       fread(InFileBuffer, 1, InFileSize, InFile);
       fclose(InFile);
       DebugMsg (NULL, 0, 9, "Input files", "the input file name is %s and the size is %u bytes", InputFileName[0], (unsigned) InFileSize);

Why not commit to Patches_to_EDK2?

Building and working fine with latest edk2 from github.

But I don't use Patches_for_EDK2.

You just didn't check those bugs.

This doesn't happen to me, but I use vanilla shell from edk2/ShellBinPkg/UefiShell/X64/Shell.efi

Full functional help command?

What about

Shell> edit config.plist

?

  • Like 1
Link to comment
Share on other sites
Sherlocks
Sherlocks
Posted
Posted

See explanation of commit 4331. Legacy clover stops at 6_ before this revision.

The bug in EDK2 BaseLib is from 2014 year.

 

I did not suspect EDK2. I mistakenly thought I could solve it only with boot0xg before. I did not know that this problem had been since 2014. clover have had these bugs for quite a long time. Glad to hear that this problem has been solved. thanks

Link to comment
Share on other sites
Philip Petev
Philip Petev
Posted
Posted

 

Ok, the problem is actually in svn commit 25815 that adds some code to GenSec.c that doesn't compile with Xcode.  The tool wasn't built, so I didn't have it which is why the build broke.

 

Here's a patch to fix GenSec.c

diff a/BaseTools/Source/C/GenSec/GenSec.c b/BaseTools/Source/C/GenSec/GenSec.c
--- a/BaseTools/Source/C/GenSec/GenSec.c
+++ b/BaseTools/Source/C/GenSec/GenSec.c
@@ -1034,9 +1034,9 @@ Returns:
   CHAR8                     *DummyFileName;
   FILE                      *DummyFile;
   UINTN                     DummyFileSize;
-  UINT8                     *DummyFileBuffer;
+  CHAR8                     *DummyFileBuffer;
   FILE                      *InFile;
-  UINT8                     *InFileBuffer;
+  CHAR8                     *InFileBuffer;
   UINTN                     InFileSize;
 
   InputFileAlign        = NULL;
@@ -1326,13 +1326,13 @@ Returns:
       DummyFile = fopen (LongFilePath (DummyFileName), "rb");
       if (DummyFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", DummyFileName);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (DummyFile, 0, SEEK_END);
       DummyFileSize = ftell (DummyFile);
       fseek (DummyFile, 0, SEEK_SET);
-      DummyFileBuffer = (UINT8 *) malloc (DummyFileSize);
+      DummyFileBuffer = (CHAR8 *) malloc (DummyFileSize);
       fread(DummyFileBuffer, 1, DummyFileSize, DummyFile);
       fclose(DummyFile);
       DebugMsg (NULL, 0, 9, "Dummy files", "the dummy file name is %s and the size is %u bytes", DummyFileName, (unsigned) DummyFileSize);
@@ -1340,13 +1340,13 @@ Returns:
       InFile = fopen(LongFilePath(InputFileName[0]), "rb");
       if (InFile == NULL) {
         Error (NULL, 0, 0001, "Error opening file", InputFileName[0]);
-        return EFI_ABORTED;
+        return STATUS_ERROR;
       }
 
       fseek (InFile, 0, SEEK_END);
       InFileSize = ftell (InFile);
       fseek (InFile, 0, SEEK_SET);
-      InFileBuffer = (UINT8 *) malloc (InFileSize);
+      InFileBuffer = (CHAR8 *) malloc (InFileSize);
       fread(InFileBuffer, 1, InFileSize, InFile);
       fclose(InFile);
       DebugMsg (NULL, 0, 9, "Input files", "the input file name is %s and the size is %u bytes", InputFileName[0], (unsigned) InFileSize);

No need for this patch anymore, the bug has been fixed upstream. Just tested the latest edk2 code with the patches applied and Clover builds fine.

  • Like 4
Link to comment
Share on other sites
Slice
Slice
Posted
Posted

What bugs are you referring to?

Each patch for EDK2 is related to some bug noticed by users.

The last one is corrected by rev 4331  #91 

The folder Patches_for_EDK2 exists because we can not correct EDK2 repository.

Link to comment
Share on other sites
Zenith432
Zenith432
Posted
Posted

@Zenith432

does clover have boot0xg now?

i saw your work

http://www.insanelymac.com/forum/topic/302938-exfat-volume-boot-record-for-chameleon

http://forge.voodooprojects.org/p/chameleon/source/tree/HEAD/branches/Bungo/CHANGES

Zenith432 : Replace boot0 with boot0xg. Now boot0xg has all features of previous boot0.

I don't know. I wrote boot0xg for Chameleon, and any import of it for Clover is not my work.

 

Why not commit to Patches_to_EDK2?

It was just temporary until edk2 fix it.

 

Full functional help command?

What about

Shell> edit config.plist

?

Didn't know about these things.
Link to comment
Share on other sites
 Share
Go to topic listing
×
×
  • Create New...