diff mbox

[RFC,v2,5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

Message ID 1430793970-11159-6-git-send-email-jiang.liu@linux.intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Jiang Liu May 5, 2015, 2:46 a.m. UTC
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.

Tested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/pci_root.c  |  214 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |   24 ++++++
 2 files changed, 238 insertions(+)

Comments

Hanjun Guo May 11, 2015, 1:36 p.m. UTC | #1
Hi Jiang,

I have comments inline.

On 2015?05?05? 10:46, Jiang Liu wrote:
> 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.
>
> Tested-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>   drivers/acpi/pci_root.c  |  214 ++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/pci-acpi.h |   24 ++++++
>   2 files changed, 238 insertions(+)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 1b5569c092c6..97c260959a54 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -656,6 +656,220 @@ 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_common *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_common *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_common *info)
> +{
> +	struct resource *res;
> +	struct resource_entry *entry, *tmp;
> +
> +	if (!info)
> +		return;
> +
> +	if (info->ops && info->ops->release_info)
> +		info->ops->release_info(info);
> +
> +	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);
> +	}
> +
> +	kfree(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,
> +				     size_t extra_size)
> +{
> +	int ret, busnum = root->secondary.start;
> +	struct acpi_device *device = root->device;
> +	struct acpi_pci_root_info_common *info;
> +	struct pci_bus *bus;
> +
> +	info = kzalloc(sizeof(*info) + extra_size, GFP_KERNEL);

For x86, the memory is allocated on its local numa node if memory
is available by using kzalloc_node(), and if
node = acpi_get_node(device->handle) is NUMA_NO_NODE, the code will
get the numa node info by using x86_pci_root_bus_node() (which you
consolidate them as a function pci_acpi_root_get_node() in later
patch).

but the code here just ignore that information, so the code
here has functional change for x86 code since you didn't use numa
information.

I'm not sure how frequently used for the info after init, so
not sure about the performance impact, but I think we should
keep consistence with the code behavior as before.

Further more, there is a implicit cast for
struct acpi_pci_root_info_common *info to arch specific
struct pci_root_info *info by using extra size, it's not
easy to understand (at least for me :) ), so how about
alloc the memory in arch specific function, and pass
struct acpi_pci_root_info_common *info fr this function?

other than that, this code is pretty good, I reworked ARM64
ACPI based PCI host bridge init code, and this patch simplified
the coed a lot, will have a test tomorrow and let you know
the result.

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu May 13, 2015, 5:36 a.m. UTC | #2
On 2015/5/11 21:36, Hanjun Guo wrote:
> Hi Jiang,
> 
> I have comments inline.
> 
> On 2015?05?05? 10:46, Jiang Liu wrote:
>> 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.
>>
>> Tested-by: Tony Luck <tony.luck@intel.com>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>> +struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>> +                     struct acpi_pci_root_ops *ops,
>> +                     size_t extra_size)
>> +{
>> +    int ret, busnum = root->secondary.start;
>> +    struct acpi_device *device = root->device;
>> +    struct acpi_pci_root_info_common *info;
>> +    struct pci_bus *bus;
>> +
>> +    info = kzalloc(sizeof(*info) + extra_size, GFP_KERNEL);
> 
> For x86, the memory is allocated on its local numa node if memory
> is available by using kzalloc_node(), and if
> node = acpi_get_node(device->handle) is NUMA_NO_NODE, the code will
> get the numa node info by using x86_pci_root_bus_node() (which you
> consolidate them as a function pci_acpi_root_get_node() in later
> patch).
> 
> but the code here just ignore that information, so the code
> here has functional change for x86 code since you didn't use numa
> information.
> 
> I'm not sure how frequently used for the info after init, so
> not sure about the performance impact, but I think we should
> keep consistence with the code behavior as before.
Hi Hanjun,
	Good catch, will change code to respect NUMA node
info when allocating memory.

> 
> Further more, there is a implicit cast for
> struct acpi_pci_root_info_common *info to arch specific
> struct pci_root_info *info by using extra size, it's not
> easy to understand (at least for me :) ), so how about
> alloc the memory in arch specific function, and pass
> struct acpi_pci_root_info_common *info fr this function?
Good suggestion.
Thanks!
Gerry

