Clover Problems and Solutions

[Zenith432]

I tried arthur-pt's CLOVERX64.efi, but can't reproduce the problem.  I'll spare the pictures, because they look the same (normal).
I checked the uploaded photos with a pixel color meter.
The blue background at the bottom has pixel (R,G,B ) = (0, 0, 0.19)
The grey background at the top has pixel (R,G,B ) = (0.75, 0.75, 0.75)
So it should be possible to locate the code that paints these two colors, and add some debug logs to see what's going on.
Notice also that in the photos with the bug - the text at the bottom (F1: Help and version) doesn't appear on top of the blue background.  To the code to paint them doesn't get done.
Anyway, I looked at egClearScreen, and it looks ok, including the external calls to Blt() which are EFIAPI functions.  So no clue yet.

And what about this one? http://www.insanelymac.com/forum/topic/284656-clover-general-discussion/page-783?p=2537527&do=findComment&comment=2537527

EDIT: Correction.  I checked again, the pixels have different color in your photo and arthur-pt's photo

arthur-pt's photo
blue (0, 0, 0.19)
grey (0.75, 0.75, 0.75)

yours
blue (0, 0, 0.13)
grey (0.8, 0.8, 0.8)

 

Also, I checked with magnification and colorometer, and the text "F1: Help" and version do appear on top of the blue background.  They're just very difficult to see.

Edited November 21, 2017 by Zenith432

[Slice]

I tried arthur-pt's CLOVERX64.efi, but can't reproduce the problem.  I'll spare the pictures, because they look the same (normal).

I checked the uploaded photos with a pixel color meter.

The blue background at the bottom has pixel (R,G,B ) = (0, 0, 0x19)

The grey background at the top has pixel (R,G,B ) = (0x75, 0x75, 0x75)

So it should be possible to locate the code that paints these two colors, and add some debug logs to see what's going on.

Notice also that in the photos with the bug - the text at the bottom (F1: Help and version) doesn't appear on top of the blue background.  To the code to paint them doesn't get done.

Anyway, I looked at egClearScreen, and it looks ok, including the external calls go Blt() which are EFIAPI functions.  So no clue yet.

I have an idea the the problem is in Blt() function which is inluded in legacy clover (boot file) or in CsmVideoDxe.

Is it true that you didn't use them? So why you have no bug.

I see the bug in QEMU which booted with legacy Clover.

[Zenith432]

I boot UEFI, not legacy.  Haven't tried legacy in months and not sure whether it still works.  Don't you have UEFI boot you can try on?

According to "drivers" list in shell, my EFI firmware also uses bios int 10 (i.e. legacy video), but it's board supplied driver.

I have an idea the the problem is in Blt() function which is inluded in legacy clover (boot file) or in CsmVideoDxe.

Is it true that you didn't use them? So why you have no bug.

I see the bug in QEMU which booted with legacy Clover.

[Slice]

I can check UEFI boot at evening.

Strange that screen filled partially every time different fill. Undefined buffer size or some timeout? If timeout then it will not affect fast graphics. It explains why it affected background but not small images. The background is largest image. But who set the timeout?

[Zenith432]

@Slice, please add following debug logs to screen.c so can tell what it's doing.  I tested with XCODE8 on my system and the logs look ok.

diff a/rEFIt_UEFI/refit/screen.c b/rEFIt_UEFI/refit/screen.c
--- a/rEFIt_UEFI/refit/screen.c
+++ b/rEFIt_UEFI/refit/screen.c
@@ -478,6 +478,9 @@ VOID BltClearScreen(IN BOOLEAN ShowBanner) //ShowBanner always TRUE
   }
   
   if (BackgroundImage == NULL) {
+    DBG("BltClearScreen(%c): calling egCreateFilledImage UGAWidth %ld, UGAHeight %ld, BlueBackgroundPixel %02x%02x%02x%02x\n",
+        ShowBanner?'Y':'N', UGAWidth, UGAHeight,
+        BlueBackgroundPixel.r, BlueBackgroundPixel.g, BlueBackgroundPixel.b, BlueBackgroundPixel.a);
     BackgroundImage = egCreateFilledImage(UGAWidth, UGAHeight, FALSE, &BlueBackgroundPixel);
   }
   
