Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1201375 > unrolled thread

Re: [PATCH v3] PCI: Only enable IO window if supported

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3] PCI: Only enable IO window if supported Yinghai Lu <yinghai@kernel.org> - 2015-08-06 03:20 +0200
    Re: [PATCH v3] PCI: Only enable IO window if supported Yinghai Lu <yinghai@kernel.org> - 2015-08-06 03:40 +0200
      Re: [PATCH v3] PCI: Only enable IO window if supported Guenter Roeck <linux@roeck-us.net> - 2015-08-06 04:30 +0200
        Re: [PATCH v3] PCI: Only enable IO window if supported Yinghai Lu <yinghai@kernel.org> - 2015-08-06 06:30 +0200
          Re: [PATCH v3] PCI: Only enable IO window if supported Guenter Roeck <linux@roeck-us.net> - 2015-08-06 07:00 +0200
            Re: [PATCH v3] PCI: Only enable IO window if supported Yinghai Lu <yinghai@kernel.org> - 2015-08-06 18:00 +0200
              Re: [PATCH v3] PCI: Only enable IO window if supported Guenter Roeck <linux@roeck-us.net> - 2015-08-06 18:20 +0200
                Re: [PATCH v3] PCI: Only enable IO window if supported Yinghai Lu <yinghai@kernel.org> - 2015-08-07 01:40 +0200
                  Re: [PATCH v3] PCI: Only enable IO window if supported Guenter Roeck <linux@roeck-us.net> - 2015-08-07 02:40 +0200
                    Re: [PATCH v3] PCI: Only enable IO window if supported Yinghai Lu <yinghai@kernel.org> - 2015-08-07 03:20 +0200
    Re: [PATCH v3] PCI: Only enable IO window if supported Guenter Roeck <linux@roeck-us.net> - 2015-08-06 03:50 +0200

#1201375 — Re: [PATCH v3] PCI: Only enable IO window if supported

On Thu, Jul 30, 2015 at 7:15 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> The PCI subsystem always assumes that I/O is supported on PCIe bridges
> and tries to assign an I/O window to each child bus even if that is not
> the case.
>
> This may result in messages such as:
>
>   pcieport 0000:02:00.0: res[7]=[io  0x1000-0x0fff] get_res_add_size add_size 1000
>   pcieport 0000:02:00.0: BAR 7: no space for [io  size 0x1000]
>   pcieport 0000:02:00.0: BAR 7: failed to assign [io  size 0x1000]
>
> for each bridge port, even if a bus or its parent does not support I/O in
> the first place.
>
> To avoid this message, check if a bus supports I/O before trying to enable
> it.  Also check if the root bus has an IO window assigned; if not, it does
> not make sense to try to assign one to any of its child busses.
>
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index cefd636681b6..d9e02ba34035 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -332,6 +332,24 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
>         }
>  }
>
> +static int pci_bridge_supports_io(struct pci_dev *bridge)
> +{
> +       u16 io;
> +
> +       pci_read_config_word(bridge, PCI_IO_BASE, &io);
> +       if (io)
> +               return 1;
> +
> +       /* IO_BASE/LIMIT is either hard-wired to zero or programmed to zero */
> +       pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0);
> +       pci_read_config_word(bridge, PCI_IO_BASE, &io);
> +       pci_write_config_word(bridge, PCI_IO_BASE, 0x0);
> +       if (io)
> +               return 1;
> +
> +       return 0;
> +}
> +
>  static void pci_read_bridge_io(struct pci_bus *child)
>  {
>         struct pci_dev *dev = child->self;
> @@ -340,6 +358,15 @@ static void pci_read_bridge_io(struct pci_bus *child)
>         struct pci_bus_region region;
>         struct resource *res;
>
> +       if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO))
> +               return;
> +
> +       if (!pci_bridge_supports_io(dev)) {
> +               dev_printk(KERN_DEBUG, &dev->dev, "  no I/O window\n");
> +               child->bus_flags &= ~PCI_BUS_FLAGS_SUPPORTS_IO;
> +               return;
> +       }
> +

It only can avoid warning with bridge, and still have warning on
devices under the bridge.

also would have problem on transparent bridges, like

BRIDGE_A ---- BRIDGE_AA----DEVICE_AA
                   |
                   \-- BRIDGE_AB ---DEVICE_AB

if only BRIDGE_A and BRIDGE_AA are transparent, and BRIDGE_AB is not.

and if pci_bridge_supports_io() return false for BRIDGE_A.

We will have all three bridges have PCI_BUS_FLAGS_SUPPORTS_IO cleared.
so all will not been sized and will not get assigned io port resource.

