Change in coreboot[master]: memrange: Enable memranges to support different alignments

[Furquan Shaikh (Code Review)]

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39483 )

Change subject: memrange: Enable memranges to support different alignments
......................................................................

memrange: Enable memranges to support different alignments

This change enables memranges library to support addresses with
different alignments. Before this change, memranges library supported
aligning addresses to 4KiB only. Though this works for most cases, it
might not be the right alignment for every use case. Example: There
are some resource allocator changes coming up that require a different
alignment when handling the range list.

This change adds a align parameter to struct memranges that determines
the alignment of all range lists in that memrange. In order to
continue supporting current users of memranges, default alignment is
maintained as 4KiB.

BUG=b:149186922

Signed-off-by: Furquan Shaikh 
Change-Id: I1da0743ff89da734c9a0972e3c56d9f512b3d1e8
---
M src/include/memrange.h
M src/lib/memrange.c
2 files changed, 34 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/39483/1

diff --git a/src/include/memrange.h b/src/include/memrange.h
index 7f42aa2..fba77af 100644
--- a/src/include/memrange.h
+++ b/src/include/memrange.h
@@ -24,6 +24,8 @@
    /* coreboot doesn't have a free() function. Therefore, keep a cache of
     * free'd entries.  */
    struct range_entry *free_list;
+	/* Alignment for base and end addresses of the range. */
+	size_t align;
 };

/* Each region within a memranges structure is represented by a
@@ -86,17 +88,32 @@
 #define memranges_each_entry(r, ranges) \
    for (r = (ranges)->entries; r != NULL; r = r->next)

+
 /* Initialize memranges structure providing an optional array of range_entry
- * to use as the free list. */
-void memranges_init_empty(struct memranges *ranges, struct range_entry *free,
-			  size_t num_free);
+ * to use as the free list. Additionally, it accepts an align parameter that
+ * determines the alignment of addresses. */
+void memranges_init_empty_with_alignment(struct memranges *ranges,
+					 struct range_entry *free,
+					 size_t num_free, size_t align);

/* Initialize and fill a memranges structure according to the
  * mask and match type for all memory resources. Tag each entry with the
- * specified type. */
-void memranges_init(struct memranges *ranges,
+ * specified type. Additionally, it accepts an align parameter that
+ * determines the alignment of addresses. */
+void memranges_init_with_alignment(struct memranges *ranges,
    	    unsigned long mask, unsigned long match,
-		    unsigned long tag);
+		    unsigned long tag, size_t align);
+
+/* Initialize memranges structure providing an optional array of range_entry
+ * to use as the free list. Addresses are default aligned to 4KiB. */
+#define memranges_init_empty(__ranges, __free, __num_free)	\
+	memranges_init_empty_with_alignment(__ranges, __free, __num_free, 4 * KiB)
+
+/* Initialize and fill a memranges structure according to the
+ * mask and match type for all memory resources. Tag each entry with the
+ * specified type. Addresses are default aligned to 4KiB. */
+#define memranges_init(__ranges, __mask, __match, __tag)	\
+	memranges_init_with_alignment(__ranges, __mask, __match, __tag, 4 * KiB)

/* Clone a memrange. The new memrange has the same entries as the old one. */
 void memranges_clone(struct memranges *newranges, struct memranges *oldranges);
diff --git a/src/lib/memrange.c b/src/lib/memrange.c
index 79a1b0e..a76f24c 100644
--- a/src/lib/memrange.c
+++ b/src/lib/memrange.c
@@ -231,9 +231,9 @@
    /* The addresses are aligned to 4096 bytes: the begin address is
     * aligned down while the end address is aligned up to be conservative
     * about the full range covered. */
-	begin = ALIGN_DOWN(base, 4096);
+	begin = ALIGN_DOWN(base, ranges->align);
    end = begin + size + (base - begin);
-	end = ALIGN_UP(end, 4096) - 1;
+	end = ALIGN_UP(end, ranges->align) - 1;
    action(ranges, begin, end, tag);
 }

@@ -290,23 +290,25 @@
    memranges_add_resources_filter(ranges, mask, match, tag, NULL);
 }

