Consolidate ACPI PCI root common code into ACPI core
v5 cover Consolidate ACPI PCI root common code into ACPI core
This patch set consolidates common code to support ACPI PCI root on x86 and IA64 platforms into ACPI core, to reproduce duplicated code and simplify maintenance. And a patch set based on previous version to support ACPI based PCIe host bridge on ARM64 has been posted at:
https://lkml.org/lkml/2015/5/26/207
It's based on latest mainstream kernel. It passes Fengguang's 0day test suite and has been tested on two IA64 platforms and one x86 platform.
V4-V5:
1) As suggested by Lorenzo Pieralisi <[email protected]>, do not introduce "struct pci_controller" into generic drivers/acpi/pci_root.c, so ARM64 doesn't need to define a useless structure. And it simplifies code a little too.
V3->V4:
1) Add patch[05/08] support solve building issue on ARM64
2) Solve an implicitly pointer cast issue.
3) Rebase to latest mainstream kernel
V2->V3:
1. Move memory allocation/free from ACPI core into arch
2. Kill the field 'segment' in struct pci_root_info on x86
Thanks!
Gerry
Jiang Liu (6):
ACPI/PCI: Enhance ACPI core to support sparse IO space ia64/PCI/ACPI: Use common ACPI resource parsing interface for host
bridge
ia64/PCI: Use common struct resource_entry to replace struct
iospace_resource
PCI/ACPI: Consolidate common PCI host bridge code into ACPI core x86/PCI/ACPI: Use common interface to support PCI host bridge ia64/PCI/ACPI: Use common interface to support PCI host bridge
arch/ia64/include/asm/pci.h | 5 - arch/ia64/pci/pci.c | 366 ++++++++++++------------------------------- arch/x86/pci/acpi.c | 294 ++++++++++------------------------ drivers/acpi/pci_root.c | 198 +++++++++++++++++++++++ drivers/acpi/resource.c | 9 +- include/linux/ioport.h | 1 + include/linux/pci-acpi.h | 23 +++ 7 files changed, 415 insertions(+), 481 deletions(-)
Signature · 1 line
1.7.10.4
PATCH v5 1/6 ACPI/PCI: Enhance ACPI core to support sparse IO space
Enhance ACPI resource parsing interfaces to support sparse IO space, which will be used to share common code between x86 and IA64 later.
--- drivers/acpi/resource.c | 9 ++++++--- include/linux/ioport.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)
drivers/acpi/resource.c +6 -3
@@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res)EXPORT_SYMBOL_GPL (acpi_dev_resource_memory);static void acpi_dev_ioresource_flags (struct resource *res,u64 len, -u8 io_decode) +u8 io_decode,u8 translation_type) { res->flags =IORESOURCE_IO ;
@@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,if (io_decode ==ACPI_DECODE_16 ) res->flags |=IORESOURCE_IO_16BIT_ADDR ; +if (translation_type ==ACPI_SPARSE_TRANSLATION ) + res->flags |=IORESOURCE_IO_SPARSE ; } static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,
@@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, { res->start = start; res->end = start + len -1 ; -acpi_dev_ioresource_flags (res, len, io_decode); +acpi_dev_ioresource_flags (res, len, io_decode,0 ); }/**
@@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win,acpi_dev_memresource_flags (res, len, wp);break ;case ACPI_IO_RANGE : -acpi_dev_ioresource_flags (res, len, iodec); +acpi_dev_ioresource_flags (res, len, iodec, + addr->info.io.translation_type);break ;case ACPI_BUS_NUMBER_RANGE : res->flags =IORESOURCE_BUS ;
include/linux/ioport.h +1 -0
@@ -94,6 +94,7 @@ struct resource {/* PnP I/O specific bits (IORESOURCE_BITS) */ #define IORESOURCE_IO_16BIT_ADDR (1<<0)#define IORESOURCE_IO_FIXED (1<<1) +#define IORESOURCE_IO_SPARSE (1<<2)/* PCI ROM control bits (IORESOURCE_BITS) */ #define IORESOURCE_ROM_ENABLE (1<<0)/* ROM is enabled, same as PCI_ROM_ADDRESS_ENABLE */
Signature · 1 line
1.7.10.4
Bjorn Helgaas
maintainerRe: [Patch v5 1/6] ACPI/PCI: Enhance ACPI core to support sparse IO space
Quoted 57 lines · Jiang Liu
Enhance ACPI resource parsing interfaces to support sparse IO space, which will be used to share common code between x86 and IA64 later.
Tested-by Tony Luck <[email protected]>Signed-off-by Jiang Liu <[email protected]>Reviewed-by Hanjun Guo <[email protected]>--- drivers/acpi/resource.c | 9 ++++++--- include/linux/ioport.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)drivers/acpi/resource.c +6
-3@@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res)EXPORT_SYMBOL_GPL (acpi_dev_resource_memory);static void acpi_dev_ioresource_flags (struct resource *res,u64 len, -u8 io_decode) +u8 io_decode,u8 translation_type) { res->flags =IORESOURCE_IO ;@@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,if (io_decode ==ACPI_DECODE_16 ) res->flags |=IORESOURCE_IO_16BIT_ADDR ; +if (translation_type ==ACPI_SPARSE_TRANSLATION ) + res->flags |=IORESOURCE_IO_SPARSE ; } static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,@@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, { res->start = start; res->end = start + len -1 ; -acpi_dev_ioresource_flags (res, len, io_decode); +acpi_dev_ioresource_flags (res, len, io_decode,0 ); }/** @@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win,acpi_dev_memresource_flags (res, len, wp);break ;case ACPI_IO_RANGE : -acpi_dev_ioresource_flags (res, len, iodec); +acpi_dev_ioresource_flags (res, len, iodec, + addr->info.io.translation_type);break ;case ACPI_BUS_NUMBER_RANGE : res->flags =IORESOURCE_BUS ;include/linux/ioport.h +0
-0
include/linux/ioport.h struct resource { · line 97
Quoted 4 lines
include/linux/ioport.h +1
-0@@ -94,6 +94,7 @@ struct resource {/* PnP I/O specific bits (IORESOURCE_BITS) */ #define IORESOURCE_IO_16BIT_ADDR (1<<0)#define IORESOURCE_IO_FIXED (1<<1) +#define IORESOURCE_IO_SPARSE (1<<2)
I don't really like this bit. We adding a new generic IORESOURCE_* bit just for a special case, and it's only used in one place, for one arch, during enumeration.
Don't we have a similar problem (struct resource can't express all the information from an ACPI resource) for the _TRA value for bridge windows? If it's a similar issue, we should solve it in a similar way.
Quoted 5 lines
include/linux/ioport.h +0
-0/* PCI ROM control bits (IORESOURCE_BITS) */ #define IORESOURCE_ROM_ENABLE (1<<0)/* ROM is enabled, same as PCI_ROM_ADDRESS_ENABLE */ Signature · 1 line
1.7.10.4
Re: [Patch v5 1/6] ACPI/PCI: Enhance ACPI core to support sparse IO space
Quoted 58 lines · Bjorn Helgaas
Quoted 57 lines · Jiang Liu
Enhance ACPI resource parsing interfaces to support sparse IO space, which will be used to share common code between x86 and IA64 later.
Tested-by Tony Luck <[email protected]>Signed-off-by Jiang Liu <[email protected]>Reviewed-by Hanjun Guo <[email protected]>--- drivers/acpi/resource.c | 9 ++++++--- include/linux/ioport.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)drivers/acpi/resource.c +6
-3@@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res)EXPORT_SYMBOL_GPL (acpi_dev_resource_memory);static void acpi_dev_ioresource_flags (struct resource *res,u64 len, -u8 io_decode) +u8 io_decode,u8 translation_type) { res->flags =IORESOURCE_IO ;@@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,if (io_decode ==ACPI_DECODE_16 ) res->flags |=IORESOURCE_IO_16BIT_ADDR ; +if (translation_type ==ACPI_SPARSE_TRANSLATION ) + res->flags |=IORESOURCE_IO_SPARSE ; } static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,@@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, { res->start = start; res->end = start + len -1 ; -acpi_dev_ioresource_flags (res, len, io_decode); +acpi_dev_ioresource_flags (res, len, io_decode,0 ); }/** @@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win,acpi_dev_memresource_flags (res, len, wp);break ;case ACPI_IO_RANGE : -acpi_dev_ioresource_flags (res, len, iodec); +acpi_dev_ioresource_flags (res, len, iodec, + addr->info.io.translation_type);break ;case ACPI_BUS_NUMBER_RANGE : res->flags =IORESOURCE_BUS ;include/linux/ioport.h +0
-0@@ -94,6 +94,7 @@ struct resource {
Quoted 8 lines
include/linux/ioport.h struct resource { · line 97
Quoted 4 lines
include/linux/ioport.h +1
-0/* PnP I/O specific bits (IORESOURCE_BITS) */ #define IORESOURCE_IO_16BIT_ADDR (1<<0)#define IORESOURCE_IO_FIXED (1<<1) +#define IORESOURCE_IO_SPARSE (1<<2)I don't really like this bit. We adding a new generic IORESOURCE_* bit just for a special case, and it's only used in one place, for one arch, during enumeration.
Hi Bjorn,
Instead of defining a formal flag IORESOURCE_IO_SPARSE, we may
reuse other field in struct resource to pass back the SPARSE flag, but that's a little dirty. For example, we may reuse res->name field to carry the SPARSE flag for the IA64 special case.
Is that OK?
Thanks!
Gerry
Quoted 4 lines
Don't we have a similar problem (struct resource can't express all the information from an ACPI resource) for the
_TRAvalue for bridge windows? If it's a similar issue, we should solve it in a similar way.
Per my understanding, we just ignore ACPI address space information which can't be expressed by struct resource.
Thanks!
Gerry
Quoted 6 lines
Quoted 6 lines
include/linux/ioport.h +0
-0/* PCI ROM control bits (IORESOURCE_BITS) */ #define IORESOURCE_ROM_ENABLE (1<<0)/* ROM is enabled, same as PCI_ROM_ADDRESS_ENABLE */ Signature · 1 line
1.7.10.4
Bjorn Helgaas
maintainerRe: [Patch v5 1/6] ACPI/PCI: Enhance ACPI core to support sparse IO space
Quoted 67 lines · Jiang Liu
Quoted 58 lines · Bjorn Helgaas
Quoted 57 lines · Jiang Liu
Enhance ACPI resource parsing interfaces to support sparse IO space, which will be used to share common code between x86 and IA64 later.
Tested-by Tony Luck <[email protected]>Signed-off-by Jiang Liu <[email protected]>Reviewed-by Hanjun Guo <[email protected]>--- drivers/acpi/resource.c | 9 ++++++--- include/linux/ioport.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)drivers/acpi/resource.c +6
-3@@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res)EXPORT_SYMBOL_GPL (acpi_dev_resource_memory);static void acpi_dev_ioresource_flags (struct resource *res,u64 len, -u8 io_decode) +u8 io_decode,u8 translation_type) { res->flags =IORESOURCE_IO ;@@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,if (io_decode ==ACPI_DECODE_16 ) res->flags |=IORESOURCE_IO_16BIT_ADDR ; +if (translation_type ==ACPI_SPARSE_TRANSLATION ) + res->flags |=IORESOURCE_IO_SPARSE ; } static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,@@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, { res->start = start; res->end = start + len -1 ; -acpi_dev_ioresource_flags (res, len, io_decode); +acpi_dev_ioresource_flags (res, len, io_decode,0 ); }/** @@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win,acpi_dev_memresource_flags (res, len, wp);break ;case ACPI_IO_RANGE : -acpi_dev_ioresource_flags (res, len, iodec); +acpi_dev_ioresource_flags (res, len, iodec, + addr->info.io.translation_type);break ;case ACPI_BUS_NUMBER_RANGE : res->flags =IORESOURCE_BUS ;include/linux/ioport.h +0
-0@@ -94,6 +94,7 @@ struct resource {Quoted 8 lines
include/linux/ioport.h struct resource { · line 97
Quoted 4 lines
include/linux/ioport.h +1
-0/* PnP I/O specific bits (IORESOURCE_BITS) */ #define IORESOURCE_IO_16BIT_ADDR (1<<0)#define IORESOURCE_IO_FIXED (1<<1) +#define IORESOURCE_IO_SPARSE (1<<2)I don't really like this bit. We adding a new generic IORESOURCE_* bit just for a special case, and it's only used in one place, for one arch, during enumeration.
Quoted 6 lines
Hi Bjorn,
Instead of defining a formal flagIORESOURCE_IO_SPARSE, we may
reuse other field in struct resource to pass back the SPARSE flag, but that's a little dirty. For example, we may reuseres->namefield to carry the SPARSE flag for the IA64 special case.
Is that OK?
No, I think that's even worse. Adding IORESOURCE_IO_SPARSE looks positively glorious now.
Bjorn
PATCH v5 2/6 ia64/PCI/ACPI: Use common ACPI resource parsing interface for host bridge
In reply to Jiang LiuUse common ACPI resource parsing interface to parse ACPI resources for PCI host bridge, so we could share more code between IA64 and x86. Later we will consolidate arch specific implementations into ACPI core.
--- arch/ia64/pci/pci.c | 414 ++++++++++++++++++++++++--------------------------- 1 file changed, 193 insertions(+), 221 deletions(-)
arch/ia64/pci/pci.c +193 -221
@@ -115,29 +115,12 @@ struct pci_ops pci_root_ops = { .write = pci_write, }; -/* Called by ACPI when it finds a new root bus. */ - -static struct pci_controller *alloc_pci_controller (int seg) -{ -struct pci_controller *controller; - - controller =kzalloc (sizeof (*controller),GFP_KERNEL ); -if (!controller) -return NULL ; - - controller->segment = seg; -return controller; -} -struct pci_root_info { +struct pci_controller controller;struct acpi_device *bridge; -struct pci_controller *controller;struct list_head resources; -struct resource *res; -resource_size_t *res_offset; -unsigned int res_num;struct list_head io_resources; -char *name; +char name[16 ]; }; static unsigned int
@@ -168,11 +151,11 @@ new_space (u64 phys_base, int sparse)return i; } -static u64 add_io_space (struct pci_root_info *info, -struct acpi_resource_address64 *addr) +static int add_io_space (struct device *dev,struct pci_root_info *info, +struct resource_entry *entry) {struct iospace_resource *iospace; -struct resource *resource; +struct resource *resource, *res = entry->res;char *name;unsigned long base, min, max, base_port;unsigned int sparse =0 , space_nr, len;
@@ -180,27 +163,24 @@ static u64 add_io_space(struct pci_root_info *info, len =strlen (info->name) +32 ; iospace =kzalloc (sizeof (*iospace) + len,GFP_KERNEL );if (!iospace) { -dev_err (&info->bridge->dev, -"PCI: No memory for %s I/O port space\n" , - info->name); - goto out; +dev_err (dev,"PCI: No memory for %s I/O port space\n" , + info->name); +return -ENOMEM ; } - name = (char *)(iospace +1 ); - - min = addr->address.minimum; - max = min + addr->address.address_length -1 ; -if (addr->info.io.translation_type ==ACPI_SPARSE_TRANSLATION ) +if (res->flags &IORESOURCE_IO_SPARSE ) sparse =1 ; - - space_nr =new_space (addr->address.translation_offset, sparse); + space_nr =new_space (entry->offset, sparse);if (space_nr == ~0 ) goto free_resource; + name = (char *)(iospace +1 ); + min = res->start - entry->offset; + max = res->end - entry->offset; base =__pa (io_space[space_nr].mmio_base); base_port =IO_SPACE_BASE (space_nr);snprintf (name, len,"%s I/O Ports %08lx-%08lx" , info->name, - base_port + min, base_port + max); + base_port + min, base_port + max);/* * The SDM guarantees the legacy 0-64K space is sparse, but if the
@@ -216,153 +196,195 @@ static u64 add_io_space(struct pci_root_info *info, resource->start = base + (sparse ?IO_SPACE_SPARSE_ENCODING (min) : min); resource->end = base + (sparse ?IO_SPACE_SPARSE_ENCODING (max) : max);if (insert_resource (&iomem_resource, resource)) { -dev_err (&info->bridge->dev, -"can't allocate host bridge io space resource %pR\n" , - resource); +dev_err (dev, +"can't allocate host bridge io space resource %pR\n" , + resource); goto free_resource; } + entry->offset = base_port; + res->start = min + base_port; + res->end = max + base_port;list_add_tail (&iospace->list, &info->io_resources); -return base_port; + +return 0 ; free_resource:kfree (iospace); -out: -return ~0 ; +return -ENOSPC ; +} + +/* +* An IO port or MMIO resource assigned to a PCI host bridge may be +* consumed by the host bridge itself or available to its child +* bus/devices. The ACPI specification defines a bit (Producer/Consumer) +* to tell whether the resource is consumed by the host bridge itself, +* but firmware hasn't used that bit consistently, so we can't rely on it. +* +* On x86 and IA64 platforms, all IO port and MMIO resources are assumed +* to be available to child bus/devices except one special case: +* IO port [0xCF8-0xCFF] is consumed by the host bridge itself +* to access PCI configuration space. +* +* So explicitly filter out PCI CFG IO ports[0xCF8-0xCFF]. +*/ +static bool resource_is_pcicfg_ioport (struct resource *res) +{ +return (res->flags &IORESOURCE_IO ) && + res->start ==0xCF8 && res->end ==0xCFF ; } -static acpi_status resource_to_window (struct acpi_resource *resource, -struct acpi_resource_address64 *addr) +static int +probe_pci_root_info (struct pci_root_info *info,struct acpi_device *device, +int busnum,int domain) { -acpi_status status; +int ret; +struct list_head *list = &info->resources; +struct resource_entry *entry, *tmp; -/* -* We're only interested in _CRS descriptors that are -* - address space descriptors for memory or I/O space -* - non-zero size -*/ - status =acpi_resource_to_address64 (resource, addr); -if (ACPI_SUCCESS (status) && - (addr->resource_type ==ACPI_MEMORY_RANGE || - addr->resource_type ==ACPI_IO_RANGE ) && - addr->address.address_length) -return AE_OK ; - -return AE_ERROR ; -} - -static acpi_status count_window (struct acpi_resource *resource,void *data) -{ -unsigned int *windows = (unsigned int *) data; -struct acpi_resource_address64 addr; -acpi_status status; - - status =resource_to_window (resource, &addr); -if (ACPI_SUCCESS (status)) - (*windows)++; - -return AE_OK ; -} - -static acpi_status add_window (struct acpi_resource *res,void *data) -{ -struct pci_root_info *info = data; -struct resource *resource; -struct acpi_resource_address64 addr; -acpi_status status; -unsigned long flags, offset =0 ; -struct resource *root; - -/* Return AE_OK for non-window resources to keep scanning for more */ - status =resource_to_window (res, &addr); -if (!ACPI_SUCCESS (status)) -return AE_OK ; - -if (addr.resource_type ==ACPI_MEMORY_RANGE ) { - flags =IORESOURCE_MEM ; - root = &iomem_resource; - offset = addr.address.translation_offset; - }else if (addr.resource_type ==ACPI_IO_RANGE ) { - flags =IORESOURCE_IO ; - root = &ioport_resource; - offset =add_io_space (info, &addr); -if (offset == ~0 ) -return AE_OK ; - }else -return AE_OK ; - - resource = &info->res[info->res_num]; - resource->name = info->name; - resource->flags = flags; - resource->start = addr.address.minimum + offset; - resource->end = resource->start + addr.address.address_length -1 ; - info->res_offset[info->res_num] = offset; - -if (insert_resource (root, resource)) { -dev_err (&info->bridge->dev, -"can't allocate host bridge window %pR\n" , - resource); - }else { -if (offset) -dev_info (&info->bridge->dev,"host bridge window %pR " -"(PCI address [%#llx-%#llx])\n" , - resource, - resource->start - offset, - resource->end - offset); -else -dev_info (&info->bridge->dev, -"host bridge window %pR\n" , resource); - } -/* HP's firmware has a hack to work around a Windows bug. -* Ignore these tiny memory ranges */ -if (!((resource->flags &IORESOURCE_MEM ) && - (resource->end - resource->start <16 ))) -pci_add_resource_offset (&info->resources, resource, - info->res_offset[info->res_num]); + ret =acpi_dev_get_resources (device, list, + acpi_dev_filter_resource_type_cb, + (void *)(IORESOURCE_IO |IORESOURCE_MEM )); +if (ret <0 ) +dev_warn (&device->dev, +"failed to parse _CRS method, error code %d\n" , ret); +else if (ret ==0 ) +dev_dbg (&device->dev, +"no IO and memory resources present in _CRS\n" ); +else +resource_list_for_each_entry_safe (entry, tmp, list) { +if ((entry->res->flags &IORESOURCE_DISABLED ) || +resource_is_pcicfg_ioport (entry->res)) +resource_list_destroy_entry (entry); +else + entry->res->name = info->name; + } - info->res_num++; -return AE_OK ; +return ret; } -static void free_pci_root_info_res (struct pci_root_info *info) -{ -struct iospace_resource *iospace, *tmp; +static void validate_resources (struct device *dev,struct list_head *resources, +unsigned long type) +{ +LIST_HEAD (list); +struct resource *res1, *res2, *root =NULL ; +struct resource_entry *tmp, *entry, *entry2; + +BUG_ON ((type & (IORESOURCE_MEM |IORESOURCE_IO )) ==0 ); + root = (type &IORESOURCE_MEM ) ? &iomem_resource : &ioport_resource; + +list_splice_init (resources, &list); +resource_list_for_each_entry_safe (entry, tmp, &list) { +bool free = false; +resource_size_t end; + + res1 = entry->res; +if (!(res1->flags & type)) + goto next; + +/* Exclude non-addressable range or non-addressable portion */ + end =min (res1->end, root->end); +if (end <= res1->start) { +dev_info (dev,"host bridge window %pR (ignored, not CPU addressable)\n" , + res1); + free = true; + goto next; + }else if (res1->end != end) { +dev_info (dev,"host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n" , + res1, (unsigned long long )end +1 , + (unsigned long long )res1->end); + res1->end = end; + } -list_for_each_entry_safe (iospace, tmp, &info->io_resources, list) -kfree (iospace); +resource_list_for_each_entry (entry2, resources) { + res2 = entry2->res; +if (!(res2->flags & type)) +continue ; + +/* +* I don't like throwing away windows because then +* our resources no longer match the ACPI _CRS, but +* the kernel resource tree doesn't allow overlaps. +*/ +if (resource_overlaps (res1, res2)) { + res2->start =min (res1->start, res2->start); + res2->end =max (res1->end, res2->end); +dev_info (dev,"host bridge window expanded to %pR; %pR ignored\n" , + res2, res1); + free = true; + goto next; + } + } -kfree (info->name); -kfree (info->res); - info->res =NULL ; -kfree (info->res_offset); - info->res_offset =NULL ; - info->res_num =0 ; -kfree (info->controller); - info->controller =NULL ; +next: +resource_list_del (entry); +if (free) +resource_list_free_entry (entry); +else +resource_list_add_tail (entry, resources); + } +} + +static void add_resources (struct pci_root_info *info,struct device *dev) +{ +struct resource_entry *entry, *tmp; +struct resource *res, *conflict, *root =NULL ; +struct list_head *list = &info->resources; + +validate_resources (dev, list,IORESOURCE_MEM ); +validate_resources (dev, list,IORESOURCE_IO ); + +resource_list_for_each_entry_safe (entry, tmp, list) { + res = entry->res; +if (res->flags &IORESOURCE_MEM ) { + root = &iomem_resource; +/* +* HP's firmware has a hack to work around a Windows +* bug. Ignore these tiny memory ranges. +*/ +if (resource_size (res) <=16 ) { +resource_list_destroy_entry (entry); +continue ; + } + }else if (res->flags &IORESOURCE_IO ) { + root = &ioport_resource; +if (add_io_space (&info->bridge->dev, info, entry)) { +resource_list_destroy_entry (entry); +continue ; + } + }else { +BUG_ON (res); + } + + conflict =insert_resource_conflict (root, res); +if (conflict) { +dev_info (dev, +"ignoring host bridge window %pR (conflicts with %s %pR)\n" , + res, conflict->name, conflict); +resource_list_destroy_entry (entry); + } + } }static void __release_pci_root_info (struct pci_root_info *info) { -int i;struct resource *res; -struct iospace_resource *iospace; +struct iospace_resource *iospace, *tmp; +struct resource_entry *entry, *tentry; -list_for_each_entry (iospace, &info->io_resources, list) +list_for_each_entry_safe (iospace, tmp, &info->io_resources, list) {release_resource (&iospace->res); +kfree (iospace); + } -for (i =0 ; i < info->res_num; i++) { - res = &info->res[i]; - -if (!res->parent) -continue ; - -if (!(res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) -continue ; - -release_resource (res); +resource_list_for_each_entry_safe (entry, tentry, &info->resources) { + res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); +resource_list_destroy_entry (entry); } -free_pci_root_info_res (info);kfree (info); }
@@ -373,99 +395,49 @@ static void release_pci_root_info(struct pci_host_bridge *bridge)__release_pci_root_info (info); } -static int -probe_pci_root_info (struct pci_root_info *info,struct acpi_device *device, -int busnum,int domain) -{ -char *name; - - name =kmalloc (16 ,GFP_KERNEL ); -if (!name) -return -ENOMEM ; - -sprintf (name,"PCI Bus %04x:%02x" , domain, busnum); - info->bridge = device; - info->name = name; - -acpi_walk_resources (device->handle,METHOD_NAME__CRS , count_window, - &info->res_num); -if (info->res_num) { - info->res = -kzalloc_node (sizeof (*info->res) * info->res_num, -GFP_KERNEL , info->controller->node); -if (!info->res) { -kfree (name); -return -ENOMEM ; - } - - info->res_offset = -kzalloc_node (sizeof (*info->res_offset) * info->res_num, -GFP_KERNEL , info->controller->node); -if (!info->res_offset) { -kfree (name); -kfree (info->res); - info->res =NULL ; -return -ENOMEM ; - } - - info->res_num =0 ; -acpi_walk_resources (device->handle,METHOD_NAME__CRS , - add_window, info); - }else -kfree (name); - -return 0 ; -} -struct pci_bus *pci_acpi_scan_root (struct acpi_pci_root *root) {struct acpi_device *device = root->device;int domain = root->segment;int bus = root->secondary.start; -struct pci_controller *controller; -struct pci_root_info *info =NULL ; -int busnum = root->secondary.start; +struct pci_root_info *info;struct pci_bus *pbus;int ret; - controller =alloc_pci_controller (domain); -if (!controller) -return NULL ; - - controller->companion = device; - controller->node =acpi_get_node (device->handle); - info =kzalloc (sizeof (*info),GFP_KERNEL );if (!info) {dev_err (&device->dev, -"pci_bus %04x:%02x: ignored (out of memory)\n" , - domain, busnum); -kfree (controller); +"pci_bus %04x:%02x: ignored (out of memory)\n" , + domain, bus);return NULL ; } - info->controller = controller; -INIT_LIST_HEAD (&info->io_resources); + info->controller.segment = domain; + info->controller.companion = device; + info->controller.node =acpi_get_node (device->handle); + info->bridge = device;INIT_LIST_HEAD (&info->resources); +INIT_LIST_HEAD (&info->io_resources); +snprintf (info->name,sizeof (info->name), +"PCI Bus %04x:%02x" , domain, bus); - ret =probe_pci_root_info (info, device, busnum, domain); -if (ret) { -kfree (info->controller); + ret =probe_pci_root_info (info, device, bus, domain); +if (ret <=0 ) {kfree (info);return NULL ; } -/* insert busn resource at first */ +add_resources (info, &info->bridge->dev);pci_add_resource (&info->resources, &root->secondary); +/* * See arch/x86/pci/acpi.c. * The desired pci bus might already be scanned in a quirk. We * should handle the case here, but it appears that IA64 hasn't * such quirk. So we just ignore the case now. */ - pbus =pci_create_root_bus (NULL , bus, &pci_root_ops, controller, - &info->resources); + pbus =pci_create_root_bus (NULL , bus, &pci_root_ops, + &info->controller, &info->resources);if (!pbus) { -pci_free_resource_list (&info->resources);__release_pci_root_info (info);return NULL ; }
Signature · 1 line
1.7.10.4
PATCH v5 3/6 ia64/PCI: Use common struct resource_entry to replace struct iospace_resource
In reply to Jiang LiuUse common struct resource_entry to replace private struct iospace_resource.
--- arch/ia64/include/asm/pci.h | 5 ----- arch/ia64/pci/pci.c | 17 ++++++++--------- 2 files changed, 8 insertions(+), 14 deletions(-)
arch/ia64/include/asm/pci.h +0 -5
@@ -83,11 +83,6 @@ extern int pci_mmap_legacy_page_range(struct pci_bus *bus,#define pci_legacy_read platform_pci_legacy_read#define pci_legacy_write platform_pci_legacy_write -struct iospace_resource { -struct list_head list; -struct resource res; -}; -struct pci_controller {struct acpi_device *companion;void *iommu;
arch/ia64/pci/pci.c +8 -9
@@ -154,14 +154,14 @@ new_space (u64 phys_base, int sparse)static int add_io_space (struct device *dev,struct pci_root_info *info,struct resource_entry *entry) { -struct iospace_resource *iospace; +struct resource_entry *iospace;struct resource *resource, *res = entry->res;char *name;unsigned long base, min, max, base_port;unsigned int sparse =0 , space_nr, len; len =strlen (info->name) +32 ; - iospace =kzalloc (sizeof (*iospace) + len,GFP_KERNEL ); + iospace =resource_list_create_entry (NULL , len);if (!iospace) {dev_err (dev,"PCI: No memory for %s I/O port space\n" , info->name);
@@ -190,7 +190,7 @@ static int add_io_space(struct device *dev, struct pci_root_info *info,if (space_nr ==0 ) sparse =1 ; - resource = &iospace->res; + resource = iospace->res; resource->name = name; resource->flags =IORESOURCE_MEM ; resource->start = base + (sparse ?IO_SPACE_SPARSE_ENCODING (min) : min);
@@ -205,12 +205,12 @@ static int add_io_space(struct device *dev, struct pci_root_info *info, entry->offset = base_port; res->start = min + base_port; res->end = max + base_port; -list_add_tail (&iospace->list, &info->io_resources); +resource_list_add_tail (iospace, &info->io_resources);return 0 ; free_resource: -kfree (iospace); +resource_list_free_entry (iospace);return -ENOSPC ; }
@@ -369,12 +369,11 @@ static void add_resources(struct pci_root_info *info, struct device *dev)static void __release_pci_root_info (struct pci_root_info *info) {struct resource *res; -struct iospace_resource *iospace, *tmp;struct resource_entry *entry, *tentry; -list_for_each_entry_safe (iospace, tmp, &info->io_resources, list) { -release_resource (&iospace->res); -kfree (iospace); +resource_list_for_each_entry_safe (entry, tentry, &info->io_resources) { +release_resource (entry->res); +resource_list_destroy_entry (entry); }resource_list_for_each_entry_safe (entry, tentry, &info->resources) {
Signature · 1 line
1.7.10.4
PATCH v5 4/6 PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
In reply to Jiang LiuIntroduce common interface acpi_pci_root_create() and related data structures to create PCI root bus for ACPI PCI host bridges. It will be used to kill duplicated arch specific code for IA64 and x86. It may also help ARM64 in future.
--- drivers/acpi/pci_root.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci-acpi.h | 23 ++++++ 2 files changed, 221 insertions(+)
drivers/acpi/pci_root.c +198 -0
@@ -656,6 +656,204 @@ static void acpi_pci_root_remove(struct acpi_device *device)kfree (root); } +static void acpi_pci_root_validate_resources (struct device *dev, +struct list_head *resources, +unsigned long type) +{ +LIST_HEAD (list); +struct resource *res1, *res2, *root =NULL ; +struct resource_entry *tmp, *entry, *entry2; + +BUG_ON ((type & (IORESOURCE_MEM |IORESOURCE_IO )) ==0 ); + root = (type &IORESOURCE_MEM ) ? &iomem_resource : &ioport_resource; + +list_splice_init (resources, &list); +resource_list_for_each_entry_safe (entry, tmp, &list) { +bool free = false; +resource_size_t end; + + res1 = entry->res; +if (!(res1->flags & type)) + goto next; + +/* Exclude non-addressable range or non-addressable portion */ + end =min (res1->end, root->end); +if (end <= res1->start) { +dev_info (dev,"host bridge window %pR (ignored, not CPU addressable)\n" , + res1); + free = true; + goto next; + }else if (res1->end != end) { +dev_info (dev,"host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n" , + res1, (unsigned long long )end +1 , + (unsigned long long )res1->end); + res1->end = end; + } + +resource_list_for_each_entry (entry2, resources) { + res2 = entry2->res; +if (!(res2->flags & type)) +continue ; + +/* +* I don't like throwing away windows because then +* our resources no longer match the ACPI _CRS, but +* the kernel resource tree doesn't allow overlaps. +*/ +if (resource_overlaps (res1, res2)) { + res2->start =min (res1->start, res2->start); + res2->end =max (res1->end, res2->end); +dev_info (dev,"host bridge window expanded to %pR; %pR ignored\n" , + res2, res1); + free = true; + goto next; + } + } + +next: +resource_list_del (entry); +if (free) +resource_list_free_entry (entry); +else +resource_list_add_tail (entry, resources); + } +} + +static int acpi_pci_probe_root_resources (struct acpi_pci_root_info *info) +{ +int ret; +struct list_head *list = &info->resources; +struct acpi_device *device = info->bridge; +struct resource_entry *entry, *tmp; +unsigned long flags; + + flags =IORESOURCE_IO |IORESOURCE_MEM |IORESOURCE_MEM_8AND16BIT ; + ret =acpi_dev_get_resources (device, list, + acpi_dev_filter_resource_type_cb, + (void *)flags); +if (ret <0 ) +dev_warn (&device->dev, +"failed to parse _CRS method, error code %d\n" , ret); +else if (ret ==0 ) +dev_dbg (&device->dev, +"no IO and memory resources present in _CRS\n" ); +else { +resource_list_for_each_entry_safe (entry, tmp, list) { +if (entry->res->flags &IORESOURCE_DISABLED ) +resource_list_destroy_entry (entry); +else + entry->res->name = info->name; + } +acpi_pci_root_validate_resources (&device->dev, list, +IORESOURCE_MEM ); +acpi_pci_root_validate_resources (&device->dev, list, +IORESOURCE_IO ); + } + +return ret; +} + +static void pci_acpi_root_add_resources (struct acpi_pci_root_info *info) +{ +struct resource_entry *entry, *tmp; +struct resource *res, *conflict, *root =NULL ; + +resource_list_for_each_entry_safe (entry, tmp, &info->resources) { + res = entry->res; +if (res->flags &IORESOURCE_MEM ) + root = &iomem_resource; +else if (res->flags &IORESOURCE_IO ) + root = &ioport_resource; +else +continue ; + + conflict =insert_resource_conflict (root, res); +if (conflict) { +dev_info (&info->bridge->dev, +"ignoring host bridge window %pR (conflicts with %s %pR)\n" , + res, conflict->name, conflict); +resource_list_destroy_entry (entry); + } + } +} + +static void __acpi_pci_root_release_info (struct acpi_pci_root_info *info) +{ +struct resource *res; +struct resource_entry *entry, *tmp; + +if (!info) +return ; + +resource_list_for_each_entry_safe (entry, tmp, &info->resources) { + res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); +resource_list_destroy_entry (entry); + } + + info->ops->release_info (info); +} + +static void acpi_pci_root_release_info (struct pci_host_bridge *bridge) +{ +struct resource *res; +struct resource_entry *entry; + +resource_list_for_each_entry (entry, &bridge->windows) { + res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); + } +__acpi_pci_root_release_info (bridge->release_data); +} + +struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node) +{ +int ret, busnum = root->secondary.start; +struct acpi_device *device = root->device; +struct pci_bus *bus; + + info->root = root; + info->bridge = device; + info->ops = ops; +INIT_LIST_HEAD (&info->resources); +snprintf (info->name,sizeof (info->name),"PCI Bus %04x:%02x" , + segment, busnum); + +if (ops->init_info && ops->init_info (info)) + goto out_release_info; + ret =acpi_pci_probe_root_resources (info); +if (ops->prepare_resources) + ret = ops->prepare_resources (info, ret); +if (ret <0 ) + goto out_release_info; +else if (ret >0 ) +pci_acpi_root_add_resources (info); +pci_add_resource (&info->resources, &root->secondary); + + bus =pci_create_root_bus (NULL , busnum, ops->pci_ops, + sysdata, &info->resources); +if (bus) { +pci_scan_child_bus (bus); +pci_set_host_bridge_release (to_pci_host_bridge (bus->bridge), + acpi_pci_root_release_info, info); +if (node !=NUMA_NO_NODE ) +dev_printk (KERN_DEBUG , &bus->dev,"on NUMA node %d\n" , + node); +return bus; + } + +out_release_info: +__acpi_pci_root_release_info (info); +return NULL ; +} +void __init acpi_pci_root_init (void ) {acpi_hest_init ();
include/linux/pci-acpi.h +23 -0
@@ -52,6 +52,29 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)return ACPI_HANDLE (dev); } +struct acpi_pci_root ; +struct acpi_pci_root_ops ; + +struct acpi_pci_root_info { +struct acpi_pci_root *root; +struct acpi_device *bridge; +struct acpi_pci_root_ops *ops; +struct list_head resources; +char name[16 ]; +}; + +struct acpi_pci_root_ops { +struct pci_ops *pci_ops; +int (*init_info)(struct acpi_pci_root_info *info); +void (*release_info)(struct acpi_pci_root_info *info); +int (*prepare_resources)(struct acpi_pci_root_info *info,int status); +}; + +extern struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sd,int seg,int node); +void acpi_pci_add_bus (struct pci_bus *bus);void acpi_pci_remove_bus (struct pci_bus *bus);
Signature · 1 line
1.7.10.4
Re: [Patch v5 4/6] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
On Mon, Jun 08, 2015 at 05:20:46PM +0100, Jiang Liu wrote:
[...]
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 730
Quoted 9 lines
drivers/acpi/pci_root.c +9
-0+static int acpi_pci_probe_root_resources (struct acpi_pci_root_info *info) +{ +int ret; +struct list_head *list = &info->resources; +struct acpi_device *device = info->bridge; +struct resource_entry *entry, *tmp; +unsigned long flags; + + flags =IORESOURCE_IO |IORESOURCE_MEM |IORESOURCE_MEM_8AND16BIT ;
Is IORESOURCE_MEM_8AND16BIT required because of some pending patches that will change ACPI resource filtering ? It does not seem to make a difference in the mainline code, AFAICT.
Quoted 70 lines
drivers/acpi/pci_root.c +69
-0+ ret =acpi_dev_get_resources (device, list, + acpi_dev_filter_resource_type_cb, + (void *)flags); +if (ret <0 ) +dev_warn (&device->dev, +"failed to parse _CRS method, error code %d\n" , ret); +else if (ret ==0 ) +dev_dbg (&device->dev, +"no IO and memory resources present in _CRS\n" ); +else { +resource_list_for_each_entry_safe (entry, tmp, list) { +if (entry->res->flags &IORESOURCE_DISABLED ) +resource_list_destroy_entry (entry); +else + entry->res->name = info->name; + } +acpi_pci_root_validate_resources (&device->dev, list, +IORESOURCE_MEM ); +acpi_pci_root_validate_resources (&device->dev, list, +IORESOURCE_IO ); + } + +return ret; +} + +static void pci_acpi_root_add_resources (struct acpi_pci_root_info *info) +{ +struct resource_entry *entry, *tmp; +struct resource *res, *conflict, *root =NULL ; + +resource_list_for_each_entry_safe (entry, tmp, &info->resources) { + res = entry->res; +if (res->flags &IORESOURCE_MEM ) + root = &iomem_resource; +else if (res->flags &IORESOURCE_IO ) + root = &ioport_resource; +else +continue ; + + conflict =insert_resource_conflict (root, res); +if (conflict) { +dev_info (&info->bridge->dev, +"ignoring host bridge window %pR (conflicts with %s %pR)\n" , + res, conflict->name, conflict); +resource_list_destroy_entry (entry); + } + } +} + +static void __acpi_pci_root_release_info (struct acpi_pci_root_info *info) +{ +struct resource *res; +struct resource_entry *entry, *tmp; + +if (!info) +return ; + +resource_list_for_each_entry_safe (entry, tmp, &info->resources) { + res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); +resource_list_destroy_entry (entry); + } + + info->ops->release_info (info); +} + +static void acpi_pci_root_release_info (struct pci_host_bridge *bridge)
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 809
Quoted 10 lines
drivers/acpi/pci_root.c +10
-0+{ +struct resource *res; +struct resource_entry *entry; + +resource_list_for_each_entry (entry, &bridge->windows) { + res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); + }
It is a question: is this loop necessary given that we are already releasing resources in __acpi_pci_root_release_info() ?
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 816
Quoted 7 lines
drivers/acpi/pci_root.c +7
-0+__acpi_pci_root_release_info (bridge->release_data); +} + +struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node)
I do not think you need to pass segment and node, they clutter the function signature when you can retrieve them from root, I would make them local variables and use root->segment and acpi_get_node in the function body to retrieve them.
Quoted 7 lines
drivers/acpi/pci_root.c +7
-0+{ +int ret, busnum = root->secondary.start; +struct acpi_device *device = root->device; +struct pci_bus *bus; + + info->root = root; + info->bridge = device;
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 833
Quoted 10 lines
drivers/acpi/pci_root.c +10
-0+ info->ops = ops; +INIT_LIST_HEAD (&info->resources); +snprintf (info->name,sizeof (info->name),"PCI Bus %04x:%02x" , + segment, busnum); + +if (ops->init_info && ops->init_info (info)) + goto out_release_info; + ret =acpi_pci_probe_root_resources (info); +if (ops->prepare_resources) + ret = ops->prepare_resources (info, ret);
You go through this ret passing song and dance because we may want to call prepare_resources even if acpi_pci_probe_root_resource failed (on x86), correct ? I will have a further look at x86 and ia64 if we can consolidate these ops function hooks even further.
Thanks,
Lorenzo
Quoted 63 lines
drivers/acpi/pci_root.c +23
-0+if (ret <0 ) + goto out_release_info; +else if (ret >0 ) +pci_acpi_root_add_resources (info); +pci_add_resource (&info->resources, &root->secondary); + + bus =pci_create_root_bus (NULL , busnum, ops->pci_ops, + sysdata, &info->resources); +if (bus) { +pci_scan_child_bus (bus); +pci_set_host_bridge_release (to_pci_host_bridge (bus->bridge), + acpi_pci_root_release_info, info); +if (node !=NUMA_NO_NODE ) +dev_printk (KERN_DEBUG , &bus->dev,"on NUMA node %d\n" , + node); +return bus; + } + +out_release_info: +__acpi_pci_root_release_info (info); +return NULL ; +} +void __init acpi_pci_root_init (void ) {acpi_hest_init ();include/linux/pci-acpi.h +23
-0@@ -52,6 +52,29 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)return ACPI_HANDLE (dev); } +struct acpi_pci_root ; +struct acpi_pci_root_ops ; + +struct acpi_pci_root_info { +struct acpi_pci_root *root; +struct acpi_device *bridge; +struct acpi_pci_root_ops *ops; +struct list_head resources; +char name[16 ]; +}; + +struct acpi_pci_root_ops { +struct pci_ops *pci_ops; +int (*init_info)(struct acpi_pci_root_info *info); +void (*release_info)(struct acpi_pci_root_info *info); +int (*prepare_resources)(struct acpi_pci_root_info *info,int status); +}; + +extern struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sd,int seg,int node); +void acpi_pci_add_bus (struct pci_bus *bus);void acpi_pci_remove_bus (struct pci_bus *bus);Signature · 1 line
1.7.10.4
Re: [Patch v5 4/6] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
Quoted 4 lines · Lorenzo Pieralisi
On Mon, Jun 08, 2015 at 05:20:46PM +0100, Jiang Liu wrote:
[...]
Quoted 13 lines
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 730
Quoted 9 lines
drivers/acpi/pci_root.c +9
-0+static int acpi_pci_probe_root_resources (struct acpi_pci_root_info *info) +{ +int ret; +struct list_head *list = &info->resources; +struct acpi_device *device = info->bridge; +struct resource_entry *entry, *tmp; +unsigned long flags; + + flags =IORESOURCE_IO |IORESOURCE_MEM |IORESOURCE_MEM_8AND16BIT ;Is
IORESOURCE_MEM_8AND16BITrequired because of some pending patches that will change ACPI resource filtering ? It does not seem to make a difference in the mainline code, AFAICT.
Hi Lorenzo,
Sorry, the 'IORESOURCE_MEM_8AND16BIT' is leaked into this patch
from another bugfix patch. It should be removed.
Quoted 6 lines
Quoted 6 lines
drivers/acpi/pci_root.c +6
-0+static void acpi_pci_root_release_info (struct pci_host_bridge *bridge) +{ +struct resource *res; +struct resource_entry *entry; + +resource_list_for_each_entry (entry, &bridge->windows) {
Quoted 8 lines
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 794
Quoted 5 lines
drivers/acpi/pci_root.c +5
-0+ res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); + }It is a question: is this loop necessary given that we are already releasing resources in
__acpi_pci_root_release_info()?
Function pci_create_root_bus() moves resources from info->resources list onto bridge->windows list, so an ACPI resource will be either on info->resources or bridge->windows. Thus we need to deal with both info->resources and bridge->windows.
Quoted 13 lines
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 816
Quoted 8 lines
drivers/acpi/pci_root.c +7
-0+__acpi_pci_root_release_info (bridge->release_data); +} + +struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node)I do not think you need to pass segment and node, they clutter the function signature when you can retrieve them from root, I would make them local variables and use
root->segmentandacpi_get_nodein the function body to retrieve them.
On x86, node and segment may be overridden under certain conditions. For example, segment will always be 0 if 'pci_ignore_seg' is set.
Quoted 14 lines
Quoted 14 lines
drivers/acpi/pci_root.c +14
-0+{ +int ret, busnum = root->secondary.start; +struct acpi_device *device = root->device; +struct pci_bus *bus; + + info->root = root; + info->bridge = device; + info->ops = ops; +INIT_LIST_HEAD (&info->resources); +snprintf (info->name,sizeof (info->name),"PCI Bus %04x:%02x" , + segment, busnum); + +if (ops->init_info && ops->init_info (info)) + goto out_release_info;
Quoted 8 lines
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 833
Quoted 3 lines
drivers/acpi/pci_root.c +3
-0+ ret =acpi_pci_probe_root_resources (info); +if (ops->prepare_resources) + ret = ops->prepare_resources (info, ret);You go through this ret passing song and dance because we may want to call
prepare_resourceseven ifacpi_pci_probe_root_resourcefailed (on x86), correct ? I will have a further look at x86 and ia64 if we can consolidate these ops function hooks even further.
Yes. X86 uses flag 'pci_use_crs' to choose ACPI parsed resources or other method parsed resources. This provides user a way to work around some bios issues.
Thanks!
Gerry
Re: [Patch v5 4/6] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
Quoted 1 line · Jiang Liu
On 2015/6/10 0:12, Lorenzo Pieralisi wrote:
[...]
Quoted 11 lines
Quoted 9 lines
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 816
Quoted 4 lines
drivers/acpi/pci_root.c +4
-0+struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node)I do not think you need to pass segment and node, they clutter the function signature when you can retrieve them from root, I would make them local variables and use
root->segmentandacpi_get_nodein the function body to retrieve them.On x86, node and segment may be overridden under certain conditions. For example, segment will always be 0 if '
pci_ignore_seg' is set.
Ok, so the question would be then why do you not override the value in root->segment then (actually, is it *correct* to leave the segment value in root-> unchanged even if it is overriden) ?
Anyway, node is just used for a printk, why do not you add segment and node to acpi_pci_root_info ? Just cosmetic stuff, trying to help you simplify the code, it is not easy to parse.
Thanks,
Lorenzo
Re: [Patch v5 4/6] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
Quoted 17 lines · Lorenzo Pieralisi
Quoted 1 line · Jiang Liu
On 2015/6/10 0:12, Lorenzo Pieralisi wrote:
[...]
Quoted 11 lines
Quoted 9 lines
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 816
Quoted 4 lines
drivers/acpi/pci_root.c +4
-0+struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node)I do not think you need to pass segment and node, they clutter the function signature when you can retrieve them from root, I would make them local variables and use
root->segmentandacpi_get_nodein the function body to retrieve them.On x86, node and segment may be overridden under certain conditions. For example, segment will always be 0 if '
pci_ignore_seg' is set.
Quoted 7 lines
Ok, so the question would be then why do you not override the value in
root->segmentthen (actually, is it *correct* to leave the segment value in root-> unchanged even if it is overriden) ?Anyway, node is just used for a printk, why do not you add segment and node to
acpi_pci_root_info? Just cosmetic stuff, trying to help you simplify the code, it is not easy to parse.
Hi Lorenzo,
I used the complex prototype to explicitly reminder callers
that you need to prepare all these information. If simpler interface is preferred, we could pack all ops, sysdata, segment, node, root into info, that there's will be only one parameter "info" left. The drawback is that it will add several lines of code to each arch using this interface. So prefer simpler interface or less lines of code?:) Thanks!
Gerry
Quoted 4 lines
Thanks,
Lorenzo
Lorenzo Pieralisi
Reviewed-byRe: [Patch v5 4/6] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
Hi Gerry,
Quoted 25 lines · Jiang Liu
Quoted 17 lines · Lorenzo Pieralisi
Quoted 1 line · Jiang Liu
On 2015/6/10 0:12, Lorenzo Pieralisi wrote:
[...]
Quoted 11 lines
Quoted 9 lines
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 816
Quoted 4 lines
drivers/acpi/pci_root.c +4
-0+struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node)I do not think you need to pass segment and node, they clutter the function signature when you can retrieve them from root, I would make them local variables and use
root->segmentandacpi_get_nodein the function body to retrieve them.On x86, node and segment may be overridden under certain conditions. For example, segment will always be 0 if '
pci_ignore_seg' is set.Quoted 7 lines
Ok, so the question would be then why do you not override the value in
root->segmentthen (actually, is it *correct* to leave the segment value in root-> unchanged even if it is overriden) ?Anyway, node is just used for a printk, why do not you add segment and node to
acpi_pci_root_info? Just cosmetic stuff, trying to help you simplify the code, it is not easy to parse.
Quoted 7 lines
Hi Lorenzo,
I used the complex prototype to explicitly reminder callers
that you need to prepare all these information. If simpler interface is preferred, we could pack all ops, sysdata, segment, node, root into info, that there's will be only one parameter "info" left. The drawback is that it will add several lines of code to each arch using this interface. So prefer simpler interface or less lines of code?:)
I was just referring to segment and node parameters, I think that having them in the parameters list is overkill and they can be encapsulated.
With the changes discussed previously feel free to add my:
Quoted 8 lines
Thanks!
GerryQuoted 4 lines
Thanks,
Lorenzo
Bjorn Helgaas
maintainerRe: [Patch v5 4/6] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
In reply to Jiang LiuQuoted 12 lines · Jiang Liu
Quoted 11 lines · Lorenzo Pieralisi
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 816
Quoted 4 lines · Jiang Liu
drivers/acpi/pci_root.c +4
-0+struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node)I do not think you need to pass segment and node, they clutter the function signature when you can retrieve them from root, I would make them local variables and use
root->segmentandacpi_get_nodein the function body to retrieve them.
Quoted 2 lines
On x86, node and segment may be overridden under certain conditions. For example, segment will always be 0 if '
pci_ignore_seg' is set.
I agree we should drop the segment and node parameters. Having the information in two places means we can make a mistake, and we can easily avoid that possibility by only keeping it in one place.
I think it is a bug that we don't set root->segment to zero when we have pci_ignore_seg. Sorry, that's my fault from 1f09b09b4de0 ("x86/PCI: Ignore _SEG on HP xw9300").
I don't understand the issue about overriding the node number. Can you explain it?
Bjorn Helgaas
maintainerRe: [Patch v5 4/6] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
In reply to Jiang LiuQuoted 4 lines · Jiang Liu
Introduce common interface
acpi_pci_root_create()and related data structures to create PCI root bus for ACPI PCI host bridges. It will be used to kill duplicated arch specific code for IA64 and x86. It may also help ARM64 in future.
I assume most of this code is copied from somewhere else. You should mention where it came from.
Quoted 98 lines
Tested-by Tony Luck <[email protected]>Signed-off-by Jiang Liu <[email protected]>--- drivers/acpi/pci_root.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci-acpi.h | 23 ++++++ 2 files changed, 221 insertions(+)drivers/acpi/pci_root.c +83
-0@@ -656,6 +656,204 @@ static void acpi_pci_root_remove(struct acpi_device *device)kfree (root); } +static void acpi_pci_root_validate_resources (struct device *dev, +struct list_head *resources, +unsigned long type) +{ +LIST_HEAD (list); +struct resource *res1, *res2, *root =NULL ; +struct resource_entry *tmp, *entry, *entry2; + +BUG_ON ((type & (IORESOURCE_MEM |IORESOURCE_IO )) ==0 ); + root = (type &IORESOURCE_MEM ) ? &iomem_resource : &ioport_resource; + +list_splice_init (resources, &list); +resource_list_for_each_entry_safe (entry, tmp, &list) { +bool free = false; +resource_size_t end; + + res1 = entry->res; +if (!(res1->flags & type)) + goto next; + +/* Exclude non-addressable range or non-addressable portion */ + end =min (res1->end, root->end); +if (end <= res1->start) { +dev_info (dev,"host bridge window %pR (ignored, not CPU addressable)\n" , + res1); + free = true; + goto next; + }else if (res1->end != end) { +dev_info (dev,"host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n" , + res1, (unsigned long long )end +1 , + (unsigned long long )res1->end); + res1->end = end; + } + +resource_list_for_each_entry (entry2, resources) { + res2 = entry2->res; +if (!(res2->flags & type)) +continue ; + +/* +* I don't like throwing away windows because then +* our resources no longer match the ACPI _CRS, but +* the kernel resource tree doesn't allow overlaps. +*/ +if (resource_overlaps (res1, res2)) { + res2->start =min (res1->start, res2->start); + res2->end =max (res1->end, res2->end); +dev_info (dev,"host bridge window expanded to %pR; %pR ignored\n" , + res2, res1); + free = true; + goto next; + } + } + +next: +resource_list_del (entry); +if (free) +resource_list_free_entry (entry); +else +resource_list_add_tail (entry, resources); + } +} + +static int acpi_pci_probe_root_resources (struct acpi_pci_root_info *info) +{ +int ret; +struct list_head *list = &info->resources; +struct acpi_device *device = info->bridge; +struct resource_entry *entry, *tmp; +unsigned long flags; + + flags =IORESOURCE_IO |IORESOURCE_MEM |IORESOURCE_MEM_8AND16BIT ; + ret =acpi_dev_get_resources (device, list, + acpi_dev_filter_resource_type_cb, + (void *)flags); +if (ret <0 ) +dev_warn (&device->dev, +"failed to parse _CRS method, error code %d\n" , ret); +else if (ret ==0 ) +dev_dbg (&device->dev, +"no IO and memory resources present in _CRS\n" ); +else { +resource_list_for_each_entry_safe (entry, tmp, list) {
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 751
Quoted 10 lines
drivers/acpi/pci_root.c +10
-0+if (entry->res->flags &IORESOURCE_DISABLED ) +resource_list_destroy_entry (entry); +else + entry->res->name = info->name; + } +acpi_pci_root_validate_resources (&device->dev, list, +IORESOURCE_MEM ); +acpi_pci_root_validate_resources (&device->dev, list, +IORESOURCE_IO ); + }
Please use this style to minimize indentation and return errors as soon as possible:
if (ret < 0) {
dev_warn(...)
return ret;
}
if (ret == 0) {
dev_dbg(...);
return 0;
}
resource_list_for_each_entry_safe(...) {
}
return ret;Quoted 24 lines
drivers/acpi/pci_root.c +24
-0+return ret; +} + +static void pci_acpi_root_add_resources (struct acpi_pci_root_info *info) +{ +struct resource_entry *entry, *tmp; +struct resource *res, *conflict, *root =NULL ; + +resource_list_for_each_entry_safe (entry, tmp, &info->resources) { + res = entry->res; +if (res->flags &IORESOURCE_MEM ) + root = &iomem_resource; +else if (res->flags &IORESOURCE_IO ) + root = &ioport_resource; +else +continue ; + + conflict =insert_resource_conflict (root, res); +if (conflict) { +dev_info (&info->bridge->dev, +"ignoring host bridge window %pR (conflicts with %s %pR)\n" , + res, conflict->name, conflict); +resource_list_destroy_entry (entry); + }
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 786
Quoted 10 lines
drivers/acpi/pci_root.c +10
-0+ } +} + +static void __acpi_pci_root_release_info (struct acpi_pci_root_info *info) +{ +struct resource *res; +struct resource_entry *entry, *tmp; + +if (!info) +return ;
In what circumstance could "info" be NULL? As far as I can tell, the only way it could be NULL is if we're called from acpi_pci_root_release_info() and the bridge->release_data element got scribbled on after we called pci_set_host_bridge_release(). In that case, I'd rather oops than silently return.
Quoted 36 lines
drivers/acpi/pci_root.c +36
-0+resource_list_for_each_entry_safe (entry, tmp, &info->resources) { + res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); +resource_list_destroy_entry (entry); + } + + info->ops->release_info (info); +} + +static void acpi_pci_root_release_info (struct pci_host_bridge *bridge) +{ +struct resource *res; +struct resource_entry *entry; + +resource_list_for_each_entry (entry, &bridge->windows) { + res = entry->res; +if (res->parent && + (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) +release_resource (res); + } +__acpi_pci_root_release_info (bridge->release_data); +} + +struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sysdata,int segment,int node) +{ +int ret, busnum = root->secondary.start; +struct acpi_device *device = root->device; +struct pci_bus *bus; + + info->root = root; + info->bridge = device;
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 833
Quoted 10 lines
drivers/acpi/pci_root.c +10
-0+ info->ops = ops; +INIT_LIST_HEAD (&info->resources); +snprintf (info->name,sizeof (info->name),"PCI Bus %04x:%02x" , + segment, busnum); + +if (ops->init_info && ops->init_info (info)) + goto out_release_info; + ret =acpi_pci_probe_root_resources (info); +if (ops->prepare_resources) + ret = ops->prepare_resources (info, ret);
It's goofy to pass the return value from acpi_pci_probe_root_resources() into prepare_resources(), especially since the local context gives no clue about what "ret" means.
Neither the x86 nor the ia64 version of pci_acpi_root_prepare_resources() actually does anything useful with "ret" except return it.
I think you should just make acpi_pci_probe_root_resources() void. It will create a list of zero or more resources from _CRS. If _CRS fails, you already print a diagnostic and the list will be empty. If _CRS succeeds and there are no windows, you also print a (debug) diagnostic and the list will be empty. The pci_acpi_root_prepare_resources() implementations will deal with empty lists just fine.
I think the change from your code here is that we would create the root bus and enumerate it even if _CRS fails. I think that's OK: we'll find devices and we'll learn what resources they want, but we won't have anything to assign them. I think that's OK and it will make this code simpler.
drivers/acpi/pci_root.c static void acpi_pci_root_remove(struct acpi_device *device) · line 842
Quoted 9 lines
drivers/acpi/pci_root.c +9
-0+if (ret <0 ) + goto out_release_info; +else if (ret >0 ) +pci_acpi_root_add_resources (info); +pci_add_resource (&info->resources, &root->secondary); + + bus =pci_create_root_bus (NULL , busnum, ops->pci_ops, + sysdata, &info->resources); +if (bus) {
if (!bus) goto out_release_info;
Then this code can be un-indented:
Quoted 37 lines
drivers/acpi/pci_root.c +14
-0+pci_scan_child_bus (bus); +pci_set_host_bridge_release (to_pci_host_bridge (bus->bridge), + acpi_pci_root_release_info, info); +if (node !=NUMA_NO_NODE ) +dev_printk (KERN_DEBUG , &bus->dev,"on NUMA node %d\n" , + node); +return bus; + } + +out_release_info: +__acpi_pci_root_release_info (info); +return NULL ; +} +void __init acpi_pci_root_init (void ) {acpi_hest_init ();include/linux/pci-acpi.h +12
-0@@ -52,6 +52,29 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)return ACPI_HANDLE (dev); } +struct acpi_pci_root ; +struct acpi_pci_root_ops ; + +struct acpi_pci_root_info { +struct acpi_pci_root *root; +struct acpi_device *bridge; +struct acpi_pci_root_ops *ops; +struct list_head resources; +char name[16 ]; +}; + +struct acpi_pci_root_ops {
include/linux/pci-acpi.h static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus) · line 76
Quoted 10 lines
include/linux/pci-acpi.h +10
-0+struct pci_ops *pci_ops; +int (*init_info)(struct acpi_pci_root_info *info); +void (*release_info)(struct acpi_pci_root_info *info); +int (*prepare_resources)(struct acpi_pci_root_info *info,int status); +}; + +extern struct pci_bus *acpi_pci_root_create (struct acpi_pci_root *root, +struct acpi_pci_root_ops *ops, +struct acpi_pci_root_info *info, +void *sd,int seg,int node);
Drop the "extern" here to match the declarations below. I know there's still a mix in this file, but my preference is to skip them in new code.
Quoted 10 lines
include/linux/pci-acpi.h +0
-0void acpi_pci_add_bus (struct pci_bus *bus);void acpi_pci_remove_bus (struct pci_bus *bus);Signature · 6 lines
1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v5 5/6 x86/PCI/ACPI: Use common interface to support PCI host bridge
In reply to Jiang LiuUse common interface to simplify ACPI PCI host bridge implementation.
--- arch/x86/pci/acpi.c | 294 +++++++++++++++------------------------------------ 1 file changed, 87 insertions(+), 207 deletions(-)
arch/x86/pci/acpi.c +87 -207
@@ -4,16 +4,15 @@#include <linux/irq.h> #include <linux/dmi.h> #include <linux/slab.h> +#include <linux/pci-acpi.h> #include <asm/numa.h> #include <asm/pci_x86.h> struct pci_root_info { -struct acpi_device *bridge; -char name[16 ]; +struct acpi_pci_root_info common;struct pci_sysdata sd;#ifdef CONFIG_PCI_MMCONFIG bool mcfg_added; -u16 segment;u8 start_bus;u8 end_bus;#endif
@@ -165,15 +164,18 @@ static int check_segment(u16 seg, struct device *dev, char *estr)return 0 ; } -static int setup_mcfg_map (struct pci_root_info *info,u16 seg,u8 start, -u8 end,phys_addr_t addr) +static int setup_mcfg_map (struct acpi_pci_root_info *ci) { -int result; -struct device *dev = &info->bridge->dev; +int result, seg; +struct pci_root_info *info; +struct acpi_pci_root *root = ci->root; +struct device *dev = &ci->bridge->dev; - info->start_bus = start; - info->end_bus = end; + info = container_of(ci, struct pci_root_info, common); + info->start_bus = (u8 )root->secondary.start; + info->end_bus = (u8 )root->secondary.end; info->mcfg_added = false; + seg = info->sd.domain;/* return success if MMCFG is not in use */ if (raw_pci_ext_ops && raw_pci_ext_ops != &pci_mmcfg)
@@ -182,7 +184,8 @@ static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,if (!(pci_probe &PCI_PROBE_MMCONF ))return check_segment (seg, dev,"MMCONFIG is disabled," ); - result =pci_mmconfig_insert (dev, seg, start, end, addr); + result =pci_mmconfig_insert (dev, seg, info->start_bus, info->end_bus, + root->mcfg_addr);if (result ==0 ) {/* enable MMCFG if it hasn't been enabled yet */ if (raw_pci_ext_ops ==NULL )
@@ -195,134 +198,55 @@ static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,return 0 ; } -static void teardown_mcfg_map (struct pci_root_info *info) +static void teardown_mcfg_map (struct acpi_pci_root_info *ci) { +struct pci_root_info *info; + + info = container_of(ci, struct pci_root_info, common);if (info->mcfg_added) { -pci_mmconfig_delete (info->segment, info->start_bus, - info->end_bus); +pci_mmconfig_delete (info->sd.domain, + info->start_bus, info->end_bus); info->mcfg_added = false; } }#else -static int setup_mcfg_map (struct pci_root_info *info, -u16 seg,u8 start,u8 end, -phys_addr_t addr) +static int setup_mcfg_map (struct acpi_pci_root_info *ci) {return 0 ; } -static void teardown_mcfg_map (struct pci_root_info *info) + +static void teardown_mcfg_map (struct acpi_pci_root_info *ci) { }#endif -static void validate_resources (struct device *dev,struct list_head *crs_res, -unsigned long type) +static int pci_acpi_root_get_node (struct acpi_pci_root *root) { -LIST_HEAD (list); -struct resource *res1, *res2, *root =NULL ; -struct resource_entry *tmp, *entry, *entry2; - -BUG_ON ((type & (IORESOURCE_MEM |IORESOURCE_IO )) ==0 ); - root = (type &IORESOURCE_MEM ) ? &iomem_resource : &ioport_resource; - -list_splice_init (crs_res, &list); -resource_list_for_each_entry_safe (entry, tmp, &list) { -bool free = false; -resource_size_t end; - - res1 = entry->res; -if (!(res1->flags & type)) - goto next; - -/* Exclude non-addressable range or non-addressable portion */ - end =min (res1->end, root->end); -if (end <= res1->start) { -dev_info (dev,"host bridge window %pR (ignored, not CPU addressable)\n" , - res1); - free = true; - goto next; - }else if (res1->end != end) { -dev_info (dev,"host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n" , - res1, (unsigned long long )end +1 , - (unsigned long long )res1->end); - res1->end = end; - } - -resource_list_for_each_entry (entry2, crs_res) { - res2 = entry2->res; -if (!(res2->flags & type)) -continue ; - -/* -* I don't like throwing away windows because then -* our resources no longer match the ACPI _CRS, but -* the kernel resource tree doesn't allow overlaps. -*/ -if (resource_overlaps (res1, res2)) { - res2->start =min (res1->start, res2->start); - res2->end =max (res1->end, res2->end); -dev_info (dev,"host bridge window expanded to %pR; %pR ignored\n" , - res2, res1); - free = true; - goto next; - } - } +int busnum = root->secondary.start; +struct acpi_device *device = root->device; +int node =acpi_get_node (device->handle); -next: -resource_list_del (entry); -if (free) -resource_list_free_entry (entry); -else -resource_list_add_tail (entry, crs_res); +if (node ==NUMA_NO_NODE ) { + node =x86_pci_root_bus_node (busnum); +if (node !=0 && node !=NUMA_NO_NODE ) +dev_info (&device->dev,FW_BUG "no _PXM; falling back to node %d from hardware (may be inconsistent with ACPI node numbers)\n" , + node); } +if (node !=NUMA_NO_NODE && !node_online (node)) + node =NUMA_NO_NODE ; + +return node; } -static void add_resources (struct pci_root_info *info, -struct list_head *resources, -struct list_head *crs_res) +static int pci_acpi_root_init_info (struct acpi_pci_root_info *ci) { -struct resource_entry *entry, *tmp; -struct resource *res, *conflict, *root =NULL ; - -validate_resources (&info->bridge->dev, crs_res,IORESOURCE_MEM ); -validate_resources (&info->bridge->dev, crs_res,IORESOURCE_IO ); - -resource_list_for_each_entry_safe (entry, tmp, crs_res) { - res = entry->res; -if (res->flags &IORESOURCE_MEM ) - root = &iomem_resource; -else if (res->flags &IORESOURCE_IO ) - root = &ioport_resource; -else -BUG_ON (res); - - conflict =insert_resource_conflict (root, res); -if (conflict) { -dev_info (&info->bridge->dev, -"ignoring host bridge window %pR (conflicts with %s %pR)\n" , - res, conflict->name, conflict); -resource_list_destroy_entry (entry); - } - } - -list_splice_tail (crs_res, resources); +return setup_mcfg_map (ci); } -static void release_pci_root_info (struct pci_host_bridge *bridge) +static void pci_acpi_root_release_info (struct acpi_pci_root_info *ci) { -struct resource *res; -struct resource_entry *entry; -struct pci_root_info *info = bridge->release_data; - -resource_list_for_each_entry (entry, &bridge->windows) { - res = entry->res; -if (res->parent && - (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) -release_resource (res); - } - -teardown_mcfg_map (info); -kfree (info); +teardown_mcfg_map (ci); +kfree (container_of(ci ,struct pci_root_info ,common )); }/*
@@ -345,47 +269,43 @@ static bool resource_is_pcicfg_ioport(struct resource *res) res->start ==0xCF8 && res->end ==0xCFF ; } -static void probe_pci_root_info (struct pci_root_info *info, -struct acpi_device *device, -int busnum,int domain, -struct list_head *list) +static int pci_acpi_root_prepare_resources (struct acpi_pci_root_info *ci, +int status) { -int ret; +struct acpi_device *device = ci->bridge; +int busnum = ci->root->secondary.start;struct resource_entry *entry, *tmp; -sprintf (info->name,"PCI Bus %04x:%02x" , domain, busnum); - info->bridge = device; - ret =acpi_dev_get_resources (device, list, - acpi_dev_filter_resource_type_cb, - (void *)(IORESOURCE_IO |IORESOURCE_MEM )); -if (ret <0 ) -dev_warn (&device->dev, -"failed to parse _CRS method, error code %d\n" , ret); -else if (ret ==0 ) -dev_dbg (&device->dev, -"no IO and memory resources present in _CRS\n" ); -else -resource_list_for_each_entry_safe (entry, tmp, list) { -if ((entry->res->flags &IORESOURCE_DISABLED ) || -resource_is_pcicfg_ioport (entry->res)) +if (pci_use_crs) { +resource_list_for_each_entry_safe (entry, tmp, &ci->resources) +if (resource_is_pcicfg_ioport (entry->res))resource_list_destroy_entry (entry); -else - entry->res->name = info->name; - } +return status; + } + +resource_list_for_each_entry_safe (entry, tmp, &ci->resources) { +dev_printk (KERN_DEBUG , &device->dev, +"host bridge window %pR (ignored)\n" , entry->res); +resource_list_destroy_entry (entry); + } +x86_pci_root_bus_resources (busnum, &ci->resources); + +return 0 ; } +static struct acpi_pci_root_ops acpi_pci_root_ops = { + .pci_ops = &pci_root_ops, + .init_info = pci_acpi_root_init_info, + .release_info = pci_acpi_root_release_info, + .prepare_resources = pci_acpi_root_prepare_resources, +}; +struct pci_bus *pci_acpi_scan_root (struct acpi_pci_root *root) { -struct acpi_device *device = root->device; -struct pci_root_info *info;int domain = root->segment;int busnum = root->secondary.start; -struct resource_entry *res_entry; -LIST_HEAD (crs_res); -LIST_HEAD (resources); +int node =pci_acpi_root_get_node (root);struct pci_bus *bus; -struct pci_sysdata *sd; -int node;if (pci_ignore_seg) domain =0 ;
@@ -397,71 +317,34 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)return NULL ; } - node =acpi_get_node (device->handle); -if (node ==NUMA_NO_NODE ) { - node =x86_pci_root_bus_node (busnum); -if (node !=0 && node !=NUMA_NO_NODE ) -dev_info (&device->dev,FW_BUG "no _PXM; falling back to node %d from hardware (may be inconsistent with ACPI node numbers)\n" , - node); - } - -if (node !=NUMA_NO_NODE && !node_online (node)) - node =NUMA_NO_NODE ; - - info =kzalloc_node (sizeof (*info),GFP_KERNEL , node); -if (!info) { -printk (KERN_WARNING "pci_bus %04x:%02x: " -"ignored (out of memory)\n" , domain, busnum); -return NULL ; - } - - sd = &info->sd; - sd->domain = domain; - sd->node = node; - sd->companion = device; - bus =pci_find_bus (domain, busnum);if (bus) {/* * If the desired bus has been scanned already, replace * its bus->sysdata. */ -memcpy (bus->sysdata, sd,sizeof (*sd)); -kfree (info); - }else { -/* insert busn res at first */ -pci_add_resource (&resources, &root->secondary); +struct pci_sysdata sd = { + .domain = domain, + .node = node, + .companion = root->device + }; -/* -* _CRS with no apertures is normal, so only fall back to -* defaults or native bridge info if we're ignoring _CRS. -*/ -probe_pci_root_info (info, device, busnum, domain, &crs_res); -if (pci_use_crs) { -add_resources (info, &resources, &crs_res); - }else { -resource_list_for_each_entry (res_entry, &crs_res) -dev_printk (KERN_DEBUG , &device->dev, -"host bridge window %pR (ignored)\n" , - res_entry->res); -resource_list_free (&crs_res); -x86_pci_root_bus_resources (busnum, &resources); - } - -if (!setup_mcfg_map (info, domain, (u8 )root->secondary.start, - (u8 )root->secondary.end, root->mcfg_addr)) - bus =pci_create_root_bus (NULL , busnum, &pci_root_ops, - sd, &resources); - -if (bus) { -pci_scan_child_bus (bus); -pci_set_host_bridge_release ( -to_pci_host_bridge (bus->bridge), - release_pci_root_info, info); - }else { -resource_list_free (&resources); -teardown_mcfg_map (info); -kfree (info); +memcpy (bus->sysdata, &sd,sizeof (sd)); + }else { +struct pci_root_info *info; + + info =kzalloc_node (sizeof (*info),GFP_KERNEL , node); +if (!info) +dev_err (&root->device->dev, +"pci_bus %04x:%02x: ignored (out of memory)\n" , + domain, busnum); +else { + info->sd.domain = domain; + info->sd.node = node; + info->sd.companion = root->device; + bus =acpi_pci_root_create (root, &acpi_pci_root_ops, + &info->common, &info->sd, + domain, node); } }
@@ -474,9 +357,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)pcie_bus_configure_settings (child); } -if (bus && node !=NUMA_NO_NODE ) -dev_printk (KERN_DEBUG , &bus->dev,"on NUMA node %d\n" , node); -return bus; }
Signature · 1 line
1.7.10.4
PATCH v5 6/6 ia64/PCI/ACPI: Use common interface to support PCI host bridge
In reply to Jiang LiuUse common interface to simplify PCI host bridge implementation.
--- arch/ia64/pci/pci.c | 229 +++++++++++---------------------------------------- 1 file changed, 46 insertions(+), 183 deletions(-)
arch/ia64/pci/pci.c +46 -183
@@ -116,15 +116,12 @@ struct pci_ops pci_root_ops = { };struct pci_root_info { +struct acpi_pci_root_info common;struct pci_controller controller; -struct acpi_device *bridge; -struct list_head resources;struct list_head io_resources; -char name[16 ]; }; -static unsigned int -new_space (u64 phys_base,int sparse) +static unsigned int new_space (u64 phys_base,int sparse) {u64 mmio_base;int i;
@@ -160,11 +157,11 @@ static int add_io_space(struct device *dev, struct pci_root_info *info,unsigned long base, min, max, base_port;unsigned int sparse =0 , space_nr, len; - len =strlen (info->name) +32 ; + len =strlen (info->common.name) +32 ; iospace =resource_list_create_entry (NULL , len);if (!iospace) {dev_err (dev,"PCI: No memory for %s I/O port space\n" , - info->name); + info->common.name);return -ENOMEM ; }
@@ -179,7 +176,7 @@ static int add_io_space(struct device *dev, struct pci_root_info *info, max = res->end - entry->offset; base =__pa (io_space[space_nr].mmio_base); base_port =IO_SPACE_BASE (space_nr); -snprintf (name, len,"%s I/O Ports %08lx-%08lx" , info->name, +snprintf (name, len,"%s I/O Ports %08lx-%08lx" , info->common.name, base_port + min, base_port + max);/*
@@ -234,217 +231,83 @@ static bool resource_is_pcicfg_ioport(struct resource *res) res->start ==0xCF8 && res->end ==0xCFF ; } -static int -probe_pci_root_info (struct pci_root_info *info,struct acpi_device *device, -int busnum,int domain) +static int pci_acpi_root_prepare_resources (struct acpi_pci_root_info *ci, +int status) { -int ret; -struct list_head *list = &info->resources; +struct device *dev = &ci->bridge->dev; +struct pci_root_info *info; +struct resource *res;struct resource_entry *entry, *tmp; - ret =acpi_dev_get_resources (device, list, - acpi_dev_filter_resource_type_cb, - (void *)(IORESOURCE_IO |IORESOURCE_MEM )); -if (ret <0 ) -dev_warn (&device->dev, -"failed to parse _CRS method, error code %d\n" , ret); -else if (ret ==0 ) -dev_dbg (&device->dev, -"no IO and memory resources present in _CRS\n" ); -else -resource_list_for_each_entry_safe (entry, tmp, list) { -if ((entry->res->flags &IORESOURCE_DISABLED ) || -resource_is_pcicfg_ioport (entry->res)) -resource_list_destroy_entry (entry); -else - entry->res->name = info->name; - } - -return ret; -} - -static void validate_resources (struct device *dev,struct list_head *resources, -unsigned long type) -{ -LIST_HEAD (list); -struct resource *res1, *res2, *root =NULL ; -struct resource_entry *tmp, *entry, *entry2; - -BUG_ON ((type & (IORESOURCE_MEM |IORESOURCE_IO )) ==0 ); - root = (type &IORESOURCE_MEM ) ? &iomem_resource : &ioport_resource; - -list_splice_init (resources, &list); -resource_list_for_each_entry_safe (entry, tmp, &list) { -bool free = false; -resource_size_t end; - - res1 = entry->res; -if (!(res1->flags & type)) - goto next; - -/* Exclude non-addressable range or non-addressable portion */ - end =min (res1->end, root->end); -if (end <= res1->start) { -dev_info (dev,"host bridge window %pR (ignored, not CPU addressable)\n" , - res1); - free = true; - goto next; - }else if (res1->end != end) { -dev_info (dev,"host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n" , - res1, (unsigned long long )end +1 , - (unsigned long long )res1->end); - res1->end = end; - } - -resource_list_for_each_entry (entry2, resources) { - res2 = entry2->res; -if (!(res2->flags & type)) -continue ; - -/* -* I don't like throwing away windows because then -* our resources no longer match the ACPI _CRS, but -* the kernel resource tree doesn't allow overlaps. -*/ -if (resource_overlaps (res1, res2)) { - res2->start =min (res1->start, res2->start); - res2->end =max (res1->end, res2->end); -dev_info (dev,"host bridge window expanded to %pR; %pR ignored\n" , - res2, res1); - free = true; - goto next; +if (status >0 ) { + info = container_of(ci, struct pci_root_info, common); +resource_list_for_each_entry_safe (entry, tmp, &ci->resources) { + res = entry->res; +if (res->flags &IORESOURCE_MEM ) { +/* +* HP's firmware has a hack to work around a +* Windows bug. Ignore these tiny memory ranges. +*/ +if (resource_size (res) <=16 ) { +resource_list_del (entry); +insert_resource (&iomem_resource, + entry->res); +resource_list_add_tail (entry, + &info->io_resources); + } + }else if (res->flags &IORESOURCE_IO ) { +if (resource_is_pcicfg_ioport (entry->res)) +resource_list_destroy_entry (entry); +else if (add_io_space (dev, info, entry)) +resource_list_destroy_entry (entry); } } - -next: -resource_list_del (entry); -if (free) -resource_list_free_entry (entry); -else -resource_list_add_tail (entry, resources); } -} -static void add_resources (struct pci_root_info *info,struct device *dev) -{ -struct resource_entry *entry, *tmp; -struct resource *res, *conflict, *root =NULL ; -struct list_head *list = &info->resources; - -validate_resources (dev, list,IORESOURCE_MEM ); -validate_resources (dev, list,IORESOURCE_IO ); - -resource_list_for_each_entry_safe (entry, tmp, list) { - res = entry->res; -if (res->flags &IORESOURCE_MEM ) { - root = &iomem_resource; -/* -* HP's firmware has a hack to work around a Windows -* bug. Ignore these tiny memory ranges. -*/ -if (resource_size (res) <=16 ) { -resource_list_destroy_entry (entry); -continue ; - } - }else if (res->flags &IORESOURCE_IO ) { - root = &ioport_resource; -if (add_io_space (&info->bridge->dev, info, entry)) { -resource_list_destroy_entry (entry); -continue ; - } - }else { -BUG_ON (res); - } - - conflict =insert_resource_conflict (root, res); -if (conflict) { -dev_info (dev, -"ignoring host bridge window %pR (conflicts with %s %pR)\n" , - res, conflict->name, conflict); -resource_list_destroy_entry (entry); - } - } +return status; } -static void __release_pci_root_info (struct pci_root_info *info) +static void pci_acpi_root_release_info (struct acpi_pci_root_info *ci) { -struct resource *res; -struct resource_entry *entry, *tentry; +struct pci_root_info *info; +struct resource_entry *entry, *tmp; -resource_list_for_each_entry_safe (entry, tentry, &info->io_resources) { + info = container_of(ci, struct pci_root_info, common); +resource_list_for_each_entry_safe (entry, tmp, &info->io_resources) {release_resource (entry->res);resource_list_destroy_entry (entry); } - -resource_list_for_each_entry_safe (entry, tentry, &info->resources) { - res = entry->res; -if (res->parent && - (res->flags & (IORESOURCE_MEM |IORESOURCE_IO ))) -release_resource (res); -resource_list_destroy_entry (entry); - } -kfree (info); } -static void release_pci_root_info (struct pci_host_bridge *bridge) -{ -struct pci_root_info *info = bridge->release_data; - -__release_pci_root_info (info); -} +static struct acpi_pci_root_ops pci_acpi_root_ops = { + .pci_ops = &pci_root_ops, + .release_info = pci_acpi_root_release_info, + .prepare_resources = pci_acpi_root_prepare_resources, +};struct pci_bus *pci_acpi_scan_root (struct acpi_pci_root *root) {struct acpi_device *device = root->device;int domain = root->segment; -int bus = root->secondary.start;struct pci_root_info *info; -struct pci_bus *pbus; -int ret; info =kzalloc (sizeof (*info),GFP_KERNEL );if (!info) {dev_err (&device->dev,"pci_bus %04x:%02x: ignored (out of memory)\n" , - domain, bus); + domain, (int )root->secondary.start);return NULL ; } info->controller.segment = domain; info->controller.companion = device; info->controller.node =acpi_get_node (device->handle); - info->bridge = device; -INIT_LIST_HEAD (&info->resources);INIT_LIST_HEAD (&info->io_resources); -snprintf (info->name,sizeof (info->name), -"PCI Bus %04x:%02x" , domain, bus); - - ret =probe_pci_root_info (info, device, bus, domain); -if (ret <=0 ) { -kfree (info); -return NULL ; - } -add_resources (info, &info->bridge->dev); -pci_add_resource (&info->resources, &root->secondary); - -/* -* See arch/x86/pci/acpi.c. -* The desired pci bus might already be scanned in a quirk. We -* should handle the case here, but it appears that IA64 hasn't -* such quirk. So we just ignore the case now. -*/ - pbus =pci_create_root_bus (NULL , bus, &pci_root_ops, - &info->controller, &info->resources); -if (!pbus) { -__release_pci_root_info (info); -return NULL ; - } -pci_set_host_bridge_release (to_pci_host_bridge (pbus->bridge), - release_pci_root_info, info); -pci_scan_child_bus (pbus); -return pbus; +return acpi_pci_root_create (root, &pci_acpi_root_ops, &info->common, + &info->controller, domain, + info->controller.node); }int pcibios_root_bridge_prepare (struct pci_host_bridge *bridge)
Signature · 1 line
1.7.10.4
Hi Bjorn, Rafael,
Just kindly ping for this patchset, it has been reviewed and tested, can it be merged in 4.3?
ARM64 ACPI PCI patches is based on this one, if this patchset is merged, there will be less dependence for ARM64 ACPI PCI.
Thanks
Hanjun
Quoted 46 lines · Jiang Liu
This patch set consolidates common code to support ACPI PCI root on x86 and IA64 platforms into ACPI core, to reproduce duplicated code and simplify maintenance. And a patch set based on previous version to support ACPI based PCIe host bridge on ARM64 has been posted at:
https://lkml.org/lkml/2015/5/26/207It's based on latest mainstream kernel. It passes Fengguang's 0day test suite and has been tested on two IA64 platforms and one x86 platform.
V4-V5:
1) As suggested by Lorenzo Pieralisi <[email protected]>, do not introduce "structpci_controller" into genericdrivers/acpi/pci_root.c, so ARM64 doesn't need to define a useless structure. And it simplifies code a little too.
V3->V4:
1) Add patch[05/08] support solve building issue on ARM64
2) Solve an implicitly pointer cast issue.
3) Rebase to latest mainstream kernel
V2->V3:
1. Move memory allocation/free from ACPI core into arch
2. Kill the field 'segment' in structpci_root_infoon x86Thanks!
GerryJiang Liu (6):
ACPI/PCI: Enhance ACPI core to support sparse IO space ia64/PCI/ACPI: Use common ACPI resource parsing interface for host
bridge
ia64/PCI: Use common structresource_entryto replace struct
iospace_resource
PCI/ACPI: Consolidate common PCI host bridge code into ACPI core x86/PCI/ACPI: Use common interface to support PCI host bridge ia64/PCI/ACPI: Use common interface to support PCI host bridgearch/ia64/include/asm/pci.h | 5 - arch/ia64/pci/pci.c | 366 ++++++++++++------------------------------- arch/x86/pci/acpi.c | 294 ++++++++++------------------------ drivers/acpi/pci_root.c | 198 +++++++++++++++++++++++ drivers/acpi/resource.c | 9 +- include/linux/ioport.h | 1 + include/linux/pci-acpi.h | 23 +++ 7 files changed, 415 insertions(+), 481 deletions(-)
Bjorn Helgaas
maintainerQuoted 4 lines · Hanjun Guo
Hi Bjorn, Rafael,
Just kindly ping for this patchset, it has been reviewed and tested, can it be merged in 4.3?
There are open review issues, e.g., IORESOURCE_MEM_8AND16BIT, so I'm not ready to merge it as-is. I'll look at it some more, but we definitely need some revisions.
Quoted 53 lines
ARM64 ACPI PCI patches is based on this one, if this patchset is merged, there will be less dependence for ARM64 ACPI PCI.
Thanks
HanjunQuoted 46 lines · Jiang Liu
This patch set consolidates common code to support ACPI PCI root on x86 and IA64 platforms into ACPI core, to reproduce duplicated code and simplify maintenance. And a patch set based on previous version to support ACPI based PCIe host bridge on ARM64 has been posted at:
https://lkml.org/lkml/2015/5/26/207It's based on latest mainstream kernel. It passes Fengguang's 0day test suite and has been tested on two IA64 platforms and one x86 platform.
V4-V5:
1) As suggested by Lorenzo Pieralisi <[email protected]>, do not introduce "structpci_controller" into genericdrivers/acpi/pci_root.c, so ARM64 doesn't need to define a useless structure. And it simplifies code a little too.
V3->V4:
1) Add patch[05/08] support solve building issue on ARM64
2) Solve an implicitly pointer cast issue.
3) Rebase to latest mainstream kernel
V2->V3:
1. Move memory allocation/free from ACPI core into arch
2. Kill the field 'segment' in structpci_root_infoon x86Thanks!
GerryJiang Liu (6):
ACPI/PCI: Enhance ACPI core to support sparse IO space ia64/PCI/ACPI: Use common ACPI resource parsing interface for host
bridge
ia64/PCI: Use common structresource_entryto replace struct
iospace_resource
PCI/ACPI: Consolidate common PCI host bridge code into ACPI core x86/PCI/ACPI: Use common interface to support PCI host bridge ia64/PCI/ACPI: Use common interface to support PCI host bridgearch/ia64/include/asm/pci.h | 5 - arch/ia64/pci/pci.c | 366 ++++++++++++------------------------------- arch/x86/pci/acpi.c | 294 ++++++++++------------------------ drivers/acpi/pci_root.c | 198 +++++++++++++++++++++++ drivers/acpi/resource.c | 9 +- include/linux/ioport.h | 1 + include/linux/pci-acpi.h | 23 +++ 7 files changed, 415 insertions(+), 481 deletions(-)
Quoted 9 lines · Bjorn Helgaas
Quoted 4 lines · Hanjun Guo
Hi Bjorn, Rafael,
Just kindly ping for this patchset, it has been reviewed and tested, can it be merged in 4.3?
There are open review issues, e.g.,
IORESOURCE_MEM_8AND16BIT, so I'm not ready to merge it as-is. I'll look at it some more, but we definitely need some revisions.
Hi Bjorn,
Thanks for review and I will handle all review comments in
coming days.
Regards!
Gerry