later DEVICE_AA could root bus io port as parent, and get io resource assigned.
but DEVICE_AB will not get assigned.

so we should get rid of pci_bridge_supports_io(), and just check root
resource IO port.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1201379

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>
> It only can avoid warning with bridge, and still have warning on
> devices under the bridge.
>
> also would have problem on transparent bridges, like
>
> BRIDGE_A ---- BRIDGE_AA----DEVICE_AA
>                    |
>                    \-- BRIDGE_AB ---DEVICE_AB
>
> if only BRIDGE_A and BRIDGE_AA are transparent, and BRIDGE_AB is not.
>
> and if pci_bridge_supports_io() return false for BRIDGE_A.
>
> We will have all three bridges have PCI_BUS_FLAGS_SUPPORTS_IO cleared.
> so all will not been sized and will not get assigned io port resource.
>
> later DEVICE_AA could root bus io port as parent, and get io resource assigned.
> but DEVICE_AB will not get assigned.
>
> so we should get rid of pci_bridge_supports_io(), and just check root
> resource IO port.

I would suggest this version instead:

Subject: [PATCH] PCI: Only try to assign io port only if root bus support that

The PCI subsystem always assumes that I/O is supported on root bus and
tries to assign an I/O window to each child bus even if that is not the
case.

This may result in messages such as:

  pcieport 0000:02:00.0: res[7]=[io  0x1000-0x0fff] get_res_add_size
add_size 1000
  pcieport 0000:02:00.0: BAR 7: no space for [io  size 0x1000]
  pcieport 0000:02:00.0: BAR 7: failed to assign [io  size 0x1000]

for each bridge port, even if root does not support I/O in the first place.

To avoid this message, check if root bus supports I/O, then child bus
will inherit the setting from parent bus, and later during sizing and
assigning, check that bus flags and skip those resources.

[bhelgaas: reverse sense of new pci_bus_flags_t value]
[yinghai: only check root bus io port, and use flag on sizing and assigning]
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

