From: Vidya Sagar <vidyas@nvidia.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: bhelgaas@google.com, rafael@kernel.org, lenb@kernel.org,
will@kernel.org, lpieralisi@kernel.org, kw@linux.com,
robh@kernel.org, frowand.list@gmail.com,
linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
treding@nvidia.com, jonathanh@nvidia.com, kthota@nvidia.com,
mmaddireddy@nvidia.com, sagar.tv@gmail.com
Subject: Re: [PATCH V3] PCI: Add support for preserving boot configuration
Date: Fri, 23 Feb 2024 02:48:24 +0530 [thread overview]
Message-ID: <17a29a7e-b013-4568-8e21-9647969b6b6d@nvidia.com> (raw)
In-Reply-To: <20240222170641.GA15593@bhelgaas>
On 22-02-2024 22:36, Bjorn Helgaas wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, Feb 22, 2024 at 06:11:10PM +0530, Vidya Sagar wrote:
>> Add support for preserving the boot configuration done by the
>> platform firmware per host bridge basis, based on the presence of
>> 'linux,pci-probe-only' property in the respective PCIe host bridge
>> device-tree node. It also unifies the ACPI and DT based boot flows
>> in this regard.
>> +/**
>> + * of_pci_bridge_check_probe_only - Return true if the boot configuration
>> + * needs to be preserved
> I don't like the "check_probe_only" name because it's a boolean
> function but the name doesn't tell me what a true/false return value
> means. Something like "preserve_resources" would be better. If you
> want "probe_only", even removing the "check" would help.
I'll change it in the next patch.
>> + * @node: Device tree node with the domain information.
>> + *
>> + * This function looks for "linux,pci-probe-only" property for a given
>> + * PCIe controller's node and returns true if found. Having this property
>> + * for a PCIe controller ensures that the kernel doesn't re-enumerate and
>> + * reconfigure the BAR resources that are already done by the platform firmware.
> This is generic PCI, not PCIe-specific (also in commit log and comment
> below).
>
> I think "enumeration" specifically refers to discovering what devices
> are present, and the kernel always does that, so drop that part.
> Reconfiguring BARs and bridge windows is what we want to prevent.
Agree and I'll address it in the next patch.
>> + * NOTE: The scope of "linux,pci-probe-only" defined within a PCIe bridge device
>> + * is limited to the hierarchy under that particular bridge device. whereas
>> + * the scope of "linux,pci-probe-only" defined within chosen node is
>> + * system wide.
>> + *
>> + * Return: true if the property exists false otherwise.
>> + */
>> +bool of_pci_bridge_check_probe_only(struct device_node *node)
>> +{
>> + return of_property_read_bool(node, "linux,pci-probe-only");
>> +}
>> +EXPORT_SYMBOL_GPL(of_pci_bridge_check_probe_only);
> Why does this need to be exported for modules and exposed via
> include/linux/pci.h?
On a second through, I think it is not required to be exported.
I'll address this also in the next patch.
>> +static void pci_check_config_preserve(struct pci_host_bridge *host_bridge)
>> +{
>> + if (&host_bridge->dev) {
> Checking &host_bridge->dev doesn't seem like the right way to
> determine whether this is an ACPI host bridge.
Honestly, I couldn't find a clear way to differentiate between an ACPI based
host bridge and a DT based host bridge. Hence, the current code tries to get
the information using both ways and since a system can only be either
ACPI or
DT based, but one at a time, preserve_config will be set only once (assuming
the system wants it to be set). Let me know if there is a better approach
for this?
I was looking at the way 'external_facing' gets set in both the boot
flows and
I see that there is no common place for it and the respective flows have
their
functions separately i.e. pci_acpi_set_external_facing() for ACPI and
pci_set_bus_of_node() for DT.
Thanks,
Vidya Sagar
>> + union acpi_object *obj;
>> +
>> + /*
>> + * Evaluate the "PCI Boot Configuration" _DSM Function. If it
>> + * exists and returns 0, we must preserve any PCI resource
>> + * assignments made by firmware for this host bridge.
>> + */
>> + obj = acpi_evaluate_dsm(ACPI_HANDLE(&host_bridge->dev), &pci_acpi_dsm_guid, 1,
>> + DSM_PCI_PRESERVE_BOOT_CONFIG, NULL);
>> + if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0)
>> + host_bridge->preserve_config = 1;
>> + ACPI_FREE(obj);
>> + }
next prev parent reply other threads:[~2024-02-22 21:18 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240110030725.710547-3-vidyas@nvidia.com>
2024-02-22 12:41 ` [PATCH V3] PCI: Add support for preserving boot configuration Vidya Sagar
2024-02-22 17:06 ` Bjorn Helgaas
2024-02-22 21:18 ` Vidya Sagar [this message]
2024-02-22 22:08 ` Bjorn Helgaas
2024-02-23 8:00 ` [PATCH V4] " Vidya Sagar
2024-03-05 14:10 ` Rob Herring
2024-03-05 14:24 ` Rob Herring
2024-04-01 7:50 ` [PATCH V5] " Vidya Sagar
2024-04-02 16:01 ` Rob Herring
2024-04-10 7:44 ` Vidya Sagar
2024-04-10 20:50 ` Bjorn Helgaas
2024-04-18 17:31 ` Vidya Sagar
2024-04-18 17:40 ` [PATCH V6] " Vidya Sagar
2024-04-21 19:03 ` Bjorn Helgaas
2024-04-21 19:09 ` [PATCH v7-incomplete 0/3] " Bjorn Helgaas
2024-04-21 19:09 ` [PATCH v7-incomplete 1/3] PCI: Move PRESERVE_BOOT_CONFIG _DSM evaluation to pci_register_host_bridge() Bjorn Helgaas
2024-04-22 7:45 ` Andy Shevchenko
2024-04-21 19:09 ` [PATCH v7-incomplete 2/3] PCI: of: Add of_pci_preserve_config() for per-host bridge support Bjorn Helgaas
2024-04-21 19:09 ` [PATCH v7-incomplete 3/3] PCI: Unify ACPI and DT 'preserve config' support Bjorn Helgaas
2024-04-22 7:43 ` Andy Shevchenko
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=17a29a7e-b013-4568-8e21-9647969b6b6d@nvidia.com \
--to=vidyas@nvidia.com \
--cc=bhelgaas@google.com \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=helgaas@kernel.org \
--cc=jonathanh@nvidia.com \
--cc=kthota@nvidia.com \
--cc=kw@linux.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=mmaddireddy@nvidia.com \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
--cc=sagar.tv@gmail.com \
--cc=treding@nvidia.com \
--cc=will@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).