@@ -531,8 +534,12 @@ VOID BltClearScreen(IN BOOLEAN ShowBanner) //ShowBanner always TRUE
   
   // Draw background
   if (BackgroundImage) {
+    DBG("BltClearScreen(%c): calling BltImage BackgroundImage %p\n",
+        ShowBanner?'Y':'N', BackgroundImage);
     BltImage(BackgroundImage, 0, 0); //if NULL then do nothing
   } else {
+    DBG("BltClearScreen(%c): calling egClearScreen StdBackgroundPixel %02x%02x%02x%02x\n",
+        ShowBanner?'Y':'N', StdBackgroundPixel.r, StdBackgroundPixel.g, StdBackgroundPixel.b, StdBackgroundPixel.a);
     egClearScreen(&StdBackgroundPixel);
   }

[RehabMan]

FYI: There are some really incorrect changes to AcpiPatcher.c in r4302.

 

In particular, this one:

-        UINTN Index = entry - &Xsdt->Entry;
+        UINTN Index = entry - (UINT64 *)(((UINT8 *)Xsdt) + OFFSET_OF(XSDT_TABLE, Entry));

And this '~(UINT)0' works, but is a very strange way to write '-1':

-      FileName = GenerateFileName(FileNamePrefix, *SsdtCount, -1, OemTableId);
+      FileName = GenerateFileName(FileNamePrefix, *SsdtCount, ~(UINTN)0, OemTableId);

(that unnecessary/confusing change happens twice)

[Zenith432]

The first one looks like an identity transformation and I see no reason for it, nor does it break anything.  The second one is right.  STDC mandates that unsigned integer be represented as radix-2 bit strings, but does not mandate that signed integers be represented using 2's complement.  So ~(UINTN)0 is always the maximum positive UINTN.  However, -1 is an int constant that gets promoted to INTN, and then has its bit-representation reinterpreted as UINTN.  This works for 2's complement, but is not STDC-compliant.

[apianti]

The first one is needed because it's getting the address of an unaligned address offset and is a compiler error in visual studio... lol. The second one is because -1 is not necessarily the largest unsigned number...

 

EDIT: Zenith432 beat me to it.

[RehabMan]

The first one is needed because it's getting the address of an unaligned address offset and is a compiler error in visual studio... lol. The second one is because -1 is not necessarily the largest unsigned number...

Strange that VS finds an error there.

It is just typical pointer math.

 

I will make a note in my fork:

        //REVIEW: same as: UINTN Index = entry - &Xsdt->Entry;
        UINTN Index = entry - (UINT64 *)(((UINT8 *)Xsdt) + OFFSET_OF(XSDT_TABLE, Entry));

The second one is right. STDC mandates that unsigned integer be represented as radix-2 bit strings, but does not mandate that signed integers be represented using 2's complement. So ~(UINTN)0 is always the maximum positive UINTN. However, -1 is an int constant that gets promoted to INTN, and then has its bit-representation reinterpreted as UINTN. This works for 2's complement, but is not STDC-compliant.

Not sure why we are concerned with portability to mythical non-2's complement computers.

Note comparisons against -1 in ScanXSDT2...

[apianti]

Strange that VS finds an error there.

It is just typical pointer math.

 

Not sure why we are concerned with portability to mythical non-2's complement computers.

Note comparisons against -1 in ScanXSDT2...

 

 

It is because the address of the struct instance + offset of the member is not aligned on 64bit boundary but is supposed to be. On certain architectures that will fail, like ARM and IA64. Not really targeting those, but none the less the error still happens, haha. As for the -1 change, it's because it needed to be typecasted, and I just automatically changed -1 to ~(UINTN)0 assuming that's what was actually intended - the highest unsigned. Plus I'm unsure what the rules of that typecasting would be from (UINTN)-1, does it get extended first and then converted to unsigned or converted unsigned and then extended? I just took the safer route that I knew the outcome.

[Zenith432]

@Slice:

 

I think I found the reason for the problem in #2636, but can't be sure because problem is not reproducible on my system.  Please update to Clover r4312, reapply patch to Conf/tools_def.txt, and then do clean rebuild with XCODE8.  If this solves it I'll explain what was going on, because it's long.

 

Edit: Ok, maybe just a little hint from here

 

Signal handlers are executed on the same stack, but 128 bytes known as the red zone is subtracted from the stack before anything is pushed to the stack. This allows small leaf functions to use 128 bytes of stack space without reserving stack space by subtracting from the stack pointer. The red zone is well-known to cause problems for x86-64 kernel developers, as the CPU itself doesn't respect the red zone when calling interrupt handlers. This leads to a subtle kernel breakage as the ABI contradicts the CPU behavior. The solution is to build all kernel code with -mno-red-zone or by handling interrupts in kernel mode on another stack than the current (and thus implementing the ABI).

Edited November 21, 2017 by Zenith432

[Zenith432]

Plus I'm unsure what the rules of that typecasting would be from (UINTN)-1, does it get extended first and then converted to unsigned or converted unsigned and then extended?

The rules of STDC are rigid enough to dictate that a signed int would first get promoted to a larger-sized signed int, and finally reinterpreted as unsigned. For 2's complement, the promotion part is to sign-extend.  For 1's-complement or sign-magnitude (which are also permitted), you'd get a different bit string as a result.

[Slice]

@Slice:

 

I think I found the reason for the problem in #2636, but can't be sure because problem is not reproducible on my system.  Please update to Clover r4312, reapply patch to Conf/tools_def.txt, and then do clean rebuild with XCODE8.  If this solves it I'll explain what was going on, because it's long.

 

Edit: Ok, maybe just a little hint from here

Yes, the problem resolved!

If still interesting there is debug.log for post #2663

3:432  0:004  Icon 21 decoded, pointer 7D60F598
3:433  0:001  BltClearScreen(Y): calling egCreateFilledImage UGAWidth 1024, UGAHeight 768, BlueBackgroundPixel BFBFBFFF
3:445  0:011  BltClearScreen(Y): calling BltImage BackgroundImage 7D60F498
3:557  0:112  Icon 22 decoded, pointer 7D60F398
3:560  0:003  Icon 23 decoded, pointer 7D60F298
4:104  0:544  Icon 11 decoded, pointer 7D5E1E98
4:106  0:001  GUI ready

Thanks for the victory!

[Zenith432]

Imagine a hypothetical leaf function that fills a buffer with a pixel color.  Imagine that this function stores the value of the pixel color just under the stack pointer, in its red zone, without adjusting the stack pointer.  Imagine that a timer interrupt drops by to visit while our leaf function is filling the buffer.  Imagine that the rude interrupt handler stomps the pixel color, leaving the red value at zero, the green value at zero, and the blue value equal to the GDT selector of the code segment that happens to be in use by UEFI firmware.  What would this buffer look like if blitted to a screen?

 

If you have a fast enough CPU, you may never get to see it.

 

I was going to paste the code and disassembly which would have made it a long explanation.  I think enough said.

 

This bug is my fault.  -mno-red-zone is in the GCC build CC_FLAGS, and I missed how important it is for writing kernel-mode code with sysv abi.

[RehabMan]

As for the -1 change, it's because it needed to be typecasted, and I just automatically changed -1 to ~(UINTN)0 assuming that's what was actually intended - the highest unsigned. Plus I'm unsure what the rules of that typecasting would be from (UINTN)-1, does it get extended first and then converted to unsigned or converted unsigned and then extended? I just took the safer route that I knew the outcome.

