* [PATCH 1/3] PCI: Fix starting basis for resource requests
2011-11-10 0:43 [PATCH 0/3] PCI: Convert maintaining FW-assigned BIOS BAR values to a list of temporary entries Myron Stowe
@ 2011-11-10 0:43 ` Myron Stowe
2011-11-10 0:43 ` [PATCH 2/3] PCI: Maintain a list of FW-assigned BIOS BAR values Myron Stowe
2011-11-10 0:43 ` [PATCH 3/3] x86/PCI: Convert maintaining FW-assigned BIOS BAR values to use a list Myron Stowe
2 siblings, 0 replies; 6+ messages in thread
From: Myron Stowe @ 2011-11-10 0:43 UTC (permalink / raw)
To: jbarnes; +Cc: linux-pci, bhelgaas, linux-kernel
From: Myron Stowe <mstowe@redhat.com>
pci_revert_fw_address() is used to reinstate a PCI device's original
FW-assigned BIOS BAR value(s) if normal resource assignment fails.
When attempting to reinstate an address, the point within the resource
tree from which to attempt the new resource request should be the parent
resource corresponding to the device, not the base of the resource tree
(ioport_resource or iomem_resource). For PCI devices this would
typically be the resource corresponding to the upstream PCI host bridge
or P2P bridge aperture.
This patch sets the point within the resource tree to attempt a new
resource assignment request to the PCI device's parent resource and only
if that fails does it fall back to the base ioport_resource or
iomem_resource.
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---
drivers/pci/setup-res.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 51a9095..ad8c4ae 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -164,15 +164,18 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
resource_size_t start, end;
int ret = 0;
- if (res->flags & IORESOURCE_IO)
- root = &ioport_resource;
- else
- root = &iomem_resource;
-
start = res->start;
end = res->end;
res->start = dev->fw_addr[resno];
res->end = res->start + size - 1;
+
+ root = pci_find_parent_resource(dev, res);
+ if (!root)
+ if (res->flags & IORESOURCE_IO)
+ root = &ioport_resource;
+ else
+ root = &iomem_resource;
+
dev_info(&dev->dev, "BAR %d: trying firmware assignment %pR\n",
resno, res);
conflict = request_resource_conflict(root, res);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] PCI: Maintain a list of FW-assigned BIOS BAR values
2011-11-10 0:43 [PATCH 0/3] PCI: Convert maintaining FW-assigned BIOS BAR values to a list of temporary entries Myron Stowe
2011-11-10 0:43 ` [PATCH 1/3] PCI: Fix starting basis for resource requests Myron Stowe
@ 2011-11-10 0:43 ` Myron Stowe
2011-11-11 17:45 ` Jesse Barnes
2011-11-10 0:43 ` [PATCH 3/3] x86/PCI: Convert maintaining FW-assigned BIOS BAR values to use a list Myron Stowe
2 siblings, 1 reply; 6+ messages in thread
From: Myron Stowe @ 2011-11-10 0:43 UTC (permalink / raw)
To: jbarnes; +Cc: linux-pci, bhelgaas, linux-kernel
From: Myron Stowe <mstowe@redhat.com>
Commit 58c84eda075 introduced functionality to try and reinstate the
original BIOS BAR addresses of a PCI device when normal resource
assignment attempts fail. To keep track of the BIOS BAR addresses,
struct pci_dev was augmented with an array to hold the BAR addresses
of the PCI device: 'resource_size_t fw_addr[DEVICE_COUNT_RESOURCE]'.
The reinstatement of BAR addresses is an uncommon event leaving the
'fw_addr' array unused normally. As the use of struct pci_dev is so
prevalent, this seems wasteful.
This patch introduces a stand alone data structure and corresponding set
of interfacing routines for maintaining a list of FW-assigned BIOS BAR
value temporary entries.
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---
drivers/pci/setup-res.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 74 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index ad8c4ae..ab45c11 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -26,6 +26,79 @@
#include "pci.h"
+/*
+ * This list of dynamic mappings is for temporarly maintaining
+ * original BIOS BAR addresses for possbile reinstatement.
+ */
+struct pci_fwaddrmap {
+ struct list_head list;
+ struct pci_dev *dev;
+ resource_size_t fw_addr[DEVICE_COUNT_RESOURCE];
+ unsigned long refcount;
+};
+
+static LIST_HEAD(pci_fwaddrmappings);
+static DEFINE_SPINLOCK(pci_fwaddrmap_lock);
+
+/* Must be called with 'pci_fwaddrmap_lock' lock held. */
+static struct pci_fwaddrmap *pci_fwaddrmap_lookup(struct pci_dev *dev)
+{
+ struct pci_fwaddrmap *map;
+
+ list_for_each_entry(map, &pci_fwaddrmappings, list)
+ if (map->dev == dev)
+ return map;
+
+ return NULL;
+}
+
+void pci_save_fw_addr(struct pci_dev *dev, int idx, resource_size_t fw_addr)
+{
+ unsigned long flags;
+ struct pci_fwaddrmap *map;
+
+ spin_lock_irqsave(&pci_fwaddrmap_lock, flags);
+ map = pci_fwaddrmap_lookup(dev);
+ if (!map) {
+ spin_unlock_irqrestore(&pci_fwaddrmap_lock, flags);
+ map = kzalloc(sizeof(*map), GFP_KERNEL);
+ if (!map)
+ return;
+
+ INIT_LIST_HEAD(&map->list);
+ map->dev = dev;
+ map->fw_addr[idx] = fw_addr;
+
+ spin_lock_irqsave(&pci_fwaddrmap_lock, flags);
+ list_add_tail(&map->list, &pci_fwaddrmappings);
+ } else {
+ map->fw_addr[idx] = fw_addr;
+ }
+ map->refcount++;
+ spin_unlock_irqrestore(&pci_fwaddrmap_lock, flags);
+}
+
+static resource_size_t pci_retrieve_fw_addr(struct pci_dev *dev, int idx)
+{
+ unsigned long flags;
+ resource_size_t fw_addr = 0;
+ struct pci_fwaddrmap *map;
+
+ spin_lock_irqsave(&pci_fwaddrmap_lock, flags);
+ map = pci_fwaddrmap_lookup(dev);
+ if (map) {
+ fw_addr = map->fw_addr[idx];
+
+ if (!--map->refcount) {
+ list_del(&map->list);
+ kfree(map);
+ }
+ }
+ spin_unlock_irqrestore(&pci_fwaddrmap_lock, flags);
+
+ return fw_addr;
+}
+
void pci_update_resource(struct pci_dev *dev, int resno)
{
struct pci_bus_region region;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 84225c7..14f2492 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -889,6 +889,7 @@ ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void
int pci_vpd_truncate(struct pci_dev *dev, size_t size);
/* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
+void pci_save_fw_addr(struct pci_dev *dev, int idx, resource_size_t fw_addr);
void pci_bus_assign_resources(const struct pci_bus *bus);
void pci_bus_size_bridges(struct pci_bus *bus);
int pci_claim_resource(struct pci_dev *, int);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] PCI: Maintain a list of FW-assigned BIOS BAR values
2011-11-10 0:43 ` [PATCH 2/3] PCI: Maintain a list of FW-assigned BIOS BAR values Myron Stowe
@ 2011-11-11 17:45 ` Jesse Barnes
2011-11-14 17:47 ` Myron Stowe
0 siblings, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2011-11-11 17:45 UTC (permalink / raw)
To: Myron Stowe; +Cc: linux-pci, bhelgaas, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]
On Wed, 09 Nov 2011 17:43:51 -0700
Myron Stowe <myron.stowe@redhat.com> wrote:
> From: Myron Stowe <mstowe@redhat.com>
>
> Commit 58c84eda075 introduced functionality to try and reinstate the
> original BIOS BAR addresses of a PCI device when normal resource
> assignment attempts fail. To keep track of the BIOS BAR addresses,
> struct pci_dev was augmented with an array to hold the BAR addresses
> of the PCI device: 'resource_size_t fw_addr[DEVICE_COUNT_RESOURCE]'.
>
> The reinstatement of BAR addresses is an uncommon event leaving the
> 'fw_addr' array unused normally. As the use of struct pci_dev is so
> prevalent, this seems wasteful.
>
> This patch introduces a stand alone data structure and corresponding set
> of interfacing routines for maintaining a list of FW-assigned BIOS BAR
> value temporary entries.
>
> Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
> ---
>
> drivers/pci/setup-res.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 1 +
> 2 files changed, 74 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index ad8c4ae..ab45c11 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -26,6 +26,79 @@
> #include "pci.h"
>
>
> +/*
> + * This list of dynamic mappings is for temporarly maintaining
> + * original BIOS BAR addresses for possbile reinstatement.
> + */
> +struct pci_fwaddrmap {
> + struct list_head list;
> + struct pci_dev *dev;
> + resource_size_t fw_addr[DEVICE_COUNT_RESOURCE];
> + unsigned long refcount;
> +};
> +
> +static LIST_HEAD(pci_fwaddrmappings);
> +static DEFINE_SPINLOCK(pci_fwaddrmap_lock);
Is the refcounting really needed? Can't we just free the whole list at
some point to simplify things a bit?
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] PCI: Maintain a list of FW-assigned BIOS BAR values
2011-11-11 17:45 ` Jesse Barnes
@ 2011-11-14 17:47 ` Myron Stowe
0 siblings, 0 replies; 6+ messages in thread
From: Myron Stowe @ 2011-11-14 17:47 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Myron Stowe, linux-pci, bhelgaas, linux-kernel
On Fri, Nov 11, 2011 at 10:45 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Wed, 09 Nov 2011 17:43:51 -0700
> Myron Stowe <myron.stowe@redhat.com> wrote:
>
>> From: Myron Stowe <mstowe@redhat.com>
>>
>> Commit 58c84eda075 introduced functionality to try and reinstate the
>> original BIOS BAR addresses of a PCI device when normal resource
>> assignment attempts fail. To keep track of the BIOS BAR addresses,
>> struct pci_dev was augmented with an array to hold the BAR addresses
>> of the PCI device: 'resource_size_t fw_addr[DEVICE_COUNT_RESOURCE]'.
>>
>> The reinstatement of BAR addresses is an uncommon event leaving the
>> 'fw_addr' array unused normally. As the use of struct pci_dev is so
>> prevalent, this seems wasteful.
>>
>> This patch introduces a stand alone data structure and corresponding set
>> of interfacing routines for maintaining a list of FW-assigned BIOS BAR
>> value temporary entries.
>>
>> Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
>> ---
>>
>> drivers/pci/setup-res.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/pci.h | 1 +
>> 2 files changed, 74 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
>> index ad8c4ae..ab45c11 100644
>> --- a/drivers/pci/setup-res.c
>> +++ b/drivers/pci/setup-res.c
>> @@ -26,6 +26,79 @@
>> #include "pci.h"
>>
>>
>> +/*
>> + * This list of dynamic mappings is for temporarly maintaining
>> + * original BIOS BAR addresses for possbile reinstatement.
>> + */
>> +struct pci_fwaddrmap {
>> + struct list_head list;
>> + struct pci_dev *dev;
>> + resource_size_t fw_addr[DEVICE_COUNT_RESOURCE];
>> + unsigned long refcount;
>> +};
>> +
>> +static LIST_HEAD(pci_fwaddrmappings);
>> +static DEFINE_SPINLOCK(pci_fwaddrmap_lock);
>
> Is the refcounting really needed? Can't we just free the whole list at
> some point to simplify things a bit?
No, refcounting is not absolutely necessary, it's just the approach I came
up with initially. I'll look into just freeing the entire list once we are done
with it.
Myron
>
> --
> Jesse Barnes, Intel Open Source Technology Center
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] x86/PCI: Convert maintaining FW-assigned BIOS BAR values to use a list
2011-11-10 0:43 [PATCH 0/3] PCI: Convert maintaining FW-assigned BIOS BAR values to a list of temporary entries Myron Stowe
2011-11-10 0:43 ` [PATCH 1/3] PCI: Fix starting basis for resource requests Myron Stowe
2011-11-10 0:43 ` [PATCH 2/3] PCI: Maintain a list of FW-assigned BIOS BAR values Myron Stowe
@ 2011-11-10 0:43 ` Myron Stowe
2 siblings, 0 replies; 6+ messages in thread
From: Myron Stowe @ 2011-11-10 0:43 UTC (permalink / raw)
To: jbarnes; +Cc: linux-pci, bhelgaas, linux-kernel
From: Myron Stowe <mstowe@redhat.com>
This patch converts the underlying maintenance aspects of FW-assigned
BIOS BAR values from a statically allocated array within struct pci_dev
to a list of temporary, stand alone, entries.
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---
arch/x86/pci/i386.c | 2 +-
drivers/pci/setup-res.c | 14 +++++++++-----
include/linux/pci.h | 1 -
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 494f2e7..99ba108 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -181,7 +181,7 @@ static void __init pcibios_allocate_resources(int pass)
idx, r, disabled, pass);
if (pci_claim_resource(dev, idx) < 0) {
/* We'll assign a new address later */
- dev->fw_addr[idx] = r->start;
+ pci_save_fw_addr(dev, idx, r->start);
r->end -= r->start;
r->start = 0;
}
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index ab45c11..7e4711e 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -231,7 +231,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
}
static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
- int resno, resource_size_t size)
+ int resno, resource_size_t fw_addr, resource_size_t size)
{
struct resource *root, *conflict;
resource_size_t start, end;
@@ -239,7 +239,7 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
start = res->start;
end = res->end;
- res->start = dev->fw_addr[resno];
+ res->start = fw_addr;
res->end = res->start + size - 1;
root = pci_find_parent_resource(dev, res);
@@ -322,7 +322,7 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsiz
int pci_assign_resource(struct pci_dev *dev, int resno)
{
struct resource *res = dev->resource + resno;
- resource_size_t align, size;
+ resource_size_t align, fw_addr, size;
struct pci_bus *bus;
int ret;
@@ -342,8 +342,12 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
* where firmware left it. That at least has a chance of
* working, which is better than just leaving it disabled.
*/
- if (ret < 0 && dev->fw_addr[resno])
- ret = pci_revert_fw_address(res, dev, resno, size);
+ if (ret < 0) {
+ fw_addr = pci_retrieve_fw_addr(dev, resno);
+ if (fw_addr)
+ ret = pci_revert_fw_address(res, dev, resno, fw_addr,
+ size);
+ }
if (!ret) {
res->flags &= ~IORESOURCE_STARTALIGN;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 14f2492..5a5d2fc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -299,7 +299,6 @@ struct pci_dev {
*/
unsigned int irq;
struct resource resource[DEVICE_COUNT_RESOURCE]; /* I/O and memory regions + expansion ROMs */
- resource_size_t fw_addr[DEVICE_COUNT_RESOURCE]; /* FW-assigned addr */
/* These fields are used by common fixups */
unsigned int transparent:1; /* Transparent PCI bridge */
^ permalink raw reply [flat|nested] 6+ messages in thread