> other than that, this code is pretty good, I reworked ARM64
> ACPI based PCI host bridge init code, and this patch simplified
> the coed a lot, will have a test tomorrow and let you know
> the result.
> 
> Thanks
> Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hanjun Guo May 13, 2015, 9:29 a.m. UTC | #3
Hi Jiang,

On 2015?05?05? 10:46, Jiang Liu wrote:
> 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.
>
[...]
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index a965efa52152..a292ee33d74b 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -52,6 +52,30 @@ 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_common {
> +	struct pci_controller		controller;

There is another problem that this patch will lead to
compile error on ARM64 since ARM64 has basic ACPI support
in 4.1.

struct pci_controller		controller is not available
on ARM64, that's the reason why compile errors happens on ARM64.

How about move struct pci_controller to this head file?

because all the related file you changed in this patch set
are only compiled when CONFI_ACPI=y, so for x86,

struct pci_controller {
#ifdef CONFIG_ACPI
         struct acpi_device *companion;  /* ACPI companion device */
#endif
#ifdef CONFIG_X86_64
         void            *iommu;         /* IOMMU private data */
#endif
         int             segment;        /* PCI domain */
         int             node;           /* NUMA node */
};

I'm sure #ifdef CONFIG_ACPI .. #endif can be removed
with no harm, and for *iommu, we can remove the #ifdef CONFIG_X86_64
with introducing little more memory on x86_32, after
that, the struct pci_controller is almost the same as ia64:

struct pci_controller {
         struct acpi_device *companion;
         void *iommu;
         int segment;
         int node;               /* nearest node with memory or 
NUMA_NO_NODE for global allocation */

         void *platform_data;
};

except void *platform_data;

On ARM64, the structure is almost the same, so how about
introduce

struct pci_controller {
         struct acpi_device *companion;  /* ACPI companion device */
         void            *iommu;         /* IOMMU private data */
         int             segment;        /* PCI domain */
         int             node;           /* NUMA node */
#ifdef CONFIG_IA64	
	void *platform_data;
#endif
};

in this file, then can be used for all architectures?

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu May 13, 2015, 12:24 p.m. UTC | #4
On 2015/5/13 17:29, Hanjun Guo wrote:
> Hi Jiang,
> 
> On 2015?05?05? 10:46, Jiang Liu wrote:
>> 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.
>>
> [...]
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index a965efa52152..a292ee33d74b 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -52,6 +52,30 @@ 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_common {
>> +    struct pci_controller        controller;
> 
> There is another problem that this patch will lead to
> compile error on ARM64 since ARM64 has basic ACPI support
> in 4.1.
> 
> struct pci_controller        controller is not available
> on ARM64, that's the reason why compile errors happens on ARM64.
> 
> How about move struct pci_controller to this head file?
> 
> because all the related file you changed in this patch set
> are only compiled when CONFI_ACPI=y, so for x86,
> 
> struct pci_controller {
> #ifdef CONFIG_ACPI
>         struct acpi_device *companion;  /* ACPI companion device */
> #endif
> #ifdef CONFIG_X86_64
>         void            *iommu;         /* IOMMU private data */
> #endif
>         int             segment;        /* PCI domain */
>         int             node;           /* NUMA node */
> };
> 
> I'm sure #ifdef CONFIG_ACPI .. #endif can be removed
> with no harm, and for *iommu, we can remove the #ifdef CONFIG_X86_64
> with introducing little more memory on x86_32, after
> that, the struct pci_controller is almost the same as ia64:

On x86, struct pci_controller may be used when CONFIG_ACPI is disabled.
So we can't move it into pci-acpi.h

> 
> struct pci_controller {
>         struct acpi_device *companion;
>         void *iommu;
>         int segment;
>         int node;               /* nearest node with memory or
> NUMA_NO_NODE for global allocation */
> 
>         void *platform_data;
> };
> 
> except void *platform_data;
> 
> On ARM64, the structure is almost the same, so how about
> introduce
> 
> struct pci_controller {
>         struct acpi_device *companion;  /* ACPI companion device */
>         void            *iommu;         /* IOMMU private data */
>         int             segment;        /* PCI domain */
>         int             node;           /* NUMA node */
> #ifdef CONFIG_IA64   
>     void *platform_data;
> #endif
> };
> 
> in this file, then can be used for all architectures?
Current mode is that architecture defines its own version of
struct pci_controller. It would be better to keep this pattern.
Thanks!
Gerry

> 
> Thanks
> Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hanjun Guo May 13, 2015, 1:25 p.m. UTC | #5
On 2015?05?13? 20:24, Jiang Liu wrote:
> On 2015/5/13 17:29, Hanjun Guo wrote:
>> Hi Jiang,
>>
>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>> 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.
>>>
>> [...]
>>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>>> index a965efa52152..a292ee33d74b 100644
>>> --- a/include/linux/pci-acpi.h
>>> +++ b/include/linux/pci-acpi.h
>>> @@ -52,6 +52,30 @@ 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_common {
>>> +    struct pci_controller        controller;
>>
>> There is another problem that this patch will lead to
>> compile error on ARM64 since ARM64 has basic ACPI support
>> in 4.1.
>>
>> struct pci_controller        controller is not available
>> on ARM64, that's the reason why compile errors happens on ARM64.
>>
>> How about move struct pci_controller to this head file?
>>
>> because all the related file you changed in this patch set
>> are only compiled when CONFI_ACPI=y, so for x86,
>>
>> struct pci_controller {
>> #ifdef CONFIG_ACPI
>>          struct acpi_device *companion;  /* ACPI companion device */
>> #endif
>> #ifdef CONFIG_X86_64
>>          void            *iommu;         /* IOMMU private data */
>> #endif
>>          int             segment;        /* PCI domain */
>>          int             node;           /* NUMA node */
>> };
>>
>> I'm sure #ifdef CONFIG_ACPI .. #endif can be removed
>> with no harm, and for *iommu, we can remove the #ifdef CONFIG_X86_64
>> with introducing little more memory on x86_32, after
>> that, the struct pci_controller is almost the same as ia64:
>
> On x86, struct pci_controller may be used when CONFIG_ACPI is disabled.
> So we can't move it into pci-acpi.h

Ah, ok, I missed this part.

>
>>
>> struct pci_controller {
>>          struct acpi_device *companion;
>>          void *iommu;
>>          int segment;
>>          int node;               /* nearest node with memory or
>> NUMA_NO_NODE for global allocation */
>>
>>          void *platform_data;
>> };
>>
>> except void *platform_data;
>>
>> On ARM64, the structure is almost the same, so how about
>> introduce
>>
>> struct pci_controller {
>>          struct acpi_device *companion;  /* ACPI companion device */
>>          void            *iommu;         /* IOMMU private data */
>>          int             segment;        /* PCI domain */
>>          int             node;           /* NUMA node */
>> #ifdef CONFIG_IA64
>>      void *platform_data;
>> #endif
>> };
>>
>> in this file, then can be used for all architectures?
> Current mode is that architecture defines its own version of
> struct pci_controller. It would be better to keep this pattern.

OK, thanks for the clarify :) So how about add my basic
PCI support patch for ARM64 on top of you patch set to fix
this problem?

Thanks
Hanjun


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu May 14, 2015, 1:09 a.m. UTC | #6
On 2015/5/13 21:25, Hanjun Guo wrote:
> On 2015?05?13? 20:24, Jiang Liu wrote:
>> On 2015/5/13 17:29, Hanjun Guo wrote:
>>> Hi Jiang,
>>>
>>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>>
>>> struct pci_controller {
>>>          struct acpi_device *companion;
>>>          void *iommu;
>>>          int segment;
>>>          int node;               /* nearest node with memory or
>>> NUMA_NO_NODE for global allocation */
>>>
>>>          void *platform_data;
>>> };
>>>
>>> except void *platform_data;
>>>
>>> On ARM64, the structure is almost the same, so how about
>>> introduce
>>>
>>> struct pci_controller {
>>>          struct acpi_device *companion;  /* ACPI companion device */
>>>          void            *iommu;         /* IOMMU private data */
>>>          int             segment;        /* PCI domain */
>>>          int             node;           /* NUMA node */
>>> #ifdef CONFIG_IA64
>>>      void *platform_data;
>>> #endif
>>> };
>>>
>>> in this file, then can be used for all architectures?
>> Current mode is that architecture defines its own version of
>> struct pci_controller. It would be better to keep this pattern.
> 
> OK, thanks for the clarify :) So how about add my basic
> PCI support patch for ARM64 on top of you patch set to fix
> this problem?

Sure, please send me the patches and I will send out v3 to
cover your review comments.
Thanks!

> 
> Thanks
> Hanjun
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hanjun Guo May 14, 2015, 4:05 a.m. UTC | #7
On 2015?05?14? 09:09, Jiang Liu wrote:
> On 2015/5/13 21:25, Hanjun Guo wrote:
>> On 2015?05?13? 20:24, Jiang Liu wrote:
>>> On 2015/5/13 17:29, Hanjun Guo wrote:
>>>> Hi Jiang,
>>>>
>>>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>>>
>>>> struct pci_controller {
>>>>           struct acpi_device *companion;
>>>>           void *iommu;
>>>>           int segment;
>>>>           int node;               /* nearest node with memory or
>>>> NUMA_NO_NODE for global allocation */
>>>>
>>>>           void *platform_data;
>>>> };
>>>>
>>>> except void *platform_data;
>>>>
>>>> On ARM64, the structure is almost the same, so how about
>>>> introduce
>>>>
>>>> struct pci_controller {
>>>>           struct acpi_device *companion;  /* ACPI companion device */
>>>>           void            *iommu;         /* IOMMU private data */
>>>>           int             segment;        /* PCI domain */
>>>>           int             node;           /* NUMA node */
>>>> #ifdef CONFIG_IA64
>>>>       void *platform_data;
>>>> #endif
>>>> };
>>>>
>>>> in this file, then can be used for all architectures?
>>> Current mode is that architecture defines its own version of
>>> struct pci_controller. It would be better to keep this pattern.
>>
>> OK, thanks for the clarify :) So how about add my basic
>> PCI support patch for ARM64 on top of you patch set to fix
>> this problem?
>
> Sure, please send me the patches and I will send out v3 to
> cover your review comments.