---
 drivers/pci/probe.c     |    5 +++++
 drivers/pci/setup-bus.c |    6 +++++-
 include/linux/pci.h     |    1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -340,6 +340,9 @@ static void pci_read_bridge_io(struct pc
     struct pci_bus_region region;
     struct resource *res;

+    if (!(child->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO))
+        return;
+
     io_mask = PCI_IO_RANGE_MASK;
     io_granularity = 0x1000;
     if (dev->io_window_1k) {
@@ -2070,6 +2073,8 @@ struct pci_bus *pci_create_root_bus(stru
         } else
             bus_addr[0] = '\0';
         dev_info(&b->dev, "root bus resource %pR%s\n", res, bus_addr);
+        if (resource_type(res) == IORESOURCE_IO)
+            b->bus_flags |= PCI_BUS_FLAGS_ROOT_SUPPORTS_IO;
     }

     down_write(&pci_bus_sem);
Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -156,6 +156,10 @@ static void pdev_sort_resources(struct p
         if (r->flags & IORESOURCE_PCI_FIXED)
             continue;

+        if ((r->flags & IORESOURCE_IO) &&
+            !(dev->bus->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO))
+            continue;
+
         if (!(r->flags) || r->parent)
             continue;

@@ -909,7 +913,7 @@ static void pbus_size_io(struct pci_bus
     resource_size_t children_add_size = 0;
     resource_size_t min_align, align;

-    if (!b_res)
+    if (!b_res || !(bus->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO))
         return;

     min_align = window_alignment(bus, IORESOURCE_IO);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -193,6 +193,7 @@ typedef unsigned short __bitwise pci_bus
 enum pci_bus_flags {
     PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,
     PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
+    PCI_BUS_FLAGS_ROOT_SUPPORTS_IO = (__force pci_bus_flags_t) 4,
 };

 /* These values come from the PCI Express Spec */

[toc] | [prev] | [next] | [standalone]

#1201392

On 08/05/2015 06:38 PM, Yinghai Lu wrote:
> On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>> It only can avoid warning with bridge, and still have warning on
>> devices under the bridge.
>>
>> also would have problem on transparent bridges, like
>>
>> BRIDGE_A ---- BRIDGE_AA----DEVICE_AA
>>                     |
>>                     \-- BRIDGE_AB ---DEVICE_AB
>>
>> if only BRIDGE_A and BRIDGE_AA are transparent, and BRIDGE_AB is not.
>>
>> and if pci_bridge_supports_io() return false for BRIDGE_A.
>>
>> We will have all three bridges have PCI_BUS_FLAGS_SUPPORTS_IO cleared.
>> so all will not been sized and will not get assigned io port resource.
>>
>> later DEVICE_AA could root bus io port as parent, and get io resource assigned.
>> but DEVICE_AB will not get assigned.
>>
>> so we should get rid of pci_bridge_supports_io(), and just check root
>> resource IO port.
>
> I would suggest this version instead:
>
> Subject: [PATCH] PCI: Only try to assign io port only if root bus support that
>

Bjorn had asked me to move the IO support check out of pci_bridge_check_ranges().
It looks like you want to keep it there.

Can you explain your reasons ? Sorry, I just don't understand.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201422

On Wed, Aug 5, 2015 at 7:22 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 08/05/2015 06:38 PM, Yinghai Lu wrote:
>>
>> On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>
>>>
>>> It only can avoid warning with bridge, and still have warning on
>>> devices under the bridge.
>>>
>>> also would have problem on transparent bridges, like
>>>
>>> BRIDGE_A ---- BRIDGE_AA----DEVICE_AA
>>>                     |
>>>                     \-- BRIDGE_AB ---DEVICE_AB
>>>
>>> if only BRIDGE_A and BRIDGE_AA are transparent, and BRIDGE_AB is not.
>>>
>>> and if pci_bridge_supports_io() return false for BRIDGE_A.
>>>
>>> We will have all three bridges have PCI_BUS_FLAGS_SUPPORTS_IO cleared.
>>> so all will not been sized and will not get assigned io port resource.
>>>
>>> later DEVICE_AA could root bus io port as parent, and get io resource
>>> assigned.
>>> but DEVICE_AB will not get assigned.
>>>
>>> so we should get rid of pci_bridge_supports_io(), and just check root
>>> resource IO port.
>>
>>
>> I would suggest this version instead:
>>
>> Subject: [PATCH] PCI: Only try to assign io port only if root bus support
>> that
>>
>
> Bjorn had asked me to move the IO support check out of
> pci_bridge_check_ranges().
> It looks like you want to keep it there.
>
> Can you explain your reasons ? Sorry, I just don't understand.

in pci_alloc_child_bus(), child bus's bus_flags will be assigned with
parent's bus_flags.

so if one parent bus has bridge's io bar can not changed, then all
children bus will also
have same bus_flags, and their bridge io bar will not used.

then if BRIDGE_AB is not transparent, we do need to use it's bridge io bar.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201427

On 08/05/2015 09:25 PM, Yinghai Lu wrote:
> On Wed, Aug 5, 2015 at 7:22 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 08/05/2015 06:38 PM, Yinghai Lu wrote:
>>>
>>> On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>>
>>>>
>>>> It only can avoid warning with bridge, and still have warning on
>>>> devices under the bridge.
>>>>
>>>> also would have problem on transparent bridges, like
>>>>
>>>> BRIDGE_A ---- BRIDGE_AA----DEVICE_AA
>>>>                      |
>>>>                      \-- BRIDGE_AB ---DEVICE_AB
>>>>
>>>> if only BRIDGE_A and BRIDGE_AA are transparent, and BRIDGE_AB is not.
>>>>
>>>> and if pci_bridge_supports_io() return false for BRIDGE_A.
>>>>
>>>> We will have all three bridges have PCI_BUS_FLAGS_SUPPORTS_IO cleared.
>>>> so all will not been sized and will not get assigned io port resource.
>>>>
>>>> later DEVICE_AA could root bus io port as parent, and get io resource
>>>> assigned.
>>>> but DEVICE_AB will not get assigned.
>>>>
>>>> so we should get rid of pci_bridge_supports_io(), and just check root
>>>> resource IO port.
>>>
>>>
>>> I would suggest this version instead:
>>>
>>> Subject: [PATCH] PCI: Only try to assign io port only if root bus support
>>> that
>>>
>>
>> Bjorn had asked me to move the IO support check out of
>> pci_bridge_check_ranges().
>> It looks like you want to keep it there.
>>
>> Can you explain your reasons ? Sorry, I just don't understand.
>
> in pci_alloc_child_bus(), child bus's bus_flags will be assigned with
> parent's bus_flags.
>
> so if one parent bus has bridge's io bar can not changed, then all
> children bus will also
> have same bus_flags, and their bridge io bar will not used.
>
> then if BRIDGE_AB is not transparent, we do need to use it's bridge io bar.
>
If I understand you correctly, we might need to change the code
in pci_read_bridge_io() as follows.

From

	if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO))
		return;

to something like

	if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO)) {
		if (dev->transparent)
			return;
		child->bus_flags |= PCI_BUS_FLAGS_SUPPORTS_IO;
	}

