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.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Yinghai Lu <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
v3: Reverse order of new flag, and name it PCI_BUS_FLAGS_SUPPORTS_IO
instead of PCI_BUS_FLAGS_NO_IO.
Don't use bool in pci_bridge_supports_io.
Drop pci_root_has_io_resource(). Instead, determine if the root bus
has an io window in pci_create_root_bus(), and clear
PCI_BUS_FLAGS_SUPPORTS_IO in its bus flags if it doesn't.
v2: Use a new bus flag to indicate if IO is supported on a bus or not.
Using IORESOURCE_DISABLED in resource flags turned out to be futile,
since the term "!res->flags" is widely used to detect if a resource
window is enabled or not, and setting IORESOURCE_DISABLED would
affect all this code.
This patch depends on 'PCI: move pci_read_bridge_bases to the generic
PCI layer' by Lorenzo Pieralisi; without it, pci_read_bridge_io()
is not always called.
---
drivers/pci/probe.c | 34 ++++++++++++++++++++++++++++++++++
drivers/pci/setup-bus.c | 9 +--------
include/linux/pci.h | 1 +
3 files changed, 36 insertions(+), 8 deletions(-)
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;
+ }
+
io_mask = PCI_IO_RANGE_MASK;
io_granularity = 0x1000;
if (dev->io_window_1k) {
@@ -496,6 +523,7 @@ static struct pci_bus *pci_alloc_bus(struct pci_bus *parent)
INIT_LIST_HEAD(&b->resources);
b->max_bus_speed = PCI_SPEED_UNKNOWN;
b->cur_bus_speed = PCI_SPEED_UNKNOWN;
+ b->bus_flags = PCI_BUS_FLAGS_SUPPORTS_IO;
#ifdef CONFIG_PCI_DOMAINS_GENERIC
if (parent)
b->domain_nr = parent->domain_nr;
@@ -1938,6 +1966,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
resource_size_t offset;
char bus_addr[64];
char *fmt;
+ bool has_io = false;
b = pci_alloc_bus(NULL);
if (!b)
@@ -2016,8 +2045,13 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
} else
bus_addr[0] = '\0';
dev_info(&b->dev, "root bus resource %pR%s\n", res, bus_addr);
+ if (resource_type(res) == IORESOURCE_IO)
+ has_io = true;
}
+ if (!has_io)
+ b->bus_flags &= ~PCI_BUS_FLAGS_SUPPORTS_IO;
+
down_write(&pci_bus_sem);
list_add_tail(&b->node, &pci_root_buses);
up_write(&pci_bus_sem);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 508cc56130e3..c3fdace30faf 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -744,7 +744,6 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
base/limit registers must be read-only and read as 0. */
static void pci_bridge_check_ranges(struct pci_bus *bus)
{
- u16 io;
u32 pmem;
struct pci_dev *bridge = bus->self;
struct resource *b_res;
@@ -752,13 +751,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
b_res[1].flags |= IORESOURCE_MEM;
- pci_read_config_word(bridge, PCI_IO_BASE, &io);
- if (!io) {
- 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)
+ if (bus->bus_flags & PCI_BUS_FLAGS_SUPPORTS_IO)
b_res[0].flags |= IORESOURCE_IO;
/* DECchip 21050 pass 2 errata: the bridge may miss an address
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8a0321a8fb59..3bbabf344bfc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -191,6 +191,7 @@ typedef unsigned short __bitwise pci_bus_flags_t;
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_SUPPORTS_IO = (__force pci_bus_flags_t) 4,
};
/* These values come from the PCI Express Spec */
--
2.1.4
On Thu, Jul 30, 2015 at 7:15 PM, Guenter Roeck <[email protected]> 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.
On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
CC: Lorenzo Pieralisi <[email protected]>
---
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 */
On 08/05/2015 06:14 PM, Yinghai Lu wrote:
> On Thu, Jul 30, 2015 at 7:15 PM, Guenter Roeck <[email protected]> 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
On 08/05/2015 06:38 PM, Yinghai Lu wrote:
> On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <[email protected]> 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
On Wed, Aug 5, 2015 at 7:22 PM, Guenter Roeck <[email protected]> wrote:
> On 08/05/2015 06:38 PM, Yinghai Lu wrote:
>>
>> On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <[email protected]> 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.
On 08/05/2015 09:25 PM, Yinghai Lu wrote:
> On Wed, Aug 5, 2015 at 7:22 PM, Guenter Roeck <[email protected]> wrote:
>> On 08/05/2015 06:38 PM, Yinghai Lu wrote:
>>>
>>> On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <[email protected]> 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
On Wed, Aug 5, 2015 at 9:50 PM, Guenter Roeck <[email protected]> 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.
On 08/06/2015 08:58 AM, Yinghai Lu wrote:
> On Wed, Aug 5, 2015 at 9:50 PM, Guenter Roeck <[email protected]> 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
On Thu, Aug 6, 2015 at 9:18 AM, Guenter Roeck <[email protected]> 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
On 08/06/2015 04:32 PM, Yinghai Lu wrote:
> On Thu, Aug 6, 2015 at 9:18 AM, Guenter Roeck <[email protected]> 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
On Thu, Aug 6, 2015 at 5:32 PM, Guenter Roeck <[email protected]> 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