Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/25703
Change subject: soc/amd/common/block/pi/heapmanager.c: Simplify code ......................................................................
soc/amd/common/block/pi/heapmanager.c: Simplify code
There are sections of code that are almost identical and they can be converted to auxiliary procedures. For allocating heap, 3 sizes are used often so they could be stored in temporary variables. These 2 changes will make code shorter, with less indentation problems and overall easier to read. The actual logic of the code is not changed.
BUG=b:77940747 TEST=Build and boot grunt.
Change-Id: Ib4c69981eab7452228ccae9ed9bc288c8baceffe Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- M src/soc/amd/common/block/pi/heapmanager.c 1 file changed, 79 insertions(+), 86 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/25703/1
diff --git a/src/soc/amd/common/block/pi/heapmanager.c b/src/soc/amd/common/block/pi/heapmanager.c index a469a45..aa3a40d 100644 --- a/src/soc/amd/common/block/pi/heapmanager.c +++ b/src/soc/amd/common/block/pi/heapmanager.c @@ -30,6 +30,44 @@ memset(BiosManagerPtr, 0, BIOS_HEAP_SIZE); }
+static AGESA_STATUS FindNode(uint32_t handle, BIOS_BUFFER_NODE **pointer) +{ + UINT32 AllocNodeOffset; + UINT8 *BiosHeapBaseAddr; + BIOS_BUFFER_NODE *AllocNodePtr; + BIOS_HEAP_MANAGER *BiosHeapBasePtr; + AGESA_STATUS Status = AGESA_SUCCESS; + + BiosHeapBaseAddr = agesa_heap_base(); + BiosHeapBasePtr = (BIOS_HEAP_MANAGER *)BiosHeapBaseAddr; + + AllocNodeOffset = BiosHeapBasePtr->StartOfAllocatedNodes; + AllocNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr + AllocNodeOffset); + + while (handle != AllocNodePtr->BufferHandle) { + if (AllocNodePtr->NextNodeOffset == 0) { + Status = AGESA_BOUNDS_CHK; + break; + } + AllocNodeOffset = AllocNodePtr->NextNodeOffset; + AllocNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr + + AllocNodeOffset); + } + *pointer = AllocNodePtr; + return Status; +} + +static void ConcatenateNodes(BIOS_BUFFER_NODE *FirstNodePtr, + BIOS_BUFFER_NODE *SecondNodePtr) +{ + FirstNodePtr->BufferSize += SecondNodePtr->BufferSize + + sizeof(BIOS_BUFFER_NODE); + FirstNodePtr->NextNodeOffset = SecondNodePtr->NextNodeOffset; + + /* Zero out the SecondNode header */ + memset((UINT8 *)SecondNodePtr, 0, sizeof(BIOS_BUFFER_NODE)); +} + #if IS_ENABLED(CONFIG_LATE_CBMEM_INIT) #error "Only EARLY_CBMEM_INIT is supported." #endif @@ -42,9 +80,12 @@ UINT32 CurrNodeOffset; UINT32 PrevNodeOffset; UINT32 FreedNodeOffset; + UINT32 FreedNodeSize; UINT32 BestFitNodeOffset; + UINT32 BestFitNodeSize; UINT32 BestFitPrevNodeOffset; UINT32 NextFreeOffset; + UINT32 MinimumSize; BIOS_BUFFER_NODE *CurrNodePtr; BIOS_BUFFER_NODE *FreedNodePtr; BIOS_BUFFER_NODE *BestFitNodePtr; @@ -52,11 +93,14 @@ BIOS_BUFFER_NODE *NextFreePtr; BIOS_HEAP_MANAGER *BiosHeapBasePtr; AGESA_BUFFER_PARAMS *AllocParams; + AGESA_STATUS Status;
AllocParams = ((AGESA_BUFFER_PARAMS *)ConfigPtr); AllocParams->BufferPointer = NULL; + MinimumSize = AllocParams->BufferLength + sizeof(BIOS_BUFFER_NODE);
AvailableHeapSize = BIOS_HEAP_SIZE - sizeof(BIOS_HEAP_MANAGER); + BestFitNodeSize = AvailableHeapSize; /* init with largest possible */ BiosHeapBaseAddr = agesa_heap_base(); BiosHeapBasePtr = (BIOS_HEAP_MANAGER *)BiosHeapBaseAddr;
@@ -85,37 +129,29 @@ BiosHeapBasePtr->StartOfAllocatedNodes = CurrNodeOffset; BiosHeapBasePtr->StartOfFreedNodes = FreedNodeOffset; } else { - /* Find out whether BufferHandle has been allocated on the heap. + /* + * Find out whether BufferHandle has been allocated on the heap. * If it has, return AGESA_BOUNDS_CHK. */ - CurrNodeOffset = BiosHeapBasePtr->StartOfAllocatedNodes; - CurrNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr - + CurrNodeOffset); + Status = FindNode(AllocParams->BufferHandle, &CurrNodePtr); + if (Status == AGESA_SUCCESS) + return AGESA_BOUNDS_CHK;
- while (CurrNodeOffset != 0) { - CurrNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr - + CurrNodeOffset); - if (CurrNodePtr->BufferHandle == - AllocParams->BufferHandle) { - return AGESA_BOUNDS_CHK; - } - CurrNodeOffset = CurrNodePtr->NextNodeOffset; - /* If BufferHandle has not been allocated on the heap, - * CurrNodePtr here points to the end of the allocated - * nodes list. - */ - } + /* + * If status ditn't returned AGESA_SUCCESS, CurrNodePtr here + * points to the end of the allocated nodes list. + */ + /* Find the node that best fits the requested buffer size */ FreedNodeOffset = BiosHeapBasePtr->StartOfFreedNodes; PrevNodeOffset = FreedNodeOffset; BestFitNodeOffset = 0; BestFitPrevNodeOffset = 0; - while (FreedNodeOffset != 0) { /* todo: simplify this */ + while (FreedNodeOffset != 0) { FreedNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr + FreedNodeOffset); - if (FreedNodePtr->BufferSize >= - (AllocParams->BufferLength + - sizeof(BIOS_BUFFER_NODE))) { + FreedNodeSize = FreedNodePtr->BufferSize; + if (FreedNodeSize >= MinimumSize) { if (BestFitNodeOffset == 0) { /* * First node that fits the requested @@ -123,21 +159,19 @@ */ BestFitNodeOffset = FreedNodeOffset; BestFitPrevNodeOffset = PrevNodeOffset; + BestFitNodeSize = FreedNodeSize; } else { /* * Find out whether current node is a * betterfit than the previous nodes */ - BestFitNodePtr = (BIOS_BUFFER_NODE *) - (BiosHeapBaseAddr + - BestFitNodeOffset); - if (BestFitNodePtr->BufferSize > - FreedNodePtr->BufferSize) { + if (BestFitNodeSize > FreedNodeSize) {
BestFitNodeOffset = FreedNodeOffset; BestFitPrevNodeOffset = PrevNodeOffset; + BestFitNodeSize = FreedNodeSize; } } } @@ -162,22 +196,19 @@ * If BestFitNode is larger than the requested buffer, * fragment the node further */ - if (BestFitNodePtr->BufferSize > - (AllocParams->BufferLength + sizeof(BIOS_BUFFER_NODE))) { - NextFreeOffset = BestFitNodeOffset + - AllocParams->BufferLength + - sizeof(BIOS_BUFFER_NODE); + if (BestFitNodePtr->BufferSize > MinimumSize) { + NextFreeOffset = BestFitNodeOffset + MinimumSize; NextFreePtr = (BIOS_BUFFER_NODE *) (BiosHeapBaseAddr + NextFreeOffset); - NextFreePtr->BufferSize = BestFitNodePtr->BufferSize - - (AllocParams->BufferLength + - sizeof(BIOS_BUFFER_NODE)); + NextFreePtr->BufferSize = BestFitNodeSize - MinimumSize; + + /* Remove BestFitNode from list of Freed nodes */ NextFreePtr->NextNodeOffset = BestFitNodePtr->NextNodeOffset; } else { /* - * Otherwise, next free node is - * NextNodeOffset of BestFitNode + * Otherwise, next free node is NextNodeOffset of + * BestFitNode. Remove it from list of Freed nodes. */ NextFreeOffset = BestFitNodePtr->NextNodeOffset; } @@ -197,7 +228,6 @@ BestFitNodePtr->BufferHandle = AllocParams->BufferHandle; BestFitNodePtr->NextNodeOffset = 0;
- /* Remove BestFitNode from list of Freed nodes */ AllocParams->BufferPointer = (UINT8 *)BestFitNodePtr + sizeof(BIOS_BUFFER_NODE); } @@ -264,14 +294,7 @@ /* If the freed node is adjacent to the first node in * the list, concatenate both nodes */ - AllocNodePtr->BufferSize += FreedNodePtr->BufferSize + - sizeof(BIOS_BUFFER_NODE); - AllocNodePtr->NextNodeOffset = - FreedNodePtr->NextNodeOffset; - - /* Zero out the FreedNode header */ - memset((UINT8 *)FreedNodePtr, 0, - sizeof(BIOS_BUFFER_NODE)); + ConcatenateNodes(AllocNodePtr, FreedNodePtr); } else { /* Otherwise, add freed node to the start of the list * Update NextNodeOffset and BufferSize to include the @@ -302,14 +325,7 @@ if (NextNodeOffset == EndNodeOffset) { NextNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr + NextNodeOffset); - AllocNodePtr->BufferSize += NextNodePtr->BufferSize + - sizeof(BIOS_BUFFER_NODE); - AllocNodePtr->NextNodeOffset = - NextNodePtr->NextNodeOffset; - - /* Zero out the NextNode header */ - memset((UINT8 *)NextNodePtr, 0, - sizeof(BIOS_BUFFER_NODE)); + ConcatenateNodes(AllocNodePtr, NextNodePtr); } else { /*AllocNodePtr->NextNodeOffset = * FreedNodePtr->NextNodeOffset; */ @@ -324,53 +340,30 @@ EndNodeOffset = PrevNodeOffset + PrevNodePtr->BufferSize + sizeof(BIOS_BUFFER_NODE);
- if (AllocNodeOffset == EndNodeOffset) { - PrevNodePtr->NextNodeOffset = - AllocNodePtr->NextNodeOffset; - PrevNodePtr->BufferSize += AllocNodePtr->BufferSize + - sizeof(BIOS_BUFFER_NODE); - - /* Zero out the AllocNode header */ - memset((UINT8 *)AllocNodePtr, 0, - sizeof(BIOS_BUFFER_NODE)); - } else { + if (AllocNodeOffset == EndNodeOffset) + ConcatenateNodes(PrevNodePtr, AllocNodePtr); + else PrevNodePtr->NextNodeOffset = AllocNodeOffset; - } } return AGESA_SUCCESS; }
AGESA_STATUS agesa_LocateBuffer (UINT32 Func, UINTN Data, VOID *ConfigPtr) { - UINT32 AllocNodeOffset; - UINT8 *BiosHeapBaseAddr; BIOS_BUFFER_NODE *AllocNodePtr; - BIOS_HEAP_MANAGER *BiosHeapBasePtr; AGESA_BUFFER_PARAMS *AllocParams; + AGESA_STATUS Status;
AllocParams = (AGESA_BUFFER_PARAMS *)ConfigPtr;
- BiosHeapBaseAddr = agesa_heap_base(); - BiosHeapBasePtr = (BIOS_HEAP_MANAGER *)BiosHeapBaseAddr; + Status = FindNode(AllocParams->BufferHandle, &AllocNodePtr);
- AllocNodeOffset = BiosHeapBasePtr->StartOfAllocatedNodes; - AllocNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr + AllocNodeOffset); - - while (AllocParams->BufferHandle != AllocNodePtr->BufferHandle) { - if (AllocNodePtr->NextNodeOffset == 0) { - AllocParams->BufferPointer = NULL; - AllocParams->BufferLength = 0; - return AGESA_BOUNDS_CHK; - } - AllocNodeOffset = AllocNodePtr->NextNodeOffset; - AllocNodePtr = (BIOS_BUFFER_NODE *)(BiosHeapBaseAddr + - AllocNodeOffset); + if (Status == AGESA_SUCCESS) { + AllocParams->BufferPointer = (UINT8 *)((UINT8 *)AllocNodePtr + + sizeof(BIOS_BUFFER_NODE)); + AllocParams->BufferLength = AllocNodePtr->BufferSize; }
- AllocParams->BufferPointer = (UINT8 *)((UINT8 *)AllocNodePtr - + sizeof(BIOS_BUFFER_NODE)); - AllocParams->BufferLength = AllocNodePtr->BufferSize; - - return AGESA_SUCCESS; + return Status;
}