[Debug 0/2] Fix regressions caused by commit 593669c2ac0f

[Debug 0/2] Fix regressions caused by commit 593669c2ac0f

[Jiang Liu]

Commit 593669c2ac0f (x86/PCI/ACPI: Use common ACPI resource interfaces
to simplify implementation) causes regression to several platforms,
which is caused by stricter checks in new ACPI resource parsing code
and BIOSes report incorrect ACPI resource descriptors.  So try to relax
the check to avoid regressions.

Jiang Liu (2):
  x86/PCI/ACPI: Ignore resources consumed by host bridge itself
  x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around
    BIOS bugs

 arch/x86/pci/acpi.c     |   11 ++++++++---
 drivers/acpi/resource.c |    4 +++-
 2 files changed, 11 insertions(+), 4 deletions(-)

-- 
1.7.10.4

[Debug 1/2] x86/PCI/ACPI: Ignore resources consumed by host bridge itself

[Jiang Liu]

When parsing resources for PCI host bridge, we should ignore resources
consumed by host bridge itself and only report window resources available
to child PCI busses.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/pci/acpi.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 6ac273832f28..e4695985f9de 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -331,7 +331,7 @@ static void probe_pci_root_info(struct pci_root_info *info,
 				struct list_head *list)
 {
 	int ret;
-	struct resource_entry *entry;
+	struct resource_entry *entry, *tmp;
 
 	sprintf(info->name, "PCI Bus %04x:%02x", domain, busnum);
 	info->bridge = device;
@@ -345,8 +345,13 @@ static void probe_pci_root_info(struct pci_root_info *info,
 		dev_dbg(&device->dev,
 			"no IO and memory resources present in _CRS\n");
 	else
-		resource_list_for_each_entry(entry, list)
-			entry->res->name = info->name;
+		resource_list_for_each_entry_safe(entry, tmp, list) {
+			if ((entry->res->flags & IORESOURCE_WINDOW) == 0 ||
+			    (entry->res->flags & IORESOURCE_DISABLED))
+				resource_list_destroy_entry(entry);
+			else
+				entry->res->name = info->name;
+		}
 }
 
 struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
-- 
1.7.10.4

[Debug 2/2] x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around BIOS bugs

[Jiang Liu]

Some BIOSes report incorrect length for ACPI address space descriptors,
so relax the checks to avoid regressions.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/resource.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index c723668e3e27..5589a6e2a023 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -42,8 +42,10 @@ static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
 	 * CHECKME: len might be required to check versus a minimum
 	 * length as well. 1 for io is fine, but for memory it does
 	 * not make any sense at all.
+	 * Note: some BIOSes report incorrect length for ACPI address space
+	 * descriptor, so remove check of 'reslen == len' to avoid regression.
 	 */
-	if (len && reslen && reslen == len && start <= end)
+	if (len && reslen && start <= end)
 		return true;
 
 	pr_debug("ACPI: invalid or unassigned resource %s [%016llx - %016llx] length [%016llx]\n",
-- 
1.7.10.4

Re: [Debug 2/2] x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around BIOS bugs

[Bjorn Helgaas]

On Mon, Mar 2, 2015 at 10:25 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
> Some BIOSes report incorrect length for ACPI address space descriptors,
> so relax the checks to avoid regressions.
>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>

It'd be nice to have a DSDT archived and referenced in this changelog
for future reference.  This sounds similar to previous issues:

3162b6f0c5e1 ("PNPACPI: truncate _CRS windows with _LEN > _MAX - _MIN + 1")
d558b483d5a7 ("x86/PCI: truncate _CRS windows with _LEN > _MAX - _MIN + 1")
f238b414a74a ("PNPACPI: compute Address Space length rather than using _LEN")
48728e077480 ("x86/PCI: compute Address Space length rather than using _LEN")

I assume your work fixes both these paths and avoids the issues we fixed above.

> ---
>  drivers/acpi/resource.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index c723668e3e27..5589a6e2a023 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -42,8 +42,10 @@ static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
>          * CHECKME: len might be required to check versus a minimum
>          * length as well. 1 for io is fine, but for memory it does
>          * not make any sense at all.
> +        * Note: some BIOSes report incorrect length for ACPI address space
> +        * descriptor, so remove check of 'reslen == len' to avoid regression.
>          */
> -       if (len && reslen && reslen == len && start <= end)
> +       if (len && reslen && start <= end)
>                 return true;
>
>         pr_debug("ACPI: invalid or unassigned resource %s [%016llx - %016llx] length [%016llx]\n",
> --
> 1.7.10.4
>

Re: [Debug 2/2] x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around BIOS bugs

[Jiang Liu]

On 2015/3/3 23:18, Bjorn Helgaas wrote:
> On Mon, Mar 2, 2015 at 10:25 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
>> Some BIOSes report incorrect length for ACPI address space descriptors,
>> so relax the checks to avoid regressions.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> 
> It'd be nice to have a DSDT archived and referenced in this changelog
> for future reference.  This sounds similar to previous issues:
Hi all,
	Could anybody help to dump an ACPI table from those failure
systems so we could archive it?
Thanks!
Gerry

> 
> 3162b6f0c5e1 ("PNPACPI: truncate _CRS windows with _LEN > _MAX - _MIN + 1")
> d558b483d5a7 ("x86/PCI: truncate _CRS windows with _LEN > _MAX - _MIN + 1")
> f238b414a74a ("PNPACPI: compute Address Space length rather than using _LEN")
> 48728e077480 ("x86/PCI: compute Address Space length rather than using _LEN")
> 
> I assume your work fixes both these paths and avoids the issues we fixed above.
> 
>> ---
>>  drivers/acpi/resource.c |    4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index c723668e3e27..5589a6e2a023 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -42,8 +42,10 @@ static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
>>          * CHECKME: len might be required to check versus a minimum
>>          * length as well. 1 for io is fine, but for memory it does
>>          * not make any sense at all.
>> +        * Note: some BIOSes report incorrect length for ACPI address space
>> +        * descriptor, so remove check of 'reslen == len' to avoid regression.
>>          */
>> -       if (len && reslen && reslen == len && start <= end)
>> +       if (len && reslen && start <= end)
>>                 return true;
>>
>>         pr_debug("ACPI: invalid or unassigned resource %s [%016llx - %016llx] length [%016llx]\n",
>> --
>> 1.7.10.4
>>

Re: [Debug 2/2] x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around BIOS bugs

[Dave Airlie]

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

 reference.  This sounds similar to previous issues:
> Hi all,
>         Could anybody help to dump an ACPI table from those failure
> systems so we could archive it?

Here is the DSDT from my amd rs780 box.

Dave.

[-- Attachment #2: DSDT --]
[-- Type: application/octet-stream, Size: 25936 bytes --]

Re: [Debug 2/2] x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around BIOS bugs

[Thomas Voegtle]

[-- Attachment #1: Type: text/plain, Size: 229 bytes --]

On Wed, 4 Mar 2015, Jiang Liu wrote:

> Hi all,
> 	Could anybody help to dump an ACPI table from those failure
> systems so we could archive it?


and here is the DSDT from a Asrock-Q1900B-ITX mainboard.

Thanks again,

   Thomas

[-- Attachment #2: Type: application/octet-stream, Size: 45609 bytes --]

Re: [Debug 2/2] x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around BIOS bugs

[Prakash Punnoor]

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

On 04.03.2015 03:29, Jiang Liu wrote:
> On 2015/3/3 23:18, Bjorn Helgaas wrote:
>> On Mon, Mar 2, 2015 at 10:25 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
>>> Some BIOSes report incorrect length for ACPI address space descriptors,
>>> so relax the checks to avoid regressions.
>>>
>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>>
>> It'd be nice to have a DSDT archived and referenced in this changelog
>> for future reference.  This sounds similar to previous issues:
> Hi all,
> 	Could anybody help to dump an ACPI table from those failure
> systems so we could archive it?

Attached: DSDT from my Abit A-S78H.

Regards,

Prakash

[-- Attachment #2: DSDT-Abit-A-S78H.xz --]
[-- Type: application/x-xz, Size: 10384 bytes --]

Re: [Debug 2/2] x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around BIOS bugs

[Hudd]

[-- Attachment #1: Type: text/plain, Size: 222 bytes --]

On Wed, 2015-03-04 at 10:29 +0800, Jiang Liu wrote:
> Hi all,
> 	Could anybody help to dump an ACPI table from those failure
> systems so we could archive it?
> Thanks!
> Gerry

Here is DSDT from Gigabyte GA-MA785G-UD3H.


[-- Attachment #2: DSDT-GA-MA785G-UD3H --]
[-- Type: application/octet-stream, Size: 29337 bytes --]

Re: [Debug 0/2] Fix regressions caused by commit 593669c2ac0f

[Dave Airlie]

On 3 March 2015 at 14:25, Jiang Liu <jiang.liu@linux.intel.com> wrote:
> Commit 593669c2ac0f (x86/PCI/ACPI: Use common ACPI resource interfaces
> to simplify implementation) causes regression to several platforms,
> which is caused by stricter checks in new ACPI resource parsing code
> and BIOSes report incorrect ACPI resource descriptors.  So try to relax
> the check to avoid regressions.
>
> Jiang Liu (2):
>   x86/PCI/ACPI: Ignore resources consumed by host bridge itself
>   x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around
>     BIOS bugs

I've booted my machine with that lost its r8169 and it appears to be
working now.

So from the regression POV:
Tested-by: Dave Airlie <airlied@redhat.com>

Dave.

Re: [Debug 0/2] Fix regressions caused by commit 593669c2ac0f

[Jiang Liu]

On 2015/3/3 12:34, Dave Airlie wrote:
> On 3 March 2015 at 14:25, Jiang Liu <jiang.liu@linux.intel.com> wrote:
>> Commit 593669c2ac0f (x86/PCI/ACPI: Use common ACPI resource interfaces
>> to simplify implementation) causes regression to several platforms,
>> which is caused by stricter checks in new ACPI resource parsing code
>> and BIOSes report incorrect ACPI resource descriptors.  So try to relax
>> the check to avoid regressions.
>>
>> Jiang Liu (2):
>>   x86/PCI/ACPI: Ignore resources consumed by host bridge itself
>>   x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around
>>     BIOS bugs
> 
> I've booted my machine with that lost its r8169 and it appears to be
> working now.
> 
> So from the regression POV:
> Tested-by: Dave Airlie <airlied@redhat.com>
Thanks for testing, Dave:)
> 
> Dave.
> 

Re: [Debug 0/2] Fix regressions caused by commit 593669c2ac0f

[Prakash Punnoor]

On 03.03.2015 05:34, Dave Airlie wrote:
> On 3 March 2015 at 14:25, Jiang Liu <jiang.liu@linux.intel.com> wrote:
>> Commit 593669c2ac0f (x86/PCI/ACPI: Use common ACPI resource interfaces
>> to simplify implementation) causes regression to several platforms,
>> which is caused by stricter checks in new ACPI resource parsing code
>> and BIOSes report incorrect ACPI resource descriptors.  So try to relax
>> the check to avoid regressions.
>>
>> Jiang Liu (2):
>>   x86/PCI/ACPI: Ignore resources consumed by host bridge itself
>>   x86/PCI/ACPI: Relax ACPI resource descriptor checks to work around
>>     BIOS bugs
> 
> I've booted my machine with that lost its r8169 and it appears to be
> working now.
> 
> So from the regression POV:
> Tested-by: Dave Airlie <airlied@redhat.com>

Works for me, as well. Thanks!

Tested-by: Prakash Punnoor <prakash@punnoor.de>


Regards,

Prakash