From: Bjorn Helgaas <bhelgaas@google.com>
To: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Ming Lei <tom.leiming@gmail.com>,
Rusty Russell <rusty@rustcorp.com.au>,
Pekka Enberg <penberg@kernel.org>,
Sasha Levin <sasha.levin@oracle.com>
Subject: Re: [PATCH 8/9] PCI: Ignore BAR contents when firmware left decoding disabled
Date: Wed, 19 Mar 2014 12:54:23 -0600 [thread overview]
Message-ID: <20140319185423.GA5514@google.com> (raw)
In-Reply-To: <20140226193757.10125.81865.stgit@bhelgaas-glaptop.roam.corp.google.com>
[+cc Ming, Rusty, Pekka, Sasha]
On Wed, Feb 26, 2014 at 12:37:57PM -0700, Bjorn Helgaas wrote:
> Don't rely on BAR contents when the command register says the BAR is
> disabled.
>
> If we receive a PCI device from firmware (or a hot-added device that was
> just powered up) with the MEMORY or IO enable bits in the PCI command
> register cleared, there's no reason to believe the BARs contain valid
> addresses.
>
> In that case, we still know the type and size of the BAR, but this
> patch marks the resource as "unset" so we have a chance to reassign it.
>
> Historically, we often used "BAR == 0" to decide the BAR is invalid. But 0
> is a legal BAR value, especially if the host bridge translates addresses,
> so I think it's better to decide based on the PCI command register, and
> store the conclusion in the IORESOURCE_UNSET bit.
I plan to replace this patch with the following, which only sets
IORESOURCE_UNSET when we already have been clearing the bus region start
address. (This probably should have been a separate patch to begin with,
mea culpa.)
This is intended for the v3.15 merge window, so I made the minimal change
to reduce risk.
Thanks to Ming Lei for prompting me to look at this; I think the issue he
reported with the original patch is really a problem somewhere else that
the patch just happened to expose, but the original patch was more
aggressive than necessary, so this revision tones it down a bit.
Bjorn
PCI: Mark 64-bit resource as IORESOURCE_UNSET if we only support 32-bit
From: Bjorn Helgaas <bhelgaas@google.com>
If we don't support 64-bit addresses, i.e., CONFIG_PHYS_ADDR_T_64BIT is not
set, we can't deal with BARs above 4GB. In this case we already pretend
the BAR contained zero; this patch also sets IORESOURCE_UNSET so we can try
to reallocate it later.
I don't think this is exactly correct: what we care about here are *bus*
addresses, not CPU addresses, so the tests of sizeof(resource_size_t)
probably should be on sizeof(dma_addr_t) instead. But this is what's been
in -next, so we'll fix that later.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/probe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6e34498ec9f0..78335efbbb74 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -252,6 +252,7 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
/* Address above 32-bit boundary; disable the BAR */
pci_write_config_dword(dev, pos, 0);
pci_write_config_dword(dev, pos + 4, 0);
+ res->flags |= IORESOURCE_UNSET;
region.start = 0;
region.end = sz64;
bar_disabled = true;
next prev parent reply other threads:[~2014-03-19 18:54 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-26 19:37 [PATCH 0/9] PCI: Use IORESOURCE_UNSET for unassigned BARs Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 1/9] resource: Add resource_contains() Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 2/9] vsprintf: Add support for IORESOURCE_UNSET in %pR Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 3/9] PCI: Remove pci_find_parent_resource() use for allocation Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 4/9] PCI: Mark resources as IORESOURCE_UNSET if we can't assign them Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 5/9] PCI: Don't clear IORESOURCE_UNSET when updating BAR Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 6/9] PCI: Check IORESOURCE_UNSET before " Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 7/9] PCI: Don't try to claim IORESOURCE_UNSET resources Bjorn Helgaas
2014-02-26 19:37 ` [PATCH 8/9] PCI: Ignore BAR contents when firmware left decoding disabled Bjorn Helgaas
2014-03-13 8:51 ` Ming Lei
2014-03-13 16:08 ` Bjorn Helgaas
2014-03-14 1:48 ` Ming Lei
2014-03-18 0:27 ` Bjorn Helgaas
2014-03-19 3:32 ` Ming Lei
2014-03-19 4:52 ` Ming Lei
2014-03-19 16:45 ` Bjorn Helgaas
2014-03-20 1:32 ` Ming Lei
2014-03-21 20:07 ` Bjorn Helgaas
2014-03-21 20:25 ` Sasha Levin
2014-03-21 20:40 ` Bjorn Helgaas
2014-03-19 18:54 ` Bjorn Helgaas [this message]
2014-03-19 21:16 ` Bjorn Helgaas
2014-03-19 21:23 ` Sasha Levin
2014-02-26 19:38 ` [PATCH 9/9] PCI: Don't enable decoding if BAR hasn't been assigned an address Bjorn Helgaas
2014-03-04 20:53 ` [PATCH 0/9] PCI: Use IORESOURCE_UNSET for unassigned BARs 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=20140319185423.GA5514@google.com \
--to=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=penberg@kernel.org \
--cc=rusty@rustcorp.com.au \
--cc=sasha.levin@oracle.com \
--cc=tom.leiming@gmail.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.