[coreboot-gerrit] Change in coreboot[master]: soc/amd/common/block/pi/heapmanager.c: Simplify code
Richard Spiegel (Code Review)
gerrit at coreboot.org
Tue Apr 17 23:25:40 CEST 2018
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 at 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;
}
--
To view, visit https://review.coreboot.org/25703
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4c69981eab7452228ccae9ed9bc288c8baceffe
Gerrit-Change-Number: 25703
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Spiegel <richard.spiegel at silverbackltd.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180417/8ccf3038/attachment-0001.html>
More information about the coreboot-gerrit
mailing list