Would that solve the problem you are concerned about ?

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201874

On Wed, Aug 5, 2015 at 9:50 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> If I understand you correctly, we might need to change the code
> in pci_read_bridge_io() as follows.
>
> From
>
>         if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO))
>                 return;
>
> to something like
>
>         if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO)) {
>                 if (dev->transparent)
>                         return;
>                 child->bus_flags |= PCI_BUS_FLAGS_SUPPORTS_IO;
>         }
>
Then how about: the root bus really does not support IO PORT resource.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201884

On 08/06/2015 08:58 AM, Yinghai Lu wrote:
> On Wed, Aug 5, 2015 at 9:50 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> If I understand you correctly, we might need to change the code
>> in pci_read_bridge_io() as follows.
>>
>> From
>>
>>          if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO))
>>                  return;
>>
>> to something like
>>
>>          if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO)) {
>>                  if (dev->transparent)
>>                          return;
>>                  child->bus_flags |= PCI_BUS_FLAGS_SUPPORTS_IO;
>>          }
>>
> Then how about: the root bus really does not support IO PORT resource.
>

Ok, I admit that I am lost. Earlier it seemed that you were concerned about
this case, where no io window is available or a bus doesn't support io,
but a non-transparent child does. Now you seem to say that the non-transparent
child would not be able to support IO either.

For my education, can you list the possible options, and how you suggest
to solve them ? I can see the following situations.

- root supports IO, but has no io window assigned
- root does not support IO and has or has not an IO window assigned
- a bridge does not support IO

For the transparent case, each of those should result in all children
not even trying to assign an IO window, which is what we want,
and what my patc set tries to do.

How should those cases be handled for non-transparent bridges ?

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202161

On Thu, Aug 6, 2015 at 9:18 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 08/06/2015 08:58 AM, Yinghai Lu wrote:

> Ok, I admit that I am lost. Earlier it seemed that you were concerned about
> this case, where no io window is available or a bus doesn't support io,
> but a non-transparent child does. Now you seem to say that the
> non-transparent
> child would not be able to support IO either.
>
> For my education, can you list the possible options, and how you suggest
> to solve them ? I can see the following situations.
>
> - root supports IO, but has no io window assigned
> - root does not support IO and has or has not an IO window assigned
> - a bridge does not support IO

should have two cases:
1. root bus does not have io window exposed.
2. pci bridge io bar is not writable.

>
> For the transparent case, each of those should result in all children
> not even trying to assign an IO window, which is what we want,
> and what my patc set tries to do.
>
> How should those cases be handled for non-transparent bridges ?

for case 2: current upstream code, no warning for the bridge itself,
but have warning for devices under the bridge.

We only need to handle case 1, aka root bus does not have io port window.

What about your setup, is it case 1 or case 2?

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202174

On 08/06/2015 04:32 PM, Yinghai Lu wrote:
> On Thu, Aug 6, 2015 at 9:18 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 08/06/2015 08:58 AM, Yinghai Lu wrote:
>
>> Ok, I admit that I am lost. Earlier it seemed that you were concerned about
>> this case, where no io window is available or a bus doesn't support io,
>> but a non-transparent child does. Now you seem to say that the
>> non-transparent
>> child would not be able to support IO either.
>>
>> For my education, can you list the possible options, and how you suggest
>> to solve them ? I can see the following situations.
>>
>> - root supports IO, but has no io window assigned
>> - root does not support IO and has or has not an IO window assigned
>> - a bridge does not support IO
>
> should have two cases:
> 1. root bus does not have io window exposed.
> 2. pci bridge io bar is not writable.
>
>>
>> For the transparent case, each of those should result in all children
>> not even trying to assign an IO window, which is what we want,
>> and what my patc set tries to do.
>>
>> How should those cases be handled for non-transparent bridges ?
>
> for case 2: current upstream code, no warning for the bridge itself,
> but have warning for devices under the bridge.
>

That warning is what I am trying to get rid of, because it is repeated
dozens of times with zero value.

> We only need to handle case 1, aka root bus does not have io port window.
>
> What about your setup, is it case 1 or case 2?
>

Assuming case 1) includes case 3), root bridge does not support IO,
I have both case 1 and 2.

Anyway, I think I am giving up. Sorry, I just fail to understand the
use case(s) you are trying to cover.

Why don't you submit a patch, I'll test it if it works for my use
case, and if it works we ask Bjorn to apply it. My problem is quite
simple: I don't want to get flooded with useless "no IO window" messages.
How it is solved doesn't matter to me, as long as it is solved.
Since you have a strong opinion on how it should be solved, we should
go with your solution and not keep turning in circles forever.

