From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: "Bjorn Helgaas" <bhelgaas@google.com>,
linux-pci@vger.kernel.org,
"Michał Winiarski" <michal.winiarski@intel.com>,
"Igor Mammedov" <imammedo@redhat.com>,
linux-kernel@vger.kernel.org
Cc: "Mika Westerberg" <mika.westerberg@linux.intel.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Jia Yao" <jia.yao@intel.com>
Subject: [PATCH 25/25] PCI: Rework optional resource handling
Date: Mon, 16 Dec 2024 19:56:32 +0200 [thread overview]
Message-ID: <20241216175632.4175-26-ilpo.jarvinen@linux.intel.com> (raw)
In-Reply-To: <20241216175632.4175-1-ilpo.jarvinen@linux.intel.com>
Remove and rescan cycle can result in not failure to assign a bridge
windows if it becomes larger than before the remove. The bridge window
size will include space for disabled Expansion ROM, which can causes
the bridge window to not fit anymore into the same address space slot
on rescan if the Expansion ROM resource was not assigned before the
remove. In addition, the optional resource handling is not internally
consistent.
The resource fitting logic supports three main types of optional
resources:
- IOV BARs
- Expansion ROMs
- Bridge window size variation due to optional resources
In addition to the above, resizable BARs beyond their current size will
require handling optional variation in resource sizes within the
resource fitting algorithm (not yet done by the resource fitting code).
There are multiple inconsistencies related to optional resource
handling:
a) The allocation failure of disabled expansion ROM requires special
case inside assign_requested_resources_sorted().
b) The optionality of disabled expansion ROM is not considered during
bridge window sizing in pbus_size_mem().
c) Setting resource size to zero for optional resource in
pbus_size_mem() is problematic because it makes also the alignment
invalid, which is checked by pdev_sort_resources().
Optional IOV resources have their size set to zero by
pbus_size_mem() but the information about size is stored externally
into the struct pci_sriov and complex call-chain trickery in
pci_resource_alignment() ensures IOV resources return a valid
alignment despite having zero resource size. A solution that is
specific to IOV resources makes it hard to use the same solution for
other types of resources such as expansion ROM.
Simply changing only pbus_size_mem() is not sufficient to fully address
the main issue because it would introduce disparity between bridge
window sizing and resource allocation. Due size based ordering of
resource list during assignment loop, an Expansion ROM resource could
steal space from some other resource and make the other resource not
fit if the Expansion ROM is larger than the other resource. Thus, the
resource assignment functions need to be changed as well.
Make optional resource handling more straightforwards. Use
pci_resource_is_optional() to determine if a resource is optional or
not in both bridge window sizing and assignment failure classification
to ensure they always align. Indicate with a parameter to
assign_requested_resources_sorted() whether it should attempt to
allocate optional resources or not.
Always try first to assign all resources (also when realloc_head is not
provided). This is required for calls from
pci_assign_unassigned_root_bus_resources() that provides realloc_head
only with some of its iterations.
Non-bridge-window optional resources in realloc_head now have add_size
0. This condition has to be detected in reassign_resources_sorted()
before reassigning them (which would fail as there is no size change).
Removing add_size=0 optional resources entirely from realloc_head might
eventually be doable but further rework in __assign_resources_sorted()
is needed first to support such a change.
Reported-by: Jia Yao <jia.yao@intel.com>
Tested-by: Jia Yao <jia.yao@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 89 +++++++++++++++++++++++++++--------------
1 file changed, 58 insertions(+), 31 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index b61f24a5cfa5..802643a4de47 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -276,13 +276,14 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
add_size = add_res->add_size;
align = add_res->min_align;
if (!res->parent) {
- resource_set_range(res, align, add_size);
+ resource_set_range(res, align,
+ resource_size(res) + add_size);
if (pci_assign_resource(dev, idx)) {
pci_dbg(dev,
"%s %pR: ignoring failure in optional allocation\n",
res_name, res);
}
- } else {
+ } else if (add_size > 0) {
res->flags |= add_res->flags &
(IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
if (pci_reassign_resource(dev, idx, add_size, align))
@@ -302,38 +303,38 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
* @head: Head of the list tracking requests for resources
* @fail_head: Head of the list tracking requests that could not be
* allocated
+ * @optional: Assign also optional resources
*
* Satisfy resource requests of each element in the list. Add requests that
* could not be satisfied to the failed_list.
*/
static void assign_requested_resources_sorted(struct list_head *head,
- struct list_head *fail_head)
+ struct list_head *fail_head,
+ bool optional)
{
struct pci_dev_resource *dev_res;
struct resource *res;
struct pci_dev *dev;
+ bool optional_res;
int idx;
list_for_each_entry(dev_res, head, list) {
res = dev_res->res;
dev = dev_res->dev;
idx = pci_resource_num(dev, res);
+ optional_res = pci_resource_is_optional(dev, idx);
if (!resource_size(res))
continue;
+ if (!optional && optional_res)
+ continue;
+
if (pci_assign_resource(dev, idx)) {
if (fail_head) {
- /*
- * If the failed resource is a ROM BAR and
- * it will be enabled later, don't add it
- * to the list.
- */
- if (!((idx == PCI_ROM_RESOURCE) &&
- (!(res->flags & IORESOURCE_ROM_ENABLE))))
- add_to_list(fail_head, dev, res,
- 0 /* don't care */,
- 0 /* don't care */);
+ add_to_list(fail_head, dev, res,
+ 0 /* don't care */,
+ 0 /* don't care */);
}
}
}
@@ -379,6 +380,20 @@ static bool pci_need_to_release(unsigned long mask, struct resource *res)
return false; /* Should not get here */
}
+/* Return: @true if assignment of a required resource failed. */
+static bool pci_required_resource_failed(struct list_head *fail_head)
+{
+ struct pci_dev_resource *fail_res;
+
+ list_for_each_entry(fail_res, fail_head, list) {
+ int idx = pci_resource_num(fail_res->dev, fail_res->res);
+
+ if (!pci_resource_is_optional(fail_res->dev, idx))
+ return true;
+ }
+ return false;
+}
+
static void __assign_resources_sorted(struct list_head *head,
struct list_head *realloc_head,
struct list_head *fail_head)
@@ -388,9 +403,11 @@ static void __assign_resources_sorted(struct list_head *head,
* adjacent, so later reassign can not reallocate them one by one in
* parent resource window.
*
- * Try to assign requested + add_size at beginning. If could do that,
- * could get out early. If could not do that, we still try to assign
- * requested at first, then try to reassign add_size for some resources.
+ * Try to assign required and any optional resources at beginning
+ * (add_size included). If all required resources were successfully
+ * assigned, get out early. If could not do that, we still try to
+ * assign required at first, then try to reassign some optional
+ * resources.
*
* Separate three resource type checking if we need to release
* assigned resource after requested + add_size try.
@@ -421,13 +438,13 @@ static void __assign_resources_sorted(struct list_head *head,
/* Check if optional add_size is there */
if (list_empty(realloc_head))
- goto requested_and_reassign;
+ goto assign;
/* Save original start, end, flags etc at first */
list_for_each_entry(dev_res, head, list) {
if (add_to_list(&save_head, dev_res->dev, dev_res->res, 0, 0)) {
free_list(&save_head);
- goto requested_and_reassign;
+ goto assign;
}
}
@@ -471,10 +488,10 @@ static void __assign_resources_sorted(struct list_head *head,
}
- /* Try updated head list with add_size added */
- assign_requested_resources_sorted(head, &local_fail_head);
+assign:
+ assign_requested_resources_sorted(head, &local_fail_head, true);
- /* All assigned with add_size? */
+ /* All non-optional resources assigned? */
if (list_empty(&local_fail_head)) {
/* Remove head list from realloc_head list */
list_for_each_entry(dev_res, head, list)
@@ -483,6 +500,22 @@ static void __assign_resources_sorted(struct list_head *head,
goto out;
}
+ /* Without realloc_head and only optional fails, nothing more to do. */
+ if (!pci_required_resource_failed(&local_fail_head) &&
+ list_empty(realloc_head)) {
+ list_for_each_entry(save_res, &save_head, list) {
+ struct resource *res = save_res->res;
+
+ if (res->parent)
+ continue;
+
+ restore_dev_resource(save_res);
+ }
+ free_list(&local_fail_head);
+ free_list(&save_head);
+ goto out;
+ }
+
/* Check failed type */
fail_type = pci_fail_res_type_mask(&local_fail_head);
/* Remove not need to be released assigned res from head list etc */
@@ -518,9 +551,8 @@ static void __assign_resources_sorted(struct list_head *head,
restore_dev_resource(save_res);
free_list(&save_head);
-requested_and_reassign:
/* Satisfy the must-have resource requests */
- assign_requested_resources_sorted(head, NULL);
+ assign_requested_resources_sorted(head, NULL, false);
/* Try to satisfy any additional optional resource requests */
if (!list_empty(realloc_head))
@@ -535,11 +567,7 @@ static void __assign_resources_sorted(struct list_head *head,
if (res->parent)
continue;
- /*
- * If the failed resource is a ROM BAR and it will
- * be enabled later, don't add it to the list.
- */
- if (fail_head && !pci_resource_is_disabled_rom(res, idx)) {
+ if (fail_head) {
add_to_list(fail_head, dev, res,
0 /* don't care */,
0 /* don't care */);
@@ -1166,10 +1194,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
r_size = resource_size(r);
/* Put SRIOV requested res to the optional list */
- if (realloc_head && pci_resource_is_iov(i)) {
+ if (realloc_head && pci_resource_is_optional(dev, i)) {
add_align = max(pci_resource_alignment(dev, r), add_align);
- resource_set_size(r, 0);
- add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
+ add_to_list(realloc_head, dev, r, 0, 0 /* Don't care */);
children_add_size += r_size;
continue;
}
--
2.39.5
next prev parent reply other threads:[~2024-12-16 18:00 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-16 17:56 [PATCH 00/25] PCI: Resource fitting/assignment fixes and cleanups Ilpo Järvinen
2025-02-14 9:59 ` Xiaochun Lee
2024-12-16 17:56 ` [PATCH 01/25] PCI: Remove add_align overwrite unrelated to size0 Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 02/25] PCI: size0 is unrelated to add_align Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 03/25] PCI: Simplify size1 assignment logic Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 04/25] PCI: Optional bridge window size too may need relaxing Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 05/25] PCI: Fix old_size lower bound in calculate_iosize() too Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 06/25] PCI: Use SZ_* instead of literals in setup-bus.c Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 07/25] PCI: resource_set_range/size() conversions Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 08/25] PCI: Add a helper to identify IOV resources Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 09/25] PCI: Check resource_size() separately Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 10/25] PCI: Add pci_resource_num() helper Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 11/25] PCI: Add dev & res local variables to resource assignment funcs Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 12/25] PCI: Converge return paths in __assign_resources_sorted() Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 13/25] PCI: Refactor pdev_sort_resources() & __dev_sort_resources() Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 14/25] PCI: Use while loop and break instead of gotos Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 15/25] PCI: Rename retval to ret Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 16/25] PCI: Consolidate assignment loop next round preparation Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 17/25] PCI: Remove wrong comment from pci_reassign_resource() Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 18/25] PCI: Add restore_dev_resource() Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 19/25] PCI: Extend enable to check for any optional resource Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 20/25] PCI: Always have realloc_head in __assign_resources_sorted() Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 21/25] PCI: Indicate optional resource assignment failures Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 22/25] PCI: Add debug print when releasing resources before retry Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 23/25] PCI: Use res->parent to check is resource is assigned Ilpo Järvinen
2024-12-16 17:56 ` [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync Ilpo Järvinen
2025-04-01 2:35 ` Guenter Roeck
2025-04-01 10:18 ` Ilpo Järvinen
2025-04-01 12:07 ` Ilpo Järvinen
2025-04-01 14:15 ` Guenter Roeck
2025-04-01 17:38 ` Ilpo Järvinen
2025-04-11 19:37 ` Ondřej Jirman
2025-04-14 9:52 ` Ilpo Järvinen
2025-04-14 12:19 ` Ondřej Jirman
2025-04-14 13:15 ` Ilpo Järvinen
2025-04-14 13:43 ` Ondřej Jirman
2025-04-14 13:52 ` Ilpo Järvinen
2025-04-01 13:28 ` Guenter Roeck
2024-12-16 17:56 ` Ilpo Järvinen [this message]
2025-02-13 21:46 ` [PATCH 00/25] PCI: Resource fitting/assignment fixes and cleanups Bjorn Helgaas
2025-02-14 8:18 ` Xiaochun XC17 Li | 李小春 Xavier
2025-02-14 11:53 ` Ilpo Järvinen
2025-02-14 21:28 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241216175632.4175-26-ilpo.jarvinen@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=imammedo@redhat.com \
--cc=jia.yao@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=michal.winiarski@intel.com \
--cc=mika.westerberg@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.