I will cleanup the code to avoid the use of -1 and instead use a clearly defined macro that evaluates to a unique value that is impossible as a real index, as per intention.

 

BTW, the ScanXSDT2 I wrote is buggy. The intention was to treat MatchIndex as an index for tables with the given signature, ignoring the OemTableId, and that is not what it is doing. My tests were evidently done on a computer that had the same OemTableId for all SSDTs. I will submit a fix when I have it done and tested.

 

diffs(untested):

diff --git a/rEFIt_UEFI/Platform/AcpiPatcher.c b/rEFIt_UEFI/Platform/AcpiPatcher.c
index 5568fc12..679e162f 100644
--- a/rEFIt_UEFI/Platform/AcpiPatcher.c
+++ b/rEFIt_UEFI/Platform/AcpiPatcher.c
@@ -37,6 +37,8 @@ Re-Work by Slice 2011.
 #define APPLE_OEM_TABLE_ID  { 'A', 'p', 'p', 'l', 'e', '0', '0', ' ' }
 #define APPLE_CREATOR_ID    { 'L', 'o', 'k', 'i' }
 
+#define IGNORE_INDEX    (-1) // index ignored for matching (not ignored for >= 0)
+
 CONST CHAR8  oemID[6]       = APPLE_OEM_ID;
 CONST CHAR8  oemTableID[8]  = APPLE_OEM_TABLE_ID;
 CONST CHAR8  creatorID[4]   = APPLE_CREATOR_ID;
