Attention is currently required from: Arthur Heymans, Himanshu Sahdev, Julius Werner, Lean Sheng Tan, Rizwan Qureshi, Tarun Tuli, Wonkyu Kim, Yu-Ping Wu.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75357?usp=email )
Change subject: {cpu, security}: Stitch multiple microcodes per CPUID into CBFS
......................................................................
Patch Set 14:
(8 comments)
File src/cpu/Kconfig:
https://review.coreboot.org/c/coreboot/+/75357/comment/43c26bdd_c4748e02 :
PS14, Line 122: CPU_MICROCODE_CBFS_SPLIT_BINS
> The code does intel specific things so this needs to be captured in the name.
Acknowledged
https://review.coreboot.org/c/coreboot/+/75357/comment/b212cd6a_0b9dbda2 :
PS14, Line 123: bool "Include microcode per CPUID into CBFS"
> This option will show on ALL platforms. Does it even make sense to have it as a user visible option?
it will allow to override the default value from site-local rather forcing to enable from SoC/MB config using upstream coreboot.
As we are not uploading the ucode blobs in coreboot upstream build hence it's easy to go by default value which is `n` and don't need to specify the split ucode blob path.
https://review.coreboot.org/c/coreboot/+/75357/comment/10b91d3c_ce5e1568 :
PS14, Line 130: if the unified microcode is large.
> Searching is not what takes a long time. […]
Acknowledged
https://review.coreboot.org/c/coreboot/+/75357/comment/7aec55b8_463ad072 :
PS14, Line 134: frequent
> frequent? what does that mean in this context? booting often, loading microcode multiple times in one boot?
The intention is to convey that having split ucode makes it easy to update a particular CPUID for in-field devices rather bothering about to stitch multiple ucode together.
lets drop this like to avoid further confusion.
https://review.coreboot.org/c/coreboot/+/75357/comment/10b1d287_93d04af9 :
PS14, Line 135: requied
> required.
Acknowledged
https://review.coreboot.org/c/coreboot/+/75357/comment/70f61165_5afe8fc1 :
PS14, Line 216: config CPU_UCODE_SPLIT_BINARIES
: string "Split microcode blob directory path"
: depends on CPU_MICROCODE_CBFS_SPLIT_BINS
: default ""
: help
: Provide the microcode blob directory path based on the configuration setting that
: allows for split microcode binaries per CPUID for both RO and RW CBFS.
:
: Some platforms have microcode in the blobs directory, and these can be hardcoded
: in the makefiles. The expected format for keeping the microcode filename in the
: directory is `cpu_microcode_$(CPUID).bin`.
:
: This should contain the full path of the microcode blob directory. For example:
: "3rdparty/blobs/mainboard/$(CONFIG_MAINBOARD_DIR)/microcode_inputs".
:
: If unsure, leave this blank.
> > Can the existing options not be reused? I don't see how fetching everything from a dir is related […]
Acknowledged
https://review.coreboot.org/c/coreboot/+/75357/comment/fe408042_b4e97f76 :
PS14, Line 216: config CPU_UCODE_SPLIT_BINARIES
: string "Split microcode blob directory path"
: depends on CPU_MICROCODE_CBFS_SPLIT_BINS
: default ""
: help
: Provide the microcode blob directory path based on the configuration setting that
: allows for split microcode binaries per CPUID for both RO and RW CBFS.
:
: Some platforms have microcode in the blobs directory, and these can be hardcoded
: in the makefiles. The expected format for keeping the microcode filename in the
: directory is `cpu_microcode_$(CPUID).bin`.
:
: This should contain the full path of the microcode blob directory. For example:
: "3rdparty/blobs/mainboard/$(CONFIG_MAINBOARD_DIR)/microcode_inputs".
:
: If unsure, leave this blank.
> > Can the existing options not be reused? I don't see how fetching everything from a dir is related […]
Acknowledged
File src/cpu/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/75357/comment/e85d3290_2948c837 :
PS14, Line 83: $(eval regions-for-file-$(params) = COREBOOT,FW_MAIN_A,FW_MAIN_B) \
> Is this line needed?
we don't need this now
--
To view, visit https://review.coreboot.org/c/coreboot/+/75357?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic7db73335ffa25399869cfb0d59129ee118f1012
Gerrit-Change-Number: 75357
Gerrit-PatchSet: 14
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Himanshu Sahdev <himanshu.sahdev(a)intel.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Himanshu Sahdev <himanshu.sahdev(a)intel.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 22 Jun 2023 19:45:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Jason Nien, Jon Murphy, Martin Roth, Matt DeVillier, Paul Menzel, Raul Rangel.
CoolStar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75874?usp=email )
Change subject: mb/google/kahlee: Enable secure OS
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75874/comment/62b602de_5a977544 :
PS5, Line 17:
> Resolves: https://ticket.coreboot. […]
does not resolve because updated PSP bootloader is needed
--
To view, visit https://review.coreboot.org/c/coreboot/+/75874?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I213aebc41cae300ecee8c01fc5c7687f7e7f5ee3
Gerrit-Change-Number: 75874
Gerrit-PatchSet: 5
Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 22 Jun 2023 19:15:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/67022?usp=email )
(
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: allocator_v4: Remove redundant parameter
......................................................................
allocator_v4: Remove redundant parameter
update_bridge_resource() already gets the type passed as part of
the resource.
Change-Id: I6b3c9809caecdd1bad5b98891a00c3392190a3e0
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/67022
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Reviewed-by: Tim Wawrzynczak <inforichland(a)gmail.com>
---
M src/device/resource_allocator_v4.c
1 file changed, 3 insertions(+), 2 deletions(-)
Approvals:
Felix Singer: Looks good to me, but someone else must approve
Tim Wawrzynczak: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/device/resource_allocator_v4.c b/src/device/resource_allocator_v4.c
index fb0320a..e64d8ad 100644
--- a/src/device/resource_allocator_v4.c
+++ b/src/device/resource_allocator_v4.c
@@ -112,12 +112,13 @@
* allocated base of the bridge resource.
*/
static void update_bridge_resource(const struct device *bridge, struct resource *bridge_res,
- unsigned long type_match, int print_depth)
+ int print_depth)
{
const struct device *child;
struct resource *child_res;
resource_t base;
const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_PREFETCH;
+ const unsigned long type_match = bridge_res->flags & type_mask;
struct bus *bus = bridge->link_list;
child_res = NULL;
@@ -228,7 +229,7 @@
* Update the window for current bridge resource now that all downstream
* requirements are gathered.
*/
- update_bridge_resource(bridge, res, type_match, print_depth);
+ update_bridge_resource(bridge, res, print_depth);
}
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/67022?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6b3c9809caecdd1bad5b98891a00c3392190a3e0
Gerrit-Change-Number: 67022
Gerrit-PatchSet: 6
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-MessageType: merged
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/65422?usp=email )
(
10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: allocator_v4: Manually inline some thin functions
......................................................................
allocator_v4: Manually inline some thin functions
Inline functions that are only called once to improve readability. The
calling functions still have rather short bodies, and the reader won't
have to look down yet another layer to understand what they are doing.
Change-Id: Ib4aa5d61dfa88c804a1aaee028185e00c5fbb923
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/65422
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/device/resource_allocator_v4.c
1 file changed, 22 insertions(+), 46 deletions(-)
Approvals:
Felix Singer: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Arthur Heymans: Looks good to me, approved
diff --git a/src/device/resource_allocator_v4.c b/src/device/resource_allocator_v4.c
index d17499c..fb0320a 100644
--- a/src/device/resource_allocator_v4.c
+++ b/src/device/resource_allocator_v4.c
@@ -268,27 +268,6 @@
}
}
-static unsigned char get_alignment_by_resource_type(const unsigned long type)
-{
- if (type & IORESOURCE_MEM)
- return 12; /* Page-aligned --> log2(4KiB) */
- else if (type & IORESOURCE_IO)
- return 0; /* No special alignment required --> log2(1) */
-
- die("Unexpected resource type: flags(%lu)!\n", type);
-}
-
-static void update_constraints(struct memranges *ranges, const struct device *dev,
- const struct resource *res)
-{
- if (!res->size)
- return;
-
- print_fixed_res(dev, res, __func__);
-
- memranges_create_hole(ranges, res->base, res->size);
-}
-
/*
* Scan the entire tree to identify any fixed resources allocated by
* any device to ensure that the address map for domain resources are
@@ -310,7 +289,10 @@
for (res = dev->resource_list; res != NULL; res = res->next) {
if ((res->flags & mask_match) != mask_match)
continue;
- update_constraints(ranges, dev, res);
+ if (!res->size)
+ continue;
+ print_fixed_res(dev, res, __func__);
+ memranges_create_hole(ranges, res->base, res->size);
}
bus = dev->link_list;
@@ -321,28 +303,6 @@
avoid_fixed_resources(ranges, child, mask_match);
}
-static void constrain_domain_resources(const struct device *domain, struct memranges *ranges,
- unsigned long type)
-{
- unsigned long mask_match = type | IORESOURCE_FIXED;
-
- if (type == IORESOURCE_IO) {
- /*
- * Don't allow allocations in the VGA I/O range. PCI has special
- * cases for that.
- */
- memranges_create_hole(ranges, 0x3b0, 0x3df - 0x3b0 + 1);
-
- /*
- * Resource allocator no longer supports the legacy behavior where
- * I/O resource allocation is guaranteed to avoid aliases over legacy
- * PCI expansion card addresses.
- */
- }
-
- avoid_fixed_resources(ranges, domain, mask_match);
-}
-
/*
* This function creates a list of memranges of given type using the
* resource that is provided. It applies additional constraints to
@@ -356,8 +316,10 @@
const unsigned long type,
struct memranges *const ranges)
{
+ /* Align mem resources to 2^12 (4KiB pages) at a minimum, so they
+ can be memory-mapped individually (e.g. for virtualization guests). */
+ const unsigned char alignment = type == IORESOURCE_MEM ? 12 : 0;
const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_FIXED;
- const unsigned char alignment = get_alignment_by_resource_type(type);
memranges_init_empty_with_alignment(ranges, NULL, 0, alignment);
@@ -368,7 +330,21 @@
memranges_insert(ranges, res->base, res->limit - res->base + 1, type);
}
- constrain_domain_resources(domain, ranges, type);
+ if (type == IORESOURCE_IO) {
+ /*
+ * Don't allow allocations in the VGA I/O range. PCI has special
+ * cases for that.
+ */
+ memranges_create_hole(ranges, 0x3b0, 0x3df - 0x3b0 + 1);
+
+ /*
+ * Resource allocator no longer supports the legacy behavior where
+ * I/O resource allocation is guaranteed to avoid aliases over legacy
+ * PCI expansion card addresses.
+ */
+ }
+
+ avoid_fixed_resources(ranges, domain, type | IORESOURCE_FIXED);
print_resource_ranges(domain, ranges);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/65422?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4aa5d61dfa88c804a1aaee028185e00c5fbb923
Gerrit-Change-Number: 65422
Gerrit-PatchSet: 13
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/65420?usp=email )
Change subject: allocator_v4: Use memranges only for toplevel
......................................................................
allocator_v4: Use memranges only for toplevel
During phase 1 of the resource allocation we gather all the size
requirements. Starting from the leafs of our devicetree, we cal-
culate the requirements per bus, until we reach the resource do-
main.
However, because alignment plays a role, we can't just accumulate
the sizes of all resources on a bus. Instead, we already sort all
the resources per bus to predict their relative placement, inclu-
ding alignment gaps. Then, phase 2 has to perform the final allo-
cations with the exact same relative placement.
This patch introduces a very simple mechanism to avoid repeating
all the calculations: In phase 1, we note the relative `base` of
each resource on a bus. And after we allocated all the resources
directly below the domain in phase 2, we add the absolute `base`
of bridge resources to the relative `base` of child resources.
This saves most of the computational complexity in phase 2. How-
ever, with a shallow devicetree with most devices directly below
the domain, this won't have a measurable impact.
Example after phase 1:
domain
|
`-- bridge #0
| res #0, base 0x000000 (relative),
| size 12M, align 8M
|
|-- device #0
| res #1, base 0x800000 (relative),
| size 4M, align 4M
|
`-- bridge #1
| res #2, base 0x000000 (relative),
| size 8M, align 8M
|
`-- device #1
res #3, base 0x000000 (relative),
size 8M, align 8M
After phase 2 allocation at the domain level (assuming res #0 got
0xa000000 assigned):
domain
|
`-- bridge #0
| res #0, base 0xa000000 (absolute),
| size 12M, align 8M
|
|-- device #0
| res #1, base 0x800000 (relative),
| size 4M, align 4M
|
`-- bridge #1
| res #2, base 0x000000 (relative),
| size 8M, align 8M
|
`-- device #1
res #3, base 0x000000 (relative),
size 8M, align 8M
Now, all we need to do is to add the `base` of bridge resources
recursively. Starting with resources on the bus below bridge #0:
domain
|
`-- bridge #0
| res #0, base 0xa000000 (absolute),
| size 12M, align 8M
|
|-- device #0
| res #1, base 0xa800000 (absolute),
| size 4M, align 4M
|
`-- bridge #1
| res #2, base 0xa000000 (absolute),
| size 8M, align 8M
|
`-- device #1
res #3, base 0x000000 (relative),
size 8M, align 8M
And finally for resources on the bus below bridge #1:
domain
|
`-- bridge #0
| res #0, base 0xa000000 (absolute),
| size 12M, align 8M
|
|-- device #0
| res #1, base 0xa800000 (absolute),
| size 4M, align 4M
|
`-- bridge #1
| res #2, base 0xa000000 (absolute),
| size 8M, align 8M
|
`-- device #1
res #3, base 0xa000000 (absolute),
size 8M, align 8M
Change-Id: I70c700318a85f6760f27597730bc9c9a86dbe6b3
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/65420
Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com>
Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
---
M src/device/resource_allocator_v4.c
1 file changed, 123 insertions(+), 125 deletions(-)
Approvals:
Felix Singer: Looks good to me, but someone else must approve
Lean Sheng Tan: Looks good to me, approved
build bot (Jenkins): Verified
Arthur Heymans: Looks good to me, approved
diff --git a/src/device/resource_allocator_v4.c b/src/device/resource_allocator_v4.c
index d758f01..9bf5aaf 100644
--- a/src/device/resource_allocator_v4.c
+++ b/src/device/resource_allocator_v4.c
@@ -49,6 +49,11 @@
* resource. This is required to guarantee that the upstream bridge/
* domain honors the limit and alignment requirements for this bridge
* based on the tightest constraints downstream.
+ *
+ * Last but not least, it stores the offset inside the bridge resource
+ * for each child resource in its base field. This simplifies pass 2
+ * for resources behind a bridge, as we only have to add offsets to the
+ * allocated base of the bridge resource.
*/
static void update_bridge_resource(const struct device *bridge, struct resource *bridge_res,
unsigned long type_match, int print_depth)
@@ -89,13 +94,8 @@
/*
* Propagate the resource alignment to the bridge resource. The
* condition can only be true for the first (largest) resource. For all
- * other children resources, alignment is taken care of by updating the
- * base to round up as per the child resource alignment. It is
- * guaranteed that pass 2 follows the exact same method of picking the
- * resource for allocation using largest_resource(). Thus, as long as
- * the alignment for the largest child resource is propagated up to the
- * bridge resource, it can be guaranteed that the alignment for all
- * resources is appropriately met.
+ * other child resources, alignment is taken care of by rounding their
+ * base up.
*/
if (child_res->align > bridge_res->align)
bridge_res->align = child_res->align;
@@ -103,10 +103,8 @@
/*
* Propagate the resource limit to the bridge resource. If a downstream
* device has stricter requirements w.r.t. limits for any resource, that
- * constraint needs to be propagated back up to the downstream bridges
- * of the domain. This guarantees that the resource allocation which
- * starts at the domain level takes into account all these constraints
- * thus working on a global view.
+ * constraint needs to be propagated back up to the bridges downstream
+ * of the domain. This way, the whole bridge resource fulfills the limit.
*/
if (effective_limit(child_res) < bridge_res->limit)
bridge_res->limit = effective_limit(child_res);
@@ -117,6 +115,14 @@
*/
base = ALIGN_UP(base, POWER_OF_2(child_res->align));
+ /*
+ * Store the relative offset inside the bridge resource for later
+ * consumption in allocate_bridge_resources(), and invalidate flags
+ * related to the base.
+ */
+ child_res->base = base;
+ child_res->flags &= ~(IORESOURCE_ASSIGNED | IORESOURCE_STORED);
+
res_printk(print_depth + 1, "%s %02lx * [0x%llx - 0x%llx] %s\n",
dev_path(child), child_res->index, base, base + child_res->size - 1,
resource2str(child_res));
@@ -237,48 +243,6 @@
}
}
-/*
- * This is where the actual allocation of resources happens during
- * pass 2. Given the list of memory ranges corresponding to the
- * resource of given type, it finds the biggest unallocated resource
- * using the type mask on the downstream bus. This continues in a
- * descending order until all resources of given type are allocated
- * address space within the current resource window.
- */
-static void allocate_child_resources(struct bus *bus, struct memranges *ranges,
- unsigned long type_mask, unsigned long type_match)
-{
- const bool allocate_top_down =
- bus->dev->path.type == DEVICE_PATH_DOMAIN &&
- CONFIG(RESOURCE_ALLOCATION_TOP_DOWN);
- struct resource *resource = NULL;
- const struct device *dev;
-
- while ((dev = largest_resource(bus, &resource, type_mask, type_match))) {
-
- if (!resource->size)
- continue;
-
- if (memranges_steal(ranges, effective_limit(resource), resource->size,
- resource->align, type_match, &resource->base,
- allocate_top_down) == false) {
- printk(BIOS_ERR, "Resource didn't fit!!! ");
- printk(BIOS_DEBUG, " %s %02lx * size: 0x%llx limit: %llx %s\n",
- dev_path(dev), resource->index, resource->size,
- effective_limit(resource), resource2str(resource));
- continue;
- }
-
- resource->limit = resource->base + resource->size - 1;
- resource->flags |= IORESOURCE_ASSIGNED;
-
- printk(BIOS_DEBUG, " %s %02lx * [0x%llx - 0x%llx] limit: %llx %s\n",
- dev_path(dev), resource->index, resource->base,
- resource->size ? resource->base + resource->size - 1 :
- resource->base, resource->limit, resource2str(resource));
- }
-}
-
static void update_constraints(struct memranges *ranges, const struct device *dev,
const struct resource *res)
{
@@ -348,49 +312,36 @@
/*
* This function creates a list of memranges of given type using the
- * resource that is provided. If the given resource is unassigned or if
- * the resource window size is 0, then it creates an empty list. This
- * results in resource allocation for that resource type failing for
- * all downstream devices since there is nothing to allocate from.
- *
- * In case of domain, it applies additional constraints to ensure that
- * the memranges do not overlap any of the fixed resources under that
- * domain. Domain typically seems to provide memrange for entire address
- * space. Thus, it is up to the chipset to add DRAM and all other
- * windows which cannot be used for resource allocation as fixed
- * resources.
+ * resource that is provided. It applies additional constraints to
+ * ensure that the memranges do not overlap any of the fixed resources
+ * under the domain. The domain typically provides a memrange for the
+ * entire address space. Thus, it is up to the chipset to add DRAM and
+ * all other windows which cannot be used for resource allocation as
+ * fixed resources.
*/
-static void setup_resource_ranges(const struct device *dev, unsigned long type,
- struct memranges *ranges)
+static void setup_resource_ranges(const struct device *const domain,
+ const unsigned long type,
+ struct memranges *const ranges)
{
- const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_FIXED |
- (dev->path.type != DEVICE_PATH_DOMAIN
- ? IORESOURCE_PREFETCH | IORESOURCE_ASSIGNED
- : 0);
- const unsigned long type_match = type |
- (dev->path.type != DEVICE_PATH_DOMAIN ? IORESOURCE_ASSIGNED : 0);
+ const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_FIXED;
const unsigned char alignment = get_alignment_by_resource_type(type);
memranges_init_empty_with_alignment(ranges, NULL, 0, alignment);
- for (struct resource *res = dev->resource_list; res != NULL; res = res->next) {
- if ((res->flags & type_mask) != type_match)
+ for (struct resource *res = domain->resource_list; res != NULL; res = res->next) {
+ if ((res->flags & type_mask) != type)
continue;
printk(BIOS_DEBUG, "%s %s: base: %llx size: %llx align: %u gran: %u limit: %llx\n",
- dev_path(dev), resource2str(res), res->base, res->size, res->align,
+ dev_path(domain), resource2str(res), res->base, res->size, res->align,
res->gran, res->limit);
memranges_insert(ranges, res->base, res->limit - res->base + 1, type);
-
- if (dev->path.type != DEVICE_PATH_DOMAIN)
- break; /* only one resource per type for bridges */
}
- if (dev->path.type == DEVICE_PATH_DOMAIN)
- constrain_domain_resources(dev, ranges, type);
+ constrain_domain_resources(domain, ranges, type);
- print_resource_ranges(dev, ranges);
+ print_resource_ranges(domain, ranges);
}
static void print_resource_done(const struct device *dev, const struct resource *res)
@@ -413,49 +364,104 @@
}
}
+static void assign_resource(struct resource *const res, const resource_t base,
+ const struct device *const dev)
+{
+ res->base = base;
+ res->limit = res->base + res->size - 1;
+ res->flags |= IORESOURCE_ASSIGNED;
+ res->flags &= ~IORESOURCE_STORED;
+
+ printk(BIOS_DEBUG, " %s %02lx * [0x%llx - 0x%llx] limit: %llx %s\n",
+ dev_path(dev), res->index, res->base, res->limit, res->limit, resource2str(res));
+}
+
+/*
+ * This is where the actual allocation of resources happens during
+ * pass 2. We construct a list of memory ranges corresponding to the
+ * resource of a given type, then look for the biggest unallocated
+ * resource on the downstream bus. This continues in a descending order
+ * until all resources of a given type have space allocated within the
+ * domain's resource window.
+ */
+static void allocate_toplevel_resources(const struct device *const domain,
+ const unsigned long type)
+{
+ const unsigned long type_mask = IORESOURCE_TYPE_MASK;
+ struct resource *res = NULL;
+ const struct device *dev;
+ struct memranges ranges;
+ resource_t base;
+
+ if (!dev_has_children(domain))
+ return;
+
+ setup_resource_ranges(domain, type, &ranges);
+
+ while ((dev = largest_resource(domain->link_list, &res, type_mask, type))) {
+
+ if (!res->size)
+ continue;
+
+ if (!memranges_steal(&ranges, res->limit, res->size, res->align, type, &base,
+ CONFIG(RESOURCE_ALLOCATION_TOP_DOWN))) {
+ printk(BIOS_ERR, "Resource didn't fit!!! ");
+ printk(BIOS_DEBUG, " %s %02lx * size: 0x%llx limit: %llx %s\n",
+ dev_path(dev), res->index, res->size,
+ res->limit, resource2str(res));
+ continue;
+ }
+
+ assign_resource(res, base, dev);
+ }
+
+ cleanup_domain_resource_ranges(domain, &ranges, type);
+}
+
/*
* Pass 2 of the resource allocator at the bridge level loops through
- * all the resources for the bridge and generates a list of memory
- * ranges similar to that at the domain level. However, there is no need
- * to apply any additional constraints since the window allocated to the
- * bridge is guaranteed to be non-overlapping by the allocator at domain
- * level.
- *
- * Allocation at the bridge level works the same as at domain level
- * (starts with the biggest resource requirement from downstream devices
- * and continues in descending order). One major difference at the
- * bridge level is that it considers prefmem resources separately from
- * mem resources.
+ * all the resources for the bridge and assigns all the base addresses
+ * of its children's resources of the same type. update_bridge_resource()
+ * of pass 1 pre-calculated the offsets of these bases inside the bridge
+ * resource. Now that the bridge resource is allocated, all we have to
+ * do is to add its final base to these offsets.
*
* Once allocation at the current bridge is complete, resource allocator
* continues walking down the downstream bridges until it hits the leaf
* devices.
*/
+static void assign_resource_cb(void *param, struct device *dev, struct resource *res)
+{
+ /* We have to filter the same resources as update_bridge_resource(). */
+ if (!res->size || !res->limit)
+ return;
+
+ assign_resource(res, *(const resource_t *)param + res->base, dev);
+}
static void allocate_bridge_resources(const struct device *bridge)
{
- struct memranges ranges;
- const struct resource *res;
- struct bus *bus = bridge->link_list;
- unsigned long type_match;
+ const unsigned long type_mask =
+ IORESOURCE_TYPE_MASK | IORESOURCE_PREFETCH | IORESOURCE_FIXED;
+ struct bus *const bus = bridge->link_list;
+ struct resource *res;
struct device *child;
- const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_PREFETCH;
- for (res = bridge->resource_list; res; res = res->next) {
+ for (res = bridge->resource_list; res != NULL; res = res->next) {
if (!res->size)
continue;
if (!(res->flags & IORESOURCE_BRIDGE))
continue;
- type_match = res->flags & type_mask;
+ if (!(res->flags & IORESOURCE_ASSIGNED))
+ continue;
- setup_resource_ranges(bridge, type_match, &ranges);
- allocate_child_resources(bus, &ranges, type_mask, type_match);
- print_resource_done(bridge, res);
- memranges_teardown(&ranges);
+ /* Run assign_resource_cb() for all downstream resources of the same type. */
+ search_bus_resources(bus, type_mask, res->flags & type_mask,
+ assign_resource_cb, &res->base);
}
- for (child = bus->children; child; child = child->sibling) {
+ for (child = bus->children; child != NULL; child = child->sibling) {
if (!dev_has_children(child))
continue;
@@ -473,19 +479,13 @@
* resource requirements of the downstream devices.
*
* Once resources are allocated to all downstream devices of the domain,
- * it walks down each downstream bridge to continue the same process
- * until resources are allocated to all devices under the domain.
+ * it walks down each downstream bridge to finish resource assignment
+ * of its children resources within its own window.
*/
static void allocate_domain_resources(const struct device *domain)
{
- struct memranges ranges;
- struct device *child;
-
/* Resource type I/O */
- setup_resource_ranges(domain, IORESOURCE_IO, &ranges);
- allocate_child_resources(domain->link_list, &ranges, IORESOURCE_TYPE_MASK,
- IORESOURCE_IO);
- cleanup_domain_resource_ranges(domain, &ranges, IORESOURCE_IO);
+ allocate_toplevel_resources(domain, IORESOURCE_IO);
/*
* Resource type Mem:
@@ -494,11 +494,9 @@
* together when finding the best fit based on the biggest resource
* requirement.
*/
- setup_resource_ranges(domain, IORESOURCE_MEM, &ranges);
- allocate_child_resources(domain->link_list, &ranges,
- IORESOURCE_TYPE_MASK, IORESOURCE_MEM);
- cleanup_domain_resource_ranges(domain, &ranges, IORESOURCE_MEM);
+ allocate_toplevel_resources(domain, IORESOURCE_MEM);
+ struct device *child;
for (child = domain->link_list->children; child; child = child->sibling) {
if (!dev_has_children(child))
continue;
@@ -537,12 +535,12 @@
* resource allocation for all immediate downstream devices is complete
* at the domain level, the resource allocator walks down the subtree
* for each downstream bridge to continue the allocation process at the
- * bridge level. Since bridges have separate windows for i/o, mem and
- * prefmem, best fit algorithm at bridge level looks for the biggest
- * requirement considering prefmem resources separately from non-prefmem
- * resources. This continues until resource allocation is performed for
- * all downstream bridges in the domain sub-tree. This is referred to as
- * pass 2 of the resource allocator.
+ * bridge level. Since bridges have either their whole window allocated
+ * or nothing, we only need to place downstream resources inside these
+ * windows by re-using offsets that were pre-calculated in pass 1. This
+ * continues until resource allocation is realized for all downstream
+ * bridges in the domain sub-tree. This is referred to as pass 2 of the
+ * resource allocator.
*
* Some rules that are followed by the resource allocator:
* - Allocate resource locations for every device as long as
@@ -566,8 +564,8 @@
post_log_path(child);
- /* Pass 1 - Gather requirements. */
- printk(BIOS_INFO, "=== Resource allocator: %s - Pass 1 (gathering requirements) ===\n",
+ /* Pass 1 - Relative placement. */
+ printk(BIOS_INFO, "=== Resource allocator: %s - Pass 1 (relative placement) ===\n",
dev_path(child));
compute_domain_resources(child);
--
To view, visit https://review.coreboot.org/c/coreboot/+/65420?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70c700318a85f6760f27597730bc9c9a86dbe6b3
Gerrit-Change-Number: 65420
Gerrit-PatchSet: 21
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-MessageType: merged
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75012?usp=email )
Change subject: allocator_v4: Treat above 4G resources more natively
......................................................................
allocator_v4: Treat above 4G resources more natively
We currently have two competing mechanisms to limit the placement of
resources:
1. the explicit `.limit` field of a resource, and
2. the IORESOURCE_ABOVE_4G flag.
This makes the resource allocator unnecessarily complex. Ideally, we
would always reduce the `.limit` field if we want to "pin" a specific
resource below 4G. However, as that's not done across the tree yet,
we will use the _absence_ of the IORESOURCE_ABOVE_4G flag as a hint
to implicitly lower the `limit` of a resource. In this patch, this
is done inside the effective_limit() function that hides the flag
from the rest of the allocator.
To automatically place resources above 4G if their limit allows it,
we have to allocate from top down. Hence, we disable the prompt for
RESOURCE_ALLOCATION_TOP_DOWN and turn it on by default. Platforms
that are incompatible should be fixed, but can also override the
default as a temporary measure.
One implication of the changes is that we act differently when a
cold-plugged device reports a prefetchable resource with 32-bit
limit. Before this change, we would fail to allocate the resource.
After this change, it forces everything on the same root port below
the 4G line.
A possible solution to get completely rid of the IORESOURCE_ABOVE_4G
flag would be rules to place resources of certain devices below 4G.
For instance, the primary VGA device and storage and HID devices
could be made available to a payload that can only address 32 bits.
For now, effective_limit() provides us enough abstraction as if the
`limit` would be the only variable to consider. With this, we get
rid of all the special handling of above 4G resources during phase 2
of the allocator. Which saves us about 20% of the code :D
An earlier version of this change (commit 117e43611548) had to be
reverted because of missing resource reservations in platform code.
This is worked around now with commit ae81497cb6c7 (device/pci:
Limit default domain memory window).
Change-Id: Ia822f0ce648c7f7afc801d9cb00b6459fe7cebea
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
Original-reviewed-on: https://review.coreboot.org/c/coreboot/+/65413
Original-reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
Original-reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/75012
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com>
Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/device/Kconfig
M src/device/resource_allocator_v4.c
2 files changed, 51 insertions(+), 189 deletions(-)
Approvals:
Felix Singer: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Arthur Heymans: Looks good to me, approved
Lean Sheng Tan: Looks good to me, approved
diff --git a/src/device/Kconfig b/src/device/Kconfig
index 9c9ecd1..2d2c6ff 100644
--- a/src/device/Kconfig
+++ b/src/device/Kconfig
@@ -994,11 +994,12 @@
is aborted and an error returned.
config RESOURCE_ALLOCATION_TOP_DOWN
- bool "Allocate resources from top down"
- default n
+ def_bool y
help
- Should be the default, but many platforms don't report resources
- correctly. Hence, the allocator might cause conflicts.
+ Top-down allocation is required to place resources above 4G by
+ default (i.e. even when there is still space below). On some
+ platforms, it might make a difference because of conflicts with
+ undeclared resources.
config XHCI_UTILS
def_bool n
diff --git a/src/device/resource_allocator_v4.c b/src/device/resource_allocator_v4.c
index b6b9fb9..d758f01 100644
--- a/src/device/resource_allocator_v4.c
+++ b/src/device/resource_allocator_v4.c
@@ -24,6 +24,17 @@
return bus && bus->children;
}
+static resource_t effective_limit(const struct resource *const res)
+{
+ /* Always allow bridge resources above 4G. */
+ if (res->flags & IORESOURCE_BRIDGE)
+ return res->limit;
+
+ const resource_t quirk_4g_limit =
+ res->flags & IORESOURCE_ABOVE_4G ? UINT64_MAX : UINT32_MAX;
+ return MIN(res->limit, quirk_4g_limit);
+}
+
#define res_printk(depth, str, ...) printk(BIOS_DEBUG, "%*c"str, depth, ' ', __VA_ARGS__)
/*
@@ -97,22 +108,8 @@
* starts at the domain level takes into account all these constraints
* thus working on a global view.
*/
- if (child_res->limit < bridge_res->limit)
- bridge_res->limit = child_res->limit;
-
- /*
- * Propagate the downstream resource request to allocate above 4G
- * boundary to upstream bridge resource. This ensures that during
- * pass 2, the resource allocator at domain level has a global view
- * of all the downstream device requirements and thus address space
- * is allocated as per updated flags in the bridge resource.
- *
- * Since the bridge resource is a single window, all the downstream
- * resources of this bridge resource will be allocated in space above
- * the 4G boundary.
- */
- if (child_res->flags & IORESOURCE_ABOVE_4G)
- bridge_res->flags |= IORESOURCE_ABOVE_4G;
+ if (effective_limit(child_res) < bridge_res->limit)
+ bridge_res->limit = effective_limit(child_res);
/*
* Alignment value of 0 means that the child resource has no alignment
@@ -225,147 +222,6 @@
die("Unexpected resource type: flags(%lu)!\n", type);
}
-/*
- * If the resource is NULL or if the resource is not assigned, then it
- * cannot be used for allocation for downstream devices.
- */
-static bool is_resource_invalid(const struct resource *res)
-{
- return (res == NULL) || !(res->flags & IORESOURCE_ASSIGNED);
-}
-
-static void initialize_domain_io_resource_memranges(struct memranges *ranges,
- const struct resource *res,
- unsigned long memrange_type)
-{
- memranges_insert(ranges, res->base, res->limit - res->base + 1, memrange_type);
-}
-
-static void initialize_domain_mem_resource_memranges(struct memranges *ranges,
- const struct resource *res,
- unsigned long memrange_type)
-{
- resource_t res_base;
- resource_t res_limit;
-
- const resource_t limit_4g = 0xffffffff;
-
- res_base = res->base;
- res_limit = res->limit;
-
- /*
- * Split the resource into two separate ranges if it crosses the 4G
- * boundary. Memrange type is set differently to ensure that memrange
- * does not merge these two ranges. For the range above 4G boundary,
- * given memrange type is ORed with IORESOURCE_ABOVE_4G.
- */
- if (res_base <= limit_4g) {
-
- resource_t range_limit;
-
- /* Clip the resource limit at 4G boundary if necessary. */
- range_limit = MIN(res_limit, limit_4g);
- memranges_insert(ranges, res_base, range_limit - res_base + 1, memrange_type);
-
- /*
- * If the resource lies completely below the 4G boundary, nothing more
- * needs to be done.
- */
- if (res_limit <= limit_4g)
- return;
-
- /*
- * If the resource window crosses the 4G boundary, then update res_base
- * to add another entry for the range above the boundary.
- */
- res_base = limit_4g + 1;
- }
-
- if (res_base > res_limit)
- return;
-
- /*
- * If resource lies completely above the 4G boundary or if the resource
- * was clipped to add two separate ranges, the range above 4G boundary
- * has the resource flag IORESOURCE_ABOVE_4G set. This allows domain to
- * handle any downstream requests for resource allocation above 4G
- * differently.
- */
- memranges_insert(ranges, res_base, res_limit - res_base + 1,
- memrange_type | IORESOURCE_ABOVE_4G);
-}
-
-/*
- * This function initializes memranges for domain device. If the
- * resource crosses 4G boundary, then this function splits it into two
- * ranges -- one for the window below 4G and the other for the window
- * above 4G. The latter range has IORESOURCE_ABOVE_4G flag set to
- * satisfy resource requests from downstream devices for allocations
- * above 4G.
- */
-static void initialize_domain_memranges(const struct device *dev, struct memranges *ranges,
- unsigned long memrange_type)
-{
- unsigned char align = get_alignment_by_resource_type(memrange_type);
-
- memranges_init_empty_with_alignment(ranges, NULL, 0, align);
-
- struct resource *res;
- for (res = dev->resource_list; res != NULL; res = res->next) {
- if (is_resource_invalid(res))
- continue;
- if (res->flags & IORESOURCE_FIXED)
- continue;
- if ((res->flags & IORESOURCE_TYPE_MASK) != memrange_type)
- continue;
-
- printk(BIOS_DEBUG, "%s %s: base: %llx size: %llx align: %u gran: %u limit: %llx\n",
- dev_path(dev), resource2str(res), res->base, res->size, res->align,
- res->gran, res->limit);
-
- if (res->flags & IORESOURCE_IO)
- initialize_domain_io_resource_memranges(ranges, res, memrange_type);
- else
- initialize_domain_mem_resource_memranges(ranges, res, memrange_type);
- }
-}
-
-/*
- * This function initializes memranges for bridge device. Unlike domain,
- * bridge does not need to care about resource window crossing 4G
- * boundary. This is handled by the resource allocator at domain level
- * to ensure that all downstream bridges are allocated space either
- * above or below 4G boundary as per the state of IORESOURCE_ABOVE_4G
- * for the respective bridge resource.
- *
- * So, this function creates a single range of the entire resource
- * window available for the bridge resource. Thus all downstream
- * resources of the bridge for the given resource type get allocated
- * space from the same window. If there is any downstream resource of
- * the bridge which requests allocation above 4G, then all other
- * downstream resources of the same type under the bridge get allocated
- * above 4G.
- */
-static void initialize_bridge_memranges(const struct device *dev, struct memranges *ranges,
- unsigned long memrange_type)
-{
- unsigned char align = get_alignment_by_resource_type(memrange_type);
-
- memranges_init_empty_with_alignment(ranges, NULL, 0, align);
-
- struct resource *res;
- for (res = dev->resource_list; res != NULL; res = res->next) {
- if (is_resource_invalid(res))
- continue;
- if (res->flags & IORESOURCE_FIXED)
- continue;
- if ((res->flags & (IORESOURCE_TYPE_MASK | IORESOURCE_PREFETCH)) == memrange_type)
- break;
- }
-
- memranges_insert(ranges, res->base, res->limit - res->base + 1, memrange_type);
-}
-
static void print_resource_ranges(const struct device *dev, const struct memranges *ranges)
{
const struct range_entry *r;
@@ -403,12 +259,13 @@
if (!resource->size)
continue;
- if (memranges_steal(ranges, resource->limit, resource->size, resource->align,
- type_match, &resource->base, allocate_top_down) == false) {
+ if (memranges_steal(ranges, effective_limit(resource), resource->size,
+ resource->align, type_match, &resource->base,
+ allocate_top_down) == false) {
printk(BIOS_ERR, "Resource didn't fit!!! ");
printk(BIOS_DEBUG, " %s %02lx * size: 0x%llx limit: %llx %s\n",
- dev_path(dev), resource->index,
- resource->size, resource->limit, resource2str(resource));
+ dev_path(dev), resource->index, resource->size,
+ effective_limit(resource), resource2str(resource));
continue;
}
@@ -491,8 +348,8 @@
/*
* This function creates a list of memranges of given type using the
- * resource that is provided. If the given resource is NULL or if the
- * resource window size is 0, then it creates an empty list. This
+ * resource that is provided. If the given resource is unassigned or if
+ * the resource window size is 0, then it creates an empty list. This
* results in resource allocation for that resource type failing for
* all downstream devices since there is nothing to allocate from.
*
@@ -506,13 +363,33 @@
static void setup_resource_ranges(const struct device *dev, unsigned long type,
struct memranges *ranges)
{
- if (dev->path.type == DEVICE_PATH_DOMAIN) {
- initialize_domain_memranges(dev, ranges, type);
- constrain_domain_resources(dev, ranges, type);
- } else {
- initialize_bridge_memranges(dev, ranges, type);
+ const unsigned long type_mask = IORESOURCE_TYPE_MASK | IORESOURCE_FIXED |
+ (dev->path.type != DEVICE_PATH_DOMAIN
+ ? IORESOURCE_PREFETCH | IORESOURCE_ASSIGNED
+ : 0);
+ const unsigned long type_match = type |
+ (dev->path.type != DEVICE_PATH_DOMAIN ? IORESOURCE_ASSIGNED : 0);
+ const unsigned char alignment = get_alignment_by_resource_type(type);
+
+ memranges_init_empty_with_alignment(ranges, NULL, 0, alignment);
+
+ for (struct resource *res = dev->resource_list; res != NULL; res = res->next) {
+ if ((res->flags & type_mask) != type_match)
+ continue;
+
+ printk(BIOS_DEBUG, "%s %s: base: %llx size: %llx align: %u gran: %u limit: %llx\n",
+ dev_path(dev), resource2str(res), res->base, res->size, res->align,
+ res->gran, res->limit);
+
+ memranges_insert(ranges, res->base, res->limit - res->base + 1, type);
+
+ if (dev->path.type != DEVICE_PATH_DOMAIN)
+ break; /* only one resource per type for bridges */
}
+ if (dev->path.type == DEVICE_PATH_DOMAIN)
+ constrain_domain_resources(dev, ranges, type);
+
print_resource_ranges(dev, ranges);
}
@@ -528,8 +405,6 @@
{
memranges_teardown(ranges);
for (struct resource *res = dev->resource_list; res != NULL; res = res->next) {
- if (is_resource_invalid(res))
- continue;
if (res->flags & IORESOURCE_FIXED)
continue;
if ((res->flags & IORESOURCE_TYPE_MASK) != type)
@@ -618,24 +493,10 @@
* the resource allocation at domain level considers mem and prefmem
* together when finding the best fit based on the biggest resource
* requirement.
- *
- * However, resource requests for allocation above 4G boundary need to
- * be handled separately if the domain resource window crosses this
- * boundary. There is a single window for resource of type
- * IORESOURCE_MEM. When creating memranges, this resource is split into
- * two separate ranges -- one for the window below 4G boundary and other
- * for the window above 4G boundary (with IORESOURCE_ABOVE_4G flag set).
- * Thus, when allocating child resources, requests for below and above
- * the 4G boundary are handled separately by setting the type_mask and
- * type_match to allocate_child_resources() accordingly.
*/
setup_resource_ranges(domain, IORESOURCE_MEM, &ranges);
allocate_child_resources(domain->link_list, &ranges,
- IORESOURCE_TYPE_MASK | IORESOURCE_ABOVE_4G,
- IORESOURCE_MEM);
- allocate_child_resources(domain->link_list, &ranges,
- IORESOURCE_TYPE_MASK | IORESOURCE_ABOVE_4G,
- IORESOURCE_MEM | IORESOURCE_ABOVE_4G);
+ IORESOURCE_TYPE_MASK, IORESOURCE_MEM);
cleanup_domain_resource_ranges(domain, &ranges, IORESOURCE_MEM);
for (child = domain->link_list->children; child; child = child->sibling) {
--
To view, visit https://review.coreboot.org/c/coreboot/+/75012?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia822f0ce648c7f7afc801d9cb00b6459fe7cebea
Gerrit-Change-Number: 75012
Gerrit-PatchSet: 7
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76007?usp=email )
Change subject: acpi/acpigen.c: Ignore compiler warning about stack overflowing
......................................................................
acpi/acpigen.c: Ignore compiler warning about stack overflowing
With arm64 -Wstack-usage= is enabled which is triggered on any use of
alloca(). Since this function basically works on x86 without wrecking
things and causing massive stack consumption it's unlikely to cause
problems on arm64.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: I5d445d151db5e6cc7b6e13bf74ce81007d819f1d
---
M src/acpi/acpigen.c
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/76007/1
diff --git a/src/acpi/acpigen.c b/src/acpi/acpigen.c
index e61954f..7ca4f02 100644
--- a/src/acpi/acpigen.c
+++ b/src/acpi/acpigen.c
@@ -1764,6 +1764,10 @@
* bit 0: other functions than 0 are supported
* bits 1-x: function x supported
*/
+/* On GCC aarch64 the compiler is worried about alloca() having unbounded stack usage. */
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic ignored "-Wstack-usage="
+#endif
static void acpigen_dsm_uuid_enum_functions(const struct dsm_uuid *id)
{
const size_t bytes = DIV_ROUND_UP(id->count, BITS_PER_BYTE);
--
To view, visit https://review.coreboot.org/c/coreboot/+/76007?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5d445d151db5e6cc7b6e13bf74ce81007d819f1d
Gerrit-Change-Number: 76007
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange