This patchset mainly converts strut resource's sibling list from singly
linked list to doubly linked list, list_head. This is suggested by
Andrew. Since I need a reversed searching on iomem_resource's
IORESOURCE_SYSTEM_RAM children, the old singly linked list way makes
the code in v1 post really ugly.
With this change, we only need one simple list_for_each_entry_reverse()
to do reversed iteration on sibling list. The relevant codes in
kernel/resource.c are more readable since those dazzling pointer
operation codes for singly linked list are replaced.
With the help of list_head, in patch 0002 we can have a very simple
walk_system_ram_res_rev(). And in patch 0003, will use it to search
available system RAM region for kexec_buffer of kexec_file from top to
down, just like we have been doing all along in kexec loading which is
done in kexec-tools utility.
Note:
This patchset passed testing on my kvm guest, x86_64 arch with network
enabling. The thing we need pay attetion to is that a root resource's
child member need be initialized specifically with LIST_HEAD_INIT() if
sttically defined or INIT_LIST_HEAD() for dynamically definition. Here
Just like we do for iomem_resource/ioport_resource, or the change in
get_pci_domain_busn_res().
And there are two places of change I am not very sure. One is in
drivers/hv/vmbus_drv.c, the other is in drivers/pci/host/vmd.c. So will
invite experts on these areas to help review.
V2 post and discussions can be found here:
https://lkml.org/lkml/2018/4/7/169
Remaining issue:
Rob raised issue that if we can make an common tree struct and helpers
defined on top of list_head or a new struct, quote his saying as below.
I didn't do much investigation, we may have a new struct like
struct tlist {
void *parent;
struct list_head sibling;
struct list_head child;
}
Since I have some rhel dev things right now, may consider this later, put
it in my TODO list. In kernel, like task_struct, task_group, they all have
this kind of tree list, and have used list_head. If anyone is interested,
feel free to have a try on this.
==========
The DT struct device_node also has the same tree structure with
parent, child, sibling pointers and converting to list_head had been
on the todo list for a while. ACPI also has some tree walking
functions (drivers/acpi/acpica/pstree.c). Perhaps there should be a
common tree struct and helpers defined either on top of list_head or a
new struct if that saves some size.
==========
Changelog:
v2->v3:
Rename resource functions first_child() and sibling() to
resource_first_chils() and resource_sibling(). Dan suggested this.
Move resource_first_chils() and resource_sibling() to linux/ioport.h
and make them as inline function. Rob suggested this. Accordingly add
linux/list.h including in linux/ioport.h, please help review if this
bring efficiency degradation or code redundancy.
The change on struct resource {} bring two pointers of size increase,
mention this in git log to make it more specifically, Rob suggested
this.
v1->v2:
Use list_head instead to link resource siblings. This is suggested by
Andrew.
Rewrite walk_system_ram_res_rev() after list_head is taken to link
resouce siblings.
Baoquan He (3):
resource: Use list_head to link sibling resource
resource: add walk_system_ram_res_rev()
kexec_file: Load kernel at top of system RAM if required
arch/sparc/kernel/ioport.c | 2 +-
drivers/gpu/drm/drm_memory.c | 3 +-
drivers/gpu/drm/gma500/gtt.c | 5 +-
drivers/hv/vmbus_drv.c | 52 +++----
drivers/input/joystick/iforce/iforce-main.c | 4 +-
drivers/nvdimm/e820.c | 2 +-
drivers/nvdimm/namespace_devs.c | 6 +-
drivers/nvdimm/nd.h | 5 +-
drivers/of/address.c | 4 +-
drivers/parisc/lba_pci.c | 4 +-
drivers/pci/host/vmd.c | 8 +-
drivers/pci/probe.c | 2 +
drivers/pci/setup-bus.c | 2 +-
include/linux/ioport.h | 20 ++-
kernel/kexec_file.c | 2 +
kernel/resource.c | 221 ++++++++++++++++------------
16 files changed, 193 insertions(+), 149 deletions(-)
--
2.13.6
This function, being a variant of walk_system_ram_res() introduced in
commit 8c86e70acead ("resource: provide new functions to walk through
resources"), walks through a list of all the resources of System RAM
in reversed order, i.e., from higher to lower.
It will be used in kexec_file code.
Signed-off-by: Baoquan He <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Brijesh Singh <[email protected]>
Cc: "Jérôme Glisse" <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Wei Yang <[email protected]>
---
include/linux/ioport.h | 3 +++
kernel/resource.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 225d13d3500a..5f2cfb460a14 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -278,6 +278,9 @@ extern int
walk_system_ram_res(u64 start, u64 end, void *arg,
int (*func)(struct resource *, void *));
extern int
+walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+ int (*func)(struct resource *, void *));
+extern int
walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
void *arg, int (*func)(struct resource *, void *));
diff --git a/kernel/resource.c b/kernel/resource.c
index 4f560991c130..6c519b06e3d6 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -23,6 +23,8 @@
#include <linux/pfn.h>
#include <linux/mm.h>
#include <linux/resource_ext.h>
+#include <linux/string.h>
+#include <linux/vmalloc.h>
#include <asm/io.h>
@@ -475,6 +477,44 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
}
/*
+ * This function, being a variant of walk_system_ram_res(), calls the @func
+ * callback against all memory ranges of type System RAM which are marked as
+ * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
+ * higher to lower.
+ */
+int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+ int (*func)(struct resource *, void *))
+{
+ unsigned long flags;
+ struct resource *res;
+ int ret = -1;
+
+ flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+ read_lock(&resource_lock);
+ list_for_each_entry_reverse(res, &iomem_resource.child, sibling) {
+ if (start >= end)
+ break;
+ if ((res->flags & flags) != flags)
+ continue;
+ if (res->desc != IORES_DESC_NONE)
+ continue;
+ if (res->end < start)
+ break;
+
+ if ((res->end >= start) && (res->start < end)) {
+ ret = (*func)(res, arg);
+ if (ret)
+ break;
+ }
+ end = res->start - 1;
+
+ }
+ read_unlock(&resource_lock);
+ return ret;
+}
+
+/*
* This function calls the @func callback against all memory ranges, which
* are ranges marked as IORESOURCE_MEM and IORESOUCE_BUSY.
*/
--
2.13.6
For kexec_file loading, if kexec_buf.top_down is 'true', the memory which
is used to load kernel/initrd/purgatory is supposed to be allocated from
top to down. This is what we have been doing all along in the old kexec
loading interface and the kexec loading is still default setting in some
distributions. However, the current kexec_file loading interface doesn't
do likt this. The function arch_kexec_walk_mem() it calls ignores checking
kexec_buf.top_down, but calls walk_system_ram_res() directly to go through
all resources of System RAM from bottom to up, to try to find memory region
which can contain the specific kexec buffer, then call locate_mem_hole_callback()
to allocate memory in that found memory region from top to down. This brings
confusion. These two interfaces need be unified on behaviour.
Here add checking if kexec_buf.top_down is 'true' in arch_kexec_walk_mem(),
if yes, call the newly added walk_system_ram_res_rev() to find memory region
from top to down to load kernel.
Signed-off-by: Baoquan He <[email protected]>
Cc: Eric Biederman <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: Dave Young <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: [email protected]
---
kernel/kexec_file.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 75d8e7cf040e..7a66d9d5a534 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -518,6 +518,8 @@ int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
crashk_res.start, crashk_res.end,
kbuf, func);
+ else if (kbuf->top_down)
+ return walk_system_ram_res_rev(0, ULONG_MAX, kbuf, func);
else
return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
}
--
2.13.6
The struct resource uses singly linked list to link siblings. It's not
easy to do reverse iteration on sibling list. So replace it with list_head.
And this makes codes in kernel/resource.c more readable after refactoring
than pointer operation.
Besides, type of member variables of struct resource, sibling and child, are
changed from 'struct resource *' to 'struct list_head'. This brings two
pointers of size increase.
Suggested-by: Andrew Morton <[email protected]>
Signed-off-by: Baoquan He <[email protected]>
Cc: Patrik Jakobsson <[email protected]>
Cc: David Airlie <[email protected]>
Cc: "K. Y. Srinivasan" <[email protected]>
Cc: Haiyang Zhang <[email protected]>
Cc: Stephen Hemminger <[email protected]>
Cc: Dmitry Torokhov <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Frank Rowand <[email protected]>
Cc: Keith Busch <[email protected]>
Cc: Jonathan Derrick <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Brijesh Singh <[email protected]>
Cc: "Jérôme Glisse" <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Yaowei Bai <[email protected]>
Cc: Wei Yang <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
v2->v3:
Rename resource functions first_child() and sibling() to
resource_first_chils() and resource_sibling(). Dan suggested this.
Move resource_first_chils() and resource_sibling() to linux/ioport.h
and make them as inline function. Rob suggested this. Accordingly add
linux/list.h including in linux/ioport.h, please help review if this
bring efficiency degradation or code redundancy.
The change on struct resource {} bring two pointers of size increase,
mention this in git log to make it more specifically, Rob suggested
this.
arch/sparc/kernel/ioport.c | 2 +-
drivers/gpu/drm/drm_memory.c | 3 +-
drivers/gpu/drm/gma500/gtt.c | 5 +-
drivers/hv/vmbus_drv.c | 52 ++++----
drivers/input/joystick/iforce/iforce-main.c | 4 +-
drivers/nvdimm/e820.c | 2 +-
drivers/nvdimm/namespace_devs.c | 6 +-
drivers/nvdimm/nd.h | 5 +-
drivers/of/address.c | 4 +-
drivers/parisc/lba_pci.c | 4 +-
drivers/pci/host/vmd.c | 8 +-
drivers/pci/probe.c | 2 +
drivers/pci/setup-bus.c | 2 +-
include/linux/ioport.h | 17 ++-
kernel/resource.c | 181 +++++++++++++---------------
15 files changed, 148 insertions(+), 149 deletions(-)
diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
index 3bcef9ce74df..4e91fbbbedcc 100644
--- a/arch/sparc/kernel/ioport.c
+++ b/arch/sparc/kernel/ioport.c
@@ -669,7 +669,7 @@ static int sparc_io_proc_show(struct seq_file *m, void *v)
struct resource *root = m->private, *r;
const char *nm;
- for (r = root->child; r != NULL; r = r->sibling) {
+ list_for_each_entry(r, &root->child, sibling) {
if ((nm = r->name) == NULL) nm = "???";
seq_printf(m, "%016llx-%016llx: %s\n",
(unsigned long long)r->start,
diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 3c54044214db..53e300a993dc 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -155,9 +155,8 @@ u64 drm_get_max_iomem(void)
struct resource *tmp;
resource_size_t max_iomem = 0;
- for (tmp = iomem_resource.child; tmp; tmp = tmp->sibling) {
+ list_for_each_entry(tmp, &iomem_resource.child, sibling)
max_iomem = max(max_iomem, tmp->end);
- }
return max_iomem;
}
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 3949b0990916..addd3bc009af 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -565,7 +565,7 @@ int psb_gtt_init(struct drm_device *dev, int resume)
int psb_gtt_restore(struct drm_device *dev)
{
struct drm_psb_private *dev_priv = dev->dev_private;
- struct resource *r = dev_priv->gtt_mem->child;
+ struct resource *r;
struct gtt_range *range;
unsigned int restored = 0, total = 0, size = 0;
@@ -573,14 +573,13 @@ int psb_gtt_restore(struct drm_device *dev)
mutex_lock(&dev_priv->gtt_mutex);
psb_gtt_init(dev, 1);
- while (r != NULL) {
+ list_for_each_entry(r, &dev_priv->gtt_mem->child, sibling) {
range = container_of(r, struct gtt_range, resource);
if (range->pages) {
psb_gtt_insert(dev, range, 1);
size += range->resource.end - range->resource.start;
restored++;
}
- r = r->sibling;
total++;
}
mutex_unlock(&dev_priv->gtt_mutex);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b10fe26c4891..d87ec5a1bc4c 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1412,9 +1412,8 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
{
resource_size_t start = 0;
resource_size_t end = 0;
- struct resource *new_res;
+ struct resource *new_res, *tmp;
struct resource **old_res = &hyperv_mmio;
- struct resource **prev_res = NULL;
switch (res->type) {
@@ -1461,44 +1460,36 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
/*
* If two ranges are adjacent, merge them.
*/
- do {
- if (!*old_res) {
- *old_res = new_res;
- break;
- }
-
- if (((*old_res)->end + 1) == new_res->start) {
- (*old_res)->end = new_res->end;
+ if (!*old_res) {
+ *old_res = new_res;
+ return AE_OK;
+ }
+ tmp = *old_res;
+ list_for_each_entry_from(tmp, &tmp->parent->child, sibling) {
+ if ((tmp->end + 1) == new_res->start) {
+ tmp->end = new_res->end;
kfree(new_res);
break;
}
- if ((*old_res)->start == new_res->end + 1) {
- (*old_res)->start = new_res->start;
+ if (tmp->start == new_res->end + 1) {
+ tmp->start = new_res->start;
kfree(new_res);
break;
}
- if ((*old_res)->start > new_res->end) {
- new_res->sibling = *old_res;
- if (prev_res)
- (*prev_res)->sibling = new_res;
- *old_res = new_res;
+ if (tmp->start > new_res->end) {
+ list_add(&new_res->sibling, tmp->sibling.prev);
break;
}
-
- prev_res = old_res;
- old_res = &(*old_res)->sibling;
-
- } while (1);
+ }
return AE_OK;
}
static int vmbus_acpi_remove(struct acpi_device *device)
{
- struct resource *cur_res;
- struct resource *next_res;
+ struct resource *res;
if (hyperv_mmio) {
if (fb_mmio) {
@@ -1507,10 +1498,9 @@ static int vmbus_acpi_remove(struct acpi_device *device)
fb_mmio = NULL;
}
- for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) {
- next_res = cur_res->sibling;
- kfree(cur_res);
- }
+ res = hyperv_mmio;
+ list_for_each_entry_from(res, &res->parent->child, sibling)
+ kfree(res);
}
return 0;
@@ -1596,7 +1586,8 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
}
}
- for (iter = hyperv_mmio; iter; iter = iter->sibling) {
+ iter = hyperv_mmio;
+ list_for_each_entry_from(iter, &iter->parent->child, sibling) {
if ((iter->start >= max) || (iter->end <= min))
continue;
@@ -1639,7 +1630,8 @@ void vmbus_free_mmio(resource_size_t start, resource_size_t size)
struct resource *iter;
down(&hyperv_mmio_lock);
- for (iter = hyperv_mmio; iter; iter = iter->sibling) {
+ iter = hyperv_mmio;
+ list_for_each_entry_from(iter, &iter->parent->child, sibling) {
if ((iter->start >= start + size) || (iter->end <= start))
continue;
diff --git a/drivers/input/joystick/iforce/iforce-main.c b/drivers/input/joystick/iforce/iforce-main.c
index daeeb4c7e3b0..5c0be27b33ff 100644
--- a/drivers/input/joystick/iforce/iforce-main.c
+++ b/drivers/input/joystick/iforce/iforce-main.c
@@ -305,8 +305,8 @@ int iforce_init_device(struct iforce *iforce)
iforce->device_memory.end = 200;
iforce->device_memory.flags = IORESOURCE_MEM;
iforce->device_memory.parent = NULL;
- iforce->device_memory.child = NULL;
- iforce->device_memory.sibling = NULL;
+ INIT_LIST_HEAD(&iforce->device_memory.child);
+ INIT_LIST_HEAD(&iforce->device_memory.sibling);
/*
* Wait until device ready - until it sends its first response.
diff --git a/drivers/nvdimm/e820.c b/drivers/nvdimm/e820.c
index 6f9a6ffd7cde..513e661bb0d8 100644
--- a/drivers/nvdimm/e820.c
+++ b/drivers/nvdimm/e820.c
@@ -53,7 +53,7 @@ static int e820_pmem_probe(struct platform_device *pdev)
goto err;
platform_set_drvdata(pdev, nvdimm_bus);
- for (p = iomem_resource.child; p ; p = p->sibling) {
+ list_for_each_entry(p, &iomem_resource.child, sibling) {
struct nd_region_desc ndr_desc;
if (p->desc != IORES_DESC_PERSISTENT_MEMORY_LEGACY)
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 28afdd668905..f53d410d9981 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -637,7 +637,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
retry:
first = 0;
for_each_dpa_resource(ndd, res) {
- struct resource *next = res->sibling, *new_res = NULL;
+ struct resource *next = resource_sibling(res), *new_res = NULL;
resource_size_t allocate, available = 0;
enum alloc_loc loc = ALLOC_ERR;
const char *action;
@@ -763,7 +763,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
* an initial "pmem-reserve pass". Only do an initial BLK allocation
* when none of the DPA space is reserved.
*/
- if ((is_pmem || !ndd->dpa.child) && n == to_allocate)
+ if ((is_pmem || list_empty(&ndd->dpa.child)) && n == to_allocate)
return init_dpa_allocation(label_id, nd_region, nd_mapping, n);
return n;
}
@@ -779,7 +779,7 @@ static int merge_dpa(struct nd_region *nd_region,
retry:
for_each_dpa_resource(ndd, res) {
int rc;
- struct resource *next = res->sibling;
+ struct resource *next = resource_sibling(res);
resource_size_t end = res->start + resource_size(res);
if (!next || strcmp(res->name, label_id->id) != 0
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 32e0364b48b9..da7da15e03e7 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -102,11 +102,10 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
(unsigned long long) (res ? res->start : 0), ##arg)
#define for_each_dpa_resource(ndd, res) \
- for (res = (ndd)->dpa.child; res; res = res->sibling)
+ list_for_each_entry(res, &(ndd)->dpa.child, sibling)
#define for_each_dpa_resource_safe(ndd, res, next) \
- for (res = (ndd)->dpa.child, next = res ? res->sibling : NULL; \
- res; res = next, next = next ? next->sibling : NULL)
+ list_for_each_entry_safe(res, next, &(ndd)->dpa.child, sibling)
struct nd_percpu_lane {
int count;
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 53349912ac75..e2e25719ab52 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -330,7 +330,9 @@ int of_pci_range_to_resource(struct of_pci_range *range,
{
int err;
res->flags = range->flags;
- res->parent = res->child = res->sibling = NULL;
+ res->parent = NULL;
+ INIT_LIST_HEAD(&res->child);
+ INIT_LIST_HEAD(&res->sibling);
res->name = np->full_name;
if (res->flags & IORESOURCE_IO) {
diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 69bd98421eb1..7482bdfd1959 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -170,8 +170,8 @@ lba_dump_res(struct resource *r, int d)
for (i = d; i ; --i) printk(" ");
printk(KERN_DEBUG "%p [%lx,%lx]/%lx\n", r,
(long)r->start, (long)r->end, r->flags);
- lba_dump_res(r->child, d+2);
- lba_dump_res(r->sibling, d);
+ lba_dump_res(resource_first_child(&r->child), d+2);
+ lba_dump_res(resource_sibling(r), d);
}
diff --git a/drivers/pci/host/vmd.c b/drivers/pci/host/vmd.c
index 930a8fa08bd6..c3000af903ea 100644
--- a/drivers/pci/host/vmd.c
+++ b/drivers/pci/host/vmd.c
@@ -520,14 +520,14 @@ static struct pci_ops vmd_ops = {
static void vmd_attach_resources(struct vmd_dev *vmd)
{
- vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[1];
- vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[2];
+ list_add(&vmd->resources[1].sibling, &vmd->dev->resource[VMD_MEMBAR1].child);
+ list_add(&vmd->resources[2].sibling, &vmd->dev->resource[VMD_MEMBAR2].child);
}
static void vmd_detach_resources(struct vmd_dev *vmd)
{
- vmd->dev->resource[VMD_MEMBAR1].child = NULL;
- vmd->dev->resource[VMD_MEMBAR2].child = NULL;
+ INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR1].child);
+ INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR2].child);
}
/*
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac91b6fd0bcd..d162c77bec29 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -59,6 +59,8 @@ static struct resource *get_pci_domain_busn_res(int domain_nr)
r->res.start = 0;
r->res.end = 0xff;
r->res.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED;
+ INIT_LIST_HEAD(&r->res.child);
+ INIT_LIST_HEAD(&r->res.sibling);
list_add_tail(&r->list, &pci_domain_busn_res_list);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 072784f55ea5..0d5e30004ca6 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2107,7 +2107,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
continue;
/* Ignore BARs which are still in use */
- if (res->child)
+ if (!list_empty(&res->child))
continue;
ret = add_to_list(&saved, bridge, res, 0, 0);
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index da0ebaec25f0..225d13d3500a 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -12,6 +12,7 @@
#ifndef __ASSEMBLY__
#include <linux/compiler.h>
#include <linux/types.h>
+#include <linux/list.h>
/*
* Resources are tree-like, allowing
* nesting etc..
@@ -22,7 +23,8 @@ struct resource {
const char *name;
unsigned long flags;
unsigned long desc;
- struct resource *parent, *sibling, *child;
+ struct list_head child, sibling;
+ struct resource *parent;
};
/*
@@ -215,7 +217,6 @@ static inline bool resource_contains(struct resource *r1, struct resource *r2)
return r1->start <= r2->start && r1->end >= r2->end;
}
-
/* Convenience shorthand with allocation */
#define request_region(start,n,name) __request_region(&ioport_resource, (start), (n), (name), 0)
#define request_muxed_region(start,n,name) __request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
@@ -286,6 +287,18 @@ static inline bool resource_overlaps(struct resource *r1, struct resource *r2)
return (r1->start <= r2->end && r1->end >= r2->start);
}
+static inline struct resource *resource_sibling(struct resource *res)
+{
+ if (res->parent && !list_is_last(&res->sibling, &res->parent->child))
+ return list_next_entry(res, sibling);
+ return NULL;
+}
+
+static inline struct resource *resource_first_child(struct list_head *head)
+{
+ return list_first_entry_or_null(head, struct resource, sibling);
+}
+
#endif /* __ASSEMBLY__ */
#endif /* _LINUX_IOPORT_H */
diff --git a/kernel/resource.c b/kernel/resource.c
index 2af6c03858b9..4f560991c130 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -31,6 +31,8 @@ struct resource ioport_resource = {
.start = 0,
.end = IO_SPACE_LIMIT,
.flags = IORESOURCE_IO,
+ .sibling = LIST_HEAD_INIT(ioport_resource.sibling),
+ .child = LIST_HEAD_INIT(ioport_resource.child),
};
EXPORT_SYMBOL(ioport_resource);
@@ -39,6 +41,8 @@ struct resource iomem_resource = {
.start = 0,
.end = -1,
.flags = IORESOURCE_MEM,
+ .sibling = LIST_HEAD_INIT(iomem_resource.sibling),
+ .child = LIST_HEAD_INIT(iomem_resource.child),
};
EXPORT_SYMBOL(iomem_resource);
@@ -57,20 +61,20 @@ static DEFINE_RWLOCK(resource_lock);
* by boot mem after the system is up. So for reusing the resource entry
* we need to remember the resource.
*/
-static struct resource *bootmem_resource_free;
+static struct list_head bootmem_resource_free = LIST_HEAD_INIT(bootmem_resource_free);
static DEFINE_SPINLOCK(bootmem_resource_lock);
static struct resource *next_resource(struct resource *p, bool sibling_only)
{
/* Caller wants to traverse through siblings only */
if (sibling_only)
- return p->sibling;
+ return resource_sibling(p);
- if (p->child)
- return p->child;
- while (!p->sibling && p->parent)
+ if (!list_empty(&p->child))
+ return resource_first_child(&p->child);
+ while (!resource_sibling(p) && p->parent)
p = p->parent;
- return p->sibling;
+ return resource_sibling(p);
}
static void *r_next(struct seq_file *m, void *v, loff_t *pos)
@@ -90,7 +94,7 @@ static void *r_start(struct seq_file *m, loff_t *pos)
struct resource *p = m->private;
loff_t l = 0;
read_lock(&resource_lock);
- for (p = p->child; p && l < *pos; p = r_next(m, p, &l))
+ for (p = resource_first_child(&p->child); p && l < *pos; p = r_next(m, p, &l))
;
return p;
}
@@ -186,8 +190,7 @@ static void free_resource(struct resource *res)
if (!PageSlab(virt_to_head_page(res))) {
spin_lock(&bootmem_resource_lock);
- res->sibling = bootmem_resource_free;
- bootmem_resource_free = res;
+ list_add(&res->sibling, &bootmem_resource_free);
spin_unlock(&bootmem_resource_lock);
} else {
kfree(res);
@@ -199,10 +202,9 @@ static struct resource *alloc_resource(gfp_t flags)
struct resource *res = NULL;
spin_lock(&bootmem_resource_lock);
- if (bootmem_resource_free) {
- res = bootmem_resource_free;
- bootmem_resource_free = res->sibling;
- }
+ res = resource_first_child(&bootmem_resource_free);
+ if (res)
+ list_del(&res->sibling);
spin_unlock(&bootmem_resource_lock);
if (res)
@@ -210,6 +212,8 @@ static struct resource *alloc_resource(gfp_t flags)
else
res = kzalloc(sizeof(struct resource), flags);
+ INIT_LIST_HEAD(&res->child);
+ INIT_LIST_HEAD(&res->sibling);
return res;
}
@@ -218,7 +222,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
{
resource_size_t start = new->start;
resource_size_t end = new->end;
- struct resource *tmp, **p;
+ struct resource *tmp;
if (end < start)
return root;
@@ -226,64 +230,62 @@ static struct resource * __request_resource(struct resource *root, struct resour
return root;
if (end > root->end)
return root;
- p = &root->child;
- for (;;) {
- tmp = *p;
- if (!tmp || tmp->start > end) {
- new->sibling = tmp;
- *p = new;
+
+ if (list_empty(&root->child)) {
+ list_add(&new->sibling, &root->child);
+ new->parent = root;
+ INIT_LIST_HEAD(&new->child);
+ return NULL;
+ }
+
+ list_for_each_entry(tmp, &root->child, sibling) {
+ if (tmp->start > end) {
+ list_add(&new->sibling, tmp->sibling.prev);
new->parent = root;
+ INIT_LIST_HEAD(&new->child);
return NULL;
}
- p = &tmp->sibling;
if (tmp->end < start)
continue;
return tmp;
}
+
+ list_add_tail(&new->sibling, &root->child);
+ new->parent = root;
+ INIT_LIST_HEAD(&new->child);
+ return NULL;
}
static int __release_resource(struct resource *old, bool release_child)
{
- struct resource *tmp, **p, *chd;
+ struct resource *tmp, *next, *chd;
- p = &old->parent->child;
- for (;;) {
- tmp = *p;
- if (!tmp)
- break;
+ list_for_each_entry_safe(tmp, next, &old->parent->child, sibling) {
if (tmp == old) {
- if (release_child || !(tmp->child)) {
- *p = tmp->sibling;
+ if (release_child || list_empty(&tmp->child)) {
+ list_del(&tmp->sibling);
} else {
- for (chd = tmp->child;; chd = chd->sibling) {
+ list_for_each_entry(chd, &tmp->child, sibling)
chd->parent = tmp->parent;
- if (!(chd->sibling))
- break;
- }
- *p = tmp->child;
- chd->sibling = tmp->sibling;
+ list_splice(&tmp->child, tmp->sibling.prev);
+ list_del(&tmp->sibling);
}
+
old->parent = NULL;
return 0;
}
- p = &tmp->sibling;
}
return -EINVAL;
}
static void __release_child_resources(struct resource *r)
{
- struct resource *tmp, *p;
+ struct resource *tmp, *next;
resource_size_t size;
- p = r->child;
- r->child = NULL;
- while (p) {
- tmp = p;
- p = p->sibling;
-
+ list_for_each_entry_safe(tmp, next, &r->child, sibling) {
tmp->parent = NULL;
- tmp->sibling = NULL;
+ INIT_LIST_HEAD(&tmp->sibling);
__release_child_resources(tmp);
printk(KERN_DEBUG "release child resource %pR\n", tmp);
@@ -292,6 +294,8 @@ static void __release_child_resources(struct resource *r)
tmp->start = 0;
tmp->end = size - 1;
}
+
+ INIT_LIST_HEAD(&tmp->child);
}
void release_child_resources(struct resource *r)
@@ -376,7 +380,8 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
read_lock(&resource_lock);
- for (p = iomem_resource.child; p; p = next_resource(p, sibling_only)) {
+ for (p = resource_first_child(&iomem_resource.child); p;
+ p = next_resource(p, sibling_only)) {
if ((p->flags & res->flags) != res->flags)
continue;
if ((desc != IORES_DESC_NONE) && (desc != p->desc))
@@ -564,7 +569,7 @@ int region_intersects(resource_size_t start, size_t size, unsigned long flags,
struct resource *p;
read_lock(&resource_lock);
- for (p = iomem_resource.child; p ; p = p->sibling) {
+ list_for_each_entry(p, &iomem_resource.child, sibling) {
bool is_type = (((p->flags & flags) == flags) &&
((desc == IORES_DESC_NONE) ||
(desc == p->desc)));
@@ -618,7 +623,7 @@ static int __find_resource(struct resource *root, struct resource *old,
resource_size_t size,
struct resource_constraint *constraint)
{
- struct resource *this = root->child;
+ struct resource *this = resource_first_child(&root->child);
struct resource tmp = *new, avail, alloc;
tmp.start = root->start;
@@ -628,7 +633,7 @@ static int __find_resource(struct resource *root, struct resource *old,
*/
if (this && this->start == root->start) {
tmp.start = (this == old) ? old->start : this->end + 1;
- this = this->sibling;
+ this = resource_sibling(this);
}
for(;;) {
if (this)
@@ -664,7 +669,7 @@ next: if (!this || this->end == root->end)
if (this != old)
tmp.start = this->end + 1;
- this = this->sibling;
+ this = resource_sibling(this);
}
return -EBUSY;
}
@@ -708,7 +713,7 @@ static int reallocate_resource(struct resource *root, struct resource *old,
goto out;
}
- if (old->child) {
+ if (!list_empty(&old->child)) {
err = -EBUSY;
goto out;
}
@@ -789,7 +794,7 @@ struct resource *lookup_resource(struct resource *root, resource_size_t start)
struct resource *res;
read_lock(&resource_lock);
- for (res = root->child; res; res = res->sibling) {
+ list_for_each_entry(res, &root->child, sibling) {
if (res->start == start)
break;
}
@@ -822,32 +827,27 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
break;
}
- for (next = first; ; next = next->sibling) {
+ for (next = first; ; next = resource_sibling(next)) {
/* Partial overlap? Bad, and unfixable */
if (next->start < new->start || next->end > new->end)
return next;
- if (!next->sibling)
+ if (!resource_sibling(next))
break;
- if (next->sibling->start > new->end)
+ if (resource_sibling(next)->start > new->end)
break;
}
-
new->parent = parent;
- new->sibling = next->sibling;
- new->child = first;
+ list_add(&new->sibling, &next->sibling);
+ INIT_LIST_HEAD(&new->child);
- next->sibling = NULL;
- for (next = first; next; next = next->sibling)
+ /*
+ * From first to next, they all fall into new's region, so change them
+ * as new's children.
+ */
+ list_cut_position(&new->child, first->sibling.prev, &next->sibling);
+ list_for_each_entry(next, &new->child, sibling)
next->parent = new;
- if (parent->child == first) {
- parent->child = new;
- } else {
- next = parent->child;
- while (next->sibling != first)
- next = next->sibling;
- next->sibling = new;
- }
return NULL;
}
@@ -969,19 +969,17 @@ static int __adjust_resource(struct resource *res, resource_size_t start,
if ((start < parent->start) || (end > parent->end))
goto out;
- if (res->sibling && (res->sibling->start <= end))
+ if (resource_sibling(res) && (resource_sibling(res)->start <= end))
goto out;
- tmp = parent->child;
- if (tmp != res) {
- while (tmp->sibling != res)
- tmp = tmp->sibling;
+ if (res->sibling.prev != &parent->child) {
+ tmp = list_prev_entry(res, sibling);
if (start <= tmp->end)
goto out;
}
skip:
- for (tmp = res->child; tmp; tmp = tmp->sibling)
+ list_for_each_entry(tmp, &res->child, sibling)
if ((tmp->start < start) || (tmp->end > end))
goto out;
@@ -1206,34 +1204,32 @@ EXPORT_SYMBOL(__request_region);
void __release_region(struct resource *parent, resource_size_t start,
resource_size_t n)
{
- struct resource **p;
+ struct resource *res;
resource_size_t end;
- p = &parent->child;
+ res = resource_first_child(&parent->child);
end = start + n - 1;
write_lock(&resource_lock);
for (;;) {
- struct resource *res = *p;
-
if (!res)
break;
if (res->start <= start && res->end >= end) {
if (!(res->flags & IORESOURCE_BUSY)) {
- p = &res->child;
+ res = resource_first_child(&res->child);
continue;
}
if (res->start != start || res->end != end)
break;
- *p = res->sibling;
+ list_del(&res->sibling);
write_unlock(&resource_lock);
if (res->flags & IORESOURCE_MUXED)
wake_up(&muxed_resource_wait);
free_resource(res);
return;
}
- p = &res->sibling;
+ res = resource_sibling(res);
}
write_unlock(&resource_lock);
@@ -1268,9 +1264,7 @@ EXPORT_SYMBOL(__release_region);
int release_mem_region_adjustable(struct resource *parent,
resource_size_t start, resource_size_t size)
{
- struct resource **p;
- struct resource *res;
- struct resource *new_res;
+ struct resource *res, *new_res;
resource_size_t end;
int ret = -EINVAL;
@@ -1281,16 +1275,16 @@ int release_mem_region_adjustable(struct resource *parent,
/* The alloc_resource() result gets checked later */
new_res = alloc_resource(GFP_KERNEL);
- p = &parent->child;
+ res = resource_first_child(&parent->child);
write_lock(&resource_lock);
- while ((res = *p)) {
+ while ((res)) {
if (res->start >= end)
break;
/* look for the next resource if it does not fit into */
if (res->start > start || res->end < end) {
- p = &res->sibling;
+ res = resource_sibling(res);
continue;
}
@@ -1298,14 +1292,14 @@ int release_mem_region_adjustable(struct resource *parent,
break;
if (!(res->flags & IORESOURCE_BUSY)) {
- p = &res->child;
+ res = resource_first_child(&res->child);
continue;
}
/* found the target resource; let's adjust accordingly */
if (res->start == start && res->end == end) {
/* free the whole entry */
- *p = res->sibling;
+ list_del(&res->sibling);
free_resource(res);
ret = 0;
} else if (res->start == start && res->end != end) {
@@ -1328,14 +1322,13 @@ int release_mem_region_adjustable(struct resource *parent,
new_res->flags = res->flags;
new_res->desc = res->desc;
new_res->parent = res->parent;
- new_res->sibling = res->sibling;
- new_res->child = NULL;
+ INIT_LIST_HEAD(&new_res->child);
ret = __adjust_resource(res, res->start,
start - res->start);
if (ret)
break;
- res->sibling = new_res;
+ list_add(&new_res->sibling, &res->sibling);
new_res = NULL;
}
@@ -1516,7 +1509,7 @@ static int __init reserve_setup(char *str)
res->end = io_start + io_num - 1;
res->flags |= IORESOURCE_BUSY;
res->desc = IORES_DESC_NONE;
- res->child = NULL;
+ INIT_LIST_HEAD(&res->child);
if (request_resource(parent, res) == 0)
reserved = x+1;
}
@@ -1536,7 +1529,7 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
loff_t l;
read_lock(&resource_lock);
- for (p = p->child; p ; p = r_next(NULL, p, &l)) {
+ for (p = resource_first_child(&p->child); p; p = r_next(NULL, p, &l)) {
/*
* We can probably skip the resources without
* IORESOURCE_IO attribute?
@@ -1592,7 +1585,7 @@ bool iomem_is_exclusive(u64 addr)
addr = addr & PAGE_MASK;
read_lock(&resource_lock);
- for (p = p->child; p ; p = r_next(NULL, p, &l)) {
+ for (p = resource_first_child(&p->child); p; p = r_next(NULL, p, &l)) {
/*
* We can probably skip the resources without
* IORESOURCE_IO attribute?
--
2.13.6
On Thu, Apr 19, 2018 at 08:18:47AM +0800, Baoquan He wrote:
> This function, being a variant of walk_system_ram_res() introduced in
> commit 8c86e70acead ("resource: provide new functions to walk through
> resources"), walks through a list of all the resources of System RAM
> in reversed order, i.e., from higher to lower.
>
> It will be used in kexec_file code.
Of course, what is missing is the big *WHY* you need this to happen this
way...
> /*
> + * This function, being a variant of walk_system_ram_res(), calls the @func
> + * callback against all memory ranges of type System RAM which are marked as
> + * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
> + * higher to lower.
> + */
> +int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> + int (*func)(struct resource *, void *))
> +{
> + unsigned long flags;
> + struct resource *res;
> + int ret = -1;
> +
> + flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +
> + read_lock(&resource_lock);
> + list_for_each_entry_reverse(res, &iomem_resource.child, sibling) {
... and the other thing that I'm not clear on is why are you
slapping this function just like that instead of extending
__walk_iomem_res_desc() to do reverse direction too?
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
>The struct resource uses singly linked list to link siblings. It's not
>easy to do reverse iteration on sibling list. So replace it with list_head.
>
Hi, Baoquan
Besides changing the data structure, I have another proposal to do the reverse
iteration. Which means it would not affect other users, if you just want a
reverse iteration.
BTW, I don't think Andrew suggest to use linked-list directly. What he wants
is a better solution to your first proposal in
https://patchwork.kernel.org/patch/10300819/.
Below is my proposal of resource reverse iteration without changing current
design.
From 5d7145d44fe48b98572a03884fa3a3aa82e3cef9 Mon Sep 17 00:00:00 2001
From: Wei Yang <[email protected]>
Date: Sat, 24 Mar 2018 23:25:46 +0800
Subject: [PATCH] kernel/resource: add walk_system_ram_res_rev()
As discussed on https://patchwork.kernel.org/patch/10300819/, this patch
comes up with a variant implementation of walk_system_ram_res_rev(), which
uses iteration instead of allocating array to store those resources.
Signed-off-by: Wei Yang <[email protected]>
---
include/linux/ioport.h | 3 ++
kernel/resource.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index da0ebaec25f0..473f1d9cb97e 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -277,6 +277,9 @@ extern int
walk_system_ram_res(u64 start, u64 end, void *arg,
int (*func)(struct resource *, void *));
extern int
+walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+ int (*func)(struct resource *, void *));
+extern int
walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
void *arg, int (*func)(struct resource *, void *));
diff --git a/kernel/resource.c b/kernel/resource.c
index 769109f20fb7..d4ec5fbc6875 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -73,6 +73,38 @@ static struct resource *next_resource(struct resource *p, bool sibling_only)
return p->sibling;
}
+static struct resource *prev_resource(struct resource *p, bool sibling_only)
+{
+ struct resource *prev;
+ if (NULL == iomem_resource.child)
+ return NULL;
+
+ if (p == NULL) {
+ prev = iomem_resource.child;
+ while (prev->sibling)
+ prev = prev->sibling;
+ } else {
+ if (p->parent->child == p) {
+ return p->parent;
+ }
+
+ for (prev = p->parent->child; prev->sibling != p;
+ prev = prev->sibling) {}
+ }
+
+ /* Caller wants to traverse through siblings only */
+ if (sibling_only)
+ return prev;
+
+ for (;prev->child;) {
+ prev = prev->child;
+
+ while (prev->sibling)
+ prev = prev->sibling;
+ }
+ return prev;
+}
+
static void *r_next(struct seq_file *m, void *v, loff_t *pos)
{
struct resource *p = v;
@@ -401,6 +433,47 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
return 0;
}
+/*
+ * Finds the highest iomem resource existing within [res->start.res->end).
+ * The caller must specify res->start, res->end, res->flags, and optionally
+ * desc. If found, returns 0, res is overwritten, if not found, returns -1.
+ * This function walks the whole tree and not just first level children until
+ * and unless first_level_children_only is true.
+ */
+static int find_prev_iomem_res(struct resource *res, unsigned long desc,
+ bool first_level_children_only)
+{
+ struct resource *p;
+
+ BUG_ON(!res);
+ BUG_ON(res->start >= res->end);
+
+ read_lock(&resource_lock);
+
+ for (p = prev_resource(NULL, first_level_children_only); p;
+ p = prev_resource(p, first_level_children_only)) {
+ if ((p->flags & res->flags) != res->flags)
+ continue;
+ if ((desc != IORES_DESC_NONE) && (desc != p->desc))
+ continue;
+ if (p->end < res->start || p->child == iomem_resource.child) {
+ p = NULL;
+ break;
+ }
+ if ((p->end >= res->start) && (p->start < res->end))
+ break;
+ }
+
+ read_unlock(&resource_lock);
+ if (!p)
+ return -1;
+ /* copy data */
+ resource_clip(res, p->start, p->end);
+ res->flags = p->flags;
+ res->desc = p->desc;
+ return 0;
+}
+
static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
bool first_level_children_only,
void *arg,
@@ -422,6 +495,27 @@ static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
return ret;
}
+static int __walk_iomem_res_rev_desc(struct resource *res, unsigned long desc,
+ bool first_level_children_only,
+ void *arg,
+ int (*func)(struct resource *, void *))
+{
+ u64 orig_start = res->start;
+ int ret = -1;
+
+ while ((res->start < res->end) &&
+ !find_prev_iomem_res(res, desc, first_level_children_only)) {
+ ret = (*func)(res, arg);
+ if (ret)
+ break;
+
+ res->end = res->start?(res->start - 1):0;
+ res->start = orig_start;
+ }
+
+ return ret;
+}
+
/*
* Walks through iomem resources and calls func() with matching resource
* ranges. This walks through whole tree and not just first level children.
@@ -468,6 +562,25 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
arg, func);
}
+/*
+ * This function, being a variant of walk_system_ram_res(), calls the @func
+ * callback against all memory ranges of type System RAM which are marked as
+ * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
+ * higher to lower.
+ */
+int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+ int (*func)(struct resource *, void *))
+{
+ struct resource res;
+
+ res.start = start;
+ res.end = end;
+ res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+ return __walk_iomem_res_rev_desc(&res, IORES_DESC_NONE, true,
+ arg, func);
+}
+
/*
* This function calls the @func callback against all memory ranges, which
* are ranges marked as IORESOURCE_MEM and IORESOUCE_BUSY.
--
2.15.1
--
Wei Yang
Help you, Help me
Hi Baoquan,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc2 next-20180424]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
config: microblaze-mmu_defconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=microblaze
All errors (new ones prefixed by >>):
arch/microblaze/pci/pci-common.c: In function 'pci_process_bridge_OF_ranges':
>> arch/microblaze/pci/pci-common.c:536:44: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
res->parent = res->child = res->sibling = NULL;
^
arch/microblaze/pci/pci-common.c: In function 'reparent_resources':
>> arch/microblaze/pci/pci-common.c:631:10: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
^
arch/microblaze/pci/pci-common.c:631:50: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
^
>> arch/microblaze/pci/pci-common.c:644:13: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
res->child = *firstpp;
^
arch/microblaze/pci/pci-common.c:645:15: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
res->sibling = *pp;
^
>> arch/microblaze/pci/pci-common.c:648:9: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
for (p = res->child; p != NULL; p = p->sibling) {
^
arch/microblaze/pci/pci-common.c:648:36: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
for (p = res->child; p != NULL; p = p->sibling) {
^
cc1: some warnings being treated as errors
vim +536 arch/microblaze/pci/pci-common.c
d3afa58c Michal Simek 2010-01-18 387
d3afa58c Michal Simek 2010-01-18 388 /**
d3afa58c Michal Simek 2010-01-18 389 * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
d3afa58c Michal Simek 2010-01-18 390 * @hose: newly allocated pci_controller to be setup
d3afa58c Michal Simek 2010-01-18 391 * @dev: device node of the host bridge
d3afa58c Michal Simek 2010-01-18 392 * @primary: set if primary bus (32 bits only, soon to be deprecated)
d3afa58c Michal Simek 2010-01-18 393 *
d3afa58c Michal Simek 2010-01-18 394 * This function will parse the "ranges" property of a PCI host bridge device
d3afa58c Michal Simek 2010-01-18 395 * node and setup the resource mapping of a pci controller based on its
d3afa58c Michal Simek 2010-01-18 396 * content.
d3afa58c Michal Simek 2010-01-18 397 *
d3afa58c Michal Simek 2010-01-18 398 * Life would be boring if it wasn't for a few issues that we have to deal
d3afa58c Michal Simek 2010-01-18 399 * with here:
d3afa58c Michal Simek 2010-01-18 400 *
d3afa58c Michal Simek 2010-01-18 401 * - We can only cope with one IO space range and up to 3 Memory space
d3afa58c Michal Simek 2010-01-18 402 * ranges. However, some machines (thanks Apple !) tend to split their
d3afa58c Michal Simek 2010-01-18 403 * space into lots of small contiguous ranges. So we have to coalesce.
d3afa58c Michal Simek 2010-01-18 404 *
d3afa58c Michal Simek 2010-01-18 405 * - We can only cope with all memory ranges having the same offset
d3afa58c Michal Simek 2010-01-18 406 * between CPU addresses and PCI addresses. Unfortunately, some bridges
d3afa58c Michal Simek 2010-01-18 407 * are setup for a large 1:1 mapping along with a small "window" which
d3afa58c Michal Simek 2010-01-18 408 * maps PCI address 0 to some arbitrary high address of the CPU space in
d3afa58c Michal Simek 2010-01-18 409 * order to give access to the ISA memory hole.
d3afa58c Michal Simek 2010-01-18 410 * The way out of here that I've chosen for now is to always set the
d3afa58c Michal Simek 2010-01-18 411 * offset based on the first resource found, then override it if we
d3afa58c Michal Simek 2010-01-18 412 * have a different offset and the previous was set by an ISA hole.
d3afa58c Michal Simek 2010-01-18 413 *
d3afa58c Michal Simek 2010-01-18 414 * - Some busses have IO space not starting at 0, which causes trouble with
d3afa58c Michal Simek 2010-01-18 415 * the way we do our IO resource renumbering. The code somewhat deals with
d3afa58c Michal Simek 2010-01-18 416 * it for 64 bits but I would expect problems on 32 bits.
d3afa58c Michal Simek 2010-01-18 417 *
d3afa58c Michal Simek 2010-01-18 418 * - Some 32 bits platforms such as 4xx can have physical space larger than
d3afa58c Michal Simek 2010-01-18 419 * 32 bits so we need to use 64 bits values for the parsing
d3afa58c Michal Simek 2010-01-18 420 */
b881bc46 Greg Kroah-Hartman 2012-12-21 421 void pci_process_bridge_OF_ranges(struct pci_controller *hose,
b881bc46 Greg Kroah-Hartman 2012-12-21 422 struct device_node *dev, int primary)
d3afa58c Michal Simek 2010-01-18 423 {
d3afa58c Michal Simek 2010-01-18 424 int memno = 0, isa_hole = -1;
d3afa58c Michal Simek 2010-01-18 425 unsigned long long isa_mb = 0;
d3afa58c Michal Simek 2010-01-18 426 struct resource *res;
4f7b6de4 Andrew Murray 2013-07-27 427 struct of_pci_range range;
4f7b6de4 Andrew Murray 2013-07-27 428 struct of_pci_range_parser parser;
d3afa58c Michal Simek 2010-01-18 429
f2b8ae0e Rob Herring 2017-06-06 430 pr_info("PCI host bridge %pOF %s ranges:\n",
f2b8ae0e Rob Herring 2017-06-06 431 dev, primary ? "(primary)" : "");
d3afa58c Michal Simek 2010-01-18 432
4f7b6de4 Andrew Murray 2013-07-27 433 /* Check for ranges property */
4f7b6de4 Andrew Murray 2013-07-27 434 if (of_pci_range_parser_init(&parser, dev))
d3afa58c Michal Simek 2010-01-18 435 return;
d3afa58c Michal Simek 2010-01-18 436
d3afa58c Michal Simek 2010-01-18 437 pr_debug("Parsing ranges property...\n");
4f7b6de4 Andrew Murray 2013-07-27 438 for_each_of_pci_range(&parser, &range) {
d3afa58c Michal Simek 2010-01-18 439 /* Read next ranges element */
6bd55f0b Michal Simek 2012-12-27 440 pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
4f7b6de4 Andrew Murray 2013-07-27 441 range.pci_space, range.pci_addr);
6bd55f0b Michal Simek 2012-12-27 442 pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
4f7b6de4 Andrew Murray 2013-07-27 443 range.cpu_addr, range.size);
d3afa58c Michal Simek 2010-01-18 444
d3afa58c Michal Simek 2010-01-18 445 /* If we failed translation or got a zero-sized region
d3afa58c Michal Simek 2010-01-18 446 * (some FW try to feed us with non sensical zero sized regions
d3afa58c Michal Simek 2010-01-18 447 * such as power3 which look like some kind of attempt
d3afa58c Michal Simek 2010-01-18 448 * at exposing the VGA memory hole)
d3afa58c Michal Simek 2010-01-18 449 */
4f7b6de4 Andrew Murray 2013-07-27 450 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
d3afa58c Michal Simek 2010-01-18 451 continue;
d3afa58c Michal Simek 2010-01-18 452
d3afa58c Michal Simek 2010-01-18 453 /* Act based on address space type */
d3afa58c Michal Simek 2010-01-18 454 res = NULL;
4f7b6de4 Andrew Murray 2013-07-27 455 switch (range.flags & IORESOURCE_TYPE_BITS) {
4f7b6de4 Andrew Murray 2013-07-27 456 case IORESOURCE_IO:
6bd55f0b Michal Simek 2012-12-27 457 pr_info(" IO 0x%016llx..0x%016llx -> 0x%016llx\n",
4f7b6de4 Andrew Murray 2013-07-27 458 range.cpu_addr, range.cpu_addr + range.size - 1,
4f7b6de4 Andrew Murray 2013-07-27 459 range.pci_addr);
d3afa58c Michal Simek 2010-01-18 460
d3afa58c Michal Simek 2010-01-18 461 /* We support only one IO range */
d3afa58c Michal Simek 2010-01-18 462 if (hose->pci_io_size) {
6bd55f0b Michal Simek 2012-12-27 463 pr_info(" \\--> Skipped (too many) !\n");
d3afa58c Michal Simek 2010-01-18 464 continue;
d3afa58c Michal Simek 2010-01-18 465 }
d3afa58c Michal Simek 2010-01-18 466 /* On 32 bits, limit I/O space to 16MB */
4f7b6de4 Andrew Murray 2013-07-27 467 if (range.size > 0x01000000)
4f7b6de4 Andrew Murray 2013-07-27 468 range.size = 0x01000000;
d3afa58c Michal Simek 2010-01-18 469
d3afa58c Michal Simek 2010-01-18 470 /* 32 bits needs to map IOs here */
4f7b6de4 Andrew Murray 2013-07-27 471 hose->io_base_virt = ioremap(range.cpu_addr,
4f7b6de4 Andrew Murray 2013-07-27 472 range.size);
d3afa58c Michal Simek 2010-01-18 473
d3afa58c Michal Simek 2010-01-18 474 /* Expect trouble if pci_addr is not 0 */
d3afa58c Michal Simek 2010-01-18 475 if (primary)
d3afa58c Michal Simek 2010-01-18 476 isa_io_base =
d3afa58c Michal Simek 2010-01-18 477 (unsigned long)hose->io_base_virt;
d3afa58c Michal Simek 2010-01-18 478 /* pci_io_size and io_base_phys always represent IO
d3afa58c Michal Simek 2010-01-18 479 * space starting at 0 so we factor in pci_addr
d3afa58c Michal Simek 2010-01-18 480 */
4f7b6de4 Andrew Murray 2013-07-27 481 hose->pci_io_size = range.pci_addr + range.size;
4f7b6de4 Andrew Murray 2013-07-27 482 hose->io_base_phys = range.cpu_addr - range.pci_addr;
d3afa58c Michal Simek 2010-01-18 483
d3afa58c Michal Simek 2010-01-18 484 /* Build resource */
d3afa58c Michal Simek 2010-01-18 485 res = &hose->io_resource;
4f7b6de4 Andrew Murray 2013-07-27 486 range.cpu_addr = range.pci_addr;
4f7b6de4 Andrew Murray 2013-07-27 487
d3afa58c Michal Simek 2010-01-18 488 break;
4f7b6de4 Andrew Murray 2013-07-27 489 case IORESOURCE_MEM:
6bd55f0b Michal Simek 2012-12-27 490 pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
4f7b6de4 Andrew Murray 2013-07-27 491 range.cpu_addr, range.cpu_addr + range.size - 1,
4f7b6de4 Andrew Murray 2013-07-27 492 range.pci_addr,
4f7b6de4 Andrew Murray 2013-07-27 493 (range.pci_space & 0x40000000) ?
4f7b6de4 Andrew Murray 2013-07-27 494 "Prefetch" : "");
d3afa58c Michal Simek 2010-01-18 495
d3afa58c Michal Simek 2010-01-18 496 /* We support only 3 memory ranges */
d3afa58c Michal Simek 2010-01-18 497 if (memno >= 3) {
6bd55f0b Michal Simek 2012-12-27 498 pr_info(" \\--> Skipped (too many) !\n");
d3afa58c Michal Simek 2010-01-18 499 continue;
d3afa58c Michal Simek 2010-01-18 500 }
d3afa58c Michal Simek 2010-01-18 501 /* Handles ISA memory hole space here */
4f7b6de4 Andrew Murray 2013-07-27 502 if (range.pci_addr == 0) {
4f7b6de4 Andrew Murray 2013-07-27 503 isa_mb = range.cpu_addr;
d3afa58c Michal Simek 2010-01-18 504 isa_hole = memno;
d3afa58c Michal Simek 2010-01-18 505 if (primary || isa_mem_base == 0)
4f7b6de4 Andrew Murray 2013-07-27 506 isa_mem_base = range.cpu_addr;
4f7b6de4 Andrew Murray 2013-07-27 507 hose->isa_mem_phys = range.cpu_addr;
4f7b6de4 Andrew Murray 2013-07-27 508 hose->isa_mem_size = range.size;
d3afa58c Michal Simek 2010-01-18 509 }
d3afa58c Michal Simek 2010-01-18 510
d3afa58c Michal Simek 2010-01-18 511 /* We get the PCI/Mem offset from the first range or
d3afa58c Michal Simek 2010-01-18 512 * the, current one if the offset came from an ISA
d3afa58c Michal Simek 2010-01-18 513 * hole. If they don't match, bugger.
d3afa58c Michal Simek 2010-01-18 514 */
d3afa58c Michal Simek 2010-01-18 515 if (memno == 0 ||
4f7b6de4 Andrew Murray 2013-07-27 516 (isa_hole >= 0 && range.pci_addr != 0 &&
d3afa58c Michal Simek 2010-01-18 517 hose->pci_mem_offset == isa_mb))
4f7b6de4 Andrew Murray 2013-07-27 518 hose->pci_mem_offset = range.cpu_addr -
4f7b6de4 Andrew Murray 2013-07-27 519 range.pci_addr;
4f7b6de4 Andrew Murray 2013-07-27 520 else if (range.pci_addr != 0 &&
4f7b6de4 Andrew Murray 2013-07-27 521 hose->pci_mem_offset != range.cpu_addr -
4f7b6de4 Andrew Murray 2013-07-27 522 range.pci_addr) {
6bd55f0b Michal Simek 2012-12-27 523 pr_info(" \\--> Skipped (offset mismatch) !\n");
d3afa58c Michal Simek 2010-01-18 524 continue;
d3afa58c Michal Simek 2010-01-18 525 }
d3afa58c Michal Simek 2010-01-18 526
d3afa58c Michal Simek 2010-01-18 527 /* Build resource */
d3afa58c Michal Simek 2010-01-18 528 res = &hose->mem_resources[memno++];
d3afa58c Michal Simek 2010-01-18 529 break;
d3afa58c Michal Simek 2010-01-18 530 }
70dcd942 Michal Simek 2014-10-27 531 if (res != NULL) {
70dcd942 Michal Simek 2014-10-27 532 res->name = dev->full_name;
70dcd942 Michal Simek 2014-10-27 533 res->flags = range.flags;
70dcd942 Michal Simek 2014-10-27 534 res->start = range.cpu_addr;
70dcd942 Michal Simek 2014-10-27 535 res->end = range.cpu_addr + range.size - 1;
70dcd942 Michal Simek 2014-10-27 @536 res->parent = res->child = res->sibling = NULL;
70dcd942 Michal Simek 2014-10-27 537 }
d3afa58c Michal Simek 2010-01-18 538 }
d3afa58c Michal Simek 2010-01-18 539
d3afa58c Michal Simek 2010-01-18 540 /* If there's an ISA hole and the pci_mem_offset is -not- matching
d3afa58c Michal Simek 2010-01-18 541 * the ISA hole offset, then we need to remove the ISA hole from
d3afa58c Michal Simek 2010-01-18 542 * the resource list for that brige
d3afa58c Michal Simek 2010-01-18 543 */
d3afa58c Michal Simek 2010-01-18 544 if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
d3afa58c Michal Simek 2010-01-18 545 unsigned int next = isa_hole + 1;
6bd55f0b Michal Simek 2012-12-27 546 pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
d3afa58c Michal Simek 2010-01-18 547 if (next < memno)
d3afa58c Michal Simek 2010-01-18 548 memmove(&hose->mem_resources[isa_hole],
d3afa58c Michal Simek 2010-01-18 549 &hose->mem_resources[next],
d3afa58c Michal Simek 2010-01-18 550 sizeof(struct resource) * (memno - next));
d3afa58c Michal Simek 2010-01-18 551 hose->mem_resources[--memno].flags = 0;
d3afa58c Michal Simek 2010-01-18 552 }
d3afa58c Michal Simek 2010-01-18 553 }
d3afa58c Michal Simek 2010-01-18 554
9413d968 Bharat Kumar Gogada 2016-09-01 555 /* Display the domain number in /proc */
d3afa58c Michal Simek 2010-01-18 556 int pci_proc_domain(struct pci_bus *bus)
d3afa58c Michal Simek 2010-01-18 557 {
9413d968 Bharat Kumar Gogada 2016-09-01 558 return pci_domain_nr(bus);
d3afa58c Michal Simek 2010-01-18 559 }
d3afa58c Michal Simek 2010-01-18 560
d3afa58c Michal Simek 2010-01-18 561 /* This header fixup will do the resource fixup for all devices as they are
d3afa58c Michal Simek 2010-01-18 562 * probed, but not for bridge ranges
d3afa58c Michal Simek 2010-01-18 563 */
b881bc46 Greg Kroah-Hartman 2012-12-21 564 static void pcibios_fixup_resources(struct pci_dev *dev)
d3afa58c Michal Simek 2010-01-18 565 {
d3afa58c Michal Simek 2010-01-18 566 struct pci_controller *hose = pci_bus_to_host(dev->bus);
d3afa58c Michal Simek 2010-01-18 567 int i;
d3afa58c Michal Simek 2010-01-18 568
d3afa58c Michal Simek 2010-01-18 569 if (!hose) {
6bd55f0b Michal Simek 2012-12-27 570 pr_err("No host bridge for PCI dev %s !\n",
d3afa58c Michal Simek 2010-01-18 571 pci_name(dev));
d3afa58c Michal Simek 2010-01-18 572 return;
d3afa58c Michal Simek 2010-01-18 573 }
d3afa58c Michal Simek 2010-01-18 574 for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
d3afa58c Michal Simek 2010-01-18 575 struct resource *res = dev->resource + i;
d3afa58c Michal Simek 2010-01-18 576 if (!res->flags)
d3afa58c Michal Simek 2010-01-18 577 continue;
e5b36841 Bjorn Helgaas 2012-02-23 578 if (res->start == 0) {
6bd55f0b Michal Simek 2012-12-27 579 pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]",
d3afa58c Michal Simek 2010-01-18 580 pci_name(dev), i,
d3afa58c Michal Simek 2010-01-18 581 (unsigned long long)res->start,
d3afa58c Michal Simek 2010-01-18 582 (unsigned long long)res->end,
d3afa58c Michal Simek 2010-01-18 583 (unsigned int)res->flags);
6bd55f0b Michal Simek 2012-12-27 584 pr_debug("is unassigned\n");
d3afa58c Michal Simek 2010-01-18 585 res->end -= res->start;
d3afa58c Michal Simek 2010-01-18 586 res->start = 0;
d3afa58c Michal Simek 2010-01-18 587 res->flags |= IORESOURCE_UNSET;
d3afa58c Michal Simek 2010-01-18 588 continue;
d3afa58c Michal Simek 2010-01-18 589 }
d3afa58c Michal Simek 2010-01-18 590
aa23bdc0 Bjorn Helgaas 2012-02-23 591 pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]\n",
d3afa58c Michal Simek 2010-01-18 592 pci_name(dev), i,
6bd55f0b Michal Simek 2012-12-27 593 (unsigned long long)res->start,
d3afa58c Michal Simek 2010-01-18 594 (unsigned long long)res->end,
d3afa58c Michal Simek 2010-01-18 595 (unsigned int)res->flags);
d3afa58c Michal Simek 2010-01-18 596 }
d3afa58c Michal Simek 2010-01-18 597 }
d3afa58c Michal Simek 2010-01-18 598 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_resources);
d3afa58c Michal Simek 2010-01-18 599
d3afa58c Michal Simek 2010-01-18 600 /*
d3afa58c Michal Simek 2010-01-18 601 * We need to avoid collisions with `mirrored' VGA ports
d3afa58c Michal Simek 2010-01-18 602 * and other strange ISA hardware, so we always want the
d3afa58c Michal Simek 2010-01-18 603 * addresses to be allocated in the 0x000-0x0ff region
d3afa58c Michal Simek 2010-01-18 604 * modulo 0x400.
d3afa58c Michal Simek 2010-01-18 605 *
d3afa58c Michal Simek 2010-01-18 606 * Why? Because some silly external IO cards only decode
d3afa58c Michal Simek 2010-01-18 607 * the low 10 bits of the IO address. The 0x00-0xff region
d3afa58c Michal Simek 2010-01-18 608 * is reserved for motherboard devices that decode all 16
d3afa58c Michal Simek 2010-01-18 609 * bits, so it's ok to allocate at, say, 0x2800-0x28ff,
d3afa58c Michal Simek 2010-01-18 610 * but we want to try to avoid allocating at 0x2900-0x2bff
d3afa58c Michal Simek 2010-01-18 611 * which might have be mirrored at 0x0100-0x03ff..
d3afa58c Michal Simek 2010-01-18 612 */
01cf9d52 Bharat Kumar Gogada 2016-02-11 613 int pcibios_add_device(struct pci_dev *dev)
01cf9d52 Bharat Kumar Gogada 2016-02-11 614 {
01cf9d52 Bharat Kumar Gogada 2016-02-11 615 dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
c86fac43 Michal Simek 2010-04-16 616
01cf9d52 Bharat Kumar Gogada 2016-02-11 617 return 0;
d3afa58c Michal Simek 2010-01-18 618 }
01cf9d52 Bharat Kumar Gogada 2016-02-11 619 EXPORT_SYMBOL(pcibios_add_device);
d3afa58c Michal Simek 2010-01-18 620
d3afa58c Michal Simek 2010-01-18 621 /*
d3afa58c Michal Simek 2010-01-18 622 * Reparent resource children of pr that conflict with res
d3afa58c Michal Simek 2010-01-18 623 * under res, and make res replace those children.
d3afa58c Michal Simek 2010-01-18 624 */
d3afa58c Michal Simek 2010-01-18 625 static int __init reparent_resources(struct resource *parent,
d3afa58c Michal Simek 2010-01-18 626 struct resource *res)
d3afa58c Michal Simek 2010-01-18 627 {
d3afa58c Michal Simek 2010-01-18 628 struct resource *p, **pp;
d3afa58c Michal Simek 2010-01-18 629 struct resource **firstpp = NULL;
d3afa58c Michal Simek 2010-01-18 630
d3afa58c Michal Simek 2010-01-18 @631 for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
d3afa58c Michal Simek 2010-01-18 632 if (p->end < res->start)
d3afa58c Michal Simek 2010-01-18 633 continue;
d3afa58c Michal Simek 2010-01-18 634 if (res->end < p->start)
d3afa58c Michal Simek 2010-01-18 635 break;
d3afa58c Michal Simek 2010-01-18 636 if (p->start < res->start || p->end > res->end)
d3afa58c Michal Simek 2010-01-18 637 return -1; /* not completely contained */
d3afa58c Michal Simek 2010-01-18 638 if (firstpp == NULL)
d3afa58c Michal Simek 2010-01-18 639 firstpp = pp;
d3afa58c Michal Simek 2010-01-18 640 }
d3afa58c Michal Simek 2010-01-18 641 if (firstpp == NULL)
d3afa58c Michal Simek 2010-01-18 642 return -1; /* didn't find any conflicting entries? */
d3afa58c Michal Simek 2010-01-18 643 res->parent = parent;
d3afa58c Michal Simek 2010-01-18 @644 res->child = *firstpp;
d3afa58c Michal Simek 2010-01-18 645 res->sibling = *pp;
d3afa58c Michal Simek 2010-01-18 646 *firstpp = res;
d3afa58c Michal Simek 2010-01-18 647 *pp = NULL;
d3afa58c Michal Simek 2010-01-18 @648 for (p = res->child; p != NULL; p = p->sibling) {
d3afa58c Michal Simek 2010-01-18 649 p->parent = res;
d3afa58c Michal Simek 2010-01-18 650 pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
d3afa58c Michal Simek 2010-01-18 651 p->name,
d3afa58c Michal Simek 2010-01-18 652 (unsigned long long)p->start,
d3afa58c Michal Simek 2010-01-18 653 (unsigned long long)p->end, res->name);
d3afa58c Michal Simek 2010-01-18 654 }
d3afa58c Michal Simek 2010-01-18 655 return 0;
d3afa58c Michal Simek 2010-01-18 656 }
d3afa58c Michal Simek 2010-01-18 657
:::::: The code at line 536 was first introduced by commit
:::::: 70dcd942dc4af3cc6c3dcc2ba499cd841c7f65a7 microblaze: Fix IO space breakage after of_pci_range_to_resource() change
:::::: TO: Michal Simek <[email protected]>
:::::: CC: Michal Simek <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Baoquan,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc2 next-20180424]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
config: xtensa-common_defconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
In file included from arch/xtensa/lib/pci-auto.c:22:0:
arch/xtensa/include/asm/pci-bridge.h: In function 'pcibios_init_resource':
>> arch/xtensa/include/asm/pci-bridge.h:74:15: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
res->sibling = NULL;
^
arch/xtensa/include/asm/pci-bridge.h:75:13: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
res->child = NULL;
^
vim +74 arch/xtensa/include/asm/pci-bridge.h
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 65
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 66 static inline void pcibios_init_resource(struct resource *res,
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 67 unsigned long start, unsigned long end, int flags, char *name)
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 68 {
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 69 res->start = start;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 70 res->end = end;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 71 res->flags = flags;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 72 res->name = name;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 73 res->parent = NULL;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 @74 res->sibling = NULL;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 75 res->child = NULL;
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 76 }
9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 77
:::::: The code at line 74 was first introduced by commit
:::::: 9a8fd5589902153a134111ed7a40f9cca1f83254 [PATCH] xtensa: Architecture support for Tensilica Xtensa Part 6
:::::: TO: Chris Zankel <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Sorry to all reviewers, I had a redhat urgent issue, this patchset
discussing is delayed.
On 04/19/18 at 12:07pm, Borislav Petkov wrote:
> On Thu, Apr 19, 2018 at 08:18:47AM +0800, Baoquan He wrote:
> > This function, being a variant of walk_system_ram_res() introduced in
> > commit 8c86e70acead ("resource: provide new functions to walk through
> > resources"), walks through a list of all the resources of System RAM
> > in reversed order, i.e., from higher to lower.
> >
> > It will be used in kexec_file code.
>
> Of course, what is missing is the big *WHY* you need this to happen this
> way...
Sorry for that, I just ran scripts/get_maintainer.pl to get expert's
name and added them into each patch. The reason this change is made is
in patch 3/3. Test robot reported a code bug on the latest kernel, will
repost and CC everyone in all patches.
Rob Herring asked the same question in v2, I explained to him. The
discussion can be found here:
https://lkml.org/lkml/2018/4/10/484
>
> > /*
> > + * This function, being a variant of walk_system_ram_res(), calls the @func
> > + * callback against all memory ranges of type System RAM which are marked as
> > + * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
> > + * higher to lower.
> > + */
> > +int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> > + int (*func)(struct resource *, void *))
> > +{
> > + unsigned long flags;
> > + struct resource *res;
> > + int ret = -1;
> > +
> > + flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> > +
> > + read_lock(&resource_lock);
> > + list_for_each_entry_reverse(res, &iomem_resource.child, sibling) {
>
> ... and the other thing that I'm not clear on is why are you
> slapping this function just like that instead of extending
> __walk_iomem_res_desc() to do reverse direction too?
Yes, I planned to do as you said when started to write code. After investigation,
I changed to use the way of this patch. Because __walk_iomem_res_desc()
provides two interfaces by calling find_next_iomem_res(). If
'first_level_children_only' is true, it only iterates system RAM
resource; if 'first_level_children_only' is false, it does depth first
iteration in iomem_resource. The 1st interface is only used by
walk_system_ram_res() and walk_mem_res(). While the 2nd interface need
call find_next_iomem_res() which resorts to next_resource().
next_resource() will check 'sibling_only' to decide if iterate
iomem_resource's children, or walk over all resources of iomem_resource
with depth first. For walk_system_ram_res_rev(), we only need to iterate
iomem_resource's children, and seems no one has requirement to
iterate all iomem_resource's children and grand children
revresedly. For simplifying code implementation, I just did as in this
patch, surely, if any suggestion or scenario given about the reversed
iteration of all resources, I can change to add below functions, and
based on them to implement walk_system_ram_res_rev().
walk_system_ram_res_rev
->__walk_iomem_res_desc_rev
->find_next_iomem_res_rev
->next_resource_rev
Thanks
Baoquan
>
> SUSE Linux GmbH, GF: Felix Imend?rffer, Jane Smithard, Graham Norton, HRB 21284 (AG N?rnberg)
> --
On Thu, Apr 26, 2018 at 04:56:49PM +0800, Baoquan He wrote:
> Sorry for that, I just ran scripts/get_maintainer.pl to get expert's
> name and added them into each patch. The reason this change is made is
> in patch 3/3. Test robot reported a code bug on the latest kernel, will
> repost and CC everyone in all patches.
>
>
> Rob Herring asked the same question in v2, I explained to him. The
> discussion can be found here:
> https://lkml.org/lkml/2018/4/10/484
... and when I open that link, the first paragraph says:
"This is the explanation I made when Andrew helped to review the v1 post:
https://lkml.org/lkml/2018/3/23/78"
Do you see the absurdity of the link chasing of your explanation?!
Instead, the explanation *WHY* should be in the commit message of the
patch - not in mail replies when people ask you about it.
Also, do not use lkml.org when referencing a mail on lkml but
use the Message-ID of the header. We have a proper redirector at
https://lkml.kernel.org/r/<Message-ID>
Now lemme read the reason finally...
"We need unify these two interfaces on behaviour since they are the same
on essense from the users' point of view... "
That's not a good enough reason for me to cause code churn. If the only
reason is: because the one does it top-down and the other bottom-up, I'm
not convinced.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
On 04/26/18 at 01:09pm, Borislav Petkov wrote:
> On Thu, Apr 26, 2018 at 04:56:49PM +0800, Baoquan He wrote:
> > Sorry for that, I just ran scripts/get_maintainer.pl to get expert's
> > name and added them into each patch. The reason this change is made is
> > in patch 3/3. Test robot reported a code bug on the latest kernel, will
> > repost and CC everyone in all patches.
> >
> >
> > Rob Herring asked the same question in v2, I explained to him. The
> > discussion can be found here:
> > https://lkml.org/lkml/2018/4/10/484
>
> ... and when I open that link, the first paragraph says:
>
> "This is the explanation I made when Andrew helped to review the v1 post:
> https://lkml.org/lkml/2018/3/23/78"
>
> Do you see the absurdity of the link chasing of your explanation?!
>
> Instead, the explanation *WHY* should be in the commit message of the
> patch - not in mail replies when people ask you about it.
It is in patch 3/3 and cover-letter. I often received one or several
patches of a big patchset. As I said, I used ./scripts/get_maintainer.pl
to each patch's reviewers, and will add all people in CC list.
>
> Also, do not use lkml.org when referencing a mail on lkml but
> use the Message-ID of the header. We have a proper redirector at
> https://lkml.kernel.org/r/<Message-ID>
I noticed maintainers merged patches with this Message-ID, could you
tell how to get this Message-ID?
>
> Now lemme read the reason finally...
>
> "We need unify these two interfaces on behaviour since they are the same
> on essense from the users' point of view... "
>
> That's not a good enough reason for me to cause code churn. If the only
> reason is: because the one does it top-down and the other bottom-up, I'm
> not convinced.
We have been using this top-down way for x86 64 since earlier 2013 in
the KEXEC loading with command:
'kexec -l bzImage --initrd initramfs-xx.img'
Two years later, Vivek added a new KEXEC_FILE loading, and most of job
is done in kernel because we need do kernel image sinature verification.
The KEXEC_FILE loading mostly is used in secure boot machine. Although
more and more users like to take secure boot machine, system using KEXEC
loading are still much more. I ever asked our kernel QE who takes care
of kernel and kdump testing, they said in their testing systems, the
proportion of legacy system vs system with secure boot is 10:1.
Before long I pinged hpa, Vivek and Yinghai who discussed and wrote the
code for KEXEC loading in x86 64, asked them about the reaon they chose
top-down, no reply from them. I guess they probably worried that low
memory under 4G are mostly reserved by system or firmware, even though
kexec/kdump have tried very hard to avoid each of them if recognized,
any missed one will cause loading and booting failure. Just a guess.
In fact, there's no spec or doc to tell which one is right or not.
So I finally decide to take the same top-down way for KEXEC_FILE loading
because on statistics KEXEC loading is used longer and wider.
This is not a thing that one is top down, the other is bottom up. For
us, they might be so different on details of code, for customers, they
just think them as a same thing. They may say I just get a new machine,
and still do kexec loading, why these top-down, bottom-up things come
up.
And this is not causing code churn. You can see that by replacing
pointer operation with list_head, code in kernel/resource.c related to
child list iteration is much easier to read, e.g __request_resource(),
__release_resource(). As Rob said in v2 reviewing, they have put
the same replacing in DT struct device_node in todo list, and ACPI tree
function in drivers/acpi/acpica/pstree.c too. I think this is why Andrew
asked to try this way and see what the patches looks like.
Thanks
Baoquan
On Thu, Apr 26, 2018 at 09:22:04PM +0800, Baoquan He wrote:
> I noticed maintainers merged patches with this Message-ID, could you
> tell how to get this Message-ID?
https://en.wikipedia.org/wiki/Message-ID
> This is not a thing that one is top down, the other is bottom up. For
> us, they might be so different on details of code, for customers, they
> just think them as a same thing. They may say I just get a new machine,
> and still do kexec loading, why these top-down, bottom-up things come
> up.
So if I read the above correctly, it doesn't matter whether top-down or
bottom-up.
> And this is not causing code churn. You can see that by replacing
> pointer operation with list_head, code in kernel/resource.c related to
> child list iteration is much easier to read,
Now *this* is starting to sound like some reason "why". If it is better
readability, then say so in the commit message.
It still doesn't justify adding walk_system_ram_res_rev() though.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
On 05/04/18 at 12:16pm, Borislav Petkov wrote:
> On Thu, Apr 26, 2018 at 09:22:04PM +0800, Baoquan He wrote:
> > I noticed maintainers merged patches with this Message-ID, could you
> > tell how to get this Message-ID?
>
> https://en.wikipedia.org/wiki/Message-ID
thx.
>
> > This is not a thing that one is top down, the other is bottom up. For
> > us, they might be so different on details of code, for customers, they
> > just think them as a same thing. They may say I just get a new machine,
> > and still do kexec loading, why these top-down, bottom-up things come
> > up.
>
> So if I read the above correctly, it doesn't matter whether top-down or
> bottom-up.
Well, kexec_file loading is added because kernel signature verification
has to be done in kernel space, it's implemented by porting code of user
space utility kexc-tools to kernel. In essence both the kexec loading
and kexec_file loading are the same, they are all used to load kernel
for the furture kexec/kdump jumping. Now they have different kernel
loading position, could you tell how you get the conclusion that it
doesn't matter? And what's your theory basis?
And KASLR also changes kernel's position, I don't think users have to
know all these details to exclude unnecessary noise.
>
> > And this is not causing code churn. You can see that by replacing
> > pointer operation with list_head, code in kernel/resource.c related to
> > child list iteration is much easier to read,
>
> Now *this* is starting to sound like some reason "why". If it is better
> readability, then say so in the commit message.
Yes, I can add it to log in next post. While the main reason I made
this is for the top down searching of system RAM in kexec_file
loading.
On 04/26/18 at 11:23am, kbuild test robot wrote:
> Hi Baoquan,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc2 next-20180424]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
> config: xtensa-common_defconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.2.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=xtensa
>
> All errors (new ones prefixed by >>):
>
> In file included from arch/xtensa/lib/pci-auto.c:22:0:
> arch/xtensa/include/asm/pci-bridge.h: In function 'pcibios_init_resource':
> >> arch/xtensa/include/asm/pci-bridge.h:74:15: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
> res->sibling = NULL;
> ^
> arch/xtensa/include/asm/pci-bridge.h:75:13: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
> res->child = NULL;
Thanks, below fix can fix it.
diff --git a/arch/xtensa/include/asm/pci-bridge.h b/arch/xtensa/include/asm/pci-bridge.h
index 0b68c76ec1e6..f487b06817df 100644
--- a/arch/xtensa/include/asm/pci-bridge.h
+++ b/arch/xtensa/include/asm/pci-bridge.h
@@ -71,8 +71,8 @@ static inline void pcibios_init_resource(struct resource *res,
res->flags = flags;
res->name = name;
res->parent = NULL;
- res->sibling = NULL;
- res->child = NULL;
+ INIT_LIST_HEAD(&res->child);
+ INIT_LIST_HEAD(&res->sibling);
}
> ^
>
> vim +74 arch/xtensa/include/asm/pci-bridge.h
>
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 65
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 66 static inline void pcibios_init_resource(struct resource *res,
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 67 unsigned long start, unsigned long end, int flags, char *name)
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 68 {
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 69 res->start = start;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 70 res->end = end;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 71 res->flags = flags;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 72 res->name = name;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 73 res->parent = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 @74 res->sibling = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 75 res->child = NULL;
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 76 }
> 9a8fd558 include/asm-xtensa/pci-bridge.h Chris Zankel 2005-06-23 77
>
> :::::: The code at line 74 was first introduced by commit
> :::::: 9a8fd5589902153a134111ed7a40f9cca1f83254 [PATCH] xtensa: Architecture support for Tensilica Xtensa Part 6
>
> :::::: TO: Chris Zankel <[email protected]>
> :::::: CC: Linus Torvalds <[email protected]>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 04/26/18 at 11:01am, kbuild test robot wrote:
> Hi Baoquan,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc2 next-20180424]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180419-223752
> config: microblaze-mmu_defconfig (attached as .config)
> compiler: microblaze-linux-gcc (GCC) 7.2.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=microblaze
>
> All errors (new ones prefixed by >>):
Thanks, below patch can fix it. Will repost including the fix.
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 161f9758c631..56d189cb4be4 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -533,7 +533,9 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
res->flags = range.flags;
res->start = range.cpu_addr;
res->end = range.cpu_addr + range.size - 1;
- res->parent = res->child = res->sibling = NULL;
+ res->parent = NULL;
+ INIT_LIST_HEAD(&res->child);
+ INIT_LIST_HEAD(&res->sibling);
}
}
@@ -625,28 +627,31 @@ EXPORT_SYMBOL(pcibios_add_device);
static int __init reparent_resources(struct resource *parent,
struct resource *res)
{
- struct resource *p, **pp;
- struct resource **firstpp = NULL;
+ struct resource *p, *first = NULL;
- for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
+ list_for_each_entry(p, &parent->child, sibling) {
if (p->end < res->start)
continue;
if (res->end < p->start)
break;
if (p->start < res->start || p->end > res->end)
return -1; /* not completely contained */
- if (firstpp == NULL)
- firstpp = pp;
+ if (first == NULL)
+ first = p;
}
- if (firstpp == NULL)
+ if (first == NULL)
return -1; /* didn't find any conflicting entries? */
res->parent = parent;
- res->child = *firstpp;
- res->sibling = *pp;
- *firstpp = res;
- *pp = NULL;
- for (p = res->child; p != NULL; p = p->sibling) {
- p->parent = res;
+ list_add(&res->sibling, &p->sibling.prev);
+ INIT_LIST_HEAD(&res->child);
+
+ /*
+ * From first to p's previous sibling, they all fall into
+ * res's region, change them as res's children.
+ */
+ list_cut_position(&res->child, first->sibling.prev, res->sibling.prev);
+ list_for_each_entry(p, &new->child, sibling) {
+ p->parent = new;
pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
p->name,
(unsigned long long)p->start,
>
> arch/microblaze/pci/pci-common.c: In function 'pci_process_bridge_OF_ranges':
> >> arch/microblaze/pci/pci-common.c:536:44: error: incompatible types when assigning to type 'struct list_head' from type 'void *'
> res->parent = res->child = res->sibling = NULL;
> ^
> arch/microblaze/pci/pci-common.c: In function 'reparent_resources':
> >> arch/microblaze/pci/pci-common.c:631:10: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
> for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> ^
> arch/microblaze/pci/pci-common.c:631:50: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
> for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> ^
> >> arch/microblaze/pci/pci-common.c:644:13: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
> res->child = *firstpp;
> ^
> arch/microblaze/pci/pci-common.c:645:15: error: incompatible types when assigning to type 'struct list_head' from type 'struct resource *'
> res->sibling = *pp;
> ^
> >> arch/microblaze/pci/pci-common.c:648:9: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
> for (p = res->child; p != NULL; p = p->sibling) {
> ^
> arch/microblaze/pci/pci-common.c:648:36: error: incompatible types when assigning to type 'struct resource *' from type 'struct list_head'
> for (p = res->child; p != NULL; p = p->sibling) {
> ^
> cc1: some warnings being treated as errors
>
> vim +536 arch/microblaze/pci/pci-common.c
>
> d3afa58c Michal Simek 2010-01-18 387
> d3afa58c Michal Simek 2010-01-18 388 /**
> d3afa58c Michal Simek 2010-01-18 389 * pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
> d3afa58c Michal Simek 2010-01-18 390 * @hose: newly allocated pci_controller to be setup
> d3afa58c Michal Simek 2010-01-18 391 * @dev: device node of the host bridge
> d3afa58c Michal Simek 2010-01-18 392 * @primary: set if primary bus (32 bits only, soon to be deprecated)
> d3afa58c Michal Simek 2010-01-18 393 *
> d3afa58c Michal Simek 2010-01-18 394 * This function will parse the "ranges" property of a PCI host bridge device
> d3afa58c Michal Simek 2010-01-18 395 * node and setup the resource mapping of a pci controller based on its
> d3afa58c Michal Simek 2010-01-18 396 * content.
> d3afa58c Michal Simek 2010-01-18 397 *
> d3afa58c Michal Simek 2010-01-18 398 * Life would be boring if it wasn't for a few issues that we have to deal
> d3afa58c Michal Simek 2010-01-18 399 * with here:
> d3afa58c Michal Simek 2010-01-18 400 *
> d3afa58c Michal Simek 2010-01-18 401 * - We can only cope with one IO space range and up to 3 Memory space
> d3afa58c Michal Simek 2010-01-18 402 * ranges. However, some machines (thanks Apple !) tend to split their
> d3afa58c Michal Simek 2010-01-18 403 * space into lots of small contiguous ranges. So we have to coalesce.
> d3afa58c Michal Simek 2010-01-18 404 *
> d3afa58c Michal Simek 2010-01-18 405 * - We can only cope with all memory ranges having the same offset
> d3afa58c Michal Simek 2010-01-18 406 * between CPU addresses and PCI addresses. Unfortunately, some bridges
> d3afa58c Michal Simek 2010-01-18 407 * are setup for a large 1:1 mapping along with a small "window" which
> d3afa58c Michal Simek 2010-01-18 408 * maps PCI address 0 to some arbitrary high address of the CPU space in
> d3afa58c Michal Simek 2010-01-18 409 * order to give access to the ISA memory hole.
> d3afa58c Michal Simek 2010-01-18 410 * The way out of here that I've chosen for now is to always set the
> d3afa58c Michal Simek 2010-01-18 411 * offset based on the first resource found, then override it if we
> d3afa58c Michal Simek 2010-01-18 412 * have a different offset and the previous was set by an ISA hole.
> d3afa58c Michal Simek 2010-01-18 413 *
> d3afa58c Michal Simek 2010-01-18 414 * - Some busses have IO space not starting at 0, which causes trouble with
> d3afa58c Michal Simek 2010-01-18 415 * the way we do our IO resource renumbering. The code somewhat deals with
> d3afa58c Michal Simek 2010-01-18 416 * it for 64 bits but I would expect problems on 32 bits.
> d3afa58c Michal Simek 2010-01-18 417 *
> d3afa58c Michal Simek 2010-01-18 418 * - Some 32 bits platforms such as 4xx can have physical space larger than
> d3afa58c Michal Simek 2010-01-18 419 * 32 bits so we need to use 64 bits values for the parsing
> d3afa58c Michal Simek 2010-01-18 420 */
> b881bc46 Greg Kroah-Hartman 2012-12-21 421 void pci_process_bridge_OF_ranges(struct pci_controller *hose,
> b881bc46 Greg Kroah-Hartman 2012-12-21 422 struct device_node *dev, int primary)
> d3afa58c Michal Simek 2010-01-18 423 {
> d3afa58c Michal Simek 2010-01-18 424 int memno = 0, isa_hole = -1;
> d3afa58c Michal Simek 2010-01-18 425 unsigned long long isa_mb = 0;
> d3afa58c Michal Simek 2010-01-18 426 struct resource *res;
> 4f7b6de4 Andrew Murray 2013-07-27 427 struct of_pci_range range;
> 4f7b6de4 Andrew Murray 2013-07-27 428 struct of_pci_range_parser parser;
> d3afa58c Michal Simek 2010-01-18 429
> f2b8ae0e Rob Herring 2017-06-06 430 pr_info("PCI host bridge %pOF %s ranges:\n",
> f2b8ae0e Rob Herring 2017-06-06 431 dev, primary ? "(primary)" : "");
> d3afa58c Michal Simek 2010-01-18 432
> 4f7b6de4 Andrew Murray 2013-07-27 433 /* Check for ranges property */
> 4f7b6de4 Andrew Murray 2013-07-27 434 if (of_pci_range_parser_init(&parser, dev))
> d3afa58c Michal Simek 2010-01-18 435 return;
> d3afa58c Michal Simek 2010-01-18 436
> d3afa58c Michal Simek 2010-01-18 437 pr_debug("Parsing ranges property...\n");
> 4f7b6de4 Andrew Murray 2013-07-27 438 for_each_of_pci_range(&parser, &range) {
> d3afa58c Michal Simek 2010-01-18 439 /* Read next ranges element */
> 6bd55f0b Michal Simek 2012-12-27 440 pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> 4f7b6de4 Andrew Murray 2013-07-27 441 range.pci_space, range.pci_addr);
> 6bd55f0b Michal Simek 2012-12-27 442 pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> 4f7b6de4 Andrew Murray 2013-07-27 443 range.cpu_addr, range.size);
> d3afa58c Michal Simek 2010-01-18 444
> d3afa58c Michal Simek 2010-01-18 445 /* If we failed translation or got a zero-sized region
> d3afa58c Michal Simek 2010-01-18 446 * (some FW try to feed us with non sensical zero sized regions
> d3afa58c Michal Simek 2010-01-18 447 * such as power3 which look like some kind of attempt
> d3afa58c Michal Simek 2010-01-18 448 * at exposing the VGA memory hole)
> d3afa58c Michal Simek 2010-01-18 449 */
> 4f7b6de4 Andrew Murray 2013-07-27 450 if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> d3afa58c Michal Simek 2010-01-18 451 continue;
> d3afa58c Michal Simek 2010-01-18 452
> d3afa58c Michal Simek 2010-01-18 453 /* Act based on address space type */
> d3afa58c Michal Simek 2010-01-18 454 res = NULL;
> 4f7b6de4 Andrew Murray 2013-07-27 455 switch (range.flags & IORESOURCE_TYPE_BITS) {
> 4f7b6de4 Andrew Murray 2013-07-27 456 case IORESOURCE_IO:
> 6bd55f0b Michal Simek 2012-12-27 457 pr_info(" IO 0x%016llx..0x%016llx -> 0x%016llx\n",
> 4f7b6de4 Andrew Murray 2013-07-27 458 range.cpu_addr, range.cpu_addr + range.size - 1,
> 4f7b6de4 Andrew Murray 2013-07-27 459 range.pci_addr);
> d3afa58c Michal Simek 2010-01-18 460
> d3afa58c Michal Simek 2010-01-18 461 /* We support only one IO range */
> d3afa58c Michal Simek 2010-01-18 462 if (hose->pci_io_size) {
> 6bd55f0b Michal Simek 2012-12-27 463 pr_info(" \\--> Skipped (too many) !\n");
> d3afa58c Michal Simek 2010-01-18 464 continue;
> d3afa58c Michal Simek 2010-01-18 465 }
> d3afa58c Michal Simek 2010-01-18 466 /* On 32 bits, limit I/O space to 16MB */
> 4f7b6de4 Andrew Murray 2013-07-27 467 if (range.size > 0x01000000)
> 4f7b6de4 Andrew Murray 2013-07-27 468 range.size = 0x01000000;
> d3afa58c Michal Simek 2010-01-18 469
> d3afa58c Michal Simek 2010-01-18 470 /* 32 bits needs to map IOs here */
> 4f7b6de4 Andrew Murray 2013-07-27 471 hose->io_base_virt = ioremap(range.cpu_addr,
> 4f7b6de4 Andrew Murray 2013-07-27 472 range.size);
> d3afa58c Michal Simek 2010-01-18 473
> d3afa58c Michal Simek 2010-01-18 474 /* Expect trouble if pci_addr is not 0 */
> d3afa58c Michal Simek 2010-01-18 475 if (primary)
> d3afa58c Michal Simek 2010-01-18 476 isa_io_base =
> d3afa58c Michal Simek 2010-01-18 477 (unsigned long)hose->io_base_virt;
> d3afa58c Michal Simek 2010-01-18 478 /* pci_io_size and io_base_phys always represent IO
> d3afa58c Michal Simek 2010-01-18 479 * space starting at 0 so we factor in pci_addr
> d3afa58c Michal Simek 2010-01-18 480 */
> 4f7b6de4 Andrew Murray 2013-07-27 481 hose->pci_io_size = range.pci_addr + range.size;
> 4f7b6de4 Andrew Murray 2013-07-27 482 hose->io_base_phys = range.cpu_addr - range.pci_addr;
> d3afa58c Michal Simek 2010-01-18 483
> d3afa58c Michal Simek 2010-01-18 484 /* Build resource */
> d3afa58c Michal Simek 2010-01-18 485 res = &hose->io_resource;
> 4f7b6de4 Andrew Murray 2013-07-27 486 range.cpu_addr = range.pci_addr;
> 4f7b6de4 Andrew Murray 2013-07-27 487
> d3afa58c Michal Simek 2010-01-18 488 break;
> 4f7b6de4 Andrew Murray 2013-07-27 489 case IORESOURCE_MEM:
> 6bd55f0b Michal Simek 2012-12-27 490 pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx %s\n",
> 4f7b6de4 Andrew Murray 2013-07-27 491 range.cpu_addr, range.cpu_addr + range.size - 1,
> 4f7b6de4 Andrew Murray 2013-07-27 492 range.pci_addr,
> 4f7b6de4 Andrew Murray 2013-07-27 493 (range.pci_space & 0x40000000) ?
> 4f7b6de4 Andrew Murray 2013-07-27 494 "Prefetch" : "");
> d3afa58c Michal Simek 2010-01-18 495
> d3afa58c Michal Simek 2010-01-18 496 /* We support only 3 memory ranges */
> d3afa58c Michal Simek 2010-01-18 497 if (memno >= 3) {
> 6bd55f0b Michal Simek 2012-12-27 498 pr_info(" \\--> Skipped (too many) !\n");
> d3afa58c Michal Simek 2010-01-18 499 continue;
> d3afa58c Michal Simek 2010-01-18 500 }
> d3afa58c Michal Simek 2010-01-18 501 /* Handles ISA memory hole space here */
> 4f7b6de4 Andrew Murray 2013-07-27 502 if (range.pci_addr == 0) {
> 4f7b6de4 Andrew Murray 2013-07-27 503 isa_mb = range.cpu_addr;
> d3afa58c Michal Simek 2010-01-18 504 isa_hole = memno;
> d3afa58c Michal Simek 2010-01-18 505 if (primary || isa_mem_base == 0)
> 4f7b6de4 Andrew Murray 2013-07-27 506 isa_mem_base = range.cpu_addr;
> 4f7b6de4 Andrew Murray 2013-07-27 507 hose->isa_mem_phys = range.cpu_addr;
> 4f7b6de4 Andrew Murray 2013-07-27 508 hose->isa_mem_size = range.size;
> d3afa58c Michal Simek 2010-01-18 509 }
> d3afa58c Michal Simek 2010-01-18 510
> d3afa58c Michal Simek 2010-01-18 511 /* We get the PCI/Mem offset from the first range or
> d3afa58c Michal Simek 2010-01-18 512 * the, current one if the offset came from an ISA
> d3afa58c Michal Simek 2010-01-18 513 * hole. If they don't match, bugger.
> d3afa58c Michal Simek 2010-01-18 514 */
> d3afa58c Michal Simek 2010-01-18 515 if (memno == 0 ||
> 4f7b6de4 Andrew Murray 2013-07-27 516 (isa_hole >= 0 && range.pci_addr != 0 &&
> d3afa58c Michal Simek 2010-01-18 517 hose->pci_mem_offset == isa_mb))
> 4f7b6de4 Andrew Murray 2013-07-27 518 hose->pci_mem_offset = range.cpu_addr -
> 4f7b6de4 Andrew Murray 2013-07-27 519 range.pci_addr;
> 4f7b6de4 Andrew Murray 2013-07-27 520 else if (range.pci_addr != 0 &&
> 4f7b6de4 Andrew Murray 2013-07-27 521 hose->pci_mem_offset != range.cpu_addr -
> 4f7b6de4 Andrew Murray 2013-07-27 522 range.pci_addr) {
> 6bd55f0b Michal Simek 2012-12-27 523 pr_info(" \\--> Skipped (offset mismatch) !\n");
> d3afa58c Michal Simek 2010-01-18 524 continue;
> d3afa58c Michal Simek 2010-01-18 525 }
> d3afa58c Michal Simek 2010-01-18 526
> d3afa58c Michal Simek 2010-01-18 527 /* Build resource */
> d3afa58c Michal Simek 2010-01-18 528 res = &hose->mem_resources[memno++];
> d3afa58c Michal Simek 2010-01-18 529 break;
> d3afa58c Michal Simek 2010-01-18 530 }
> 70dcd942 Michal Simek 2014-10-27 531 if (res != NULL) {
> 70dcd942 Michal Simek 2014-10-27 532 res->name = dev->full_name;
> 70dcd942 Michal Simek 2014-10-27 533 res->flags = range.flags;
> 70dcd942 Michal Simek 2014-10-27 534 res->start = range.cpu_addr;
> 70dcd942 Michal Simek 2014-10-27 535 res->end = range.cpu_addr + range.size - 1;
> 70dcd942 Michal Simek 2014-10-27 @536 res->parent = res->child = res->sibling = NULL;
> 70dcd942 Michal Simek 2014-10-27 537 }
> d3afa58c Michal Simek 2010-01-18 538 }
> d3afa58c Michal Simek 2010-01-18 539
> d3afa58c Michal Simek 2010-01-18 540 /* If there's an ISA hole and the pci_mem_offset is -not- matching
> d3afa58c Michal Simek 2010-01-18 541 * the ISA hole offset, then we need to remove the ISA hole from
> d3afa58c Michal Simek 2010-01-18 542 * the resource list for that brige
> d3afa58c Michal Simek 2010-01-18 543 */
> d3afa58c Michal Simek 2010-01-18 544 if (isa_hole >= 0 && hose->pci_mem_offset != isa_mb) {
> d3afa58c Michal Simek 2010-01-18 545 unsigned int next = isa_hole + 1;
> 6bd55f0b Michal Simek 2012-12-27 546 pr_info(" Removing ISA hole at 0x%016llx\n", isa_mb);
> d3afa58c Michal Simek 2010-01-18 547 if (next < memno)
> d3afa58c Michal Simek 2010-01-18 548 memmove(&hose->mem_resources[isa_hole],
> d3afa58c Michal Simek 2010-01-18 549 &hose->mem_resources[next],
> d3afa58c Michal Simek 2010-01-18 550 sizeof(struct resource) * (memno - next));
> d3afa58c Michal Simek 2010-01-18 551 hose->mem_resources[--memno].flags = 0;
> d3afa58c Michal Simek 2010-01-18 552 }
> d3afa58c Michal Simek 2010-01-18 553 }
> d3afa58c Michal Simek 2010-01-18 554
> 9413d968 Bharat Kumar Gogada 2016-09-01 555 /* Display the domain number in /proc */
> d3afa58c Michal Simek 2010-01-18 556 int pci_proc_domain(struct pci_bus *bus)
> d3afa58c Michal Simek 2010-01-18 557 {
> 9413d968 Bharat Kumar Gogada 2016-09-01 558 return pci_domain_nr(bus);
> d3afa58c Michal Simek 2010-01-18 559 }
> d3afa58c Michal Simek 2010-01-18 560
> d3afa58c Michal Simek 2010-01-18 561 /* This header fixup will do the resource fixup for all devices as they are
> d3afa58c Michal Simek 2010-01-18 562 * probed, but not for bridge ranges
> d3afa58c Michal Simek 2010-01-18 563 */
> b881bc46 Greg Kroah-Hartman 2012-12-21 564 static void pcibios_fixup_resources(struct pci_dev *dev)
> d3afa58c Michal Simek 2010-01-18 565 {
> d3afa58c Michal Simek 2010-01-18 566 struct pci_controller *hose = pci_bus_to_host(dev->bus);
> d3afa58c Michal Simek 2010-01-18 567 int i;
> d3afa58c Michal Simek 2010-01-18 568
> d3afa58c Michal Simek 2010-01-18 569 if (!hose) {
> 6bd55f0b Michal Simek 2012-12-27 570 pr_err("No host bridge for PCI dev %s !\n",
> d3afa58c Michal Simek 2010-01-18 571 pci_name(dev));
> d3afa58c Michal Simek 2010-01-18 572 return;
> d3afa58c Michal Simek 2010-01-18 573 }
> d3afa58c Michal Simek 2010-01-18 574 for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> d3afa58c Michal Simek 2010-01-18 575 struct resource *res = dev->resource + i;
> d3afa58c Michal Simek 2010-01-18 576 if (!res->flags)
> d3afa58c Michal Simek 2010-01-18 577 continue;
> e5b36841 Bjorn Helgaas 2012-02-23 578 if (res->start == 0) {
> 6bd55f0b Michal Simek 2012-12-27 579 pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]",
> d3afa58c Michal Simek 2010-01-18 580 pci_name(dev), i,
> d3afa58c Michal Simek 2010-01-18 581 (unsigned long long)res->start,
> d3afa58c Michal Simek 2010-01-18 582 (unsigned long long)res->end,
> d3afa58c Michal Simek 2010-01-18 583 (unsigned int)res->flags);
> 6bd55f0b Michal Simek 2012-12-27 584 pr_debug("is unassigned\n");
> d3afa58c Michal Simek 2010-01-18 585 res->end -= res->start;
> d3afa58c Michal Simek 2010-01-18 586 res->start = 0;
> d3afa58c Michal Simek 2010-01-18 587 res->flags |= IORESOURCE_UNSET;
> d3afa58c Michal Simek 2010-01-18 588 continue;
> d3afa58c Michal Simek 2010-01-18 589 }
> d3afa58c Michal Simek 2010-01-18 590
> aa23bdc0 Bjorn Helgaas 2012-02-23 591 pr_debug("PCI:%s Resource %d %016llx-%016llx [%x]\n",
> d3afa58c Michal Simek 2010-01-18 592 pci_name(dev), i,
> 6bd55f0b Michal Simek 2012-12-27 593 (unsigned long long)res->start,
> d3afa58c Michal Simek 2010-01-18 594 (unsigned long long)res->end,
> d3afa58c Michal Simek 2010-01-18 595 (unsigned int)res->flags);
> d3afa58c Michal Simek 2010-01-18 596 }
> d3afa58c Michal Simek 2010-01-18 597 }
> d3afa58c Michal Simek 2010-01-18 598 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_resources);
> d3afa58c Michal Simek 2010-01-18 599
> d3afa58c Michal Simek 2010-01-18 600 /*
> d3afa58c Michal Simek 2010-01-18 601 * We need to avoid collisions with `mirrored' VGA ports
> d3afa58c Michal Simek 2010-01-18 602 * and other strange ISA hardware, so we always want the
> d3afa58c Michal Simek 2010-01-18 603 * addresses to be allocated in the 0x000-0x0ff region
> d3afa58c Michal Simek 2010-01-18 604 * modulo 0x400.
> d3afa58c Michal Simek 2010-01-18 605 *
> d3afa58c Michal Simek 2010-01-18 606 * Why? Because some silly external IO cards only decode
> d3afa58c Michal Simek 2010-01-18 607 * the low 10 bits of the IO address. The 0x00-0xff region
> d3afa58c Michal Simek 2010-01-18 608 * is reserved for motherboard devices that decode all 16
> d3afa58c Michal Simek 2010-01-18 609 * bits, so it's ok to allocate at, say, 0x2800-0x28ff,
> d3afa58c Michal Simek 2010-01-18 610 * but we want to try to avoid allocating at 0x2900-0x2bff
> d3afa58c Michal Simek 2010-01-18 611 * which might have be mirrored at 0x0100-0x03ff..
> d3afa58c Michal Simek 2010-01-18 612 */
> 01cf9d52 Bharat Kumar Gogada 2016-02-11 613 int pcibios_add_device(struct pci_dev *dev)
> 01cf9d52 Bharat Kumar Gogada 2016-02-11 614 {
> 01cf9d52 Bharat Kumar Gogada 2016-02-11 615 dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> c86fac43 Michal Simek 2010-04-16 616
> 01cf9d52 Bharat Kumar Gogada 2016-02-11 617 return 0;
> d3afa58c Michal Simek 2010-01-18 618 }
> 01cf9d52 Bharat Kumar Gogada 2016-02-11 619 EXPORT_SYMBOL(pcibios_add_device);
> d3afa58c Michal Simek 2010-01-18 620
> d3afa58c Michal Simek 2010-01-18 621 /*
> d3afa58c Michal Simek 2010-01-18 622 * Reparent resource children of pr that conflict with res
> d3afa58c Michal Simek 2010-01-18 623 * under res, and make res replace those children.
> d3afa58c Michal Simek 2010-01-18 624 */
> d3afa58c Michal Simek 2010-01-18 625 static int __init reparent_resources(struct resource *parent,
> d3afa58c Michal Simek 2010-01-18 626 struct resource *res)
> d3afa58c Michal Simek 2010-01-18 627 {
> d3afa58c Michal Simek 2010-01-18 628 struct resource *p, **pp;
> d3afa58c Michal Simek 2010-01-18 629 struct resource **firstpp = NULL;
> d3afa58c Michal Simek 2010-01-18 630
> d3afa58c Michal Simek 2010-01-18 @631 for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> d3afa58c Michal Simek 2010-01-18 632 if (p->end < res->start)
> d3afa58c Michal Simek 2010-01-18 633 continue;
> d3afa58c Michal Simek 2010-01-18 634 if (res->end < p->start)
> d3afa58c Michal Simek 2010-01-18 635 break;
> d3afa58c Michal Simek 2010-01-18 636 if (p->start < res->start || p->end > res->end)
> d3afa58c Michal Simek 2010-01-18 637 return -1; /* not completely contained */
> d3afa58c Michal Simek 2010-01-18 638 if (firstpp == NULL)
> d3afa58c Michal Simek 2010-01-18 639 firstpp = pp;
> d3afa58c Michal Simek 2010-01-18 640 }
> d3afa58c Michal Simek 2010-01-18 641 if (firstpp == NULL)
> d3afa58c Michal Simek 2010-01-18 642 return -1; /* didn't find any conflicting entries? */
> d3afa58c Michal Simek 2010-01-18 643 res->parent = parent;
> d3afa58c Michal Simek 2010-01-18 @644 res->child = *firstpp;
> d3afa58c Michal Simek 2010-01-18 645 res->sibling = *pp;
> d3afa58c Michal Simek 2010-01-18 646 *firstpp = res;
> d3afa58c Michal Simek 2010-01-18 647 *pp = NULL;
> d3afa58c Michal Simek 2010-01-18 @648 for (p = res->child; p != NULL; p = p->sibling) {
> d3afa58c Michal Simek 2010-01-18 649 p->parent = res;
> d3afa58c Michal Simek 2010-01-18 650 pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
> d3afa58c Michal Simek 2010-01-18 651 p->name,
> d3afa58c Michal Simek 2010-01-18 652 (unsigned long long)p->start,
> d3afa58c Michal Simek 2010-01-18 653 (unsigned long long)p->end, res->name);
> d3afa58c Michal Simek 2010-01-18 654 }
> d3afa58c Michal Simek 2010-01-18 655 return 0;
> d3afa58c Michal Simek 2010-01-18 656 }
> d3afa58c Michal Simek 2010-01-18 657
>
> :::::: The code at line 536 was first introduced by commit
> :::::: 70dcd942dc4af3cc6c3dcc2ba499cd841c7f65a7 microblaze: Fix IO space breakage after of_pci_range_to_resource() change
>
> :::::: TO: Michal Simek <[email protected]>
> :::::: CC: Michal Simek <[email protected]>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Wei Yang,
On 04/26/18 at 09:18am, Wei Yang wrote:
> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
> >The struct resource uses singly linked list to link siblings. It's not
> >easy to do reverse iteration on sibling list. So replace it with list_head.
> >
>
> Hi, Baoquan
>
> Besides changing the data structure, I have another proposal to do the reverse
> iteration. Which means it would not affect other users, if you just want a
> reverse iteration.
>
> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
> is a better solution to your first proposal in
> https://patchwork.kernel.org/patch/10300819/.
>
> Below is my proposal of resource reverse iteration without changing current
> design.
I got your mail and read it, then interrupted by other thing and forgot
replying, sorry.
I am fine with your code change. As I said before, I have tried to change
code per reviewers' comment, then let reviewers decide which way is
better. Please feel free to post formal patches and joining discussion
about this issue.
Thanks
Baoquan
>
> From 5d7145d44fe48b98572a03884fa3a3aa82e3cef9 Mon Sep 17 00:00:00 2001
> From: Wei Yang <[email protected]>
> Date: Sat, 24 Mar 2018 23:25:46 +0800
> Subject: [PATCH] kernel/resource: add walk_system_ram_res_rev()
>
> As discussed on https://patchwork.kernel.org/patch/10300819/, this patch
> comes up with a variant implementation of walk_system_ram_res_rev(), which
> uses iteration instead of allocating array to store those resources.
>
> Signed-off-by: Wei Yang <[email protected]>
> ---
> include/linux/ioport.h | 3 ++
> kernel/resource.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index da0ebaec25f0..473f1d9cb97e 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -277,6 +277,9 @@ extern int
> walk_system_ram_res(u64 start, u64 end, void *arg,
> int (*func)(struct resource *, void *));
> extern int
> +walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> + int (*func)(struct resource *, void *));
> +extern int
> walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
> void *arg, int (*func)(struct resource *, void *));
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 769109f20fb7..d4ec5fbc6875 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -73,6 +73,38 @@ static struct resource *next_resource(struct resource *p, bool sibling_only)
> return p->sibling;
> }
>
> +static struct resource *prev_resource(struct resource *p, bool sibling_only)
> +{
> + struct resource *prev;
> + if (NULL == iomem_resource.child)
> + return NULL;
> +
> + if (p == NULL) {
> + prev = iomem_resource.child;
> + while (prev->sibling)
> + prev = prev->sibling;
> + } else {
> + if (p->parent->child == p) {
> + return p->parent;
> + }
> +
> + for (prev = p->parent->child; prev->sibling != p;
> + prev = prev->sibling) {}
> + }
> +
> + /* Caller wants to traverse through siblings only */
> + if (sibling_only)
> + return prev;
> +
> + for (;prev->child;) {
> + prev = prev->child;
> +
> + while (prev->sibling)
> + prev = prev->sibling;
> + }
> + return prev;
> +}
> +
> static void *r_next(struct seq_file *m, void *v, loff_t *pos)
> {
> struct resource *p = v;
> @@ -401,6 +433,47 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
> return 0;
> }
>
> +/*
> + * Finds the highest iomem resource existing within [res->start.res->end).
> + * The caller must specify res->start, res->end, res->flags, and optionally
> + * desc. If found, returns 0, res is overwritten, if not found, returns -1.
> + * This function walks the whole tree and not just first level children until
> + * and unless first_level_children_only is true.
> + */
> +static int find_prev_iomem_res(struct resource *res, unsigned long desc,
> + bool first_level_children_only)
> +{
> + struct resource *p;
> +
> + BUG_ON(!res);
> + BUG_ON(res->start >= res->end);
> +
> + read_lock(&resource_lock);
> +
> + for (p = prev_resource(NULL, first_level_children_only); p;
> + p = prev_resource(p, first_level_children_only)) {
> + if ((p->flags & res->flags) != res->flags)
> + continue;
> + if ((desc != IORES_DESC_NONE) && (desc != p->desc))
> + continue;
> + if (p->end < res->start || p->child == iomem_resource.child) {
> + p = NULL;
> + break;
> + }
> + if ((p->end >= res->start) && (p->start < res->end))
> + break;
> + }
> +
> + read_unlock(&resource_lock);
> + if (!p)
> + return -1;
> + /* copy data */
> + resource_clip(res, p->start, p->end);
> + res->flags = p->flags;
> + res->desc = p->desc;
> + return 0;
> +}
> +
> static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
> bool first_level_children_only,
> void *arg,
> @@ -422,6 +495,27 @@ static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
> return ret;
> }
>
> +static int __walk_iomem_res_rev_desc(struct resource *res, unsigned long desc,
> + bool first_level_children_only,
> + void *arg,
> + int (*func)(struct resource *, void *))
> +{
> + u64 orig_start = res->start;
> + int ret = -1;
> +
> + while ((res->start < res->end) &&
> + !find_prev_iomem_res(res, desc, first_level_children_only)) {
> + ret = (*func)(res, arg);
> + if (ret)
> + break;
> +
> + res->end = res->start?(res->start - 1):0;
> + res->start = orig_start;
> + }
> +
> + return ret;
> +}
> +
> /*
> * Walks through iomem resources and calls func() with matching resource
> * ranges. This walks through whole tree and not just first level children.
> @@ -468,6 +562,25 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
> arg, func);
> }
>
> +/*
> + * This function, being a variant of walk_system_ram_res(), calls the @func
> + * callback against all memory ranges of type System RAM which are marked as
> + * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
> + * higher to lower.
> + */
> +int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
> + int (*func)(struct resource *, void *))
> +{
> + struct resource res;
> +
> + res.start = start;
> + res.end = end;
> + res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +
> + return __walk_iomem_res_rev_desc(&res, IORES_DESC_NONE, true,
> + arg, func);
> +}
> +
> /*
> * This function calls the @func callback against all memory ranges, which
> * are ranges marked as IORESOURCE_MEM and IORESOUCE_BUSY.
> --
> 2.15.1
>
>
> --
> Wei Yang
> Help you, Help me
On Mon, May 07, 2018 at 09:14:29AM +0800, Baoquan He wrote:
>Hi Wei Yang,
>
>On 04/26/18 at 09:18am, Wei Yang wrote:
>> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
>> >The struct resource uses singly linked list to link siblings. It's not
>> >easy to do reverse iteration on sibling list. So replace it with list_head.
>> >
>>
>> Hi, Baoquan
>>
>> Besides changing the data structure, I have another proposal to do the reverse
>> iteration. Which means it would not affect other users, if you just want a
>> reverse iteration.
>>
>> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
>> is a better solution to your first proposal in
>> https://patchwork.kernel.org/patch/10300819/.
>>
>> Below is my proposal of resource reverse iteration without changing current
>> design.
>
>I got your mail and read it, then interrupted by other thing and forgot
>replying, sorry.
>
>I am fine with your code change. As I said before, I have tried to change
>code per reviewers' comment, then let reviewers decide which way is
>better. Please feel free to post formal patches and joining discussion
>about this issue.
Yep, while I don't have a real requirement to add the reverse version, so what
is the proper way to send a patch?
A patch reply to this thread is ok?
>
>Thanks
>Baoquan
>
On 05/08/18 at 08:48pm, Wei Yang wrote:
> On Mon, May 07, 2018 at 09:14:29AM +0800, Baoquan He wrote:
> >Hi Wei Yang,
> >
> >On 04/26/18 at 09:18am, Wei Yang wrote:
> >> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
> >> >The struct resource uses singly linked list to link siblings. It's not
> >> >easy to do reverse iteration on sibling list. So replace it with list_head.
> >> >
> >>
> >> Hi, Baoquan
> >>
> >> Besides changing the data structure, I have another proposal to do the reverse
> >> iteration. Which means it would not affect other users, if you just want a
> >> reverse iteration.
> >>
> >> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
> >> is a better solution to your first proposal in
> >> https://patchwork.kernel.org/patch/10300819/.
> >>
> >> Below is my proposal of resource reverse iteration without changing current
> >> design.
> >
> >I got your mail and read it, then interrupted by other thing and forgot
> >replying, sorry.
> >
> >I am fine with your code change. As I said before, I have tried to change
> >code per reviewers' comment, then let reviewers decide which way is
> >better. Please feel free to post formal patches and joining discussion
> >about this issue.
>
> Yep, while I don't have a real requirement to add the reverse version, so what
> is the proper way to send a patch?
>
> A patch reply to this thread is ok?
I am not sure either. Since my patches are still under reviewing. And
you have pasted your patch. It depends on maintainers, mainly Andrew and
other reviewers who have concerns.
On Tue, May 08, 2018 at 08:11:23PM +0800, Baoquan He wrote:
>On 05/08/18 at 08:48pm, Wei Yang wrote:
>> On Mon, May 07, 2018 at 09:14:29AM +0800, Baoquan He wrote:
>> >Hi Wei Yang,
>> >
>> >On 04/26/18 at 09:18am, Wei Yang wrote:
>> >> On Thu, Apr 19, 2018 at 08:18:46AM +0800, Baoquan He wrote:
>> >> >The struct resource uses singly linked list to link siblings. It's not
>> >> >easy to do reverse iteration on sibling list. So replace it with list_head.
>> >> >
>> >>
>> >> Hi, Baoquan
>> >>
>> >> Besides changing the data structure, I have another proposal to do the reverse
>> >> iteration. Which means it would not affect other users, if you just want a
>> >> reverse iteration.
>> >>
>> >> BTW, I don't think Andrew suggest to use linked-list directly. What he wants
>> >> is a better solution to your first proposal in
>> >> https://patchwork.kernel.org/patch/10300819/.
>> >>
>> >> Below is my proposal of resource reverse iteration without changing current
>> >> design.
>> >
>> >I got your mail and read it, then interrupted by other thing and forgot
>> >replying, sorry.
>> >
>> >I am fine with your code change. As I said before, I have tried to change
>> >code per reviewers' comment, then let reviewers decide which way is
>> >better. Please feel free to post formal patches and joining discussion
>> >about this issue.
>>
>> Yep, while I don't have a real requirement to add the reverse version, so what
>> is the proper way to send a patch?
>>
>> A patch reply to this thread is ok?
>
>I am not sure either. Since my patches are still under reviewing. And
>you have pasted your patch. It depends on maintainers, mainly Andrew and
>other reviewers who have concerns.
Ok, thanks.
--
Wei Yang
Help you, Help me