On Sat, May 18, 2013 at 8:24 PM, Kevin Hao <haokexin@xxxxxxxxx> wrote:
> On Fri, May 17, 2013 at 10:51:20PM +0800, Liu Jiang wrote:
>> On Fri 17 May 2013 10:11:29 AM CST, Kevin Hao wrote:
>> >On Fri, May 17, 2013 at 12:18:07AM +0800, Liu Jiang wrote:
>> >>On Tue 14 May 2013 09:07:56 PM CST, Kevin Hao wrote:
>> >>>In the current kernel, we just set the CPU address to the bus address
>> >>>if we can't find the match region for one specific bus address. If BAR
>> >>>of one pci device is set to address which happens to be a legitimate
>> >>>CPU address by firmware, the kernel will think this resource is legal
>> >>>and will not try to reassign later. In cases the CPU address and bus
>> >>>address isn't equal, the device will not work. So we should check
>> >>>if we can translate the bus address to CPU address correctly. If not,
>> >>>we should unset this resource and wish the kernel will reassign it
>> >>>later.
>> >>>
>> >>>Since we will invoke pcibios_bus_to_resource unconditionally if we
>> >>>don't goto fail, move it out of if/else wrap.
>> >>>
>> >>>Signed-off-by: Kevin Hao <haokexin@xxxxxxxxx>
>> >>>---
>> >>> drivers/pci/probe.c | 9 ++++++---
>> >>> 1 file changed, 6 insertions(+), 3 deletions(-)
>> >>>
>> >>>diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> >>>index 70f10fa..c96772f 100644
>> >>>--- a/drivers/pci/probe.c
>> >>>+++ b/drivers/pci/probe.c
>> >>>@@ -250,12 +250,10 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
>> >>> pci_write_config_dword(dev, pos + 4, 0);
>> >>> region.start = 0;
>> >>> region.end = sz64;
>> >>>- pcibios_bus_to_resource(dev, res, ®ion);
>> >>> bar_disabled = true;
>> >>> } else {
>> >>> region.start = l64;
>> >>> region.end = l64 + sz64;
>> >>>- pcibios_bus_to_resource(dev, res, ®ion);
>> >>> }
>> >>> } else {
>> >>> sz = pci_size(l, sz, mask);
>> >>>@@ -265,7 +263,12 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
>> >>>
>> >>> region.start = l;
>> >>> region.end = l + sz;
>> >>>- pcibios_bus_to_resource(dev, res, ®ion);
>> >>>+ }
>> >>>+
>> >>>+ if (!pcibios_bus_to_resource(dev, res, ®ion)) {
>> >>>+ res->flags |= IORESOURCE_UNSET;
>> >>>+ res->end -= res->start;
>> >>>+ res->start = 0;
>> >>> }
>> >>>
>> >>> goto out;
>> >>
>> >>Hi Kevin,
>> >> Will this break subtractive decode PCI bridges and devices?
>> >
>> >No. A subtractive decode occurs only when no other pci bridge or device
>> >claim the transactions. As you can see that when we are trying to translate
>> >a bus address to cpu address we search the address regions of pci host
>> >bridge instead of a pci bridge. The pci host bridge address regions should
>> >cover all the bus address we can use for the pci device or bridge under this
>> >pci controller. This definitely also include the bus address regions used by
>> >the expansion bus for subtractive decode. So if a pci device use a bus address
>> >that is unknown to this pci host bridge, there is definitely something wrong
>> >here. This is the case we are trying to fix.
>> >
>> Hi Kevin,
>> I'm not sure about this assumption "the pci host bridge
>> address regions should cover
>> all the bus address we can use for the pci device or bridge under
>> this pci controller".
>> I have a fear that it may cause regressions to some legacy x86
>> platforms, such as an AMD
>> platform with PCI based IOAPICs, though I have no evidences for it.
>
> Does any x86 expert can confirm whether the assumption that "the pci host
> bridge address regions should cover all the bus address we can use for the
> pci device or bridge under this pci host bridge" is true or not on x86?
> It seems definitely wrong in logical. But things always happen out of
> expectation in reality. :-)
You're changing generic code, not x86-specific code, so you need the
assumption to hold on all architectures. I don't know off-hand
whether it's true or not.
Can you save me some time by collecting a full dmesg showing the
problem, and maybe one with your fix? I know there are cases where we
notice a BAR assignment that we think is incorrect, e.g.,
https://bugzilla.kernel.org/show_bug.cgi?id=31602 (fixed by
eb31aae8cb5) and we reassign it, and I want to know how your case is
different from that.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
