|
Re: [PATCH 2/5] Platform/RaspberryPi: Expand locking to cover return data
Andrei Warkentin
Yikes - that's embarrassing. Thanks for the fix.
Reviewed-by: Andrei Warkentin <awarkentin@...>
From: Jeremy Linton <jeremy.linton@...>
Sent: Friday, October 1, 2021 7:52 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: pete@... <pete@...>; ardb+tianocore@... <ardb+tianocore@...>; leif@... <leif@...>; Andrei Warkentin <awarkentin@...>; Sunny.Wang@... <Sunny.Wang@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Jeremy Linton <jeremy.linton@...> Subject: [PATCH 2/5] Platform/RaspberryPi: Expand locking to cover return data Â
While debugging problems with the GET/SET_CLOCK mailbox calls it appeared
that the locking in most of the mailbox commands isn't perfectly correct. All UEFI firmware calls to the RPi mailbox share a single mDmaBuffer which is used to fill out the command passed to the vc firmware, and record its response. The buffer is protected by mMailboxLock, yet in many cases the result of the request is copied from the buffer after the lock has been released. This doesn't currently appear to be causing any problems, but should probably be fixed anyway. There are a couple other minor tweaks in this patch that are hard to justify on their own, one is a bit of whitespace cleanup, and the other is the addition of a debug message to print the returned clock rate for the requested clock. This latter print would have immediatly shown that the vc firmware was returning 0 as the emmc clock rate rather than something reasonable. Signed-off-by: Jeremy Linton <jeremy.linton@...> ---  .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c       | 102 ++++++++++++---------  1 file changed, 59 insertions(+), 43 deletions(-) diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c index bf74148bbb..29719aa5ec 100644 --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c @@ -203,7 +203,6 @@ RpiFirmwareSetPowerState (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock);    if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) { @@ -219,6 +218,7 @@ RpiFirmwareSetPowerState (       __FUNCTION__, PowerState ? "en" : "dis", DeviceId));     Status = EFI_DEVICE_ERROR;   } + ReleaseSpinLock (&mMailboxLock);    return Status;  } @@ -266,18 +266,20 @@ RpiFirmwareGetArmMemory (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock);    if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *Base = Cmd->TagBody.Base;   *Size = Cmd->TagBody.Size; + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -323,17 +325,18 @@ RpiFirmwareGetMacAddress (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    CopyMem (MacAddress, Cmd->TagBody.MacAddress, sizeof (Cmd->TagBody.MacAddress)); + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -378,17 +381,17 @@ RpiFirmwareGetSerial (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *Serial = Cmd->TagBody.Serial; + ReleaseSpinLock (&mMailboxLock);   // Some platforms return 0 or 0x0000000010000000 for serial.   // For those, try to use the MAC address.   if ((*Serial == 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) == 0)) { @@ -441,17 +444,18 @@ RpiFirmwareGetModel (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *Model = Cmd->TagBody.Model; + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -496,17 +500,18 @@ RpiFirmwareGetModelRevision (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *Revision = Cmd->TagBody.Revision; + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -538,17 +543,18 @@ RpiFirmwareGetFirmwareRevision (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *Revision = Cmd->TagBody.Revision; + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -831,18 +837,19 @@ RpiFirmwareGetFbSize (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *Width = Cmd->TagBody.Width;   *Height = Cmd->TagBody.Height; + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -872,16 +879,18 @@ RpiFirmwareFreeFb (VOID)   Cmd->EndTag                 = 0;    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result); - ReleaseSpinLock (&mMailboxLock);    if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }  + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -935,19 +944,20 @@ RpiFirmwareAllocFb (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *Pitch = Cmd->Pitch.Pitch;   *FbBase = Cmd->AllocFb.AlignmentBase - BCM2836_DMA_DEVICE_OFFSET;   *FbSize = Cmd->AllocFb.Size; + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -999,13 +1009,12 @@ RpiFirmwareGetCommmandLine (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }  @@ -1013,6 +1022,7 @@ RpiFirmwareGetCommmandLine (   if (Cmd->TagHead.TagValueSize >= BufferSize &&       Cmd->CommandLine[Cmd->TagHead.TagValueSize - 1] != '\0') {     DEBUG ((DEBUG_ERROR, "%a: insufficient buffer size\n", __FUNCTION__)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_OUT_OF_RESOURCES;   }  @@ -1026,6 +1036,7 @@ RpiFirmwareGetCommmandLine (     CommandLine[Cmd->TagHead.TagValueSize] = '\0';   }  + ReleaseSpinLock (&mMailboxLock);   return EFI_SUCCESS;  }  @@ -1075,18 +1086,20 @@ RpiFirmwareSetClockRate (   Cmd->TagBody.SkipTurbo     = SkipTurbo ? 1 : 0;   Cmd->EndTag                = 0;  + DEBUG ((DEBUG_ERROR, "%a: Request clock rate %X = %d\n", __FUNCTION__, ClockId, ClockRate));   Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }  + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -1131,20 +1144,23 @@ RpiFirmwareGetClockRate (   Cmd->TagHead.TagValueSize  = 0;   Cmd->TagBody.ClockId       = ClockId;   Cmd->EndTag                = 0; - +   Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }    *ClockRate = Cmd->TagBody.ClockRate; + ReleaseSpinLock (&mMailboxLock); + + DEBUG ((DEBUG_ERROR, "%a: Get Clock Rate return: ClockRate=%d ClockId=%X\n", __FUNCTION__, *ClockRate, ClockId)); +   return EFI_SUCCESS;  }  @@ -1191,7 +1207,7 @@ RpiFirmwareGetMinClockRate (  {   return RpiFirmwareGetClockRate (ClockId, RPI_MBOX_GET_MIN_CLOCK_RATE, ClockRate);  } - +  #pragma pack()  typedef struct {   UINT32                   ClockId; @@ -1236,16 +1252,17 @@ RpiFirmwareSetClockState (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response)); +   ReleaseSpinLock (&mMailboxLock);     return EFI_DEVICE_ERROR;   }  + ReleaseSpinLock (&mMailboxLock); +   return EFI_SUCCESS;  }  @@ -1297,16 +1314,15 @@ RpiFirmwareSetGpio (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response));   } + ReleaseSpinLock (&mMailboxLock);  } - +  STATIC  VOID  EFIAPI @@ -1361,8 +1377,6 @@ RpiFirmwareNotifyXhciReset (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR, @@ -1370,6 +1384,8 @@ RpiFirmwareNotifyXhciReset (       __FUNCTION__, Status, Cmd->BufferHead.Response));   }  + ReleaseSpinLock (&mMailboxLock); +   return Status;  }  @@ -1424,8 +1440,6 @@ RpiFirmwareNotifyGpioGetCfg (    *Polarity = Cmd->TagBody.Polarity;  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR, @@ -1433,6 +1447,8 @@ RpiFirmwareNotifyGpioGetCfg (       __FUNCTION__, Status, Cmd->BufferHead.Response));   }  + ReleaseSpinLock (&mMailboxLock); +   return Status;  }  @@ -1471,8 +1487,8 @@ RpiFirmwareNotifyGpioSetCfg (    Status = RpiFirmwareNotifyGpioGetCfg (Gpio, &Result);   if (EFI_ERROR (Status)) { -        DEBUG ((DEBUG_ERROR, "%a: Failed to get GPIO polarity\n", __FUNCTION__)); -        Result = 0; //default polarity +     DEBUG ((DEBUG_ERROR, "%a: Failed to get GPIO polarity\n", __FUNCTION__)); +     Result = 0; //default polarity   }   @@ -1488,7 +1504,7 @@ RpiFirmwareNotifyGpioSetCfg (   Cmd->BufferHead.Response   = 0;   Cmd->TagHead.TagId         = RPI_MBOX_SET_GPIO_CONFIG;   Cmd->TagHead.TagSize       = sizeof (Cmd->TagBody); - +   Cmd->TagBody.Gpio = 128 + Gpio;   Cmd->TagBody.Direction = Direction;   Cmd->TagBody.Polarity = Result; @@ -1501,17 +1517,17 @@ RpiFirmwareNotifyGpioSetCfg (    Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);  - ReleaseSpinLock (&mMailboxLock); -   if (EFI_ERROR (Status) ||       Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {     DEBUG ((DEBUG_ERROR,       "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",       __FUNCTION__, Status, Cmd->BufferHead.Response));   } - - RpiFirmwareSetGpio (Gpio,!State); - + + ReleaseSpinLock (&mMailboxLock); + + RpiFirmwareSetGpio (Gpio,!State); +    return Status;  } @@ -1540,7 +1556,7 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {   RPiFirmwareGetModelInstalledMB,   RpiFirmwareNotifyXhciReset,   RpiFirmwareGetCurrentClockState, - RpiFirmwareSetClockState, + RpiFirmwareSetClockState,   RpiFirmwareNotifyGpioSetCfg  };  -- 2.13.7
|
|
|
|
Re: [PATCH 5/5] Platform/RaspberryPi: Disconnect/shutdown all drivers before reboot
Andrei Warkentin
Seems smart to do.
Reviewed-by: Andrei Warkentin <awarkentin@...>
From: Jeremy Linton <jeremy.linton@...>
Sent: Friday, October 1, 2021 7:52 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: pete@... <pete@...>; ardb+tianocore@... <ardb+tianocore@...>; leif@... <leif@...>; Andrei Warkentin <awarkentin@...>; Sunny.Wang@... <Sunny.Wang@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Jeremy Linton <jeremy.linton@...> Subject: [PATCH 5/5] Platform/RaspberryPi: Disconnect/shutdown all drivers before reboot Â
In theory we should be properly cleaning up all the device drivers before
pulling the big switch. Particularly the partition mgr will issue flush commands to attached disks as it goes down. This assures that devices running in WB mode (that correctly handle flush/sync/etc) commands are persisted to physical media before we hit reset. Without this, there are definitly cases where the relevant specifications don't guarantee persistence of data in their buffers in the face of reset conditions. We can't really do anything about the many devices that don't honor persistance requests but we can start here. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Â Platform/RaspberryPi/Library/ResetLib/ResetLib.c | 44 ++++++++++++++++++++++++ Â 1 file changed, 44 insertions(+) diff --git a/Platform/RaspberryPi/Library/ResetLib/ResetLib.c b/Platform/RaspberryPi/Library/ResetLib/ResetLib.c index a70eee485d..036f619cb5 100644 --- a/Platform/RaspberryPi/Library/ResetLib/ResetLib.c +++ b/Platform/RaspberryPi/Library/ResetLib/ResetLib.c @@ -19,11 +19,54 @@ Â #include <Library/TimerLib.h> Â #include <Library/EfiResetSystemLib.h> Â #include <Library/ArmSmcLib.h> +#include <Library/UefiBootServicesTableLib.h> Â #include <Library/UefiLib.h> Â #include <Library/UefiRuntimeLib.h> Â Â #include <IndustryStandard/ArmStdSmc.h> Â + +/** +Â Disconnect everything. +Â Modified from the UEFI 2.3 spec (May 2009 version) + +Â @retval EFI_SUCCESSÂ Â Â Â The operation was successful. + +**/ +EFI_STATUS +DisconnectAll( +Â VOID +Â ) +{ +Â EFI_STATUS Status; +Â UINTN HandleCount; +Â EFI_HANDLE *HandleBuffer; +Â UINTN HandleIndex; + +Â // +Â // Retrieve the list of all handles from the handle database +Â // +Â Status = gBS->LocateHandleBuffer ( +Â Â Â AllHandles, +Â Â Â NULL, +Â Â Â NULL, +Â Â Â &HandleCount, +Â Â Â &HandleBuffer +Â Â ); +Â if (!EFI_ERROR (Status)) { +Â Â Â for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) { +Â Â Â Â Â Status = gBS->DisconnectController ( +Â Â Â Â Â Â Â HandleBuffer[HandleIndex], +Â Â Â Â Â Â Â NULL, +Â Â Â Â Â Â Â NULL +Â Â Â Â Â Â ); +Â Â Â } +Â Â Â gBS->FreePool(HandleBuffer); +Â } +Â return (EFI_SUCCESS); +} + + Â /** Â Â Resets the entire platform. Â @@ -57,6 +100,7 @@ LibResetSystem ( Â Â Â Â if (Delay != 0) { Â Â Â Â Â Â DEBUG ((DEBUG_INFO, "Platform will be reset in %d.%d seconds...\n", Â Â Â Â Â Â Â Â Â Â Â Â Â Â Delay / 1000000, (Delay % 1000000) / 100000)); +Â Â Â Â Â DisconnectAll (); Â Â Â Â Â Â MicroSecondDelay (Delay); Â Â Â Â } Â Â } -- 2.13.7
|
|
|
|
Re: [PATCH 4/5] Platform/RaspberryPi: Normal memory should not be marked as uncached
Andrei Warkentin
I may have misunderstood the flags as being valid ways of mapping the added range. Should we also then take out WC and WT?
From: Jeremy Linton <jeremy.linton@...>
Sent: Friday, October 1, 2021 7:52 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: pete@... <pete@...>; ardb+tianocore@... <ardb+tianocore@...>; leif@... <leif@...>; Andrei Warkentin <awarkentin@...>; Sunny.Wang@... <Sunny.Wang@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Jeremy Linton <jeremy.linton@...> Subject: [PATCH 4/5] Platform/RaspberryPi: Normal memory should not be marked as uncached Â
The efi spec seems to indicate that the efi uncacheable attribute
should be mapped to device memory rather than normal-nc. This means that the uefi mem attribute for the >3G ram doesn't match the remainder of the RAM in the machine. So, lets remove the uncacheable attribute to make it more consistent. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Â Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 4 ++-- Â 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c index 2ef7da67bd..415d99fadb 100644 --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c @@ -499,7 +499,7 @@ ApplyVariables ( Â Â Â Â Â Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, 3UL * BASE_1GB, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â SystemMemorySizeBelow4GB - (3UL * SIZE_1GB), -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB); +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB); Â Â Â Â ASSERT_EFI_ERROR (Status); Â Â Â Â Status = gDS->SetMemorySpaceAttributes (3UL * BASE_1GB, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â SystemMemorySizeBelow4GB - (3UL * SIZE_1GB), EFI_MEMORY_WB); @@ -511,7 +511,7 @@ ApplyVariables ( Â Â Â Â Â Â // Â Â Â Â Â Â Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, 4UL * BASE_1GB, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â SystemMemorySize - (4UL * SIZE_1GB), -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB); +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB); Â Â Â Â Â Â ASSERT_EFI_ERROR (Status); Â Â Â Â Â Â Status = gDS->SetMemorySpaceAttributes (4UL * BASE_1GB, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â SystemMemorySize - (4UL * SIZE_1GB), EFI_MEMORY_WB); -- 2.13.7
|
|
|
|
Re: [edk2-platforms][PATCH v2 1/1] MinPlatformPkg/AcpiTables: Update structures for ACPI 6.3
toggle quoted messageShow quoted text
-----Original Message-----
|
|
|
|
Re: [PATCH 3/5] Platform/RaspberryPi: Update Linux quirk name
Andrei Warkentin
Reviewed-by: Andrei Warkentin <awarkentin@...>
From: Jeremy Linton <jeremy.linton@...>
Sent: Friday, October 1, 2021 7:52 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: pete@... <pete@...>; ardb+tianocore@... <ardb+tianocore@...>; leif@... <leif@...>; Andrei Warkentin <awarkentin@...>; Sunny.Wang@... <Sunny.Wang@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Jeremy Linton <jeremy.linton@...> Subject: [PATCH 3/5] Platform/RaspberryPi: Update Linux quirk name Â
During review/merge of the linux ecam quirk, some logic
was added to require the quirk name to be exactly 6 characters, matching the MADT field its overriding. As such, the rpi quirk here needed to be shorted by a character to avoid confusion. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Â Platform/RaspberryPi/AcpiTables/Pci.asl | 2 +- Â 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Platform/RaspberryPi/AcpiTables/Pci.asl b/Platform/RaspberryPi/AcpiTables/Pci.asl index ee37b7a21e..e5fe755923 100644 --- a/Platform/RaspberryPi/AcpiTables/Pci.asl +++ b/Platform/RaspberryPi/AcpiTables/Pci.asl @@ -65,7 +65,7 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4PCIE", 2) Â Â Â Â Â Â Name (_DSD, Package () { Â Â Â Â Â Â Â Â ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Â Â Â Â Â Â Â Â Â Â Package () { -Â Â Â Â Â Â Â Â Â Â Â Package () { "linux-ecam-quirk-id", "bcm2711" }, +Â Â Â Â Â Â Â Â Â Â Â Package () { "linux-ecam-quirk-id", "bc2711" }, Â Â Â Â Â Â Â Â Â Â } Â Â Â Â Â Â }) Â -- 2.13.7
|
|
|
|
Re: [edk2-platforms][PATCH v2 1/1] MinPlatformPkg/AcpiTables: Update structures for ACPI 6.3
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
toggle quoted messageShow quoted text
-----Original Message-----
|
|
|
|
Re: [PATCH 1/5] Platform/RaspberryPi: Fix vfr warning caused by two defaults
Andrei Warkentin
Reviewed-by: Andrei Warkentin <awarkentin@...>
From: Jeremy Linton <jeremy.linton@...>
Sent: Friday, October 1, 2021 7:52 PM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: pete@... <pete@...>; ardb+tianocore@... <ardb+tianocore@...>; leif@... <leif@...>; Andrei Warkentin <awarkentin@...>; Sunny.Wang@... <Sunny.Wang@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Jeremy Linton <jeremy.linton@...> Subject: [PATCH 1/5] Platform/RaspberryPi: Fix vfr warning caused by two defaults Â
The build has been tossing a warning about having two defaults
for a while now, lets fix it. Signed-off-by: Jeremy Linton <jeremy.linton@...> ---  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 2 +-  1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr index 18b3ec726e..e8bf16312d 100644 --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr @@ -192,7 +192,7 @@ formset             flags      = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,             option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_ACPI), value = SYSTEM_TABLE_MODE_ACPI, flags = 0;             option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_BOTH), value = SYSTEM_TABLE_MODE_BOTH, flags = DEFAULT; -           option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT; +           option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags =0;         endoneof;   #if (RPI_MODEL == 4) -- 2.13.7
|
|
|
|
[PATCH 5/5] Platform/RaspberryPi: Disconnect/shutdown all drivers before reboot
Jeremy Linton
In theory we should be properly cleaning up all the device drivers before
pulling the big switch. Particularly the partition mgr will issue flush commands to attached disks as it goes down. This assures that devices running in WB mode (that correctly handle flush/sync/etc) command= s are persisted to physical media before we hit reset. Without this, there are definitly cases where the relevant specifications don't guarantee persistence of data in their buffers in the face of reset conditions. We can't really do anything about the many devices that don't honor persistance requests but we can start here. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Platform/RaspberryPi/Library/ResetLib/ResetLib.c | 44 ++++++++++++++++++= ++++++ 1 file changed, 44 insertions(+) diff --git a/Platform/RaspberryPi/Library/ResetLib/ResetLib.c b/Platform/= RaspberryPi/Library/ResetLib/ResetLib.c index a70eee485d..036f619cb5 100644 --- a/Platform/RaspberryPi/Library/ResetLib/ResetLib.c +++ b/Platform/RaspberryPi/Library/ResetLib/ResetLib.c @@ -19,11 +19,54 @@ #include <Library/TimerLib.h> #include <Library/EfiResetSystemLib.h> #include <Library/ArmSmcLib.h> +#include <Library/UefiBootServicesTableLib.h> #include <Library/UefiLib.h> #include <Library/UefiRuntimeLib.h> =20 #include <IndustryStandard/ArmStdSmc.h> =20 + +/** + Disconnect everything. + Modified from the UEFI 2.3 spec (May 2009 version) + + @retval EFI_SUCCESS The operation was successful. + +**/ +EFI_STATUS +DisconnectAll( + VOID + ) +{ + EFI_STATUS Status; + UINTN HandleCount; + EFI_HANDLE *HandleBuffer; + UINTN HandleIndex; + + // + // Retrieve the list of all handles from the handle database + // + Status =3D gBS->LocateHandleBuffer ( + AllHandles, + NULL, + NULL, + &HandleCount, + &HandleBuffer + ); + if (!EFI_ERROR (Status)) { + for (HandleIndex =3D 0; HandleIndex < HandleCount; HandleIndex++) { + Status =3D gBS->DisconnectController ( + HandleBuffer[HandleIndex], + NULL, + NULL + ); + } + gBS->FreePool(HandleBuffer); + } + return (EFI_SUCCESS); +} + + /** Resets the entire platform. =20 @@ -57,6 +100,7 @@ LibResetSystem ( if (Delay !=3D 0) { DEBUG ((DEBUG_INFO, "Platform will be reset in %d.%d seconds...\n"= , Delay / 1000000, (Delay % 1000000) / 100000)); + DisconnectAll (); MicroSecondDelay (Delay); } } --=20 2.13.7
|
|
|
|
[PATCH 4/5] Platform/RaspberryPi: Normal memory should not be marked as uncached
Jeremy Linton
The efi spec seems to indicate that the efi uncacheable attribute
should be mapped to device memory rather than normal-nc. This means that the uefi mem attribute for the >3G ram doesn't match the remainder of the RAM in the machine. So, lets remove the uncacheable attribute to make it more consistent. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platfor= m/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c index 2ef7da67bd..415d99fadb 100644 --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c @@ -499,7 +499,7 @@ ApplyVariables ( =20 Status =3D gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, 3UL * = BASE_1GB, SystemMemorySizeBelow4GB - (3UL * SIZE_1GB), - EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_= MEMORY_WB); + EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB); ASSERT_EFI_ERROR (Status); Status =3D gDS->SetMemorySpaceAttributes (3UL * BASE_1GB, SystemMemorySizeBelow4GB - (3UL * SIZE_1GB), EFI_MEM= ORY_WB); @@ -511,7 +511,7 @@ ApplyVariables ( // Status =3D gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, 4UL = * BASE_1GB, SystemMemorySize - (4UL * SIZE_1GB), - EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EF= I_MEMORY_WB); + EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB); ASSERT_EFI_ERROR (Status); Status =3D gDS->SetMemorySpaceAttributes (4UL * BASE_1GB, SystemMemorySize - (4UL * SIZE_1GB), EFI_MEMORY_WB= ); --=20 2.13.7
|
|
|
|
[PATCH 3/5] Platform/RaspberryPi: Update Linux quirk name
Jeremy Linton
During review/merge of the linux ecam quirk, some logic
was added to require the quirk name to be exactly 6 characters, matching the MADT field its overriding. As such, the rpi quirk here needed to be shorted by a character to avoid confusion. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Platform/RaspberryPi/AcpiTables/Pci.asl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Platform/RaspberryPi/AcpiTables/Pci.asl b/Platform/Raspberry= Pi/AcpiTables/Pci.asl index ee37b7a21e..e5fe755923 100644 --- a/Platform/RaspberryPi/AcpiTables/Pci.asl +++ b/Platform/RaspberryPi/AcpiTables/Pci.asl @@ -65,7 +65,7 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4PC= IE", 2) Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package () { - Package () { "linux-ecam-quirk-id", "bcm2711" }, + Package () { "linux-ecam-quirk-id", "bc2711" }, } }) =20 --=20 2.13.7
|
|
|
|
[PATCH 2/5] Platform/RaspberryPi: Expand locking to cover return data
Jeremy Linton
While debugging problems with the GET/SET_CLOCK mailbox calls it appeared
that the locking in most of the mailbox commands isn't perfectly correct. All UEFI firmware calls to the RPi mailbox share a single mDmaBuffer which is used to fill out the command passed to the vc firmwar= e, and record its response. The buffer is protected by mMailboxLock, yet in many cases the result of the request is copied from the buffer after the lock has been released. This doesn't currently appear to be causing any problems, but should probably be fixed anyway. There are a couple other minor tweaks in this patch that are hard to justify on their own, one is a bit of whitespace cleanup, and the other i= s the addition of a debug message to print the returned clock rate for the requested clock. This latter print would have immediatly shown that the v= c firmware was returning 0 as the emmc clock rate rather than something reasonable. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 102 ++++++++++++---= ------ 1 file changed, 59 insertions(+), 43 deletions(-) diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c= b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c index bf74148bbb..29719aa5ec 100644 --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c @@ -203,7 +203,6 @@ RpiFirmwareSetPowerState ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); =20 if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { @@ -219,6 +218,7 @@ RpiFirmwareSetPowerState ( __FUNCTION__, PowerState ? "en" : "dis", DeviceId)); Status =3D EFI_DEVICE_ERROR; } + ReleaseSpinLock (&mMailboxLock); =20 return Status; } @@ -266,18 +266,20 @@ RpiFirmwareGetArmMemory ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); =20 if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *Base =3D Cmd->TagBody.Base; *Size =3D Cmd->TagBody.Size; + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -323,17 +325,18 @@ RpiFirmwareGetMacAddress ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 CopyMem (MacAddress, Cmd->TagBody.MacAddress, sizeof (Cmd->TagBody.Mac= Address)); + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -378,17 +381,17 @@ RpiFirmwareGetSerial ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *Serial =3D Cmd->TagBody.Serial; + ReleaseSpinLock (&mMailboxLock); // Some platforms return 0 or 0x0000000010000000 for serial. // For those, try to use the MAC address. if ((*Serial =3D=3D 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) =3D=3D 0)= ) { @@ -441,17 +444,18 @@ RpiFirmwareGetModel ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *Model =3D Cmd->TagBody.Model; + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -496,17 +500,18 @@ RpiFirmwareGetModelRevision ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *Revision =3D Cmd->TagBody.Revision; + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -538,17 +543,18 @@ RpiFirmwareGetFirmwareRevision ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *Revision =3D Cmd->TagBody.Revision; + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -831,18 +837,19 @@ RpiFirmwareGetFbSize ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D= 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *Width =3D Cmd->TagBody.Width; *Height =3D Cmd->TagBody.Height; + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -872,16 +879,18 @@ RpiFirmwareFreeFb (VOID) Cmd->EndTag =3D 0; =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); - ReleaseSpinLock (&mMailboxLock); =20 if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -935,19 +944,20 @@ RpiFirmwareAllocFb ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *Pitch =3D Cmd->Pitch.Pitch; *FbBase =3D Cmd->AllocFb.AlignmentBase - BCM2836_DMA_DEVICE_OFFSET; *FbSize =3D Cmd->AllocFb.Size; + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -999,13 +1009,12 @@ RpiFirmwareGetCommmandLine ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 @@ -1013,6 +1022,7 @@ RpiFirmwareGetCommmandLine ( if (Cmd->TagHead.TagValueSize >=3D BufferSize && Cmd->CommandLine[Cmd->TagHead.TagValueSize - 1] !=3D '\0') { DEBUG ((DEBUG_ERROR, "%a: insufficient buffer size\n", __FUNCTION__)= ); + ReleaseSpinLock (&mMailboxLock); return EFI_OUT_OF_RESOURCES; } =20 @@ -1026,6 +1036,7 @@ RpiFirmwareGetCommmandLine ( CommandLine[Cmd->TagHead.TagValueSize] =3D '\0'; } =20 + ReleaseSpinLock (&mMailboxLock); return EFI_SUCCESS; } =20 @@ -1075,18 +1086,20 @@ RpiFirmwareSetClockRate ( Cmd->TagBody.SkipTurbo =3D SkipTurbo ? 1 : 0; Cmd->EndTag =3D 0; =20 + DEBUG ((DEBUG_ERROR, "%a: Request clock rate %X =3D %d\n", __FUNCTION_= _, ClockId, ClockRate)); Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -1131,20 +1144,23 @@ RpiFirmwareGetClockRate ( Cmd->TagHead.TagValueSize =3D 0; Cmd->TagBody.ClockId =3D ClockId; Cmd->EndTag =3D 0; - +=20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 *ClockRate =3D Cmd->TagBody.ClockRate; + ReleaseSpinLock (&mMailboxLock); + + DEBUG ((DEBUG_ERROR, "%a: Get Clock Rate return: ClockRate=3D%d ClockI= d=3D%X\n", __FUNCTION__, *ClockRate, ClockId)); + return EFI_SUCCESS; } =20 @@ -1191,7 +1207,7 @@ RpiFirmwareGetMinClockRate ( { return RpiFirmwareGetClockRate (ClockId, RPI_MBOX_GET_MIN_CLOCK_RATE, = ClockRate); } - + #pragma pack() typedef struct { UINT32 ClockId; @@ -1236,16 +1252,17 @@ RpiFirmwareSetClockState ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D = 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); + ReleaseSpinLock (&mMailboxLock); return EFI_DEVICE_ERROR; } =20 + ReleaseSpinLock (&mMailboxLock); + return EFI_SUCCESS; } =20 @@ -1297,16 +1314,15 @@ RpiFirmwareSetGpio ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D= 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); } + ReleaseSpinLock (&mMailboxLock); } - + STATIC VOID EFIAPI @@ -1361,8 +1377,6 @@ RpiFirmwareNotifyXhciReset ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, @@ -1370,6 +1384,8 @@ RpiFirmwareNotifyXhciReset ( __FUNCTION__, Status, Cmd->BufferHead.Response)); } =20 + ReleaseSpinLock (&mMailboxLock); + return Status; } =20 @@ -1424,8 +1440,6 @@ RpiFirmwareNotifyGpioGetCfg ( =20 *Polarity =3D Cmd->TagBody.Polarity; =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, @@ -1433,6 +1447,8 @@ RpiFirmwareNotifyGpioGetCfg ( __FUNCTION__, Status, Cmd->BufferHead.Response)); } =20 + ReleaseSpinLock (&mMailboxLock); + return Status; } =20 @@ -1471,8 +1487,8 @@ RpiFirmwareNotifyGpioSetCfg ( =20 Status =3D RpiFirmwareNotifyGpioGetCfg (Gpio, &Result); if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Failed to get GPIO polarity\n", __FUNCTION_= _)); - Result =3D 0; //default polarity + DEBUG ((DEBUG_ERROR, "%a: Failed to get GPIO polarity\n", __FUNCTI= ON__)); + Result =3D 0; //default polarity } =20 =20 @@ -1488,7 +1504,7 @@ RpiFirmwareNotifyGpioSetCfg ( Cmd->BufferHead.Response =3D 0; Cmd->TagHead.TagId =3D RPI_MBOX_SET_GPIO_CONFIG; Cmd->TagHead.TagSize =3D sizeof (Cmd->TagBody); - + Cmd->TagBody.Gpio =3D 128 + Gpio; Cmd->TagBody.Direction =3D Direction; Cmd->TagBody.Polarity =3D Result; @@ -1501,17 +1517,17 @@ RpiFirmwareNotifyGpioSetCfg ( =20 Status =3D MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC= _CHANNEL, &Result); =20 - ReleaseSpinLock (&mMailboxLock); - if (EFI_ERROR (Status) || Cmd->BufferHead.Response !=3D RPI_MBOX_RESP_SUCCESS) { DEBUG ((DEBUG_ERROR, "%a: mailbox transaction error: Status =3D=3D %r, Response =3D=3D= 0x%x\n", __FUNCTION__, Status, Cmd->BufferHead.Response)); } - - RpiFirmwareSetGpio (Gpio,!State); - + + ReleaseSpinLock (&mMailboxLock); + + RpiFirmwareSetGpio (Gpio,!State); + =20 return Status; } @@ -1540,7 +1556,7 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareP= rotocol =3D { RPiFirmwareGetModelInstalledMB, RpiFirmwareNotifyXhciReset, RpiFirmwareGetCurrentClockState, - RpiFirmwareSetClockState, + RpiFirmwareSetClockState, RpiFirmwareNotifyGpioSetCfg }; =20 --=20 2.13.7
|
|
|
|
[PATCH 1/5] Platform/RaspberryPi: Fix vfr warning caused by two defaults
Jeremy Linton
The build has been tossing a warning about having two defaults
for a while now, lets fix it. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Pl= atform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr index 18b3ec726e..e8bf16312d 100644 --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr @@ -192,7 +192,7 @@ formset flags =3D NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRE= D, option text =3D STRING_TOKEN(STR_ADVANCED_SYSTAB_ACPI), valu= e =3D SYSTEM_TABLE_MODE_ACPI, flags =3D 0; option text =3D STRING_TOKEN(STR_ADVANCED_SYSTAB_BOTH), valu= e =3D SYSTEM_TABLE_MODE_BOTH, flags =3D DEFAULT; - option text =3D STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = =3D SYSTEM_TABLE_MODE_DT, flags =3D DEFAULT; + option text =3D STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = =3D SYSTEM_TABLE_MODE_DT, flags =3D0; endoneof; =20 #if (RPI_MODEL =3D=3D 4) --=20 2.13.7
|
|
|
|
[PATCH 0/5] Platform/Rpi: Various cleanups
Jeremy Linton
This set is a few patches I've been collecting to fix minor issues I've s=
een while debugging other problems, or just various things I think should pro= bably be changed. Generally I don't think they fix anything that is currently u= ser visible, although its quite possible some mysterious failures go away. I've been running all of them in some form or another for a few months an= d generally nothing has broken because of them AFAIK. So its probably time = to start getting a few of them out of my private tree. The first is just a compiler warning. The second is mostly expanding the mailbox lock to cover the return data. The third is an update to make the hopefully merged soon CM4 quirk actually work with the patches currently = on LKML. Number 4 is an odd one because it just looks wrong, and I'm worried= its causing random bugs. The final is a corrected shutdown sequence that was discussed months ago. It looks right. but didn't actually fix the data persistence problems that resulted in the couple second reboot delay that is currently in place. Jeremy Linton (5): Platform/RaspberryPi: Fix vfr warning caused by two defaults Platform/RaspberryPi: Expand locking to cover return data Platform/RaspberryPi: Update Linux quirk name Platform/RaspberryPi: Normal memory should not be marked as uncached Platform/RaspberryPi: Disconnect/shutdown all drivers before reboot Platform/RaspberryPi/AcpiTables/Pci.asl | 2 +- Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 4 +- .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 2 +- .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 102 ++++++++++++---= ------ Platform/RaspberryPi/Library/ResetLib/ResetLib.c | 44 +++++++++ 5 files changed, 107 insertions(+), 47 deletions(-) --=20 2.13.7
|
|
|
|
[PATCH v2 1/1] Platform/RaspberryPi: Always use non translating DMA in DT mode
Jeremy Linton
One of the many issues with the PCIe on this platform is
its inbound DMA is either constrained to the lower 3G, or on later SoC's a translation can be used. That translation was problematic with some of the OS's expected to boot on this platform. So, across the board a 3G DMA limit is enforced during boot. This itself causes problems because the later boards removed the SPI EEPROM used by the onboard XHCI controller, instead favoring using a block of RAM to load its firmware. Hence it is the lower level firmware's responsibility via a mailbox call, to read the bridge translation/configuration before telling the XHCI controller where it can find its firmware. Everything is great, except that it appears that Linux after reprogramming the bridge to match the DT (when using a translation) can't actually get the XHCI/quirk/reset to function. Apparently, because the firmware only reads the bridge configuration the first time its called(?). Worse with just the DMA ranges corrected, the XHCI/QUIRK itself then causes the controller to start having what appear to be DMA issues. So again, simplify the situation and make all DT's provided by this firmware have a 3G DMA limit on the PCIe bus. Then remove the ability for linux/etc to trigger the quirk by remove the DT node attaching the reset controller to the XHCI. The latter seems somewhat questionable, since the DT/PCIe host bridge driver is doing what appears to be a PERST which in theory might then require a firmware reload, but at the moment seems to work without. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 75 ++++++++++++++++++++++= ++++++ 1 file changed, 75 insertions(+) diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/Rasp= berryPi/Drivers/FdtDxe/FdtDxe.c index 0472d8ecf6..1a8fc7a134 100644 --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c @@ -334,6 +334,76 @@ CleanSimpleFramebuffer ( return EFI_SUCCESS; } =20 +STATIC +EFI_STATUS +SyncPcie ( + VOID + ) +{ +#if (RPI_MODEL =3D=3D 4) + INTN Node; + INTN Retval; + UINT32 DmaRanges[7]; + + Node =3D fdt_path_offset (mFdtImage, "pcie0"); + if (Node < 0) { + DEBUG ((DEBUG_ERROR, "%a: failed to locate 'pcie0' alias\n", __FUNCT= ION__)); + return EFI_NOT_FOUND; + } + + // non translated 32-bit DMA window with a limit of 0xc0000000 + DmaRanges[0] =3D cpu_to_fdt32 (0x02000000);=20 + DmaRanges[1] =3D cpu_to_fdt32 (0x00000000);=20 + DmaRanges[2] =3D cpu_to_fdt32 (0x00000000); + DmaRanges[3] =3D cpu_to_fdt32 (0x00000000); + DmaRanges[4] =3D cpu_to_fdt32 (0x00000000); + DmaRanges[5] =3D cpu_to_fdt32 (0x00000000); + DmaRanges[6] =3D cpu_to_fdt32 (0xc0000000); + + DEBUG ((DEBUG_INFO, "%a: Updating PCIe dma-ranges\n", __FUNCTION__)); + + // Match dma-ranges with the edk2+ACPI setup we are using. + // This works around a failure in linux to reset the XHCI correctly + // when in DT mode following the linux kernel reprogramming the PCIe + // subsystem to match the DT supplied inbound PCIe/DMA translation. + // It appears the lower level firmware honors whatever it reads + // during the first PCI/XHCI quirk call and uses that value until + // rebooted rather than re-reading it on every mailbox command. + Retval =3D fdt_setprop (mFdtImage, Node, "dma-ranges", + DmaRanges, sizeof DmaRanges); + if (Retval !=3D 0) { + DEBUG ((DEBUG_ERROR, "%a: failed to update 'dma-ranges' property (%d= )\n", + __FUNCTION__, Retval)); + return EFI_NOT_FOUND; + } + + // Now that we are always running without DMA translation, and with + // a 3G DMA limit, there shouldn't be a need to reset/reload + // the XHCI for it to operate. The possible problem is that the PCIe r= oot + // port is itself being reset (by linux+DT), The rpi foundation claims + // this is needed as a pre-req to reloading the XHCI firmware, which + // also implies that a PCI fundamental reset should cause the XHCI its= elf + // to reset. For sure isn't happening fully, otherwise reloading the + // firmware would be mandatory. As it is, recent kernels actually star= t to + // have problems following the XHCI reset notification mailbox! + // So lets just stop the kernel from triggering the mailbox by removin= g + // the node. + Node =3D fdt_path_offset (mFdtImage, "/scb/pcie@7d500000/pci@1,0"); + if (Node < 0) { + // This can happen on CM4/etc which doesn't have an onboard XHCI + DEBUG ((DEBUG_ERROR, "%a: failed to locate /scb/pcie@7d500000/pci@1/= usb@1\n", __FUNCTION__)); + } else { + Retval =3D fdt_del_node (mFdtImage, Node); + if (Retval !=3D 0) { + DEBUG ((DEBUG_ERROR, "Failed to remove /scb/pcie@7d500000/pci@1/us= b@1\n")); + return EFI_NOT_FOUND; + } + } + +#endif + return EFI_SUCCESS; +} + /** @param ImageHandle of the loaded driver @param SystemTable Pointer to the System Table @@ -431,6 +501,11 @@ FdtDxeInitialize ( Print (L"Failed to update USB compatible properties: %r\n", Status); } =20 + SyncPcie (); + if (EFI_ERROR (Status)) { + Print (L"Failed to update PCIe address ranges: %r\n", Status); + } + DEBUG ((DEBUG_INFO, "Installed devicetree at address %p\n", mFdtImage)= ); Status =3D gBS->InstallConfigurationTable (&gFdtTableGuid, mFdtImage); if (EFI_ERROR (Status)) { --=20 2.13.7
|
|
|
|
[PATCH v2 0/1] Fix DT boot on rpi4
Jeremy Linton
Some rpi4's haven't booted cleanly using DT for a few months. The primar=
ly problem is that the XHCI_NOTIFY_RESET mailbox call doesn't seem to be abl= e to correctly read the pcie translation state after we have set it up. So, while linux appears to be able to adjust the PCIe subsystem to the demand= s of it's DT, the XHCI won't function because its looking for an untranslat= ed DMA range to access(?) its firmware image in main RAM. So, lets apply the same non-translated 3G limit to DT's we hand off, as well as remove the XHCI reset controller info so that linux/etc doesn't t= ry to reload the firmware. This latter tweak is a bit non obvious, but seems to cure the general problem. V1->V2: Also remove the pci/usb node to keep Linux from trying to call the XHCI_NOTIFY_RESET mailbox. Jeremy Linton (1): Platform/RaspberryPi: Always use non translating DMA in DT mode Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 75 ++++++++++++++++++++++= ++++++ 1 file changed, 75 insertions(+) --=20 2.13.7
|
|
|
|
Re: [PATCH] IntelFsp2Pkg: Adopt FSP 2.3 specification.
Hi Chasel,
toggle quoted messageShow quoted text
My only feedback is let's not add the EDK1 style GUID definition to FspNonVolatileStorageHob2.h. It is redundant with the EDK2 style definition. Please see inline. With that change... Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
-----Original Message-----Please delete #define FSP_NON_VOLATILE_STORAGE_HOB2_GUID. This is the old EDK1 method of defining GUIDs and is redundant with the EDK2 style definition (extern EFI_GUID gFspNonVolatileStorageHob2Guid).
|
|
|
|
Re: [PATCH v1 9/9] ArmVirtPkg: Kvmtool: Add RNG support using FW-TRNG interface
Joey Gouly
Hi Sami,
Tested on Juno running kvmtool. Tested-by: Joey Gouly <joey.gouly@...> Thanks, Joey
|
|
|
|
Re: [PATCH edk2-platforms v1 2/2] Platform/ARM/Juno: Add RNG support using FW-TRNG interface
Joey Gouly
Hi Sami,
TF-A for Juno has been updated to implement the Arm FW-TRNG interfaceTested on Juno. Tested-by: Joey Gouly <joey.gouly@...> Thanks, Joey IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
|
|
|
Re: [PATCH v1 11/13] DynamicTablesPkg: AML code generation to add an _LPI state
Sami Mujawar
Hi Pierre, Please find my response inline marked [SAMI]. Regards, Sami Mujawar [SAMI] Can we use EFI_ACPI_6_4_SYSTEM_MEMORY here instead?From: Pierre Gondois <Pierre.Gondois@...> Add AmlAddLpiState() to generates AML code to add an _LPI state to an _LPI object created using AmlCreateLpiNode(). AmlAddLpiState increments the count of LPI states in the LPI node by one, and adds the following package: Package() { MinResidency, WorstCaseWakeLatency, Flags, ArchFlags, ResCntFreq, EnableParentState, (GenericRegisterDescriptor != NULL) ? // Entry method. If a ResourceTemplate(GenericRegisterDescriptor) : // Register is given, Integer, // use it. Use the // Integer otherwise ResourceTemplate() { // NULL Residency Register (SystemMemory, 0, 0, 0, 0) // Counter }, ResourceTemplate() { // NULL Usage Counter Register (SystemMemory, 0, 0, 0, 0) }, "" // NULL State Name }, Signed-off-by: Pierre Gondois <Pierre.Gondois@...> --- .../Include/Library/AmlLib/AmlLib.h | 71 +++ .../Common/AmlLib/CodeGen/AmlCodeGen.c | 439 ++++++++++++++++++ 2 files changed, 510 insertions(+) diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h index 40c45073d303..4932f6fd9c8b 100644 --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h @@ -716,6 +716,77 @@ AmlCreateLpiNode ( OUT AML_OBJECT_NODE_HANDLE * NewLpiNode OPTIONAL ); +/** Add an _LPI state to a LPI node created using AmlCreateLpiNode (). + + AmlAddLpiState () increments the Count of LPI states in the LPI node by one, + and adds the following package: + Package() { + MinResidency, + WorstCaseWakeLatency, + Flags, + ArchFlags, + ResCntFreq, + EnableParentState, + (GenericRegisterDescriptor != NULL) ? // Entry method. If a + ResourceTemplate(GenericRegisterDescriptor) : // Register is given, + Integer, // use it. Use the + // Integer otherwise. + ResourceTemplate() { // NULL Residency Counter + Register (SystemMemory, 0, 0, 0, 0) + }, + ResourceTemplate() { // NULL Usage Counter + Register (SystemMemory, 0, 0, 0, 0) + }, + "" // NULL State Name + }, + + Cf ACPI 6.3 specification, s8.4.4 "Lower Power Idle States". + + @ingroup CodeGenApis + + @param [in] MinResidency Minimum Residency. + @param [in] WorstCaseWakeLatency Worst case wake-up latency. + @param [in] Flags Flags. + @param [in] ArchFlags Architectural flags. + @param [in] ResCntFreq Residency Counter Frequency. + @param [in] EnableParentState Enabled Parent State. + @param [in] GenericRegisterDescriptor Entry Method. + If not NULL, use this Register to + describe the entry method address. + @param [in] Integer Entry Method. + If GenericRegisterDescriptor is NULL, + take this value. + @param [in] ResidencyCounterRegister If not NULL, use it to populate the + residency counter register. + @param [in] UsageCounterRegister If not NULL, use it to populate the + usage counter register. + @param [in] StateName If not NULL, use it to populate the + state name. + @param [in] LpiNode Lpi node created with the function + AmlCreateLpiNode to which the new LPI + state is appended. + + @retval EFI_SUCCESS The function completed successfully. + @retval EFI_INVALID_PARAMETER Invalid parameter. + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. +**/ +EFI_STATUS +EFIAPI +AmlAddLpiState ( + IN UINT32 MinResidency, + IN UINT32 WorstCaseWakeLatency, + IN UINT32 Flags, + IN UINT32 ArchFlags, + IN UINT32 ResCntFreq, + IN UINT32 EnableParentState, + IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE * GenericRegisterDescriptor, OPTIONAL + IN UINT64 Integer, OPTIONAL + IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE * ResidencyCounterRegister, OPTIONAL + IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE * UsageCounterRegister, OPTIONAL + IN CHAR8 * StateName, OPTIONAL + IN AML_OBJECT_NODE_HANDLE LpiNode + ); + // DEPRECATED APIS #ifndef DISABLE_NEW_DEPRECATED_INTERFACES diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c index 89350f65f5df..f0861040191f 100644 --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c @@ -12,6 +12,7 @@ #include <AmlCoreInterface.h> #include <AmlEncoding/Aml.h> +#include <Api/AmlApiHelper.h> #include <CodeGen/AmlResourceDataCodeGen.h> #include <Tree/AmlNode.h> #include <Tree/AmlTree.h> @@ -1554,3 +1555,441 @@ error_handler: } return Status; } + +/** Add an _LPI state to a LPI node created using AmlCreateLpiNode. + + AmlAddLpiState increments the Count of LPI states in the LPI node by one, + and adds the following package: + Package() { + MinResidency, + WorstCaseWakeLatency, + Flags, + ArchFlags, + ResCntFreq, + EnableParentState, + (GenericRegisterDescriptor != NULL) ? // Entry method. If a + ResourceTemplate(GenericRegisterDescriptor) : // Register is given, + Integer, // use it. Use the + // Integer otherwise. + ResourceTemplate() { // NULL Residency Counter + Register (SystemMemory, 0, 0, 0, 0) + }, + ResourceTemplate() { // NULL Usage Counter + Register (SystemMemory, 0, 0, 0, 0) + }, + "" // NULL State Name + }, + + Cf ACPI 6.3 specification, s8.4.4 "Lower Power Idle States". + + @param [in] MinResidency Minimum Residency. + @param [in] WorstCaseWakeLatency Worst case wake-up latency. + @param [in] Flags Flags. + @param [in] ArchFlags Architectural flags. + @param [in] ResCntFreq Residency Counter Frequency. + @param [in] EnableParentState Enabled Parent State. + @param [in] GenericRegisterDescriptor Entry Method. + If not NULL, use this Register to + describe the entry method address. + @param [in] Integer Entry Method. + If GenericRegisterDescriptor is NULL, + take this value. + @param [in] ResidencyCounterRegister If not NULL, use it to populate the + residency counter register. + @param [in] UsageCounterRegister If not NULL, use it to populate the + usage counter register. + @param [in] StateName If not NULL, use it to populate the + state name. + @param [in] LpiNode Lpi node created with the function + AmlCreateLpiNode to which the new LPI + state is appended. + + @retval EFI_SUCCESS The function completed successfully. + @retval EFI_INVALID_PARAMETER Invalid parameter. + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. +**/ +EFI_STATUS +EFIAPI +AmlAddLpiState ( + IN UINT32 MinResidency, + IN UINT32 WorstCaseWakeLatency, + IN UINT32 Flags, + IN UINT32 ArchFlags, + IN UINT32 ResCntFreq, + IN UINT32 EnableParentState, + IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE * GenericRegisterDescriptor, OPTIONAL + IN UINT64 Integer, OPTIONAL + IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE * ResidencyCounterRegister, OPTIONAL + IN EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE * UsageCounterRegister, OPTIONAL + IN CHAR8 * StateName, OPTIONAL + IN AML_OBJECT_NODE_HANDLE LpiNode + ) +{ + EFI_STATUS Status; + AML_DATA_NODE_HANDLE RdNode; + AML_OBJECT_NODE_HANDLE PackageNode; + AML_OBJECT_NODE_HANDLE IntegerNode; + AML_OBJECT_NODE_HANDLE StringNode; + AML_OBJECT_NODE_HANDLE NewLpiPackageNode; + AML_OBJECT_NODE_HANDLE ResourceTemplateNode; + + UINT32 Index; + AML_OBJECT_NODE_HANDLE CountNode; + UINT64 Count; + + if ((LpiNode == NULL) || + (AmlGetNodeType ((AML_NODE_HANDLE)LpiNode) != EAmlNodeObject) || + (!AmlNodeHasOpCode (LpiNode, AML_NAME_OP, 0))) { + ASSERT (0); + return EFI_INVALID_PARAMETER; + } + + RdNode = 0; + StringNode = NULL; + IntegerNode = NULL; + ResourceTemplateNode = NULL; + + // Get the LPI value which is represented as a PackageOp object node + // which is the 2nd fixed argument (i.e. index 1). + PackageNode = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument ( + LpiNode, + EAmlParseIndexTerm1 + ); + if ((PackageNode == NULL) || + (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) != EAmlNodeObject) || + (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0))) { + ASSERT (0); + return EFI_INVALID_PARAMETER; + } + + CountNode = NULL; + // The third variable argument is the LPI Count node. + for (Index = 0; Index < 3; Index++) { + CountNode = (AML_OBJECT_NODE_HANDLE)AmlGetNextVariableArgument ( + (AML_NODE_HANDLE)PackageNode, + (AML_NODE_HANDLE)CountNode + ); + if (CountNode == NULL) { + ASSERT (0); + return EFI_INVALID_PARAMETER; + } + } + + Status = AmlNodeGetIntegerValue (CountNode, &Count); + if (EFI_ERROR (Status)) { + ASSERT (0); + return Status; + } + Status = AmlUpdateInteger (CountNode, Count + 1); + if (EFI_ERROR (Status)) { + ASSERT (0); + return Status; + } + + Status = AmlCodeGenPackage (&NewLpiPackageNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + return Status; + } + + // MinResidency + Status = AmlCodeGenInteger (MinResidency, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + IntegerNode = NULL; + + // WorstCaseWakeLatency + Status = AmlCodeGenInteger (WorstCaseWakeLatency, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + IntegerNode = NULL; + + // Flags + Status = AmlCodeGenInteger (Flags, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + IntegerNode = NULL; + + // ArchFlags + Status = AmlCodeGenInteger (ArchFlags, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + IntegerNode = NULL; + + // ResCntFreq + Status = AmlCodeGenInteger (ResCntFreq, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + IntegerNode = NULL; + + // EnableParentState + Status = AmlCodeGenInteger (EnableParentState, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + IntegerNode = NULL; + + // Entry Method + if (GenericRegisterDescriptor != NULL) { + // Entry Method: As a Register resource data + Status = AmlCodeGenResourceTemplate (&ResourceTemplateNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlCodeGenRdRegister ( + GenericRegisterDescriptor->AddressSpaceId, + GenericRegisterDescriptor->RegisterBitWidth, + GenericRegisterDescriptor->RegisterBitOffset, + GenericRegisterDescriptor->Address, + GenericRegisterDescriptor->AccessSize, + NULL, + &RdNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + + Status = AmlAppendRdNode (ResourceTemplateNode, RdNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + RdNode = NULL; + + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)ResourceTemplateNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + ResourceTemplateNode = NULL; + } else { + // Entry Method: As an integer + Status = AmlCodeGenInteger (Integer, &IntegerNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)IntegerNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + IntegerNode = NULL; + } + + // Residency Counter Register. + Status = AmlCodeGenResourceTemplate (&ResourceTemplateNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + if (ResidencyCounterRegister != NULL) { + Status = AmlCodeGenRdRegister ( + ResidencyCounterRegister->AddressSpaceId, + ResidencyCounterRegister->RegisterBitWidth, + ResidencyCounterRegister->RegisterBitOffset, + ResidencyCounterRegister->Address, + ResidencyCounterRegister->AccessSize, + NULL, + &RdNode + ); + } else { + Status = AmlCodeGenRdRegister ( + EFI_ACPI_2_0_SYSTEM_MEMORY, + 0, + 0, + 0, + 0, + NULL, + &RdNode + ); + } + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + + Status = AmlAppendRdNode (ResourceTemplateNode, RdNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + RdNode = NULL; + + Status = AmlVarListAddTail ( + (AML_NODE_HANDLE)NewLpiPackageNode, + (AML_NODE_HANDLE)ResourceTemplateNode + ); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + ResourceTemplateNode = NULL; + + // Usage Counter Register. + Status = AmlCodeGenResourceTemplate (&ResourceTemplateNode); + if (EFI_ERROR (Status)) { + ASSERT (0); + goto error_handler; + } + if (UsageCounterRegister != NULL) { + Status = AmlCodeGenRdRegister ( + UsageCounterRegister->AddressSpaceId, + UsageCounterRegister->RegisterBitWidth, + UsageCounterRegister->RegisterBitOffset, + UsageCounterRegister->Address, + UsageCounterRegister->AccessSize, + NULL, + &RdNode + ); + } else { + Status = AmlCodeGenRdRegister ( + EFI_ACPI_2_0_SYSTEM_MEMORY,
|
|
|
|
Re: [PATCH v1 10/13] DynamicTablesPkg: AML code generation for a _LPI object
Sami Mujawar
Hi Pierre,
Please find my response inline marked [SAMI]. Regards, Sami Mujawar On 23/06/2021 12:40 PM, Pierre.Gondois@... wrote: From: Pierre Gondois <Pierre.Gondois@...>[SAMI] Can you check if setting the IntegerNode to NULL is needed here? + goto error_handler;
|
|
|