OK, I need to rework my patches because my patch set is dependent
on top of another MMCFG refactor patch set [1], so I need to remove
MMCONFIG first then will speed up the upstream process of your patch
set, will send you the patches soon.

[1]: https://lkml.org/lkml/2015/4/17/29

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu May 14, 2015, 4:42 a.m. UTC | #8
On 2015/5/14 12:05, Hanjun Guo wrote:
> On 2015?05?14? 09:09, Jiang Liu wrote:
>> On 2015/5/13 21:25, Hanjun Guo wrote:
>>> On 2015?05?13? 20:24, Jiang Liu wrote:
>>>> On 2015/5/13 17:29, Hanjun Guo wrote:
>>>>> Hi Jiang,
>>>>>
>>>>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>>>>
>>>>> struct pci_controller {
>>>>>           struct acpi_device *companion;
>>>>>           void *iommu;
>>>>>           int segment;
>>>>>           int node;               /* nearest node with memory or
>>>>> NUMA_NO_NODE for global allocation */
>>>>>
>>>>>           void *platform_data;
>>>>> };
>>>>>
>>>>> except void *platform_data;
>>>>>
>>>>> On ARM64, the structure is almost the same, so how about
>>>>> introduce
>>>>>
>>>>> struct pci_controller {
>>>>>           struct acpi_device *companion;  /* ACPI companion device */
>>>>>           void            *iommu;         /* IOMMU private data */
>>>>>           int             segment;        /* PCI domain */
>>>>>           int             node;           /* NUMA node */
>>>>> #ifdef CONFIG_IA64
>>>>>       void *platform_data;
>>>>> #endif
>>>>> };
>>>>>
>>>>> in this file, then can be used for all architectures?
>>>> Current mode is that architecture defines its own version of
>>>> struct pci_controller. It would be better to keep this pattern.
>>>
>>> OK, thanks for the clarify :) So how about add my basic
>>> PCI support patch for ARM64 on top of you patch set to fix
>>> this problem?
>>
>> Sure, please send me the patches and I will send out v3 to
>> cover your review comments.
> 
> OK, I need to rework my patches because my patch set is dependent
> on top of another MMCFG refactor patch set [1], so I need to remove
> MMCONFIG first then will speed up the upstream process of your patch
> set, will send you the patches soon.
Hi Hanjun,
	I will send out v3 soon , so you could rebase onto v3.