-void memranges_init_empty(struct memranges *ranges, struct range_entry *to_free,
-			  size_t num_free)
+void memranges_init_empty_with_alignment(struct memranges *ranges,
+					 struct range_entry *to_free,
+					 size_t num_free, size_t align)
 {
    size_t i;

ranges->entries = NULL;
    ranges->free_list = NULL;
+	ranges->align = align;

for (i = 0; i < num_free; i++)
    	range_entry_link(&ranges->free_list, &to_free[i]);
 }

-void memranges_init(struct memranges *ranges,
-		    unsigned long mask, unsigned long match,
-		    unsigned long tag)
+void memranges_init_with_alignment(struct memranges *ranges,
+				   unsigned long mask, unsigned long match,
+				   unsigned long tag, size_t align)
 {
-	memranges_init_empty(ranges, NULL, 0);
+	memranges_init_empty_with_alignment(ranges, NULL, 0, align);
    memranges_add_resources(ranges, mask, match, tag);
 }

@@ -316,7 +318,7 @@
    struct range_entry *r, *cur;
    struct range_entry **prev_ptr;

-	memranges_init_empty(newranges, NULL, 0);
+	memranges_init_empty_with_alignment(newranges, NULL, 0, oldranges->align);

prev_ptr = &newranges->entries;
    memranges_each_entry(r, oldranges) {

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39483 )

Change subject: memrange: Enable memranges to support different alignments
......................................................................

Patch Set 1: Code-Review+2

[Furquan Shaikh (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Duncan Laurie, Subrata Banik, Brandon Breitenstein, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39483

to look at the new patch set (#2).

Change subject: memrange: Enable memranges to support different alignments
......................................................................

memrange: Enable memranges to support different alignments

This change enables memranges library to support addresses with
different alignments. Before this change, memranges library supported
aligning addresses to 4KiB only. Though this works for most cases, it
might not be the right alignment for every use case. Example: There
are some resource allocator changes coming up that require a different
alignment when handling the range list.

This change adds a align parameter to struct memranges that determines
the alignment of all range lists in that memrange. In order to
continue supporting current users of memranges, default alignment is
maintained as 4KiB.

BUG=b:149186922

Signed-off-by: Furquan Shaikh 
Change-Id: I1da0743ff89da734c9a0972e3c56d9f512b3d1e8
---
M src/include/memrange.h
M src/lib/memrange.c
2 files changed, 40 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/39483/2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39483 )

Change subject: memrange: Enable memranges to support different alignments
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39483 )

Change subject: memrange: Enable memranges to support different alignments
......................................................................

memrange: Enable memranges to support different alignments

This change enables memranges library to support addresses with
different alignments. Before this change, memranges library supported
aligning addresses to 4KiB only. Though this works for most cases, it
might not be the right alignment for every use case. Example: There
are some resource allocator changes coming up that require a different
alignment when handling the range list.

This change adds a align parameter to struct memranges that determines
the alignment of all range lists in that memrange. In order to
continue supporting current users of memranges, default alignment is
maintained as 4KiB.

BUG=b:149186922

Signed-off-by: Furquan Shaikh 
Change-Id: I1da0743ff89da734c9a0972e3c56d9f512b3d1e8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39483
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Georgi 
---
M src/include/memrange.h
M src/lib/memrange.c
2 files changed, 40 insertions(+), 15 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved

diff --git a/src/include/memrange.h b/src/include/memrange.h
index 7f42aa2..0d20236 100644
--- a/src/include/memrange.h
+++ b/src/include/memrange.h
@@ -24,6 +24,8 @@
    /* coreboot doesn't have a free() function. Therefore, keep a cache of
     * free'd entries.  */
    struct range_entry *free_list;
+	/* Alignment for base and end addresses of the range. (Must be power of 2). */
+	size_t align;
 };

