diff mbox

[v3,5/7] PCI: Do not use IORESOURCE_STARTALIGN to identify bridge resources

Message ID 1467283993-3185-6-git-send-email-xyjxie@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Yongji Xie June 30, 2016, 10:53 a.m. UTC
Now we use the IORESOURCE_STARTALIGN to identify bridge resources
in __assign_resources_sorted(). That's quite fragile. We may also
set flag IORESOURCE_STARTALIGN for SR-IOV resources in some cases,
for example, using the option "noresize" of parameter
"pci=resource_alignment".

In this patch, we try to use a more robust way to identify
bridge resources.

Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
---
 drivers/pci/setup-bus.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Gavin Shan July 1, 2016, 2:34 a.m. UTC | #1
On Thu, Jun 30, 2016 at 06:53:11PM +0800, Yongji Xie wrote:
>Now we use the IORESOURCE_STARTALIGN to identify bridge resources
>in __assign_resources_sorted(). That's quite fragile. We may also
>set flag IORESOURCE_STARTALIGN for SR-IOV resources in some cases,
>for example, using the option "noresize" of parameter
>"pci=resource_alignment".
>
>In this patch, we try to use a more robust way to identify
>bridge resources.
>
>Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>

Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Yongji, I think this doesn't have to be part of this series, meaning
it can be sent or merged separately.

>---
> drivers/pci/setup-bus.c |    9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>index 55641a3..216ddbc 100644
>--- a/drivers/pci/setup-bus.c
>+++ b/drivers/pci/setup-bus.c
>@@ -390,6 +390,7 @@ static void __assign_resources_sorted(struct list_head *head,
> 	struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
> 	unsigned long fail_type;
> 	resource_size_t add_align, align;
>+	int index;
>
> 	/* Check if optional add_size is there */
> 	if (!realloc_head || list_empty(realloc_head))
>@@ -410,11 +411,13 @@ static void __assign_resources_sorted(struct list_head *head,
>
> 		/*
> 		 * There are two kinds of additional resources in the list:
>-		 * 1. bridge resource  -- IORESOURCE_STARTALIGN
>-		 * 2. SR-IOV resource   -- IORESOURCE_SIZEALIGN
>+		 * 1. bridge resource
>+		 * 2. SR-IOV resource
> 		 * Here just fix the additional alignment for bridge
> 		 */
>-		if (!(dev_res->res->flags & IORESOURCE_STARTALIGN))
>+		index = dev_res->res - dev_res->dev->resource;
>+		if (index < PCI_BRIDGE_RESOURCES ||
>+			index > PCI_BRIDGE_RESOURCE_END)
> 			continue;
>
> 		add_align = get_res_add_align(realloc_head, dev_res->res);

Thanks,
Gavin

--
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
Yongji Xie July 1, 2016, 7:04 a.m. UTC | #2
Hi Gavin,

On 2016/7/1 10:34, Gavin Shan wrote:

> On Thu, Jun 30, 2016 at 06:53:11PM +0800, Yongji Xie wrote:
>> Now we use the IORESOURCE_STARTALIGN to identify bridge resources
>> in __assign_resources_sorted(). That's quite fragile. We may also
>> set flag IORESOURCE_STARTALIGN for SR-IOV resources in some cases,
>> for example, using the option "noresize" of parameter
>> "pci=resource_alignment".
>>
>> In this patch, we try to use a more robust way to identify
>> bridge resources.
>>
>> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
> Yongji, I think this doesn't have to be part of this series, meaning
> it can be sent or merged separately.
>

Seems like I give a wrong example in my changelog.  The
parameter "pci=resource_alignment" would not set flag
IORESOURCE_STARTALIGN for SR-IOV resources as I replied
to you in previous patch. So this patch has nothing to do
with this series now, I will remove it as you suggested.

Thanks,
Yongji

--
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
Gavin Shan July 2, 2016, 12:37 a.m. UTC | #3
On Fri, Jul 01, 2016 at 03:04:10PM +0800, Yongji Xie wrote:
>Hi Gavin,
>
>On 2016/7/1 10:34, Gavin Shan wrote:
>
>>On Thu, Jun 30, 2016 at 06:53:11PM +0800, Yongji Xie wrote:
>>>Now we use the IORESOURCE_STARTALIGN to identify bridge resources
>>>in __assign_resources_sorted(). That's quite fragile. We may also
>>>set flag IORESOURCE_STARTALIGN for SR-IOV resources in some cases,
>>>for example, using the option "noresize" of parameter
>>>"pci=resource_alignment".
>>>
>>>In this patch, we try to use a more robust way to identify
>>>bridge resources.
>>>
>>>Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
>>Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>
>>Yongji, I think this doesn't have to be part of this series, meaning
>>it can be sent or merged separately.
>>
>
>Seems like I give a wrong example in my changelog.  The
>parameter "pci=resource_alignment" would not set flag
>IORESOURCE_STARTALIGN for SR-IOV resources as I replied
>to you in previous patch. So this patch has nothing to do
>with this series now, I will remove it as you suggested.
>

Thanks, please refine the changelog as it's not related to the
enforced alignment as you claimed when resending it separately.

Thanks,
Gavin

--
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/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 55641a3..216ddbc 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -390,6 +390,7 @@  static void __assign_resources_sorted(struct list_head *head,
 	struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
 	unsigned long fail_type;
 	resource_size_t add_align, align;
+	int index;
 
 	/* Check if optional add_size is there */
 	if (!realloc_head || list_empty(realloc_head))
@@ -410,11 +411,13 @@  static void __assign_resources_sorted(struct list_head *head,
 
 		/*
 		 * There are two kinds of additional resources in the list:
-		 * 1. bridge resource  -- IORESOURCE_STARTALIGN
-		 * 2. SR-IOV resource   -- IORESOURCE_SIZEALIGN
+		 * 1. bridge resource
+		 * 2. SR-IOV resource
 		 * Here just fix the additional alignment for bridge
 		 */
-		if (!(dev_res->res->flags & IORESOURCE_STARTALIGN))
+		index = dev_res->res - dev_res->dev->resource;
+		if (index < PCI_BRIDGE_RESOURCES ||
+			index > PCI_BRIDGE_RESOURCE_END)
 			continue;
 
 		add_align = get_res_add_align(realloc_head, dev_res->res);