Thanks
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202185

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 6, 2015 at 5:32 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> Assuming case 1) includes case 3), root bridge does not support IO,
> I have both case 1 and 2.
>
> Anyway, I think I am giving up. Sorry, I just fail to understand the
> use case(s) you are trying to cover.
>
> Why don't you submit a patch, I'll test it if it works for my use
> case, and if it works we ask Bjorn to apply it. My problem is quite
> simple: I don't want to get flooded with useless "no IO window" messages.
> How it is solved doesn't matter to me, as long as it is solved.
> Since you have a strong opinion on how it should be solved, we should
> go with your solution and not keep turning in circles forever.

Please check attached patch and post boot log.

Thanks

Yinghai

[toc] | [prev] | [next] | [standalone]

#1201384

On 08/05/2015 06:14 PM, Yinghai Lu wrote:
> On Thu, Jul 30, 2015 at 7:15 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> The PCI subsystem always assumes that I/O is supported on PCIe bridges
>> and tries to assign an I/O window to each child bus even if that is not
>> the case.
>>
>> This may result in messages such as:
>>
>>    pcieport 0000:02:00.0: res[7]=[io  0x1000-0x0fff] get_res_add_size add_size 1000
>>    pcieport 0000:02:00.0: BAR 7: no space for [io  size 0x1000]
>>    pcieport 0000:02:00.0: BAR 7: failed to assign [io  size 0x1000]
>>
>> for each bridge port, even if a bus or its parent does not support I/O in
>> the first place.
>>
>> To avoid this message, check if a bus supports I/O before trying to enable
>> it.  Also check if the root bus has an IO window assigned; if not, it does
>> not make sense to try to assign one to any of its child busses.
>>
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index cefd636681b6..d9e02ba34035 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -332,6 +332,24 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
>>          }
>>   }
>>
>> +static int pci_bridge_supports_io(struct pci_dev *bridge)
>> +{
>> +       u16 io;
>> +
>> +       pci_read_config_word(bridge, PCI_IO_BASE, &io);
>> +       if (io)
>> +               return 1;
>> +
>> +       /* IO_BASE/LIMIT is either hard-wired to zero or programmed to zero */
>> +       pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0);
>> +       pci_read_config_word(bridge, PCI_IO_BASE, &io);
>> +       pci_write_config_word(bridge, PCI_IO_BASE, 0x0);
>> +       if (io)
>> +               return 1;
>> +
>> +       return 0;
>> +}
>> +
>>   static void pci_read_bridge_io(struct pci_bus *child)
>>   {
>>          struct pci_dev *dev = child->self;
>> @@ -340,6 +358,15 @@ static void pci_read_bridge_io(struct pci_bus *child)
>>          struct pci_bus_region region;
>>          struct resource *res;
>>
>> +       if (!(child->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO))
>> +               return;
>> +
>> +       if (!pci_bridge_supports_io(dev)) {
>> +               dev_printk(KERN_DEBUG, &dev->dev, "  no I/O window\n");
>> +               child->bus_flags &= ~PCI_BUS_FLAGS_SUPPORTS_IO;
>> +               return;
>> +       }
>> +
>
> It only can avoid warning with bridge, and still have warning on
> devices under the bridge.
>
Not really, because children inherit bus_flags.

> also would have problem on transparent bridges, like
>
> BRIDGE_A ---- BRIDGE_AA----DEVICE_AA
>                     |
>                     \-- BRIDGE_AB ---DEVICE_AB
>
> if only BRIDGE_A and BRIDGE_AA are transparent, and BRIDGE_AB is not.
>
> and if pci_bridge_supports_io() return false for BRIDGE_A.
>
> We will have all three bridges have PCI_BUS_FLAGS_SUPPORTS_IO cleared.
> so all will not been sized and will not get assigned io port resource.
>
What is wrong with that ? Doesn't it reflect reality ?

> later DEVICE_AA could root bus io port as parent, and get io resource assigned.
> but DEVICE_AB will not get assigned.
>
> so we should get rid of pci_bridge_supports_io(), and just check root
> resource IO port.
>
You lost me here. That would mean that we would not clear the flag
and claim that a bridge supports IO even if it doesn't, and all ports
and bridges connected to that bridge would try (and fail) to get
IO address assignments.

That does not make much sense to me. What do you expect to do in this case ?

I must admit I have no idea how non-transparent bridges fit
into this picture. However, if Bridge A doesn't support IO,
I don't see how we can get IO anywhere, transparent or not.

Guenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web