There are changes which will affect your rebase.
Thanks!
Gerry
> 
> [1]: https://lkml.org/lkml/2015/4/17/29
> 
> Thanks
> Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 1b5569c092c6..97c260959a54 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -656,6 +656,220 @@  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_common *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_common *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_common *info)
+{
+	struct resource *res;
+	struct resource_entry *entry, *tmp;
+
+	if (!info)
+		return;
+
+	if (info->ops && info->ops->release_info)
+		info->ops->release_info(info);
+
+	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);
+	}
+
+	kfree(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,
+				     size_t extra_size)
+{
+	int ret, busnum = root->secondary.start;
+	struct acpi_device *device = root->device;
+	struct acpi_pci_root_info_common *info;
+	struct pci_bus *bus;
+
+	info = kzalloc(sizeof(*info) + extra_size, GFP_KERNEL);
+	if (!info) {
+		dev_err(&device->dev,
+			"pci_bus %04x:%02x: ignored (out of memory)\n",
+			root->segment, busnum);
+		return NULL;
+	}
+
+	info->controller.segment = root->segment;
+	info->controller.node = acpi_get_node(device->handle);
+	info->controller.companion = device;
+	info->root = root;
+	info->bridge = device;
+	info->ops = ops;
+	INIT_LIST_HEAD(&info->resources);
+	snprintf(info->name, sizeof(info->name), "PCI Bus %04x:%02x",
+		 info->controller.segment, busnum);
+	if (ops->init_info && ops->init_info(info)) {
+		kfree(info);
+		return NULL;
+	}
+
+	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,
+				  &info->controller, &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 (info->controller.node != NUMA_NO_NODE)
+			dev_printk(KERN_DEBUG, &bus->dev, "on NUMA node %d\n",
+				   info->controller.node);
+		return bus;
+	}
+
+out_release_info:
+	__acpi_pci_root_release_info(info);
+	return NULL;
+}
+
 void __init acpi_pci_root_init(void)
 {
 	acpi_hest_init();
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index a965efa52152..a292ee33d74b 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -52,6 +52,30 @@  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_common {
+	struct pci_controller		controller;
+	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_common *info);
+	void (*release_info)(struct acpi_pci_root_info_common *info);
+	int (*prepare_resources)(struct acpi_pci_root_info_common *info,
+				 int status);
+};
+
+extern struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
+					    struct acpi_pci_root_ops *ops,
+					    size_t extra_size);
+
 void acpi_pci_add_bus(struct pci_bus *bus);
 void acpi_pci_remove_bus(struct pci_bus *bus);