@@ -219,11 +221,10 @@ UINT32* ScanRSDT2(UINT32 Signature, UINT64 TableId, INTN MatchIndex)
   EntryCount = (Rsdt->Header.Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
   EntryPtr = &Rsdt->Entry;
   for (Index = 0; Index < EntryCount; Index++, EntryPtr++) {
-    TableEntry = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
-    if (((Signature == 0) || (TableEntry->Signature == Signature)) &&
-      ((TableId == 0) || (TableEntry->OemTableId == TableId))) {
-      if (-1 == MatchIndex || Count == MatchIndex) {
-        return EntryPtr; //point to TableEntry entry
+    TableEntry = (EFI_ACPI_DESCRIPTION_HEADER*)(UINTN)*EntryPtr;
+    if (0 == Signature || TableEntry->Signature == Signature) {
+      if ((0 == TableId || TableEntry->OemTableId == TableId) && (IGNORE_INDEX == MatchIndex || Count == MatchIndex)) {
+        return EntryPtr; //pointer to the TableEntry entry
       }
       ++Count;
     }
@@ -234,7 +235,7 @@ UINT32* ScanRSDT2(UINT32 Signature, UINT64 TableId, INTN MatchIndex)
 
 UINT32* ScanRSDT(UINT32 Signature, UINT64 TableId)
 {
-  return ScanRSDT2(Signature, TableId, -1);
+  return ScanRSDT2(Signature, TableId, IGNORE_INDEX);
 }
 
 UINT64* ScanXSDT2(UINT32 Signature, UINT64 TableId, INTN MatchIndex)
@@ -252,13 +253,12 @@ UINT64* ScanXSDT2(UINT32 Signature, UINT64 TableId, INTN MatchIndex)
 
   EntryCount = (Xsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
   BasePtr = (CHAR8*)(&(Xsdt->Entry));
-  for (Index = 0; Index < EntryCount; Index ++, BasePtr+=sizeof(UINT64)) {
-    CopyMem (&Entry64, (VOID*)BasePtr, sizeof(UINT64)); //value from BasePtr->
-    TableEntry = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(Entry64));
-    if (((Signature == 0) || (TableEntry->Signature == Signature)) &&
-      ((TableId == 0) || (TableEntry->OemTableId == TableId))) {
-      if (-1 == MatchIndex || Count == MatchIndex) {
-        return (UINT64 *)BasePtr; //pointer to the TableEntry entry
+  for (Index = 0; Index < EntryCount; Index++, BasePtr += sizeof(UINT64)) {
+    CopyMem(&Entry64, BasePtr, sizeof Entry64); //value from BasePtr->
+    TableEntry = (EFI_ACPI_DESCRIPTION_HEADER*)(UINTN)Entry64;
+    if (0 == Signature || TableEntry->Signature == Signature) {
+      if ((0 == TableId || TableEntry->OemTableId == TableId) && (IGNORE_INDEX == MatchIndex || Count == MatchIndex)) {
+        return (UINT64*)BasePtr; //pointer to the TableEntry entry
       }
       ++Count;
     }
@@ -268,7 +268,7 @@ UINT64* ScanXSDT2(UINT32 Signature, UINT64 TableId, INTN MatchIndex)
 
 UINT64* ScanXSDT(UINT32 Signature, UINT64 TableId)
 {
-  return ScanXSDT2(Signature, TableId, -1);
+  return ScanXSDT2(Signature, TableId, IGNORE_INDEX);
 }
 
 
@@ -701,7 +701,7 @@ INTN IndexFromFileName(CHAR16* FileName)
   // FileName must start as "XXXX-number...", such as "SSDT-9.aml", or "SSDT-11-SaSsdt.aml"
 
   // search for '-'
-  INTN Result = -1;
+  INTN Result = IGNORE_INDEX;
   CHAR16* temp = FileName;
   for (; *temp != 0 && *temp != '-'; temp++);
   if ('-' == *temp && 4 == temp-FileName) {
@@ -741,7 +741,7 @@ EFI_STATUS ReplaceOrInsertTable(VOID* TableEntry, UINTN Length, INTN MatchIndex)
     //insert/modify into RSDT
     if (Rsdt) {
       UINT32* entry = NULL;
-      if (hdr->Signature != EFI_ACPI_4_0_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE || MatchIndex != -1) {
+      if (hdr->Signature != EFI_ACPI_4_0_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE || MatchIndex != IGNORE_INDEX) {
         // SSDT with target index or non-SSDT, try to find matching entry
         entry = ScanRSDT2(hdr->Signature, hdr->OemTableId, MatchIndex);
       }
@@ -758,12 +758,13 @@ EFI_STATUS ReplaceOrInsertTable(VOID* TableEntry, UINTN Length, INTN MatchIndex)
     //insert/modify into XSDT
     if (Xsdt) {
       UINT64* entry = NULL;
-      if (hdr->Signature != EFI_ACPI_4_0_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE || MatchIndex != -1) {
+      if (hdr->Signature != EFI_ACPI_4_0_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE || MatchIndex != IGNORE_INDEX) {
         // SSDT with target index or non-SSDT, try to find matching entry
         entry = ScanXSDT2(hdr->Signature, hdr->OemTableId, MatchIndex);
       }
       if (entry) {
         WriteUnaligned64(entry, (UINT64)BufferPtr);
+        //REVIEW: same as: UINTN Index = entry - &Xsdt->Entry;
         UINTN Index = entry - (UINT64 *)(((UINT8 *)Xsdt) + OFFSET_OF(XSDT_TABLE, Entry));
         if (XsdtReplaceSizes && Index < XsdtOriginalEntryCount) {
           XsdtReplaceSizes[Index] = Length;  // mark as replaced, as it should be freed if patched later
@@ -895,9 +896,9 @@ STATIC UINT8 NameCSDT2[] = {0x80, 0x43, 0x53, 0x44, 0x54};
 
 //UINT32 get_size(UINT8 * An, UINT32 ); // Let borrow from FixBiosDsdt.
 
-static CHAR16* GenerateFileName(CHAR16* FileNamePrefix, UINTN SsdtCount, UINTN ChildCount, CHAR8 OemTableId[9])
-// ChildCount == -1 indicates normal SSDT
-// SsdtCount == -1 indicates dynamic SSDT in DSDT
+static CHAR16* GenerateFileName(CHAR16* FileNamePrefix, INTN SsdtCount, INTN ChildCount, CHAR8 OemTableId[9])
+// ChildCount == IGNORE_INDEX indicates normal SSDT
+// SsdtCount == IGNORE_INDEX indicates dynamic SSDT in DSDT
 // otherwise is child SSDT from normal SSDT
 {
   CHAR16* FileName;
@@ -908,10 +909,10 @@ static CHAR16* GenerateFileName(CHAR16* FileNamePrefix, UINTN SsdtCount, UINTN C
     Suffix[0] = '-';
     CopyMem(Suffix+1, OemTableId, 9);
   }
-  if (-1 == ChildCount) {
+  if (IGNORE_INDEX == ChildCount) {
     // normal SSDT
     FileName = PoolPrint(L"%sSSDT-%d%a.aml", FileNamePrefix, SsdtCount, Suffix);
-  } else if (-1 == SsdtCount) {
+  } else if (IGNORE_INDEX == SsdtCount) {
     // dynamic SSDT in DSDT
     FileName = PoolPrint(L"%sSSDT-xDSDT_%d%a.aml", FileNamePrefix, ChildCount, Suffix);
   } else {
@@ -933,7 +934,7 @@ VOID DumpChildSsdt(EFI_ACPI_DESCRIPTION_HEADER *TableEntry, CHAR16 *DirName, CHA
   UINT8         *Entry;
   UINT8         *End;
   UINT8         *pacBody;
-  UINTN         ChildCount = 0;
+  INTN          ChildCount = 0;
 
   if (gSettings.NoDynamicExtract) {
     return;
@@ -1054,7 +1055,7 @@ VOID DumpChildSsdt(EFI_ACPI_DESCRIPTION_HEADER *TableEntry, CHAR16 *DirName, CHA
 /** Saves Table to disk as DirName\\FileName (DirName != NULL)
  *  or just prints basic table data to log (DirName == NULL).
  */
-EFI_STATUS DumpTable(EFI_ACPI_DESCRIPTION_HEADER *TableEntry, CHAR8 *CheckSignature, CHAR16 *DirName, CHAR16 *FileName, CHAR16 *FileNamePrefix, UINTN *SsdtCount)
+EFI_STATUS DumpTable(EFI_ACPI_DESCRIPTION_HEADER *TableEntry, CHAR8 *CheckSignature, CHAR16 *DirName, CHAR16 *FileName, CHAR16 *FileNamePrefix, INTN *SsdtCount)
 {
   EFI_STATUS    Status;
   CHAR8         Signature[5];
@@ -1125,7 +1126,7 @@ EFI_STATUS DumpTable(EFI_ACPI_DESCRIPTION_HEADER *TableEntry, CHAR8 *CheckSignat
   if (FileName == NULL) {
     // take the name from the signature
     if (TableEntry->Signature == EFI_ACPI_1_0_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE && SsdtCount != NULL) {
-      FileName = GenerateFileName(FileNamePrefix, *SsdtCount, -1, OemTableId);
+      FileName = GenerateFileName(FileNamePrefix, *SsdtCount, IGNORE_INDEX, OemTableId);
       DumpChildSsdt(TableEntry, DirName, FileNamePrefix, *SsdtCount);
       *SsdtCount += 1;
     } else {
@@ -1151,7 +1152,7 @@ EFI_STATUS DumpTable(EFI_ACPI_DESCRIPTION_HEADER *TableEntry, CHAR8 *CheckSignat
 }
 
 /** Saves to disk (DirName != NULL) or prints to log (DirName == NULL) Fadt tables: Dsdt and Facs. */
-EFI_STATUS DumpFadtTables(EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt, CHAR16 *DirName, CHAR16 *FileNamePrefix, UINTN *SsdtCount)
+EFI_STATUS DumpFadtTables(EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt, CHAR16 *DirName, CHAR16 *FileNamePrefix, INTN *SsdtCount)
 {
   EFI_ACPI_DESCRIPTION_HEADER                   *TableEntry;
   EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
@@ -1197,7 +1198,7 @@ EFI_STATUS DumpFadtTables(EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt, CHAR1
       return Status;
     }
     DBG("\n");
-    DumpChildSsdt(TableEntry, DirName, FileNamePrefix, ~(UINTN)0);
+    DumpChildSsdt(TableEntry, DirName, FileNamePrefix, IGNORE_INDEX);
   }
   //
   // Save Facs
@@ -1310,7 +1311,7 @@ VOID DumpTables(VOID *RsdPtrVoid, CHAR16 *DirName)
   CHAR8           *EntryPtr;
   UINTN           EntryCount;
   UINTN           Index;
-  UINTN           SsdtCount;
+  INTN            SsdtCount;
   CHAR16          *FileNamePrefix;
 
   //

[Slice]

Imagine a hypothetical leaf function that fills a buffer with a pixel color.  Imagine that this function stores the value of the pixel color just under the stack pointer, in its red zone, without adjusting the stack pointer.  Imagine that a timer interrupt drops by to visit while our leaf function is filling the buffer.  Imagine that the rude interrupt handler stomps the pixel color, leaving the red value at zero, the green value at zero, and the blue value equal to the GDT selector of the code segment that happens to be in use by UEFI firmware.  What would this buffer look like if blitted to a screen?

 

If you have a fast enough CPU, you may never get to see it.

 

I was going to paste the code and disassembly which would have made it a long explanation.  I think enough said.

 

This bug is my fault.  -mno-red-zone is in the GCC build CC_FLAGS, and I missed how important it is for writing kernel-mode code with sysv abi.

Yes. See post #2651

I disassebled two compilation XCODE5 and XCODE8.

The difference is a line

000000000000006c 4883EC20                        sub        rsp, 0x20

It is what you said.

[Sherlocks]

@Slice

i checked block kext(no caches) part.

strangely, i'm not remember that use of prelinkedkernel start(maybe 10.11+?)

https://sourceforge.net/p/cloverefiboot/code/4313/tree//rEFIt_UEFI/Platform/Settings.c#l7128

after uncomment

if we block prelinkedkernel file, we can't get boot with does printf work? message hang

old caches location is no problem.

[Slice]

This is bad idea to block cache or prelinkedkernel.

[Sherlocks]

This is bad idea to block cache or prelinkedkernel.

 

strangely, we can use this in old macos. 

kernelcaches is shortcut by PrelinkedKernel file.

so can't we use no caches on 10.11+?

 

EDIT1.

i don't know whether chameleon also can't use no cache option in 10.11+

i will ask this for chameleon user

 

EDIT2.

i will test this option on each macos. i will find macos that not work this option

 

EDIT3.

i tested all with hard cases from DVD of 10.6.7 to 10.13

completely done for without cache option.

[Sherlocks]

i found case that osversion null in 10.7/10.8. restore Base System method is no problem

 

directly restore InstallESD by using diskutility or DiskMaker X or InstallDiskCreator. 

then 1st stage, recognized osversion, copy files to install mac(ex,/Volumes/MAC/OS X Install Data), reboot

and 2nd stage, osversion is null, because there is no SystemVersion.plist file.

only can check os version in ia.log.

 

in this case, if user don't have fakesmc.kext in Other folder, will fail boot in 2nd stage.

 

there is no plist file to recognize os version. only ia.log.

clover need to find this pattern "Running OS Build: Mac OS X 10.8.5 (12F45)" in ia.log txt file. then read os version.

 

ia.log location

10.7

\\Mac OS X Install Data\\ia.log

10.8

\\OS X Install Data\\ia.log

 

thanks

10.8.5 2nd stage.zip

10.7.5 2nd stage.zip

[Slice]

 

i found case that osversion null in 10.7/10.8. restore Base System method is no problem

 

directly restore InstallESD by using diskutility or DiskMaker X or InstallDiskCreator. 

then 1st stage, recognized osversion, copy files to install mac(ex,/Volumes/MAC/OS X Install Data), reboot

and 2nd stage, osversion is null, because there is no SystemVersion.plist file.

only can check os version in ia.log.

 

in this case, if user don't have fakesmc.kext in Other folder, will fail boot in 2nd stage.

 

there is no plist file to recognize os version. only ia.log.

clover need to find this pattern "Running OS Build: Mac OS X 10.8.5 (12F45)" in ia.log txt file. then read os version.

 

ia.log location

10.7

\\Mac OS X Install Data\\ia.log

10.8

\\OS X Install Data\\ia.log

 

thanks

 

boot.efi you attached already contains mac OS version as usual for Clover

[Sherlocks]

boot.efi you attached already contains mac OS version as usual for Clover

bootefi.PNG

I see. But strangely, clover can't detect os version. I don't know why. I just found it when testing "-f" option.

I think clover still depend on SystemVersion.plist.

 

I'm ready. If you give me file, i will test.

 

EDIT1.

Check string in boot.efi after gui? Can't we directly read os version from boot.efi in GUI? If there is no systemversion.plist.

Null case

In GUI, shown NULL, enter partition, read os version from boot.efi. is this process right?

 

EDIT2.

okay. I understand this process. I have a idea for null case in GUI. I will test.

 

EDIT3.

Apple still use 10.12 os version in boot.efi.

6:442  0:011   Mac OS X 10.12

6:442  0:000  Corrected OSVersion: 10.12

6:442  0:000   10.12 (17C79a) <-buildversion is 10.13.2 beta4

나의 LG-F800S 의 Tapatalk에서 보냄

[mhaeuser]

Can only repeat myself, please don't use boot.efi's OSVersion, it is often inaccurate and not even available pre-Lion.

[Slice]

Something new here.

boot6 compiled with XCODE8 toolset works fine

but not with XCODE5 toolset. Just 6_ blinking. But why?

boot-bad.zip

[RehabMan]

I have made a few fixes for AutoMerge=true.

The changes are checked into my github fork.

 

bug: AutoMerge=true, SSDTs for merge in ACPI/patched, and SSDTs with non-unique OEM table IDs. The patched SSDTs would not be merged properly due to the code thinking the SSDT index did not match.

fix: https://github.com/RehabMan/Clover/commit/5f0464140511f69d451e5ac0b13591b77cc084c0

(that fix also includes a cleanup/portable fix for the whole UINTN vs. INTN, two's complement -1 issue)

 

bug: AutoMerge=true, SSDTs for merge in ACPI/patched, and DropTables used to drop one or more SSDTs. The patched SSDTs would not be merged due to the table dropping process upsetting the numbering scheme for the SSDTs.

fix: Use a two-pass load process for SSDTs in ACPI/patched, such that dropping is done after merging.

first, refactor: https://github.com/RehabMan/Clover/commit/2fbe7f1906da852e1c4019a20165404bdb9fcc9a

then, actual fix: https://github.com/RehabMan/Clover/commit/5c5a44499cc0dcbfdf4999d8c37d2997e0d24d81

final, cleanup with memory leak fixes: https://github.com/RehabMan/Clover/commit/5d7f2f47aebb49ef69a845d45abc2f369cca7f64

 

It is probably easiest to just import my AcpiPatcher.c into the svn repo:

https://github.com/RehabMan/Clover/raw/master/rEFIt_UEFI/Platform/AcpiPatcher.c