Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Monday, April 13, 2015 12:31:20 PM Jiang Liu wrote: > On 2015/4/10 8:28, Rafael J. Wysocki wrote: > > On Fri, Apr 10, 2015 at 12:37 AM, Rafael J. Wysocki > > wrote: > >> On Thursday, April 09, 2015 05:00:08 PM Bjorn Helgaas wrote: > >>> On Thu, Apr 9, 2015 at 4:37 PM, Rafael J. Wysocki > >>> wrote: > On Thursday, April 09, 2015 10:50:02 AM Jiang Liu wrote: > > On 2015/4/9 7:44, Rafael J. Wysocki wrote: > >> On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote: > >>> On 2015/4/7 8:28, Rafael J. Wysocki wrote: > On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: > > Hi Jiang, > >>> > >> Currently acpi_dev_filter_resource_type() is only used by ACPI pci > >> host bridge and IOAPIC driver, so it shouldn't affect other > >> drivers. > > > > We should assume it will eventually be used for all ACPI devices, > > shouldn't we? > > I'm not sure about that, really. In fact, I'd restrict its use to > devices > types that actually can "produce" resources (ie. do not require the > resources > to be provided by their ancestors or to be available from a global > pool). > > Otherwise we're pretty much guaranteed to get into trouble. > > And all of the above rules need to be documented in the kernel > source tree > or people will get confused. > >>> Hi Rafael, > >>> How about following comments for acpi_dev_filter_resource_type()? > >>> Thanks! > >>> Gerry > >>> > >>> /** > >>> * According to ACPI specifications, Consumer/Producer flag in ACPI > >>> resource > >>> * descriptor means: > >>> * 1(CONSUMER): This device consumes this resource > >>> * 0(PRODUCER): This device produces and consumes this resource > >>> * But for ACPI PCI host bridge, it is interpreted in another way: > >> > >> So first of all, this leads to a question: Why is it interpreted for > >> ACPI PCI > >> host bridges differently? > >> > >> Is it something we've figured out from experience, or is there a > >> standard > >> mandating that? > > Hi Rafael, > > I think we got this knowledge by experiences. PCI FW spec only > > states _CRS reports resources assigned to the host bridge by firmware. > > There's no statement about whether the resource is consumed by host > > bridge itself or provided to it's child bus/devices. On x86/IA64 side, > > the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF, > > but not sure about ARM64 side. So how about: > > This: > > > PCI Firmware specification states that _CRS reports resources > > assigned to the host bridge, but there's no way to tell whether > > the resource is consumed by host bridge itself or provided to > > its child bus/devices. > > looks OK to me, but I'd replace the below with something like: > > "However, experience shows, that in the PCI host bridge case firmware > writers > expect the resource to be provided to devices on the bus (below the > bridge) for > consumption entirely if its Consumer/Producer flag is clear. That > expectation > is reflected by the code in this routine as follows:". > >>> > >>> What a mess. The spec is regrettably lacking in Consumer/Producer > >>> specifics. But I don't see what's confusing about the descriptors > >>> that have Consumer/Producer bits. > >>> > >>> QWord, DWord, and Word descriptors don't seem to have a > >>> Consumer/Producer bit, but they do contain _TRA, so they must be > >>> intended for bridge windows. Can they also be used for device > >>> registers? I don't know. > >>> > >>> The Extended Address descriptor has a Consumer/Producer bit, and I > >>> would interpret the spec to mean that if the flag is clear (the device > >>> produces and consumes this resource), the resource is available on the > >>> bus below the bridge, i.e., the bridge consumes the resource on its > >>> upstream side and produces it on its downstream side. > >> > >> OK, that makes sense for bridges. > > >>> If the bit were > >>> set (the device only consumes the resource), I would expect that to > >>> mean the descriptor is for bridge registers like 0xcf8/0xcfc that > >>> never appear on the downstream side. > >> > >> That part is clear. What is not clear is whether or not we can *always* > >> expect the resources with Consumer/Producer *clear* to be produced on the > >> downstram side. That appears to be the case for host bridges if my > >> understanding of things is correct, but is it the case in general? > >> > >>> Maybe I'm reading the spec too naively, but this doesn't seem a matter > >>> of experience. > >> > >> Well, the specification is not clear
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Monday, April 13, 2015 12:31:20 PM Jiang Liu wrote: On 2015/4/10 8:28, Rafael J. Wysocki wrote: On Fri, Apr 10, 2015 at 12:37 AM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Thursday, April 09, 2015 05:00:08 PM Bjorn Helgaas wrote: On Thu, Apr 9, 2015 at 4:37 PM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Thursday, April 09, 2015 10:50:02 AM Jiang Liu wrote: On 2015/4/9 7:44, Rafael J. Wysocki wrote: On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote: On 2015/4/7 8:28, Rafael J. Wysocki wrote: On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: Hi Jiang, snip Currently acpi_dev_filter_resource_type() is only used by ACPI pci host bridge and IOAPIC driver, so it shouldn't affect other drivers. We should assume it will eventually be used for all ACPI devices, shouldn't we? I'm not sure about that, really. In fact, I'd restrict its use to devices types that actually can produce resources (ie. do not require the resources to be provided by their ancestors or to be available from a global pool). Otherwise we're pretty much guaranteed to get into trouble. And all of the above rules need to be documented in the kernel source tree or people will get confused. Hi Rafael, How about following comments for acpi_dev_filter_resource_type()? Thanks! Gerry /** * According to ACPI specifications, Consumer/Producer flag in ACPI resource * descriptor means: * 1(CONSUMER): This device consumes this resource * 0(PRODUCER): This device produces and consumes this resource * But for ACPI PCI host bridge, it is interpreted in another way: So first of all, this leads to a question: Why is it interpreted for ACPI PCI host bridges differently? Is it something we've figured out from experience, or is there a standard mandating that? Hi Rafael, I think we got this knowledge by experiences. PCI FW spec only states _CRS reports resources assigned to the host bridge by firmware. There's no statement about whether the resource is consumed by host bridge itself or provided to it's child bus/devices. On x86/IA64 side, the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF, but not sure about ARM64 side. So how about: This: PCI Firmware specification states that _CRS reports resources assigned to the host bridge, but there's no way to tell whether the resource is consumed by host bridge itself or provided to its child bus/devices. looks OK to me, but I'd replace the below with something like: However, experience shows, that in the PCI host bridge case firmware writers expect the resource to be provided to devices on the bus (below the bridge) for consumption entirely if its Consumer/Producer flag is clear. That expectation is reflected by the code in this routine as follows:. What a mess. The spec is regrettably lacking in Consumer/Producer specifics. But I don't see what's confusing about the descriptors that have Consumer/Producer bits. QWord, DWord, and Word descriptors don't seem to have a Consumer/Producer bit, but they do contain _TRA, so they must be intended for bridge windows. Can they also be used for device registers? I don't know. The Extended Address descriptor has a Consumer/Producer bit, and I would interpret the spec to mean that if the flag is clear (the device produces and consumes this resource), the resource is available on the bus below the bridge, i.e., the bridge consumes the resource on its upstream side and produces it on its downstream side. OK, that makes sense for bridges. If the bit were set (the device only consumes the resource), I would expect that to mean the descriptor is for bridge registers like 0xcf8/0xcfc that never appear on the downstream side. That part is clear. What is not clear is whether or not we can *always* expect the resources with Consumer/Producer *clear* to be produced on the downstram side. That appears to be the case for host bridges if my understanding of things is correct, but is it the case in general? Maybe I'm reading the spec too naively, but this doesn't seem a matter of experience. Well, the specification is not clear enough in that respect, at least as far as I can say, or maybe it is, but then does firmware always follow that interpretation? So I guess I'd like to propose to go back to the 3.19 behavior for PCI host bridges and then to handle the IOAPIC as a separate case. We can think about consolidating the two later. Any objections to doing that? Hi Rafael, When reverting to the behavior before v3.19, I found a simpler solution. The logic before v3.19 is: on x86 and IA64, all IO port and MMIO resources assigned to PCI host bridge are available to child bus/devices, except one
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/10 8:28, Rafael J. Wysocki wrote: > On Fri, Apr 10, 2015 at 12:37 AM, Rafael J. Wysocki > wrote: >> On Thursday, April 09, 2015 05:00:08 PM Bjorn Helgaas wrote: >>> On Thu, Apr 9, 2015 at 4:37 PM, Rafael J. Wysocki >>> wrote: On Thursday, April 09, 2015 10:50:02 AM Jiang Liu wrote: > On 2015/4/9 7:44, Rafael J. Wysocki wrote: >> On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote: >>> On 2015/4/7 8:28, Rafael J. Wysocki wrote: On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: > Hi Jiang, >>> >> Currently acpi_dev_filter_resource_type() is only used by ACPI pci >> host bridge and IOAPIC driver, so it shouldn't affect other drivers. > > We should assume it will eventually be used for all ACPI devices, > shouldn't we? I'm not sure about that, really. In fact, I'd restrict its use to devices types that actually can "produce" resources (ie. do not require the resources to be provided by their ancestors or to be available from a global pool). Otherwise we're pretty much guaranteed to get into trouble. And all of the above rules need to be documented in the kernel source tree or people will get confused. >>> Hi Rafael, >>> How about following comments for acpi_dev_filter_resource_type()? >>> Thanks! >>> Gerry >>> >>> /** >>> * According to ACPI specifications, Consumer/Producer flag in ACPI >>> resource >>> * descriptor means: >>> * 1(CONSUMER): This device consumes this resource >>> * 0(PRODUCER): This device produces and consumes this resource >>> * But for ACPI PCI host bridge, it is interpreted in another way: >> >> So first of all, this leads to a question: Why is it interpreted for >> ACPI PCI >> host bridges differently? >> >> Is it something we've figured out from experience, or is there a standard >> mandating that? > Hi Rafael, > I think we got this knowledge by experiences. PCI FW spec only > states _CRS reports resources assigned to the host bridge by firmware. > There's no statement about whether the resource is consumed by host > bridge itself or provided to it's child bus/devices. On x86/IA64 side, > the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF, > but not sure about ARM64 side. So how about: This: > PCI Firmware specification states that _CRS reports resources > assigned to the host bridge, but there's no way to tell whether > the resource is consumed by host bridge itself or provided to > its child bus/devices. looks OK to me, but I'd replace the below with something like: "However, experience shows, that in the PCI host bridge case firmware writers expect the resource to be provided to devices on the bus (below the bridge) for consumption entirely if its Consumer/Producer flag is clear. That expectation is reflected by the code in this routine as follows:". >>> >>> What a mess. The spec is regrettably lacking in Consumer/Producer >>> specifics. But I don't see what's confusing about the descriptors >>> that have Consumer/Producer bits. >>> >>> QWord, DWord, and Word descriptors don't seem to have a >>> Consumer/Producer bit, but they do contain _TRA, so they must be >>> intended for bridge windows. Can they also be used for device >>> registers? I don't know. >>> >>> The Extended Address descriptor has a Consumer/Producer bit, and I >>> would interpret the spec to mean that if the flag is clear (the device >>> produces and consumes this resource), the resource is available on the >>> bus below the bridge, i.e., the bridge consumes the resource on its >>> upstream side and produces it on its downstream side. >> >> OK, that makes sense for bridges. >>> If the bit were >>> set (the device only consumes the resource), I would expect that to >>> mean the descriptor is for bridge registers like 0xcf8/0xcfc that >>> never appear on the downstream side. >> >> That part is clear. What is not clear is whether or not we can *always* >> expect the resources with Consumer/Producer *clear* to be produced on the >> downstram side. That appears to be the case for host bridges if my >> understanding of things is correct, but is it the case in general? >> >>> Maybe I'm reading the spec too naively, but this doesn't seem a matter >>> of experience. >> >> Well, the specification is not clear enough in that respect, at least as >> far as I can say, or maybe it is, but then does firmware always follow that >> interpretation? > > So I guess I'd like to propose to go back to the 3.19 behavior for PCI host > bridges and then to handle the IOAPIC as a separate case. > > We can think
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/10 8:28, Rafael J. Wysocki wrote: On Fri, Apr 10, 2015 at 12:37 AM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Thursday, April 09, 2015 05:00:08 PM Bjorn Helgaas wrote: On Thu, Apr 9, 2015 at 4:37 PM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Thursday, April 09, 2015 10:50:02 AM Jiang Liu wrote: On 2015/4/9 7:44, Rafael J. Wysocki wrote: On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote: On 2015/4/7 8:28, Rafael J. Wysocki wrote: On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: Hi Jiang, snip Currently acpi_dev_filter_resource_type() is only used by ACPI pci host bridge and IOAPIC driver, so it shouldn't affect other drivers. We should assume it will eventually be used for all ACPI devices, shouldn't we? I'm not sure about that, really. In fact, I'd restrict its use to devices types that actually can produce resources (ie. do not require the resources to be provided by their ancestors or to be available from a global pool). Otherwise we're pretty much guaranteed to get into trouble. And all of the above rules need to be documented in the kernel source tree or people will get confused. Hi Rafael, How about following comments for acpi_dev_filter_resource_type()? Thanks! Gerry /** * According to ACPI specifications, Consumer/Producer flag in ACPI resource * descriptor means: * 1(CONSUMER): This device consumes this resource * 0(PRODUCER): This device produces and consumes this resource * But for ACPI PCI host bridge, it is interpreted in another way: So first of all, this leads to a question: Why is it interpreted for ACPI PCI host bridges differently? Is it something we've figured out from experience, or is there a standard mandating that? Hi Rafael, I think we got this knowledge by experiences. PCI FW spec only states _CRS reports resources assigned to the host bridge by firmware. There's no statement about whether the resource is consumed by host bridge itself or provided to it's child bus/devices. On x86/IA64 side, the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF, but not sure about ARM64 side. So how about: This: PCI Firmware specification states that _CRS reports resources assigned to the host bridge, but there's no way to tell whether the resource is consumed by host bridge itself or provided to its child bus/devices. looks OK to me, but I'd replace the below with something like: However, experience shows, that in the PCI host bridge case firmware writers expect the resource to be provided to devices on the bus (below the bridge) for consumption entirely if its Consumer/Producer flag is clear. That expectation is reflected by the code in this routine as follows:. What a mess. The spec is regrettably lacking in Consumer/Producer specifics. But I don't see what's confusing about the descriptors that have Consumer/Producer bits. QWord, DWord, and Word descriptors don't seem to have a Consumer/Producer bit, but they do contain _TRA, so they must be intended for bridge windows. Can they also be used for device registers? I don't know. The Extended Address descriptor has a Consumer/Producer bit, and I would interpret the spec to mean that if the flag is clear (the device produces and consumes this resource), the resource is available on the bus below the bridge, i.e., the bridge consumes the resource on its upstream side and produces it on its downstream side. OK, that makes sense for bridges. If the bit were set (the device only consumes the resource), I would expect that to mean the descriptor is for bridge registers like 0xcf8/0xcfc that never appear on the downstream side. That part is clear. What is not clear is whether or not we can *always* expect the resources with Consumer/Producer *clear* to be produced on the downstram side. That appears to be the case for host bridges if my understanding of things is correct, but is it the case in general? Maybe I'm reading the spec too naively, but this doesn't seem a matter of experience. Well, the specification is not clear enough in that respect, at least as far as I can say, or maybe it is, but then does firmware always follow that interpretation? So I guess I'd like to propose to go back to the 3.19 behavior for PCI host bridges and then to handle the IOAPIC as a separate case. We can think about consolidating the two later. Any objections to doing that? Hi Rafael, When reverting to the behavior before v3.19, I found a simpler solution. The logic before v3.19 is: on x86 and IA64, all IO port and MMIO resources assigned to PCI host bridge are available to child bus/devices, except one special case to filter out IO port[0xCF8-0xCFF] for PCI configuration space access. And with pre-v3.19 kernels, all IO port defined by IO and fixed_IO resource descriptors are ignored
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Thursday, April 09, 2015 10:50:02 AM Jiang Liu wrote: > On 2015/4/9 7:44, Rafael J. Wysocki wrote: > > On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote: > >> On 2015/4/7 8:28, Rafael J. Wysocki wrote: > >>> On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: > Hi Jiang, > >> > > Currently acpi_dev_filter_resource_type() is only used by ACPI pci > > host bridge and IOAPIC driver, so it shouldn't affect other drivers. > > We should assume it will eventually be used for all ACPI devices, > shouldn't we? > >>> > >>> I'm not sure about that, really. In fact, I'd restrict its use to devices > >>> types that actually can "produce" resources (ie. do not require the > >>> resources > >>> to be provided by their ancestors or to be available from a global pool). > >>> > >>> Otherwise we're pretty much guaranteed to get into trouble. > >>> > >>> And all of the above rules need to be documented in the kernel source tree > >>> or people will get confused. > >> Hi Rafael, > >> How about following comments for acpi_dev_filter_resource_type()? > >> Thanks! > >> Gerry > >> > >> /** > >> * According to ACPI specifications, Consumer/Producer flag in ACPI > >> resource > >> * descriptor means: > >> * 1(CONSUMER): This device consumes this resource > >> * 0(PRODUCER): This device produces and consumes this resource > >> * But for ACPI PCI host bridge, it is interpreted in another way: > > > > So first of all, this leads to a question: Why is it interpreted for ACPI > > PCI > > host bridges differently? > > > > Is it something we've figured out from experience, or is there a standard > > mandating that? > Hi Rafael, > I think we got this knowledge by experiences. PCI FW spec only > states _CRS reports resources assigned to the host bridge by firmware. > There's no statement about whether the resource is consumed by host > bridge itself or provided to it's child bus/devices. On x86/IA64 side, > the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF, > but not sure about ARM64 side. So how about: This: > PCI Firmware specification states that _CRS reports resources > assigned to the host bridge, but there's no way to tell whether > the resource is consumed by host bridge itself or provided to > its child bus/devices. looks OK to me, but I'd replace the below with something like: "However, experience shows, that in the PCI host bridge case firmware writers expect the resource to be provided to devices on the bus (below the bridge) for consumption entirely if its Consumer/Producer flag is clear. That expectation is reflected by the code in this routine as follows:". > So according to experiences, PCI host bridge > interprets Consumer/Producer flag as below to tell whether the resource > is consumed by host bridge itself. > > > > >> * 1(CONSUMER): PCI host bridge itself consumes the resource, such as > >> * IOPORT 0xCF8-0xCFF to access PCI configuraiton space. > >> * 0(PRODUCER): PCI host bridge provides this resource to its child > >> * bus and devices. > >> * > >> * So this is a specially designed helper function to support ACPI PCI host > >> * bridge and ACPI IOAPIC, and its usage should be limited to those > >> specific > > > > And this will make the reader wonder why the IOAPIC should be treated the > > same > > way as a PCI host bridge in that respect. Your responses would be easier to follow if they were clearly separate from the original message text (ie. add an empty line between the paragraph you're responding to and the response). > If a hot-pluggable IOAPIC is represented as an ACPI device, its _CRS > reports MMIO address assigned to the IOAPIC. And an IOAPIC device > won't produce MMIO resources by itself. So we could reuse > acpi_dev_filter_resource_type() here. > How about: > * So this is a specially designed helper function to support: > * 1) ACPI PCI host bridge, as explained above > * 2) ACPI IOAPIC, its _CRS reports only one MMIO resource and > *it won't produce MMIO resources by itself. OK, so I'd like to see how the code would look like if those two cases had their own "filter" routines, if that's not a problem. Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Thursday, April 09, 2015 10:50:02 AM Jiang Liu wrote: On 2015/4/9 7:44, Rafael J. Wysocki wrote: On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote: On 2015/4/7 8:28, Rafael J. Wysocki wrote: On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: Hi Jiang, snip Currently acpi_dev_filter_resource_type() is only used by ACPI pci host bridge and IOAPIC driver, so it shouldn't affect other drivers. We should assume it will eventually be used for all ACPI devices, shouldn't we? I'm not sure about that, really. In fact, I'd restrict its use to devices types that actually can produce resources (ie. do not require the resources to be provided by their ancestors or to be available from a global pool). Otherwise we're pretty much guaranteed to get into trouble. And all of the above rules need to be documented in the kernel source tree or people will get confused. Hi Rafael, How about following comments for acpi_dev_filter_resource_type()? Thanks! Gerry /** * According to ACPI specifications, Consumer/Producer flag in ACPI resource * descriptor means: * 1(CONSUMER): This device consumes this resource * 0(PRODUCER): This device produces and consumes this resource * But for ACPI PCI host bridge, it is interpreted in another way: So first of all, this leads to a question: Why is it interpreted for ACPI PCI host bridges differently? Is it something we've figured out from experience, or is there a standard mandating that? Hi Rafael, I think we got this knowledge by experiences. PCI FW spec only states _CRS reports resources assigned to the host bridge by firmware. There's no statement about whether the resource is consumed by host bridge itself or provided to it's child bus/devices. On x86/IA64 side, the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF, but not sure about ARM64 side. So how about: This: PCI Firmware specification states that _CRS reports resources assigned to the host bridge, but there's no way to tell whether the resource is consumed by host bridge itself or provided to its child bus/devices. looks OK to me, but I'd replace the below with something like: However, experience shows, that in the PCI host bridge case firmware writers expect the resource to be provided to devices on the bus (below the bridge) for consumption entirely if its Consumer/Producer flag is clear. That expectation is reflected by the code in this routine as follows:. So according to experiences, PCI host bridge interprets Consumer/Producer flag as below to tell whether the resource is consumed by host bridge itself. * 1(CONSUMER): PCI host bridge itself consumes the resource, such as * IOPORT 0xCF8-0xCFF to access PCI configuraiton space. * 0(PRODUCER): PCI host bridge provides this resource to its child * bus and devices. * * So this is a specially designed helper function to support ACPI PCI host * bridge and ACPI IOAPIC, and its usage should be limited to those specific And this will make the reader wonder why the IOAPIC should be treated the same way as a PCI host bridge in that respect. Your responses would be easier to follow if they were clearly separate from the original message text (ie. add an empty line between the paragraph you're responding to and the response). If a hot-pluggable IOAPIC is represented as an ACPI device, its _CRS reports MMIO address assigned to the IOAPIC. And an IOAPIC device won't produce MMIO resources by itself. So we could reuse acpi_dev_filter_resource_type() here. How about: * So this is a specially designed helper function to support: * 1) ACPI PCI host bridge, as explained above * 2) ACPI IOAPIC, its _CRS reports only one MMIO resource and *it won't produce MMIO resources by itself. OK, so I'd like to see how the code would look like if those two cases had their own filter routines, if that's not a problem. Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/9 7:44, Rafael J. Wysocki wrote:
> On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote:
>> On 2015/4/7 8:28, Rafael J. Wysocki wrote:
>>> On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
Hi Jiang,
>>
> Currently acpi_dev_filter_resource_type() is only used by ACPI pci
> host bridge and IOAPIC driver, so it shouldn't affect other drivers.
We should assume it will eventually be used for all ACPI devices,
shouldn't we?
>>>
>>> I'm not sure about that, really. In fact, I'd restrict its use to devices
>>> types that actually can "produce" resources (ie. do not require the
>>> resources
>>> to be provided by their ancestors or to be available from a global pool).
>>>
>>> Otherwise we're pretty much guaranteed to get into trouble.
>>>
>>> And all of the above rules need to be documented in the kernel source tree
>>> or people will get confused.
>> Hi Rafael,
>> How about following comments for acpi_dev_filter_resource_type()?
>> Thanks!
>> Gerry
>>
>> /**
>> * According to ACPI specifications, Consumer/Producer flag in ACPI resource
>> * descriptor means:
>> * 1(CONSUMER): This device consumes this resource
>> * 0(PRODUCER): This device produces and consumes this resource
>> * But for ACPI PCI host bridge, it is interpreted in another way:
>
> So first of all, this leads to a question: Why is it interpreted for ACPI PCI
> host bridges differently?
>
> Is it something we've figured out from experience, or is there a standard
> mandating that?
Hi Rafael,
I think we got this knowledge by experiences. PCI FW spec only
states _CRS reports resources assigned to the host bridge by firmware.
There's no statement about whether the resource is consumed by host
bridge itself or provided to it's child bus/devices. On x86/IA64 side,
the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF,
but not sure about ARM64 side. So how about:
PCI Firmware specification states that _CRS reports resources
assigned to the host bridge, but there's no way to tell whether
the resource is consumed by host bridge itself or provided to
its child bus/devices. So according to experiences, PCI host bridge
interprets Consumer/Producer flag as below to tell whether the resource
is consumed by host bridge itself.
>
>> * 1(CONSUMER): PCI host bridge itself consumes the resource, such as
>> * IOPORT 0xCF8-0xCFF to access PCI configuraiton space.
>> * 0(PRODUCER): PCI host bridge provides this resource to its child
>> * bus and devices.
>> *
>> * So this is a specially designed helper function to support ACPI PCI host
>> * bridge and ACPI IOAPIC, and its usage should be limited to those specific
>
> And this will make the reader wonder why the IOAPIC should be treated the same
> way as a PCI host bridge in that respect.
If a hot-pluggable IOAPIC is represented as an ACPI device, its _CRS
reports MMIO address assigned to the IOAPIC. And an IOAPIC device
won't produce MMIO resources by itself. So we could reuse
acpi_dev_filter_resource_type() here.
How about:
* So this is a specially designed helper function to support:
* 1) ACPI PCI host bridge, as explained above
* 2) ACPI IOAPIC, its _CRS reports only one MMIO resource and
*it won't produce MMIO resources by itself.
Thanks!
Gerry
>
>> * scenarioso only. It filters ACPI resource descriptors as below:
>> * 1) If flag IORESOURCE_WINDOW is not specified, it's querying resources
>> *consumed by device. That is to return:
>> * a) ACPI resources without producer_consumer flag
>> * b) ACPI resources with producer_consumer flag setting to CONSUMER.
>> * 2) If flag IORESOURCE_WINDOW is specified, it's querying resources
>> provided
>> *by device. That is to return:
>> * a) ACPI resources with producer_consumer flag setting to PRODUCER.
>> * 3) But there's an exception. Some platforms, such as PC Engines APU.1C,
>> *report PCI host bridge resource provision by Memory32Fixed().
>> *Please refer to https://bugzilla.kernel.org/show_bug.cgi?id=94221
>> *So a special flag IORESOURCE_MEM_8AND16BIT is used to work around this
>> *BIOS issue.
>> */
>>
>>>
> Another possible fix is to only ignore IO resource consumed by host
> bridge and keep IOMEM resource consumed by host bridge, please refer to:
> http://www.spinics.net/lists/linux-pci/msg39706.html
>
> Sample ACPI table are archived at:
> https://bugzilla.kernel.org/show_bug.cgi?id=94221
>
> V2->V3:
> Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
>
> Fixes: 63f1789ec716("Ignore resources consumed by host bridge itself")
> Reported-and-Tested-by: Bernhard Thaler
> Signed-off-by: Jiang Liu
> ---
> arch/x86/pci/acpi.c |5 ++---
> drivers/acpi/resource.c | 33
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote:
> On 2015/4/7 8:28, Rafael J. Wysocki wrote:
> > On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
> >> Hi Jiang,
>
> >>> Currently acpi_dev_filter_resource_type() is only used by ACPI pci
> >>> host bridge and IOAPIC driver, so it shouldn't affect other drivers.
> >>
> >> We should assume it will eventually be used for all ACPI devices,
> >> shouldn't we?
> >
> > I'm not sure about that, really. In fact, I'd restrict its use to devices
> > types that actually can "produce" resources (ie. do not require the
> > resources
> > to be provided by their ancestors or to be available from a global pool).
> >
> > Otherwise we're pretty much guaranteed to get into trouble.
> >
> > And all of the above rules need to be documented in the kernel source tree
> > or people will get confused.
> Hi Rafael,
> How about following comments for acpi_dev_filter_resource_type()?
> Thanks!
> Gerry
>
> /**
> * According to ACPI specifications, Consumer/Producer flag in ACPI resource
> * descriptor means:
> * 1(CONSUMER): This device consumes this resource
> * 0(PRODUCER): This device produces and consumes this resource
> * But for ACPI PCI host bridge, it is interpreted in another way:
So first of all, this leads to a question: Why is it interpreted for ACPI PCI
host bridges differently?
Is it something we've figured out from experience, or is there a standard
mandating that?
> * 1(CONSUMER): PCI host bridge itself consumes the resource, such as
> * IOPORT 0xCF8-0xCFF to access PCI configuraiton space.
> * 0(PRODUCER): PCI host bridge provides this resource to its child
> * bus and devices.
> *
> * So this is a specially designed helper function to support ACPI PCI host
> * bridge and ACPI IOAPIC, and its usage should be limited to those specific
And this will make the reader wonder why the IOAPIC should be treated the same
way as a PCI host bridge in that respect.
> * scenarioso only. It filters ACPI resource descriptors as below:
> * 1) If flag IORESOURCE_WINDOW is not specified, it's querying resources
> *consumed by device. That is to return:
> * a) ACPI resources without producer_consumer flag
> * b) ACPI resources with producer_consumer flag setting to CONSUMER.
> * 2) If flag IORESOURCE_WINDOW is specified, it's querying resources
> provided
> *by device. That is to return:
> * a) ACPI resources with producer_consumer flag setting to PRODUCER.
> * 3) But there's an exception. Some platforms, such as PC Engines APU.1C,
> *report PCI host bridge resource provision by Memory32Fixed().
> *Please refer to https://bugzilla.kernel.org/show_bug.cgi?id=94221
> *So a special flag IORESOURCE_MEM_8AND16BIT is used to work around this
> *BIOS issue.
> */
>
> >
> >>> Another possible fix is to only ignore IO resource consumed by host
> >>> bridge and keep IOMEM resource consumed by host bridge, please refer to:
> >>> http://www.spinics.net/lists/linux-pci/msg39706.html
> >>>
> >>> Sample ACPI table are archived at:
> >>> https://bugzilla.kernel.org/show_bug.cgi?id=94221
> >>>
> >>> V2->V3:
> >>> Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
> >>>
> >>> Fixes: 63f1789ec716("Ignore resources consumed by host bridge itself")
> >>> Reported-and-Tested-by: Bernhard Thaler
> >>> Signed-off-by: Jiang Liu
> >>> ---
> >>> arch/x86/pci/acpi.c |5 ++---
> >>> drivers/acpi/resource.c | 33 +
> >>> 2 files changed, 31 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> >>> index e4695985f9de..8c4b1201f340 100644
> >>> --- a/arch/x86/pci/acpi.c
> >>> +++ b/arch/x86/pci/acpi.c
> >>> @@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info
> >>> *info,
> >>> info->bridge = device;
> >>> ret = acpi_dev_get_resources(device, list,
> >>>acpi_dev_filter_resource_type_cb,
> >>> - (void *)(IORESOURCE_IO | IORESOURCE_MEM));
> >>> + (void *)(IORESOURCE_IO | IORESOURCE_MEM |
> >>> IORESOURCE_WINDOW));
> >>> if (ret < 0)
> >>> dev_warn(>dev,
> >>>"failed to parse _CRS method, error code %d\n", ret);
> >>> @@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info
> >>> *info,
> >>> "no IO and memory resources present in _CRS\n");
> >>> else
> >>> resource_list_for_each_entry_safe(entry, tmp, list) {
> >>> - if ((entry->res->flags & IORESOURCE_WINDOW) == 0 ||
> >>> - (entry->res->flags & IORESOURCE_DISABLED))
> >>> + if (entry->res->flags & IORESOURCE_DISABLED)
> >>> resource_list_destroy_entry(entry);
> >>>
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote:
On 2015/4/7 8:28, Rafael J. Wysocki wrote:
On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
Hi Jiang,
snip
Currently acpi_dev_filter_resource_type() is only used by ACPI pci
host bridge and IOAPIC driver, so it shouldn't affect other drivers.
We should assume it will eventually be used for all ACPI devices,
shouldn't we?
I'm not sure about that, really. In fact, I'd restrict its use to devices
types that actually can produce resources (ie. do not require the
resources
to be provided by their ancestors or to be available from a global pool).
Otherwise we're pretty much guaranteed to get into trouble.
And all of the above rules need to be documented in the kernel source tree
or people will get confused.
Hi Rafael,
How about following comments for acpi_dev_filter_resource_type()?
Thanks!
Gerry
/**
* According to ACPI specifications, Consumer/Producer flag in ACPI resource
* descriptor means:
* 1(CONSUMER): This device consumes this resource
* 0(PRODUCER): This device produces and consumes this resource
* But for ACPI PCI host bridge, it is interpreted in another way:
So first of all, this leads to a question: Why is it interpreted for ACPI PCI
host bridges differently?
Is it something we've figured out from experience, or is there a standard
mandating that?
* 1(CONSUMER): PCI host bridge itself consumes the resource, such as
* IOPORT 0xCF8-0xCFF to access PCI configuraiton space.
* 0(PRODUCER): PCI host bridge provides this resource to its child
* bus and devices.
*
* So this is a specially designed helper function to support ACPI PCI host
* bridge and ACPI IOAPIC, and its usage should be limited to those specific
And this will make the reader wonder why the IOAPIC should be treated the same
way as a PCI host bridge in that respect.
* scenarioso only. It filters ACPI resource descriptors as below:
* 1) If flag IORESOURCE_WINDOW is not specified, it's querying resources
*consumed by device. That is to return:
* a) ACPI resources without producer_consumer flag
* b) ACPI resources with producer_consumer flag setting to CONSUMER.
* 2) If flag IORESOURCE_WINDOW is specified, it's querying resources
provided
*by device. That is to return:
* a) ACPI resources with producer_consumer flag setting to PRODUCER.
* 3) But there's an exception. Some platforms, such as PC Engines APU.1C,
*report PCI host bridge resource provision by Memory32Fixed().
*Please refer to https://bugzilla.kernel.org/show_bug.cgi?id=94221
*So a special flag IORESOURCE_MEM_8AND16BIT is used to work around this
*BIOS issue.
*/
Another possible fix is to only ignore IO resource consumed by host
bridge and keep IOMEM resource consumed by host bridge, please refer to:
http://www.spinics.net/lists/linux-pci/msg39706.html
Sample ACPI table are archived at:
https://bugzilla.kernel.org/show_bug.cgi?id=94221
V2-V3:
Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
Fixes: 63f1789ec716(Ignore resources consumed by host bridge itself)
Reported-and-Tested-by: Bernhard Thaler bernhard.tha...@wvnet.at
Signed-off-by: Jiang Liu jiang@linux.intel.com
---
arch/x86/pci/acpi.c |5 ++---
drivers/acpi/resource.c | 33 +
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index e4695985f9de..8c4b1201f340 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info
*info,
info-bridge = device;
ret = acpi_dev_get_resources(device, list,
acpi_dev_filter_resource_type_cb,
- (void *)(IORESOURCE_IO | IORESOURCE_MEM));
+ (void *)(IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_WINDOW));
if (ret 0)
dev_warn(device-dev,
failed to parse _CRS method, error code %d\n, ret);
@@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info
*info,
no IO and memory resources present in _CRS\n);
else
resource_list_for_each_entry_safe(entry, tmp, list) {
- if ((entry-res-flags IORESOURCE_WINDOW) == 0 ||
- (entry-res-flags IORESOURCE_DISABLED))
+ if (entry-res-flags IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
else
entry-res-name = info-name;
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 5589a6e2a023..e761a868bdba 100644
--- a/drivers/acpi/resource.c
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/9 7:44, Rafael J. Wysocki wrote: On Wednesday, April 08, 2015 01:48:46 PM Jiang Liu wrote: On 2015/4/7 8:28, Rafael J. Wysocki wrote: On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: Hi Jiang, snip Currently acpi_dev_filter_resource_type() is only used by ACPI pci host bridge and IOAPIC driver, so it shouldn't affect other drivers. We should assume it will eventually be used for all ACPI devices, shouldn't we? I'm not sure about that, really. In fact, I'd restrict its use to devices types that actually can produce resources (ie. do not require the resources to be provided by their ancestors or to be available from a global pool). Otherwise we're pretty much guaranteed to get into trouble. And all of the above rules need to be documented in the kernel source tree or people will get confused. Hi Rafael, How about following comments for acpi_dev_filter_resource_type()? Thanks! Gerry /** * According to ACPI specifications, Consumer/Producer flag in ACPI resource * descriptor means: * 1(CONSUMER): This device consumes this resource * 0(PRODUCER): This device produces and consumes this resource * But for ACPI PCI host bridge, it is interpreted in another way: So first of all, this leads to a question: Why is it interpreted for ACPI PCI host bridges differently? Is it something we've figured out from experience, or is there a standard mandating that? Hi Rafael, I think we got this knowledge by experiences. PCI FW spec only states _CRS reports resources assigned to the host bridge by firmware. There's no statement about whether the resource is consumed by host bridge itself or provided to it's child bus/devices. On x86/IA64 side, the main resource consumed by PCI host bridge is IOPORT 0xCF8-0xCFF, but not sure about ARM64 side. So how about: PCI Firmware specification states that _CRS reports resources assigned to the host bridge, but there's no way to tell whether the resource is consumed by host bridge itself or provided to its child bus/devices. So according to experiences, PCI host bridge interprets Consumer/Producer flag as below to tell whether the resource is consumed by host bridge itself. * 1(CONSUMER): PCI host bridge itself consumes the resource, such as * IOPORT 0xCF8-0xCFF to access PCI configuraiton space. * 0(PRODUCER): PCI host bridge provides this resource to its child * bus and devices. * * So this is a specially designed helper function to support ACPI PCI host * bridge and ACPI IOAPIC, and its usage should be limited to those specific And this will make the reader wonder why the IOAPIC should be treated the same way as a PCI host bridge in that respect. If a hot-pluggable IOAPIC is represented as an ACPI device, its _CRS reports MMIO address assigned to the IOAPIC. And an IOAPIC device won't produce MMIO resources by itself. So we could reuse acpi_dev_filter_resource_type() here. How about: * So this is a specially designed helper function to support: * 1) ACPI PCI host bridge, as explained above * 2) ACPI IOAPIC, its _CRS reports only one MMIO resource and *it won't produce MMIO resources by itself. Thanks! Gerry * scenarioso only. It filters ACPI resource descriptors as below: * 1) If flag IORESOURCE_WINDOW is not specified, it's querying resources *consumed by device. That is to return: * a) ACPI resources without producer_consumer flag * b) ACPI resources with producer_consumer flag setting to CONSUMER. * 2) If flag IORESOURCE_WINDOW is specified, it's querying resources provided *by device. That is to return: * a) ACPI resources with producer_consumer flag setting to PRODUCER. * 3) But there's an exception. Some platforms, such as PC Engines APU.1C, *report PCI host bridge resource provision by Memory32Fixed(). *Please refer to https://bugzilla.kernel.org/show_bug.cgi?id=94221 *So a special flag IORESOURCE_MEM_8AND16BIT is used to work around this *BIOS issue. */ Another possible fix is to only ignore IO resource consumed by host bridge and keep IOMEM resource consumed by host bridge, please refer to: http://www.spinics.net/lists/linux-pci/msg39706.html Sample ACPI table are archived at: https://bugzilla.kernel.org/show_bug.cgi?id=94221 V2-V3: Refine function acpi_dev_match_producer_consumer() as suggested by Rafael Fixes: 63f1789ec716(Ignore resources consumed by host bridge itself) Reported-and-Tested-by: Bernhard Thaler bernhard.tha...@wvnet.at Signed-off-by: Jiang Liu jiang@linux.intel.com --- arch/x86/pci/acpi.c |5 ++--- drivers/acpi/resource.c | 33 + 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index e4695985f9de..8c4b1201f340 100644 ---
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/7 8:28, Rafael J. Wysocki wrote:
> On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
>> Hi Jiang,
>>> Currently acpi_dev_filter_resource_type() is only used by ACPI pci
>>> host bridge and IOAPIC driver, so it shouldn't affect other drivers.
>>
>> We should assume it will eventually be used for all ACPI devices,
>> shouldn't we?
>
> I'm not sure about that, really. In fact, I'd restrict its use to devices
> types that actually can "produce" resources (ie. do not require the resources
> to be provided by their ancestors or to be available from a global pool).
>
> Otherwise we're pretty much guaranteed to get into trouble.
>
> And all of the above rules need to be documented in the kernel source tree
> or people will get confused.
Hi Rafael,
How about following comments for acpi_dev_filter_resource_type()?
Thanks!
Gerry
/**
* According to ACPI specifications, Consumer/Producer flag in ACPI resource
* descriptor means:
* 1(CONSUMER): This device consumes this resource
* 0(PRODUCER): This device produces and consumes this resource
* But for ACPI PCI host bridge, it is interpreted in another way:
* 1(CONSUMER): PCI host bridge itself consumes the resource, such as
* IOPORT 0xCF8-0xCFF to access PCI configuraiton space.
* 0(PRODUCER): PCI host bridge provides this resource to its child
* bus and devices.
*
* So this is a specially designed helper function to support ACPI PCI host
* bridge and ACPI IOAPIC, and its usage should be limited to those specific
* scenarioso only. It filters ACPI resource descriptors as below:
* 1) If flag IORESOURCE_WINDOW is not specified, it's querying resources
*consumed by device. That is to return:
* a) ACPI resources without producer_consumer flag
* b) ACPI resources with producer_consumer flag setting to CONSUMER.
* 2) If flag IORESOURCE_WINDOW is specified, it's querying resources
provided
*by device. That is to return:
* a) ACPI resources with producer_consumer flag setting to PRODUCER.
* 3) But there's an exception. Some platforms, such as PC Engines APU.1C,
*report PCI host bridge resource provision by Memory32Fixed().
*Please refer to https://bugzilla.kernel.org/show_bug.cgi?id=94221
*So a special flag IORESOURCE_MEM_8AND16BIT is used to work around this
*BIOS issue.
*/
>
>>> Another possible fix is to only ignore IO resource consumed by host
>>> bridge and keep IOMEM resource consumed by host bridge, please refer to:
>>> http://www.spinics.net/lists/linux-pci/msg39706.html
>>>
>>> Sample ACPI table are archived at:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=94221
>>>
>>> V2->V3:
>>> Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
>>>
>>> Fixes: 63f1789ec716("Ignore resources consumed by host bridge itself")
>>> Reported-and-Tested-by: Bernhard Thaler
>>> Signed-off-by: Jiang Liu
>>> ---
>>> arch/x86/pci/acpi.c |5 ++---
>>> drivers/acpi/resource.c | 33 +
>>> 2 files changed, 31 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>>> index e4695985f9de..8c4b1201f340 100644
>>> --- a/arch/x86/pci/acpi.c
>>> +++ b/arch/x86/pci/acpi.c
>>> @@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info
>>> *info,
>>> info->bridge = device;
>>> ret = acpi_dev_get_resources(device, list,
>>> acpi_dev_filter_resource_type_cb,
>>> -(void *)(IORESOURCE_IO | IORESOURCE_MEM));
>>> +(void *)(IORESOURCE_IO | IORESOURCE_MEM |
>>> IORESOURCE_WINDOW));
>>> if (ret < 0)
>>> dev_warn(>dev,
>>> "failed to parse _CRS method, error code %d\n", ret);
>>> @@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info
>>> *info,
>>> "no IO and memory resources present in _CRS\n");
>>> else
>>> resource_list_for_each_entry_safe(entry, tmp, list) {
>>> - if ((entry->res->flags & IORESOURCE_WINDOW) == 0 ||
>>> - (entry->res->flags & IORESOURCE_DISABLED))
>>> + if (entry->res->flags & IORESOURCE_DISABLED)
>>> resource_list_destroy_entry(entry);
>>> else
>>> entry->res->name = info->name;
>>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>>> index 5589a6e2a023..e761a868bdba 100644
>>> --- a/drivers/acpi/resource.c
>>> +++ b/drivers/acpi/resource.c
>>> @@ -567,6 +567,12 @@ int acpi_dev_get_resources(struct acpi_device *adev,
>>> struct list_head *list,
>>> }
>>> EXPORT_SYMBOL_GPL(acpi_dev_get_resources);
>>>
>>> +static bool acpi_dev_match_producer_consumer(unsigned long types, int
>>> producer)
>>> +{
>>> + return
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/7 8:28, Rafael J. Wysocki wrote:
> On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
>> Hi Jiang,
>>
>> Sorry for my delayed response. I've been on vacation for a week and am
>> still trying to catch up.
>>
>> On Mon, Mar 30, 2015 at 10:40:43AM +0800, Jiang Liu wrote:
>>> Then commit 63f1789ec716("Ignore resources consumed by host bridge
>>> itself") ignores resources consumed by host bridge itself by checking
>>> IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
>>> above for BIOS bug .
>>
>> This is probably partly my fault.
>>
>> I think the ACPI spec intention is that every _CRS resource descriptor
>> should be interpreted as "Consumer," i.e., as resources consumed by the
>> device itself, unless it's marked otherwise. Only the following types can
>> be marked as "Producer":
>>
>> - Word/DWord/QWord/Extended address space descriptors,
>> - Extended interrupt descriptors,
>> - GPIO interrupt and I/O connections,
>> - I2C/SPI/UART serial bus resource descriptors
>
> So we're talking about the consumer/producer flag in those extended resource
> type descriptors, right?
>
> My understanding of that flag is that it doesn't say whether or not the device
> is a producer of a resource in a general sense. It only says whether the
> device
> consumes a resource provided by someone else (1) or it consumes a resources
> provided by itself (0).
Hi Rafael,
I have read the ACPI spec again.
You are right, the spec states that:
Consumer/Producer:
1–This device consumes this resource
0–This device produces and consumes this resource
This is different from my previous understanding.
Previously I thought "CONSUMER" means the device consumes the resource
by itself and "PRODUCER" means the device provides resource to other
devices.
So seems PCI host bridge has different interpretation of
CONSUMER/PRODUCER flag from ACPI spec.
>
>> With 66528fdd45b0 ("x86/PCI: parse additional host bridge window resource
>> types"), I made Linux treat Memory24, Memory32, and Memory32Fixed
>> descriptors in PCI host bridge _CRS as Producers. I did it because Windows
>> apparently does that (there are details in
>> https://bugzilla.kernel.org/show_bug.cgi?id=15817),
>
> It looks like it does that to indicate that those resources are provided
> by the host bridge itself (which is consistent with the consumer/producer
> flag interpretation above)
>
>> but I wasn't aware of any machines that required it. That was probably a
>> mistake because it didn't fix anything and it covered up ASL usage errors
>> like what PC Engines did.
>
> I don't think it is required. It only seems to allow Windows to consolidate
> the handling of host bridge resources.
>
>>> It's really costed us much time to figure out this whole picture.
>>> So we refine interface acpi_dev_filter_resource_type as below,
>>> which should be easier for maintence:
>>> 1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
>>>for bridge(PRODUCER), otherwise it's querying resource for
>>>device(CONSUMER).
>>
>> Sounds good to me.
>>
>>> 2) Ignore IO port resources defined by acpi_resource_io and
>>>acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
>>
>> Sounds good to me.
>>
>>> 3) Accpet IOMEM resource defined by acpi_resource_memory24,
>>>acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
>>>bugs, with comment to state it's workaround for BIOS bug.
>>
>> I don't like the fact that this is the behavior for all ACPI devices.
>> Prior to 593669c2ac0f, we had this behavior for PCI host bridges only.
>> I don't think this is what the spec envisioned, so I don't really like
>> doing it for all devices.
>
> Agreed.
>
>>> 4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
>>>a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
>>>b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
>>
>> Sounds good to me.
>>
>>> Currently acpi_dev_filter_resource_type() is only used by ACPI pci
>>> host bridge and IOAPIC driver, so it shouldn't affect other drivers.
>>
>> We should assume it will eventually be used for all ACPI devices,
>> shouldn't we?
>
> I'm not sure about that, really. In fact, I'd restrict its use to devices
> types that actually can "produce" resources (ie. do not require the resources
> to be provided by their ancestors or to be available from a global pool).
>
> Otherwise we're pretty much guaranteed to get into trouble.
>
> And all of the above rules need to be documented in the kernel source tree
> or people will get confused.
You are right, we should limit acpi_dev_filter_resource_type() usages
to PCI host bridges and IOAPIC only.
>
>>> Another possible fix is to only ignore IO resource consumed by host
>>> @@ -585,27 +591,46 @@ int acpi_dev_filter_resource_type(struct
>>> acpi_resource *ares,
>>> case ACPI_RESOURCE_TYPE_MEMORY24:
>>> case ACPI_RESOURCE_TYPE_MEMORY32:
>>> case
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/7 8:28, Rafael J. Wysocki wrote: On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote: Hi Jiang, Sorry for my delayed response. I've been on vacation for a week and am still trying to catch up. On Mon, Mar 30, 2015 at 10:40:43AM +0800, Jiang Liu wrote: snip Then commit 63f1789ec716(Ignore resources consumed by host bridge itself) ignores resources consumed by host bridge itself by checking IORESOURCE_WINDOW flag, which accidently removed the workaround in 2) above for BIOS bug . This is probably partly my fault. I think the ACPI spec intention is that every _CRS resource descriptor should be interpreted as Consumer, i.e., as resources consumed by the device itself, unless it's marked otherwise. Only the following types can be marked as Producer: - Word/DWord/QWord/Extended address space descriptors, - Extended interrupt descriptors, - GPIO interrupt and I/O connections, - I2C/SPI/UART serial bus resource descriptors So we're talking about the consumer/producer flag in those extended resource type descriptors, right? My understanding of that flag is that it doesn't say whether or not the device is a producer of a resource in a general sense. It only says whether the device consumes a resource provided by someone else (1) or it consumes a resources provided by itself (0). Hi Rafael, I have read the ACPI spec again. You are right, the spec states that: Consumer/Producer: 1–This device consumes this resource 0–This device produces and consumes this resource This is different from my previous understanding. Previously I thought CONSUMER means the device consumes the resource by itself and PRODUCER means the device provides resource to other devices. So seems PCI host bridge has different interpretation of CONSUMER/PRODUCER flag from ACPI spec. With 66528fdd45b0 (x86/PCI: parse additional host bridge window resource types), I made Linux treat Memory24, Memory32, and Memory32Fixed descriptors in PCI host bridge _CRS as Producers. I did it because Windows apparently does that (there are details in https://bugzilla.kernel.org/show_bug.cgi?id=15817), It looks like it does that to indicate that those resources are provided by the host bridge itself (which is consistent with the consumer/producer flag interpretation above) but I wasn't aware of any machines that required it. That was probably a mistake because it didn't fix anything and it covered up ASL usage errors like what PC Engines did. I don't think it is required. It only seems to allow Windows to consolidate the handling of host bridge resources. It's really costed us much time to figure out this whole picture. So we refine interface acpi_dev_filter_resource_type as below, which should be easier for maintence: 1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource for bridge(PRODUCER), otherwise it's querying resource for device(CONSUMER). Sounds good to me. 2) Ignore IO port resources defined by acpi_resource_io and acpi_resource_fixed_io if IORESOURCE_WINDOW is specified. Sounds good to me. 3) Accpet IOMEM resource defined by acpi_resource_memory24, acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS bugs, with comment to state it's workaround for BIOS bug. I don't like the fact that this is the behavior for all ACPI devices. Prior to 593669c2ac0f, we had this behavior for PCI host bridges only. I don't think this is what the spec envisioned, so I don't really like doing it for all devices. Agreed. 4) Accept IO port and IOMEM defined by acpi_resource_addressxx if a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false Sounds good to me. Currently acpi_dev_filter_resource_type() is only used by ACPI pci host bridge and IOAPIC driver, so it shouldn't affect other drivers. We should assume it will eventually be used for all ACPI devices, shouldn't we? I'm not sure about that, really. In fact, I'd restrict its use to devices types that actually can produce resources (ie. do not require the resources to be provided by their ancestors or to be available from a global pool). Otherwise we're pretty much guaranteed to get into trouble. And all of the above rules need to be documented in the kernel source tree or people will get confused. You are right, we should limit acpi_dev_filter_resource_type() usages to PCI host bridges and IOAPIC only. Another possible fix is to only ignore IO resource consumed by host snip @@ -585,27 +591,46 @@ int acpi_dev_filter_resource_type(struct acpi_resource *ares, case ACPI_RESOURCE_TYPE_MEMORY24: case ACPI_RESOURCE_TYPE_MEMORY32: case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: + /* +* These types of resource descriptor should be used to +* describe resource consumption instead of resource provision. +*
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On 2015/4/7 8:28, Rafael J. Wysocki wrote:
On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
Hi Jiang,
snip
Currently acpi_dev_filter_resource_type() is only used by ACPI pci
host bridge and IOAPIC driver, so it shouldn't affect other drivers.
We should assume it will eventually be used for all ACPI devices,
shouldn't we?
I'm not sure about that, really. In fact, I'd restrict its use to devices
types that actually can produce resources (ie. do not require the resources
to be provided by their ancestors or to be available from a global pool).
Otherwise we're pretty much guaranteed to get into trouble.
And all of the above rules need to be documented in the kernel source tree
or people will get confused.
Hi Rafael,
How about following comments for acpi_dev_filter_resource_type()?
Thanks!
Gerry
/**
* According to ACPI specifications, Consumer/Producer flag in ACPI resource
* descriptor means:
* 1(CONSUMER): This device consumes this resource
* 0(PRODUCER): This device produces and consumes this resource
* But for ACPI PCI host bridge, it is interpreted in another way:
* 1(CONSUMER): PCI host bridge itself consumes the resource, such as
* IOPORT 0xCF8-0xCFF to access PCI configuraiton space.
* 0(PRODUCER): PCI host bridge provides this resource to its child
* bus and devices.
*
* So this is a specially designed helper function to support ACPI PCI host
* bridge and ACPI IOAPIC, and its usage should be limited to those specific
* scenarioso only. It filters ACPI resource descriptors as below:
* 1) If flag IORESOURCE_WINDOW is not specified, it's querying resources
*consumed by device. That is to return:
* a) ACPI resources without producer_consumer flag
* b) ACPI resources with producer_consumer flag setting to CONSUMER.
* 2) If flag IORESOURCE_WINDOW is specified, it's querying resources
provided
*by device. That is to return:
* a) ACPI resources with producer_consumer flag setting to PRODUCER.
* 3) But there's an exception. Some platforms, such as PC Engines APU.1C,
*report PCI host bridge resource provision by Memory32Fixed().
*Please refer to https://bugzilla.kernel.org/show_bug.cgi?id=94221
*So a special flag IORESOURCE_MEM_8AND16BIT is used to work around this
*BIOS issue.
*/
Another possible fix is to only ignore IO resource consumed by host
bridge and keep IOMEM resource consumed by host bridge, please refer to:
http://www.spinics.net/lists/linux-pci/msg39706.html
Sample ACPI table are archived at:
https://bugzilla.kernel.org/show_bug.cgi?id=94221
V2-V3:
Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
Fixes: 63f1789ec716(Ignore resources consumed by host bridge itself)
Reported-and-Tested-by: Bernhard Thaler bernhard.tha...@wvnet.at
Signed-off-by: Jiang Liu jiang@linux.intel.com
---
arch/x86/pci/acpi.c |5 ++---
drivers/acpi/resource.c | 33 +
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index e4695985f9de..8c4b1201f340 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info
*info,
info-bridge = device;
ret = acpi_dev_get_resources(device, list,
acpi_dev_filter_resource_type_cb,
-(void *)(IORESOURCE_IO | IORESOURCE_MEM));
+(void *)(IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_WINDOW));
if (ret 0)
dev_warn(device-dev,
failed to parse _CRS method, error code %d\n, ret);
@@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info
*info,
no IO and memory resources present in _CRS\n);
else
resource_list_for_each_entry_safe(entry, tmp, list) {
- if ((entry-res-flags IORESOURCE_WINDOW) == 0 ||
- (entry-res-flags IORESOURCE_DISABLED))
+ if (entry-res-flags IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
else
entry-res-name = info-name;
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 5589a6e2a023..e761a868bdba 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -567,6 +567,12 @@ int acpi_dev_get_resources(struct acpi_device *adev,
struct list_head *list,
}
EXPORT_SYMBOL_GPL(acpi_dev_get_resources);
+static bool acpi_dev_match_producer_consumer(unsigned long types, int
producer)
+{
+ return ((types IORESOURCE_WINDOW) producer == ACPI_PRODUCER) ||
+ ((types IORESOURCE_WINDOW) == 0 producer == ACPI_CONSUMER);
+}
+
/**
*
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
> Hi Jiang,
>
> Sorry for my delayed response. I've been on vacation for a week and am
> still trying to catch up.
>
> On Mon, Mar 30, 2015 at 10:40:43AM +0800, Jiang Liu wrote:
> > Before commit 593669c2ac0f("Use common ACPI resource interfaces to
> > simplify implementation"), arch/x86/pci/acpi.c applies following
> > rules when parsing ACPI resources for PCI host bridge:
> > 1) Ignore IO port resources defined by acpi_resource_io and
> >acpi_resource_fixed_io, which should be used to define resource
> >for PCI device instead of PCI bridge.
> > 2) Accept IOMEM resource defined by acpi_resource_memory24,
> >acpi_resource_memory32 and acpi_resource_fixed_memory32.
> >These IOMEM resources are accepted to workaround some BIOS issue,
> >though they should be ignored. For example, PC Engines APU.1C
> >platform defines PCI host bridge IOMEM resources as:
> > Memory32Fixed (ReadOnly,
> > 0x000A, // Address Base
> > 0x0002, // Address Length
> > )
> > Memory32Fixed (ReadOnly,
> > 0x, // Address Base
> > 0x, // Address Length
> > _Y00)
> > 3) Accept all IO port and IOMEM resources defined by
> >acpi_resource_address{16,32,64,extended64}, no matter it's marked as
> >ACPI_CONSUMER or ACPI_PRODUCER.
> >
> > Commit 593669c2ac0f("Use common ACPI resource interfaces to
> > simplify implementation") accept all IO port and IOMEM resources
> > defined by acpi_resource_io, acpi_resource_fixed_io,
> > acpi_resource_memory24, acpi_resource_memory32,
> > acpi_resource_fixed_memory32 and
> > acpi_resource_address{16,32,64,extended64}, which causes IO port
> > resources consumed by host bridge itself are listed in to host bridge
> > resource list.
> >
> > Then commit 63f1789ec716("Ignore resources consumed by host bridge
> > itself") ignores resources consumed by host bridge itself by checking
> > IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
> > above for BIOS bug .
>
> This is probably partly my fault.
>
> I think the ACPI spec intention is that every _CRS resource descriptor
> should be interpreted as "Consumer," i.e., as resources consumed by the
> device itself, unless it's marked otherwise. Only the following types can
> be marked as "Producer":
>
> - Word/DWord/QWord/Extended address space descriptors,
> - Extended interrupt descriptors,
> - GPIO interrupt and I/O connections,
> - I2C/SPI/UART serial bus resource descriptors
So we're talking about the consumer/producer flag in those extended resource
type descriptors, right?
My understanding of that flag is that it doesn't say whether or not the device
is a producer of a resource in a general sense. It only says whether the device
consumes a resource provided by someone else (1) or it consumes a resources
provided by itself (0).
> With 66528fdd45b0 ("x86/PCI: parse additional host bridge window resource
> types"), I made Linux treat Memory24, Memory32, and Memory32Fixed
> descriptors in PCI host bridge _CRS as Producers. I did it because Windows
> apparently does that (there are details in
> https://bugzilla.kernel.org/show_bug.cgi?id=15817),
It looks like it does that to indicate that those resources are provided
by the host bridge itself (which is consistent with the consumer/producer
flag interpretation above)
> but I wasn't aware of any machines that required it. That was probably a
> mistake because it didn't fix anything and it covered up ASL usage errors
> like what PC Engines did.
I don't think it is required. It only seems to allow Windows to consolidate
the handling of host bridge resources.
> > It's really costed us much time to figure out this whole picture.
> > So we refine interface acpi_dev_filter_resource_type as below,
> > which should be easier for maintence:
> > 1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
> >for bridge(PRODUCER), otherwise it's querying resource for
> >device(CONSUMER).
>
> Sounds good to me.
>
> > 2) Ignore IO port resources defined by acpi_resource_io and
> >acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
>
> Sounds good to me.
>
> > 3) Accpet IOMEM resource defined by acpi_resource_memory24,
> >acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
> >bugs, with comment to state it's workaround for BIOS bug.
>
> I don't like the fact that this is the behavior for all ACPI devices.
> Prior to 593669c2ac0f, we had this behavior for PCI host bridges only.
> I don't think this is what the spec envisioned, so I don't really like
> doing it for all devices.
Agreed.
> > 4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
> >a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
> >b)
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Friday, April 03, 2015 10:04:11 PM Bjorn Helgaas wrote:
Hi Jiang,
Sorry for my delayed response. I've been on vacation for a week and am
still trying to catch up.
On Mon, Mar 30, 2015 at 10:40:43AM +0800, Jiang Liu wrote:
Before commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation), arch/x86/pci/acpi.c applies following
rules when parsing ACPI resources for PCI host bridge:
1) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io, which should be used to define resource
for PCI device instead of PCI bridge.
2) Accept IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32.
These IOMEM resources are accepted to workaround some BIOS issue,
though they should be ignored. For example, PC Engines APU.1C
platform defines PCI host bridge IOMEM resources as:
Memory32Fixed (ReadOnly,
0x000A, // Address Base
0x0002, // Address Length
)
Memory32Fixed (ReadOnly,
0x, // Address Base
0x, // Address Length
_Y00)
3) Accept all IO port and IOMEM resources defined by
acpi_resource_address{16,32,64,extended64}, no matter it's marked as
ACPI_CONSUMER or ACPI_PRODUCER.
Commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation) accept all IO port and IOMEM resources
defined by acpi_resource_io, acpi_resource_fixed_io,
acpi_resource_memory24, acpi_resource_memory32,
acpi_resource_fixed_memory32 and
acpi_resource_address{16,32,64,extended64}, which causes IO port
resources consumed by host bridge itself are listed in to host bridge
resource list.
Then commit 63f1789ec716(Ignore resources consumed by host bridge
itself) ignores resources consumed by host bridge itself by checking
IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
above for BIOS bug .
This is probably partly my fault.
I think the ACPI spec intention is that every _CRS resource descriptor
should be interpreted as Consumer, i.e., as resources consumed by the
device itself, unless it's marked otherwise. Only the following types can
be marked as Producer:
- Word/DWord/QWord/Extended address space descriptors,
- Extended interrupt descriptors,
- GPIO interrupt and I/O connections,
- I2C/SPI/UART serial bus resource descriptors
So we're talking about the consumer/producer flag in those extended resource
type descriptors, right?
My understanding of that flag is that it doesn't say whether or not the device
is a producer of a resource in a general sense. It only says whether the device
consumes a resource provided by someone else (1) or it consumes a resources
provided by itself (0).
With 66528fdd45b0 (x86/PCI: parse additional host bridge window resource
types), I made Linux treat Memory24, Memory32, and Memory32Fixed
descriptors in PCI host bridge _CRS as Producers. I did it because Windows
apparently does that (there are details in
https://bugzilla.kernel.org/show_bug.cgi?id=15817),
It looks like it does that to indicate that those resources are provided
by the host bridge itself (which is consistent with the consumer/producer
flag interpretation above)
but I wasn't aware of any machines that required it. That was probably a
mistake because it didn't fix anything and it covered up ASL usage errors
like what PC Engines did.
I don't think it is required. It only seems to allow Windows to consolidate
the handling of host bridge resources.
It's really costed us much time to figure out this whole picture.
So we refine interface acpi_dev_filter_resource_type as below,
which should be easier for maintence:
1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
for bridge(PRODUCER), otherwise it's querying resource for
device(CONSUMER).
Sounds good to me.
2) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
Sounds good to me.
3) Accpet IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
bugs, with comment to state it's workaround for BIOS bug.
I don't like the fact that this is the behavior for all ACPI devices.
Prior to 593669c2ac0f, we had this behavior for PCI host bridges only.
I don't think this is what the spec envisioned, so I don't really like
doing it for all devices.
Agreed.
4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
Sounds good to me.
Currently acpi_dev_filter_resource_type() is only used by ACPI pci
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
Hi Jiang,
Sorry for my delayed response. I've been on vacation for a week and am
still trying to catch up.
On Mon, Mar 30, 2015 at 10:40:43AM +0800, Jiang Liu wrote:
> Before commit 593669c2ac0f("Use common ACPI resource interfaces to
> simplify implementation"), arch/x86/pci/acpi.c applies following
> rules when parsing ACPI resources for PCI host bridge:
> 1) Ignore IO port resources defined by acpi_resource_io and
>acpi_resource_fixed_io, which should be used to define resource
>for PCI device instead of PCI bridge.
> 2) Accept IOMEM resource defined by acpi_resource_memory24,
>acpi_resource_memory32 and acpi_resource_fixed_memory32.
>These IOMEM resources are accepted to workaround some BIOS issue,
>though they should be ignored. For example, PC Engines APU.1C
>platform defines PCI host bridge IOMEM resources as:
> Memory32Fixed (ReadOnly,
> 0x000A, // Address Base
> 0x0002, // Address Length
> )
> Memory32Fixed (ReadOnly,
> 0x, // Address Base
> 0x, // Address Length
> _Y00)
> 3) Accept all IO port and IOMEM resources defined by
>acpi_resource_address{16,32,64,extended64}, no matter it's marked as
>ACPI_CONSUMER or ACPI_PRODUCER.
>
> Commit 593669c2ac0f("Use common ACPI resource interfaces to
> simplify implementation") accept all IO port and IOMEM resources
> defined by acpi_resource_io, acpi_resource_fixed_io,
> acpi_resource_memory24, acpi_resource_memory32,
> acpi_resource_fixed_memory32 and
> acpi_resource_address{16,32,64,extended64}, which causes IO port
> resources consumed by host bridge itself are listed in to host bridge
> resource list.
>
> Then commit 63f1789ec716("Ignore resources consumed by host bridge
> itself") ignores resources consumed by host bridge itself by checking
> IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
> above for BIOS bug .
This is probably partly my fault.
I think the ACPI spec intention is that every _CRS resource descriptor
should be interpreted as "Consumer," i.e., as resources consumed by the
device itself, unless it's marked otherwise. Only the following types can
be marked as "Producer":
- Word/DWord/QWord/Extended address space descriptors,
- Extended interrupt descriptors,
- GPIO interrupt and I/O connections,
- I2C/SPI/UART serial bus resource descriptors
With 66528fdd45b0 ("x86/PCI: parse additional host bridge window resource
types"), I made Linux treat Memory24, Memory32, and Memory32Fixed
descriptors in PCI host bridge _CRS as Producers. I did it because Windows
apparently does that (there are details in
https://bugzilla.kernel.org/show_bug.cgi?id=15817), but I wasn't aware of
any machines that required it. That was probably a mistake because it
didn't fix anything and it covered up ASL usage errors like what PC Engines
did.
> It's really costed us much time to figure out this whole picture.
> So we refine interface acpi_dev_filter_resource_type as below,
> which should be easier for maintence:
> 1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
>for bridge(PRODUCER), otherwise it's querying resource for
>device(CONSUMER).
Sounds good to me.
> 2) Ignore IO port resources defined by acpi_resource_io and
>acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
Sounds good to me.
> 3) Accpet IOMEM resource defined by acpi_resource_memory24,
>acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
>bugs, with comment to state it's workaround for BIOS bug.
I don't like the fact that this is the behavior for all ACPI devices.
Prior to 593669c2ac0f, we had this behavior for PCI host bridges only.
I don't think this is what the spec envisioned, so I don't really like
doing it for all devices.
> 4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
>a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
>b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
Sounds good to me.
> Currently acpi_dev_filter_resource_type() is only used by ACPI pci
> host bridge and IOAPIC driver, so it shouldn't affect other drivers.
We should assume it will eventually be used for all ACPI devices,
shouldn't we?
> Another possible fix is to only ignore IO resource consumed by host
> bridge and keep IOMEM resource consumed by host bridge, please refer to:
> http://www.spinics.net/lists/linux-pci/msg39706.html
>
> Sample ACPI table are archived at:
> https://bugzilla.kernel.org/show_bug.cgi?id=94221
>
> V2->V3:
> Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
>
> Fixes: 63f1789ec716("Ignore resources consumed by host bridge itself")
> Reported-and-Tested-by: Bernhard Thaler
> Signed-off-by: Jiang Liu
> ---
> arch/x86/pci/acpi.c |5 ++---
> drivers/acpi/resource.c |
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Monday, March 30, 2015 10:40:43 AM Jiang Liu wrote:
> Before commit 593669c2ac0f("Use common ACPI resource interfaces to
> simplify implementation"), arch/x86/pci/acpi.c applies following
> rules when parsing ACPI resources for PCI host bridge:
> 1) Ignore IO port resources defined by acpi_resource_io and
>acpi_resource_fixed_io, which should be used to define resource
>for PCI device instead of PCI bridge.
> 2) Accept IOMEM resource defined by acpi_resource_memory24,
>acpi_resource_memory32 and acpi_resource_fixed_memory32.
>These IOMEM resources are accepted to workaround some BIOS issue,
>though they should be ignored. For example, PC Engines APU.1C
>platform defines PCI host bridge IOMEM resources as:
> Memory32Fixed (ReadOnly,
> 0x000A, // Address Base
> 0x0002, // Address Length
> )
> Memory32Fixed (ReadOnly,
> 0x, // Address Base
> 0x, // Address Length
> _Y00)
> 3) Accept all IO port and IOMEM resources defined by
>acpi_resource_address{16,32,64,extended64}, no matter it's marked as
>ACPI_CONSUMER or ACPI_PRODUCER.
>
> Commit 593669c2ac0f("Use common ACPI resource interfaces to
> simplify implementation") accept all IO port and IOMEM resources
> defined by acpi_resource_io, acpi_resource_fixed_io,
> acpi_resource_memory24, acpi_resource_memory32,
> acpi_resource_fixed_memory32 and
> acpi_resource_address{16,32,64,extended64}, which causes IO port
> resources consumed by host bridge itself are listed in to host bridge
> resource list.
>
> Then commit 63f1789ec716("Ignore resources consumed by host bridge
> itself") ignores resources consumed by host bridge itself by checking
> IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
> above for BIOS bug .
>
> It's really costed us much time to figure out this whole picture.
> So we refine interface acpi_dev_filter_resource_type as below,
> which should be easier for maintence:
> 1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
>for bridge(PRODUCER), otherwise it's querying resource for
>device(CONSUMER).
> 2) Ignore IO port resources defined by acpi_resource_io and
>acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
> 3) Accpet IOMEM resource defined by acpi_resource_memory24,
>acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
>bugs, with comment to state it's workaround for BIOS bug.
> 4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
>a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
>b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
>
> Currently acpi_dev_filter_resource_type() is only used by ACPI pci
> host bridge and IOAPIC driver, so it shouldn't affect other drivers.
>
> Another possible fix is to only ignore IO resource consumed by host
> bridge and keep IOMEM resource consumed by host bridge, please refer to:
> http://www.spinics.net/lists/linux-pci/msg39706.html
>
> Sample ACPI table are archived at:
> https://bugzilla.kernel.org/show_bug.cgi?id=94221
>
> V2->V3:
> Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
>
> Fixes: 63f1789ec716("Ignore resources consumed by host bridge itself")
> Reported-and-Tested-by: Bernhard Thaler
> Signed-off-by: Jiang Liu
Bjorn, any comments here? We still have this regression in 4.0-rc ...
> ---
> arch/x86/pci/acpi.c |5 ++---
> drivers/acpi/resource.c | 33 +
> 2 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index e4695985f9de..8c4b1201f340 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info
> *info,
> info->bridge = device;
> ret = acpi_dev_get_resources(device, list,
>acpi_dev_filter_resource_type_cb,
> - (void *)(IORESOURCE_IO | IORESOURCE_MEM));
> + (void *)(IORESOURCE_IO | IORESOURCE_MEM |
> IORESOURCE_WINDOW));
> if (ret < 0)
> dev_warn(>dev,
>"failed to parse _CRS method, error code %d\n", ret);
> @@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info
> *info,
> "no IO and memory resources present in _CRS\n");
> else
> resource_list_for_each_entry_safe(entry, tmp, list) {
> - if ((entry->res->flags & IORESOURCE_WINDOW) == 0 ||
> - (entry->res->flags & IORESOURCE_DISABLED))
> + if (entry->res->flags & IORESOURCE_DISABLED)
> resource_list_destroy_entry(entry);
> else
>
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
On Monday, March 30, 2015 10:40:43 AM Jiang Liu wrote:
Before commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation), arch/x86/pci/acpi.c applies following
rules when parsing ACPI resources for PCI host bridge:
1) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io, which should be used to define resource
for PCI device instead of PCI bridge.
2) Accept IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32.
These IOMEM resources are accepted to workaround some BIOS issue,
though they should be ignored. For example, PC Engines APU.1C
platform defines PCI host bridge IOMEM resources as:
Memory32Fixed (ReadOnly,
0x000A, // Address Base
0x0002, // Address Length
)
Memory32Fixed (ReadOnly,
0x, // Address Base
0x, // Address Length
_Y00)
3) Accept all IO port and IOMEM resources defined by
acpi_resource_address{16,32,64,extended64}, no matter it's marked as
ACPI_CONSUMER or ACPI_PRODUCER.
Commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation) accept all IO port and IOMEM resources
defined by acpi_resource_io, acpi_resource_fixed_io,
acpi_resource_memory24, acpi_resource_memory32,
acpi_resource_fixed_memory32 and
acpi_resource_address{16,32,64,extended64}, which causes IO port
resources consumed by host bridge itself are listed in to host bridge
resource list.
Then commit 63f1789ec716(Ignore resources consumed by host bridge
itself) ignores resources consumed by host bridge itself by checking
IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
above for BIOS bug .
It's really costed us much time to figure out this whole picture.
So we refine interface acpi_dev_filter_resource_type as below,
which should be easier for maintence:
1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
for bridge(PRODUCER), otherwise it's querying resource for
device(CONSUMER).
2) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
3) Accpet IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
bugs, with comment to state it's workaround for BIOS bug.
4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
Currently acpi_dev_filter_resource_type() is only used by ACPI pci
host bridge and IOAPIC driver, so it shouldn't affect other drivers.
Another possible fix is to only ignore IO resource consumed by host
bridge and keep IOMEM resource consumed by host bridge, please refer to:
http://www.spinics.net/lists/linux-pci/msg39706.html
Sample ACPI table are archived at:
https://bugzilla.kernel.org/show_bug.cgi?id=94221
V2-V3:
Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
Fixes: 63f1789ec716(Ignore resources consumed by host bridge itself)
Reported-and-Tested-by: Bernhard Thaler bernhard.tha...@wvnet.at
Signed-off-by: Jiang Liu jiang@linux.intel.com
Bjorn, any comments here? We still have this regression in 4.0-rc ...
---
arch/x86/pci/acpi.c |5 ++---
drivers/acpi/resource.c | 33 +
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index e4695985f9de..8c4b1201f340 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info
*info,
info-bridge = device;
ret = acpi_dev_get_resources(device, list,
acpi_dev_filter_resource_type_cb,
- (void *)(IORESOURCE_IO | IORESOURCE_MEM));
+ (void *)(IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_WINDOW));
if (ret 0)
dev_warn(device-dev,
failed to parse _CRS method, error code %d\n, ret);
@@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info
*info,
no IO and memory resources present in _CRS\n);
else
resource_list_for_each_entry_safe(entry, tmp, list) {
- if ((entry-res-flags IORESOURCE_WINDOW) == 0 ||
- (entry-res-flags IORESOURCE_DISABLED))
+ if (entry-res-flags IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
else
entry-res-name = info-name;
diff --git
Re: [Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
Hi Jiang,
Sorry for my delayed response. I've been on vacation for a week and am
still trying to catch up.
On Mon, Mar 30, 2015 at 10:40:43AM +0800, Jiang Liu wrote:
Before commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation), arch/x86/pci/acpi.c applies following
rules when parsing ACPI resources for PCI host bridge:
1) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io, which should be used to define resource
for PCI device instead of PCI bridge.
2) Accept IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32.
These IOMEM resources are accepted to workaround some BIOS issue,
though they should be ignored. For example, PC Engines APU.1C
platform defines PCI host bridge IOMEM resources as:
Memory32Fixed (ReadOnly,
0x000A, // Address Base
0x0002, // Address Length
)
Memory32Fixed (ReadOnly,
0x, // Address Base
0x, // Address Length
_Y00)
3) Accept all IO port and IOMEM resources defined by
acpi_resource_address{16,32,64,extended64}, no matter it's marked as
ACPI_CONSUMER or ACPI_PRODUCER.
Commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation) accept all IO port and IOMEM resources
defined by acpi_resource_io, acpi_resource_fixed_io,
acpi_resource_memory24, acpi_resource_memory32,
acpi_resource_fixed_memory32 and
acpi_resource_address{16,32,64,extended64}, which causes IO port
resources consumed by host bridge itself are listed in to host bridge
resource list.
Then commit 63f1789ec716(Ignore resources consumed by host bridge
itself) ignores resources consumed by host bridge itself by checking
IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
above for BIOS bug .
This is probably partly my fault.
I think the ACPI spec intention is that every _CRS resource descriptor
should be interpreted as Consumer, i.e., as resources consumed by the
device itself, unless it's marked otherwise. Only the following types can
be marked as Producer:
- Word/DWord/QWord/Extended address space descriptors,
- Extended interrupt descriptors,
- GPIO interrupt and I/O connections,
- I2C/SPI/UART serial bus resource descriptors
With 66528fdd45b0 (x86/PCI: parse additional host bridge window resource
types), I made Linux treat Memory24, Memory32, and Memory32Fixed
descriptors in PCI host bridge _CRS as Producers. I did it because Windows
apparently does that (there are details in
https://bugzilla.kernel.org/show_bug.cgi?id=15817), but I wasn't aware of
any machines that required it. That was probably a mistake because it
didn't fix anything and it covered up ASL usage errors like what PC Engines
did.
It's really costed us much time to figure out this whole picture.
So we refine interface acpi_dev_filter_resource_type as below,
which should be easier for maintence:
1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
for bridge(PRODUCER), otherwise it's querying resource for
device(CONSUMER).
Sounds good to me.
2) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
Sounds good to me.
3) Accpet IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
bugs, with comment to state it's workaround for BIOS bug.
I don't like the fact that this is the behavior for all ACPI devices.
Prior to 593669c2ac0f, we had this behavior for PCI host bridges only.
I don't think this is what the spec envisioned, so I don't really like
doing it for all devices.
4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
Sounds good to me.
Currently acpi_dev_filter_resource_type() is only used by ACPI pci
host bridge and IOAPIC driver, so it shouldn't affect other drivers.
We should assume it will eventually be used for all ACPI devices,
shouldn't we?
Another possible fix is to only ignore IO resource consumed by host
bridge and keep IOMEM resource consumed by host bridge, please refer to:
http://www.spinics.net/lists/linux-pci/msg39706.html
Sample ACPI table are archived at:
https://bugzilla.kernel.org/show_bug.cgi?id=94221
V2-V3:
Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
Fixes: 63f1789ec716(Ignore resources consumed by host bridge itself)
Reported-and-Tested-by: Bernhard Thaler bernhard.tha...@wvnet.at
Signed-off-by: Jiang Liu jiang@linux.intel.com
---
arch/x86/pci/acpi.c |5 ++---
drivers/acpi/resource.c | 33
[Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
Before commit 593669c2ac0f("Use common ACPI resource interfaces to
simplify implementation"), arch/x86/pci/acpi.c applies following
rules when parsing ACPI resources for PCI host bridge:
1) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io, which should be used to define resource
for PCI device instead of PCI bridge.
2) Accept IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32.
These IOMEM resources are accepted to workaround some BIOS issue,
though they should be ignored. For example, PC Engines APU.1C
platform defines PCI host bridge IOMEM resources as:
Memory32Fixed (ReadOnly,
0x000A, // Address Base
0x0002, // Address Length
)
Memory32Fixed (ReadOnly,
0x, // Address Base
0x, // Address Length
_Y00)
3) Accept all IO port and IOMEM resources defined by
acpi_resource_address{16,32,64,extended64}, no matter it's marked as
ACPI_CONSUMER or ACPI_PRODUCER.
Commit 593669c2ac0f("Use common ACPI resource interfaces to
simplify implementation") accept all IO port and IOMEM resources
defined by acpi_resource_io, acpi_resource_fixed_io,
acpi_resource_memory24, acpi_resource_memory32,
acpi_resource_fixed_memory32 and
acpi_resource_address{16,32,64,extended64}, which causes IO port
resources consumed by host bridge itself are listed in to host bridge
resource list.
Then commit 63f1789ec716("Ignore resources consumed by host bridge
itself") ignores resources consumed by host bridge itself by checking
IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
above for BIOS bug .
It's really costed us much time to figure out this whole picture.
So we refine interface acpi_dev_filter_resource_type as below,
which should be easier for maintence:
1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
for bridge(PRODUCER), otherwise it's querying resource for
device(CONSUMER).
2) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
3) Accpet IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
bugs, with comment to state it's workaround for BIOS bug.
4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
Currently acpi_dev_filter_resource_type() is only used by ACPI pci
host bridge and IOAPIC driver, so it shouldn't affect other drivers.
Another possible fix is to only ignore IO resource consumed by host
bridge and keep IOMEM resource consumed by host bridge, please refer to:
http://www.spinics.net/lists/linux-pci/msg39706.html
Sample ACPI table are archived at:
https://bugzilla.kernel.org/show_bug.cgi?id=94221
V2->V3:
Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
Fixes: 63f1789ec716("Ignore resources consumed by host bridge itself")
Reported-and-Tested-by: Bernhard Thaler
Signed-off-by: Jiang Liu
---
arch/x86/pci/acpi.c |5 ++---
drivers/acpi/resource.c | 33 +
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index e4695985f9de..8c4b1201f340 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info *info,
info->bridge = device;
ret = acpi_dev_get_resources(device, list,
acpi_dev_filter_resource_type_cb,
-(void *)(IORESOURCE_IO | IORESOURCE_MEM));
+(void *)(IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_WINDOW));
if (ret < 0)
dev_warn(>dev,
"failed to parse _CRS method, error code %d\n", ret);
@@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info *info,
"no IO and memory resources present in _CRS\n");
else
resource_list_for_each_entry_safe(entry, tmp, list) {
- if ((entry->res->flags & IORESOURCE_WINDOW) == 0 ||
- (entry->res->flags & IORESOURCE_DISABLED))
+ if (entry->res->flags & IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
else
entry->res->name = info->name;
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 5589a6e2a023..e761a868bdba 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -567,6 +567,12 @@ int acpi_dev_get_resources(struct acpi_device *adev,
[Bugfix v3] x86/PCI/ACPI: Fix regression caused by commit 63f1789ec716
Before commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation), arch/x86/pci/acpi.c applies following
rules when parsing ACPI resources for PCI host bridge:
1) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io, which should be used to define resource
for PCI device instead of PCI bridge.
2) Accept IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32.
These IOMEM resources are accepted to workaround some BIOS issue,
though they should be ignored. For example, PC Engines APU.1C
platform defines PCI host bridge IOMEM resources as:
Memory32Fixed (ReadOnly,
0x000A, // Address Base
0x0002, // Address Length
)
Memory32Fixed (ReadOnly,
0x, // Address Base
0x, // Address Length
_Y00)
3) Accept all IO port and IOMEM resources defined by
acpi_resource_address{16,32,64,extended64}, no matter it's marked as
ACPI_CONSUMER or ACPI_PRODUCER.
Commit 593669c2ac0f(Use common ACPI resource interfaces to
simplify implementation) accept all IO port and IOMEM resources
defined by acpi_resource_io, acpi_resource_fixed_io,
acpi_resource_memory24, acpi_resource_memory32,
acpi_resource_fixed_memory32 and
acpi_resource_address{16,32,64,extended64}, which causes IO port
resources consumed by host bridge itself are listed in to host bridge
resource list.
Then commit 63f1789ec716(Ignore resources consumed by host bridge
itself) ignores resources consumed by host bridge itself by checking
IORESOURCE_WINDOW flag, which accidently removed the workaround in 2)
above for BIOS bug .
It's really costed us much time to figure out this whole picture.
So we refine interface acpi_dev_filter_resource_type as below,
which should be easier for maintence:
1) Caller specifies IORESOURCE_WINDOW flag to explicitly query resource
for bridge(PRODUCER), otherwise it's querying resource for
device(CONSUMER).
2) Ignore IO port resources defined by acpi_resource_io and
acpi_resource_fixed_io if IORESOURCE_WINDOW is specified.
3) Accpet IOMEM resource defined by acpi_resource_memory24,
acpi_resource_memory32 and acpi_resource_fixed_memory32 for BIOS
bugs, with comment to state it's workaround for BIOS bug.
4) Accept IO port and IOMEM defined by acpi_resource_addressxx if
a) IORESOURCE_WINDOW is specified and ACPI_PRODUCER is true
b) IORESOURCE_WINDOW is not specified and ACPI_PRODUCER is false
Currently acpi_dev_filter_resource_type() is only used by ACPI pci
host bridge and IOAPIC driver, so it shouldn't affect other drivers.
Another possible fix is to only ignore IO resource consumed by host
bridge and keep IOMEM resource consumed by host bridge, please refer to:
http://www.spinics.net/lists/linux-pci/msg39706.html
Sample ACPI table are archived at:
https://bugzilla.kernel.org/show_bug.cgi?id=94221
V2-V3:
Refine function acpi_dev_match_producer_consumer() as suggested by Rafael
Fixes: 63f1789ec716(Ignore resources consumed by host bridge itself)
Reported-and-Tested-by: Bernhard Thaler bernhard.tha...@wvnet.at
Signed-off-by: Jiang Liu jiang@linux.intel.com
---
arch/x86/pci/acpi.c |5 ++---
drivers/acpi/resource.c | 33 +
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index e4695985f9de..8c4b1201f340 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -337,7 +337,7 @@ static void probe_pci_root_info(struct pci_root_info *info,
info-bridge = device;
ret = acpi_dev_get_resources(device, list,
acpi_dev_filter_resource_type_cb,
-(void *)(IORESOURCE_IO | IORESOURCE_MEM));
+(void *)(IORESOURCE_IO | IORESOURCE_MEM |
IORESOURCE_WINDOW));
if (ret 0)
dev_warn(device-dev,
failed to parse _CRS method, error code %d\n, ret);
@@ -346,8 +346,7 @@ static void probe_pci_root_info(struct pci_root_info *info,
no IO and memory resources present in _CRS\n);
else
resource_list_for_each_entry_safe(entry, tmp, list) {
- if ((entry-res-flags IORESOURCE_WINDOW) == 0 ||
- (entry-res-flags IORESOURCE_DISABLED))
+ if (entry-res-flags IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
else
entry-res-name = info-name;
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 5589a6e2a023..e761a868bdba 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -567,6 +567,12 @@ int