/* Each region within a memranges structure is represented by a
@@ -86,17 +88,32 @@
 #define memranges_each_entry(r, ranges) \
    for (r = (ranges)->entries; r != NULL; r = r->next)

+
 /* Initialize memranges structure providing an optional array of range_entry
- * to use as the free list. */
-void memranges_init_empty(struct memranges *ranges, struct range_entry *free,
-			  size_t num_free);
+ * to use as the free list. Additionally, it accepts an align parameter that
+ * determines the alignment of addresses. (Alignment must be a power of 2). */
+void memranges_init_empty_with_alignment(struct memranges *ranges,
+					 struct range_entry *free,
+					 size_t num_free, size_t align);

/* Initialize and fill a memranges structure according to the
  * mask and match type for all memory resources. Tag each entry with the
- * specified type. */
-void memranges_init(struct memranges *ranges,
+ * specified type. Additionally, it accepts an align parameter that
+ * determines the alignment of addresses. (Alignment must be a power of 2). */
+void memranges_init_with_alignment(struct memranges *ranges,
    	    unsigned long mask, unsigned long match,
-		    unsigned long tag);
+		    unsigned long tag, size_t align);
+
+/* Initialize memranges structure providing an optional array of range_entry
+ * to use as the free list. Addresses are default aligned to 4KiB. */
+#define memranges_init_empty(__ranges, __free, __num_free)	\
+	memranges_init_empty_with_alignment(__ranges, __free, __num_free, 4 * KiB)
+
+/* Initialize and fill a memranges structure according to the
+ * mask and match type for all memory resources. Tag each entry with the
+ * specified type. Addresses are default aligned to 4KiB. */
+#define memranges_init(__ranges, __mask, __match, __tag)	\
+	memranges_init_with_alignment(__ranges, __mask, __match, __tag, 4 * KiB)

/* Clone a memrange. The new memrange has the same entries as the old one. */
 void memranges_clone(struct memranges *newranges, struct memranges *oldranges);
diff --git a/src/lib/memrange.c b/src/lib/memrange.c
index 79a1b0e..21fff00 100644
--- a/src/lib/memrange.c
+++ b/src/lib/memrange.c
@@ -12,7 +12,10 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
+
+#include <assert.h>
 #include <stdlib.h>
+#include <commonlib/helpers.h>
 #include <console/console.h>
 #include <memrange.h>

@@ -231,9 +234,9 @@
    /* The addresses are aligned to 4096 bytes: the begin address is
     * aligned down while the end address is aligned up to be conservative
     * about the full range covered. */
-	begin = ALIGN_DOWN(base, 4096);
+	begin = ALIGN_DOWN(base, ranges->align);
    end = begin + size + (base - begin);
-	end = ALIGN_UP(end, 4096) - 1;
+	end = ALIGN_UP(end, ranges->align) - 1;
    action(ranges, begin, end, tag);
 }

@@ -290,23 +293,28 @@
    memranges_add_resources_filter(ranges, mask, match, tag, NULL);
 }

-void memranges_init_empty(struct memranges *ranges, struct range_entry *to_free,
-			  size_t num_free)
+void memranges_init_empty_with_alignment(struct memranges *ranges,
+					 struct range_entry *to_free,
+					 size_t num_free, size_t align)
 {
    size_t i;

+	/* Alignment must be a power of 2. */
+	assert(IS_POWER_OF_2(align));
+
    ranges->entries = NULL;
    ranges->free_list = NULL;
+	ranges->align = align;

for (i = 0; i < num_free; i++)
    	range_entry_link(&ranges->free_list, &to_free[i]);
 }

-void memranges_init(struct memranges *ranges,
-		    unsigned long mask, unsigned long match,
-		    unsigned long tag)
+void memranges_init_with_alignment(struct memranges *ranges,
+				   unsigned long mask, unsigned long match,
+				   unsigned long tag, size_t align)
 {
-	memranges_init_empty(ranges, NULL, 0);
+	memranges_init_empty_with_alignment(ranges, NULL, 0, align);
    memranges_add_resources(ranges, mask, match, tag);
 }

@@ -316,7 +324,7 @@
    struct range_entry *r, *cur;
    struct range_entry **prev_ptr;

-	memranges_init_empty(newranges, NULL, 0);
+	memranges_init_empty_with_alignment(newranges, NULL, 0, oldranges->align);

prev_ptr = &newranges->entries;
    memranges_each_entry(r, oldranges) {