[PATCH v1 0/3] [RFC] resource, PCI: work around pci=use_crs conflicts

[PATCH v1 0/3] [RFC] resource, PCI: work around pci=use_crs conflicts

[Bjorn Helgaas]

These patches are to fix this pci=use_crs regression:

    http://bugzilla.kernel.org/show_bug.cgi?id=15480

The problem is that BIOS reported a PCI host bridge window that overlaps
system RAM.  This workaround trims the window to avoid the overlap, which
requires information about the conflicting resource.

Yanko, I reworked these a bit, so if you have a chance to retest them
and collect another dmesg log, I'd appreciate it.

I'm still hoping for someone to find out how Windows deals with this, but
haven't gotten any data yet.

---

Bjorn Helgaas (3):
      resources: add interfaces that return conflict information
      x86/PCI: trim _CRS windows when they conflict with previous reservations
      PCI: for address space collisions, show conflicting resource


 arch/x86/pci/acpi.c     |   48 ++++++++++++++++++++++++++++++++++-------------
 drivers/pci/setup-res.c |   14 ++++++++------
 include/linux/ioport.h  |    2 ++
 kernel/resource.c       |   44 ++++++++++++++++++++++++++++++++++++-------
 4 files changed, 82 insertions(+), 26 deletions(-)

[PATCH v1 1/3] resources: add interfaces that return conflict information

[Bjorn Helgaas]

request_resource() and insert_resource() only return success or failure,
which no information about what existing resource conflicted with the
proposed new reservation.  This patch adds request_resource_conflict()
and insert_resource_conflict(), which return the conflicting resource.

Callers may use this for better error messages or to adjust the new
resource and retry the request.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---

 include/linux/ioport.h |    2 ++
 kernel/resource.c      |   44 +++++++++++++++++++++++++++++++++++++-------
 2 files changed, 39 insertions(+), 7 deletions(-)


diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index dda9841..1d95f75 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -110,12 +110,14 @@ struct resource_list {
 extern struct resource ioport_resource;
 extern struct resource iomem_resource;
 
+extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
 extern int request_resource(struct resource *root, struct resource *new);
 extern int release_resource(struct resource *new);
 void release_child_resources(struct resource *new);
 extern void reserve_region_with_split(struct resource *root,
 			     resource_size_t start, resource_size_t end,
 			     const char *name);
+extern struct resource *insert_resource_conflict(struct resource *parent, struct resource *new);
 extern int insert_resource(struct resource *parent, struct resource *new);
 extern void insert_resource_expand_to_fit(struct resource *root, struct resource *new);
 extern int allocate_resource(struct resource *root, struct resource *new,
diff --git a/kernel/resource.c b/kernel/resource.c
index 2d5be5d..9c358e2 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -219,19 +219,34 @@ void release_child_resources(struct resource *r)
 }
 
 /**
- * request_resource - request and reserve an I/O or memory resource
+ * request_resource_conflict - request and reserve an I/O or memory resource
  * @root: root resource descriptor
  * @new: resource descriptor desired by caller
  *
- * Returns 0 for success, negative error code on error.
+ * Returns 0 for success, conflict resource on error.
  */
-int request_resource(struct resource *root, struct resource *new)
+struct resource *request_resource_conflict(struct resource *root, struct resource *new)
 {
 	struct resource *conflict;
 
 	write_lock(&resource_lock);
 	conflict = __request_resource(root, new);
 	write_unlock(&resource_lock);
+	return conflict;
+}
+
+/**
+ * request_resource - request and reserve an I/O or memory resource
+ * @root: root resource descriptor
+ * @new: resource descriptor desired by caller
+ *
+ * Returns 0 for success, negative error code on error.
+ */
+int request_resource(struct resource *root, struct resource *new)
+{
+	struct resource *conflict;
+
+	conflict = request_resource_conflict(root, new);
 	return conflict ? -EBUSY : 0;
 }
 
@@ -474,25 +489,40 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
 }
 
 /**
- * insert_resource - Inserts a resource in the resource tree
+ * insert_resource_conflict - Inserts resource in the resource tree
  * @parent: parent of the new resource
  * @new: new resource to insert
  *
- * Returns 0 on success, -EBUSY if the resource can't be inserted.
+ * Returns 0 on success, conflict resource if the resource can't be inserted.
  *
- * This function is equivalent to request_resource when no conflict
+ * This function is equivalent to request_resource_conflict when no conflict
  * happens. If a conflict happens, and the conflicting resources
  * entirely fit within the range of the new resource, then the new
  * resource is inserted and the conflicting resources become children of
  * the new resource.
  */
-int insert_resource(struct resource *parent, struct resource *new)
+struct resource *insert_resource_conflict(struct resource *parent, struct resource *new)
 {
 	struct resource *conflict;
 
 	write_lock(&resource_lock);
 	conflict = __insert_resource(parent, new);
 	write_unlock(&resource_lock);
+	return conflict;
+}
+
+/**
+ * insert_resource - Inserts a resource in the resource tree
+ * @parent: parent of the new resource
+ * @new: new resource to insert
+ *
+ * Returns 0 on success, -EBUSY if the resource can't be inserted.
+ */
+int insert_resource(struct resource *parent, struct resource *new)
+{
+	struct resource *conflict;
+
+	conflict = insert_resource_conflict(parent, new);
 	return conflict ? -EBUSY : 0;
 }
 

Re: [PATCH v1 1/3] resources: add interfaces that return conflict information

[Jesse Barnes]

On Thu, 11 Mar 2010 17:01:09 -0700
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> 
> request_resource() and insert_resource() only return success or failure,
> which no information about what existing resource conflicted with the
> proposed new reservation.  This patch adds request_resource_conflict()
> and insert_resource_conflict(), which return the conflicting resource.
> 
> Callers may use this for better error messages or to adjust the new
> resource and retry the request.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
> 

Applied this series to my for-linus branch, thanks.

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH v1 2/3] x86/PCI: trim _CRS windows when they conflict with previous reservations

[Bjorn Helgaas]

Yanko's GA-MA78GM-S2H (BIOS F11) reports a host bridge window that overlaps
system memory:

    PCI window: [mem 0xcff00000-0x10ed0ffff]
    System RAM: [mem 0x100000000-0x22fffffff]

We can be pretty confident that the System RAM region is correct (if it
were wrong, we'd crash as soon as we tried to use any memory in that area),
so this patch tries to correct the PCI window by trimming it so it doesn't
conflict with any previous reservations.

http://bugzilla.kernel.org/show_bug.cgi?id=15480

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reported-by: Yanko Kaneti <yaneti@declera.com>
---

 arch/x86/pci/acpi.c |   48 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 35 insertions(+), 13 deletions(-)


diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 6e22454..d255ce8 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -114,11 +114,19 @@ align_resource(struct acpi_device *bridge, struct resource *res)
 	}
 }
 
+static bool
+resource_contains(struct resource *res, resource_size_t n)
+{
+	if (n < res->start || n > res->end)
+		return false;
+	return true;
+}
+
 static acpi_status
 setup_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
-	struct resource *res;
+	struct resource *res, *conflict;
 	struct acpi_resource_address64 addr;
 	acpi_status status;
 	unsigned long flags;
@@ -157,21 +165,35 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
 		return AE_OK;
 	}
 
-	if (insert_resource(root, res)) {
+	conflict = insert_resource_conflict(root, res);
+	while (conflict) {
 		dev_err(&info->bridge->dev,
-			"can't allocate host bridge window %pR\n", res);
-	} else {
-		pci_bus_add_resource(info->bus, res, 0);
-		info->res_num++;
-		if (addr.translation_offset)
-			dev_info(&info->bridge->dev, "host bridge window %pR "
-				 "(PCI address [%#llx-%#llx])\n",
-				 res, res->start - addr.translation_offset,
-				 res->end - addr.translation_offset);
+		        "host bridge window %pR conflicts with %s %pR\n",
+			res, conflict->name, conflict);
+
+		if (resource_contains(res, conflict->end))
+			res->start = conflict->end + 1;
+		else if (resource_contains(res, conflict->start))
+			res->end = conflict->start - 1;
 		else
-			dev_info(&info->bridge->dev,
-				 "host bridge window %pR\n", res);
+			return AE_OK;
+
+		if (res->start >= res->end)
+			return AE_OK;
+
+		conflict = insert_resource_conflict(root, res);
 	}
+
+	pci_bus_add_resource(info->bus, res, 0);
+	info->res_num++;
+	if (addr.translation_offset)
+		dev_info(&info->bridge->dev, "host bridge window %pR "
+			 "(PCI address [%#llx-%#llx])\n",
+			 res, res->start - addr.translation_offset,
+			 res->end - addr.translation_offset);
+	else
+		dev_info(&info->bridge->dev,
+			 "host bridge window %pR\n", res);
 	return AE_OK;
 }
 

Re: [PATCH v1 2/3] x86/PCI: trim _CRS windows when they conflict with previous reservations

[Kenji Kaneshige]

Bjorn Helgaas wrote:
> Yanko's GA-MA78GM-S2H (BIOS F11) reports a host bridge window that overlaps
> system memory:
> 
>     PCI window: [mem 0xcff00000-0x10ed0ffff]
>     System RAM: [mem 0x100000000-0x22fffffff]
> 
> We can be pretty confident that the System RAM region is correct (if it
> were wrong, we'd crash as soon as we tried to use any memory in that area),
> so this patch tries to correct the PCI window by trimming it so it doesn't
> conflict with any previous reservations.
> 

Though I might misunderstand something, it looks Yanko's machine specific
workaround. I'm wondering if trimming _CRS is a generic workaround for
broken _CRS machine.

How about doing this when GA-MA78GM-S2H (BIOS F11) (and known machines
that have the same problem) is detected? Or how about switching nocrs
mode if the problem (resource conflict) is detected?

Thanks,
Kenji Kaneshige



> http://bugzilla.kernel.org/show_bug.cgi?id=15480
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Reported-by: Yanko Kaneti <yaneti@declera.com>
> ---
> 
>  arch/x86/pci/acpi.c |   48 +++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 35 insertions(+), 13 deletions(-)
> 
> 
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index 6e22454..d255ce8 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -114,11 +114,19 @@ align_resource(struct acpi_device *bridge, struct resource *res)
>  	}
>  }
>  
> +static bool
> +resource_contains(struct resource *res, resource_size_t n)
> +{
> +	if (n < res->start || n > res->end)
> +		return false;
> +	return true;
> +}
> +
>  static acpi_status
>  setup_resource(struct acpi_resource *acpi_res, void *data)
>  {
>  	struct pci_root_info *info = data;
> -	struct resource *res;
> +	struct resource *res, *conflict;
>  	struct acpi_resource_address64 addr;
>  	acpi_status status;
>  	unsigned long flags;
> @@ -157,21 +165,35 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
>  		return AE_OK;
>  	}
>  
> -	if (insert_resource(root, res)) {
> +	conflict = insert_resource_conflict(root, res);
> +	while (conflict) {
>  		dev_err(&info->bridge->dev,
> -			"can't allocate host bridge window %pR\n", res);
> -	} else {
> -		pci_bus_add_resource(info->bus, res, 0);
> -		info->res_num++;
> -		if (addr.translation_offset)
> -			dev_info(&info->bridge->dev, "host bridge window %pR "
> -				 "(PCI address [%#llx-%#llx])\n",
> -				 res, res->start - addr.translation_offset,
> -				 res->end - addr.translation_offset);
> +		        "host bridge window %pR conflicts with %s %pR\n",
> +			res, conflict->name, conflict);
> +
> +		if (resource_contains(res, conflict->end))
> +			res->start = conflict->end + 1;
> +		else if (resource_contains(res, conflict->start))
> +			res->end = conflict->start - 1;
>  		else
> -			dev_info(&info->bridge->dev,
> -				 "host bridge window %pR\n", res);
> +			return AE_OK;
> +
> +		if (res->start >= res->end)
> +			return AE_OK;
> +
> +		conflict = insert_resource_conflict(root, res);
>  	}
> +
> +	pci_bus_add_resource(info->bus, res, 0);
> +	info->res_num++;
> +	if (addr.translation_offset)
> +		dev_info(&info->bridge->dev, "host bridge window %pR "
> +			 "(PCI address [%#llx-%#llx])\n",
> +			 res, res->start - addr.translation_offset,
> +			 res->end - addr.translation_offset);
> +	else
> +		dev_info(&info->bridge->dev,
> +			 "host bridge window %pR\n", res);
>  	return AE_OK;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v1 2/3] x86/PCI: trim _CRS windows when they conflict with previous reservations

[Bjorn Helgaas]

On Wed, 2010-03-17 at 12:25 +0900, Kenji Kaneshige wrote:
> Bjorn Helgaas wrote:
> > Yanko's GA-MA78GM-S2H (BIOS F11) reports a host bridge window that overlaps
> > system memory:
> > 
> >     PCI window: [mem 0xcff00000-0x10ed0ffff]
> >     System RAM: [mem 0x100000000-0x22fffffff]
> > 
> > We can be pretty confident that the System RAM region is correct (if it
> > were wrong, we'd crash as soon as we tried to use any memory in that area),
> > so this patch tries to correct the PCI window by trimming it so it doesn't
> > conflict with any previous reservations.
> 
> Though I might misunderstand something, it looks Yanko's machine specific
> workaround. I'm wondering if trimming _CRS is a generic workaround for
> broken _CRS machine.
> 
> How about doing this when GA-MA78GM-S2H (BIOS F11) (and known machines
> that have the same problem) is detected? Or how about switching nocrs
> mode if the problem (resource conflict) is detected?

It's certainly a possibility to do this only for specific machines, but
I'd like to avoid tripping over issues one-by-one.

I think there are three ways to address BIOS _CRS defects:

  1) Ship an OEM-specific host bridge driver
  2) Put a platform- or BIOS-specific quirk into Windows
  3) Change the BIOS

The first two sound like such a hassle to me that I doubt they would be
practical.

But it's clear that there are systems like this with what appear to be
_CRS defects.  It's quite possible that it's not really a defect, and we
just don't understand how to parse _CRS correctly yet.  Or, Windows
might have a few heuristics to clean up obvious errors.

For example, I think Windows aligns host bridge windows, as documented
here: http://bugzilla.kernel.org/show_bug.cgi?id=14337

I think Windows also knows to ignore the Consumer/Producer bit in
Address Space Descriptors, and assume that all resources on bridges are
Producers.

Hmm, what we really need is a way to run Windows in a virtualized
environment where we could manipulate the _CRS method and see what
Windows does with it...

Anyway, I'd like to make Linux behave as much like Windows as possible
in this area so we can take advantage of all the testing that's done
with Windows.

Bjorn

> > http://bugzilla.kernel.org/show_bug.cgi?id=15480
> > 
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > Reported-by: Yanko Kaneti <yaneti@declera.com>
> > ---
> > 
> >  arch/x86/pci/acpi.c |   48 +++++++++++++++++++++++++++++++++++-------------
> >  1 files changed, 35 insertions(+), 13 deletions(-)
> > 
> > 
> > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> > index 6e22454..d255ce8 100644
> > --- a/arch/x86/pci/acpi.c
> > +++ b/arch/x86/pci/acpi.c
> > @@ -114,11 +114,19 @@ align_resource(struct acpi_device *bridge, struct resource *res)
> >  	}
> >  }
> >  
> > +static bool
> > +resource_contains(struct resource *res, resource_size_t n)
> > +{
> > +	if (n < res->start || n > res->end)
> > +		return false;
> > +	return true;
> > +}
> > +
> >  static acpi_status
> >  setup_resource(struct acpi_resource *acpi_res, void *data)
> >  {
> >  	struct pci_root_info *info = data;
> > -	struct resource *res;
> > +	struct resource *res, *conflict;
> >  	struct acpi_resource_address64 addr;
> >  	acpi_status status;
> >  	unsigned long flags;
> > @@ -157,21 +165,35 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
> >  		return AE_OK;
> >  	}
> >  
> > -	if (insert_resource(root, res)) {
> > +	conflict = insert_resource_conflict(root, res);
> > +	while (conflict) {
> >  		dev_err(&info->bridge->dev,
> > -			"can't allocate host bridge window %pR\n", res);
> > -	} else {
> > -		pci_bus_add_resource(info->bus, res, 0);
> > -		info->res_num++;
> > -		if (addr.translation_offset)
> > -			dev_info(&info->bridge->dev, "host bridge window %pR "
> > -				 "(PCI address [%#llx-%#llx])\n",
> > -				 res, res->start - addr.translation_offset,
> > -				 res->end - addr.translation_offset);
> > +		        "host bridge window %pR conflicts with %s %pR\n",
> > +			res, conflict->name, conflict);
> > +
> > +		if (resource_contains(res, conflict->end))
> > +			res->start = conflict->end + 1;
> > +		else if (resource_contains(res, conflict->start))
> > +			res->end = conflict->start - 1;
> >  		else
> > -			dev_info(&info->bridge->dev,
> > -				 "host bridge window %pR\n", res);
> > +			return AE_OK;
> > +
> > +		if (res->start >= res->end)
> > +			return AE_OK;
> > +
> > +		conflict = insert_resource_conflict(root, res);
> >  	}
> > +
> > +	pci_bus_add_resource(info->bus, res, 0);
> > +	info->res_num++;
> > +	if (addr.translation_offset)
> > +		dev_info(&info->bridge->dev, "host bridge window %pR "
> > +			 "(PCI address [%#llx-%#llx])\n",
> > +			 res, res->start - addr.translation_offset,
> > +			 res->end - addr.translation_offset);
> > +	else
> > +		dev_info(&info->bridge->dev,
> > +			 "host bridge window %pR\n", res);
> >  	return AE_OK;
> >  }
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> 
> 

Re: [PATCH v1 2/3] x86/PCI: trim _CRS windows when they conflict with previous reservations

[Kenji Kaneshige]

Bjorn Helgaas wrote:
> On Wed, 2010-03-17 at 12:25 +0900, Kenji Kaneshige wrote:
>> Bjorn Helgaas wrote:
>>> Yanko's GA-MA78GM-S2H (BIOS F11) reports a host bridge window that overlaps
>>> system memory:
>>>
>>>     PCI window: [mem 0xcff00000-0x10ed0ffff]
>>>     System RAM: [mem 0x100000000-0x22fffffff]
>>>
>>> We can be pretty confident that the System RAM region is correct (if it
>>> were wrong, we'd crash as soon as we tried to use any memory in that area),
>>> so this patch tries to correct the PCI window by trimming it so it doesn't
>>> conflict with any previous reservations.
>> Though I might misunderstand something, it looks Yanko's machine specific
>> workaround. I'm wondering if trimming _CRS is a generic workaround for
>> broken _CRS machine.
>>
>> How about doing this when GA-MA78GM-S2H (BIOS F11) (and known machines
>> that have the same problem) is detected? Or how about switching nocrs
>> mode if the problem (resource conflict) is detected?
> 
> It's certainly a possibility to do this only for specific machines, but
> I'd like to avoid tripping over issues one-by-one.
> 
> I think there are three ways to address BIOS _CRS defects:
> 
>   1) Ship an OEM-specific host bridge driver
>   2) Put a platform- or BIOS-specific quirk into Windows

"into Linux"?

>   3) Change the BIOS
> 
> The first two sound like such a hassle to me that I doubt they would be
> practical.

I agree.
For 1), we need OEM-specific driver, not chipset specific driver.

For 2), I thought it depends on how many machines with broken _CRS are
there, and I didn't think there are so many, but...

> 
> But it's clear that there are systems like this with what appear to be
> _CRS defects.  It's quite possible that it's not really a defect, and we
> just don't understand how to parse _CRS correctly yet.  Or, Windows
> might have a few heuristics to clean up obvious errors.

Indeed, it might be true. Now I realize I need to change my opinion
about 2).

> 
> For example, I think Windows aligns host bridge windows, as documented
> here: http://bugzilla.kernel.org/show_bug.cgi?id=14337
> 
> I think Windows also knows to ignore the Consumer/Producer bit in
> Address Space Descriptors, and assume that all resources on bridges are
> Producers.
> 
> Hmm, what we really need is a way to run Windows in a virtualized
> environment where we could manipulate the _CRS method and see what
> Windows does with it...
> 
> Anyway, I'd like to make Linux behave as much like Windows as possible
> in this area so we can take advantage of all the testing that's done
> with Windows.

Okey, thank you very much for explanation. I understood the background
of your change.

Thanks,
Kenji Kaneshige

Re: [PATCH v1 2/3] x86/PCI: trim _CRS windows when they conflict with previous reservations

[Bjorn Helgaas]

On Wed, 2010-03-17 at 17:47 +0900, Kenji Kaneshige wrote:
> Bjorn Helgaas wrote:
> > On Wed, 2010-03-17 at 12:25 +0900, Kenji Kaneshige wrote:
> >> Bjorn Helgaas wrote:
> >>> Yanko's GA-MA78GM-S2H (BIOS F11) reports a host bridge window that overlaps
> >>> system memory:
> >>>
> >>>     PCI window: [mem 0xcff00000-0x10ed0ffff]
> >>>     System RAM: [mem 0x100000000-0x22fffffff]
> >>>
> >>> We can be pretty confident that the System RAM region is correct (if it
> >>> were wrong, we'd crash as soon as we tried to use any memory in that area),
> >>> so this patch tries to correct the PCI window by trimming it so it doesn't
> >>> conflict with any previous reservations.
> >> Though I might misunderstand something, it looks Yanko's machine specific
> >> workaround. I'm wondering if trimming _CRS is a generic workaround for
> >> broken _CRS machine.
> >>
> >> How about doing this when GA-MA78GM-S2H (BIOS F11) (and known machines
> >> that have the same problem) is detected? Or how about switching nocrs
> >> mode if the problem (resource conflict) is detected?
> > 
> > It's certainly a possibility to do this only for specific machines, but
> > I'd like to avoid tripping over issues one-by-one.
> > 
> > I think there are three ways to address BIOS _CRS defects:
> > 
> >   1) Ship an OEM-specific host bridge driver
> >   2) Put a platform- or BIOS-specific quirk into Windows
> 
> "into Linux"?

No, I forgot to say that all this is my speculation about how Windows
works.  I really can't imagine OEMs shipping OEM-specific host bridge
drivers for Windows, because I think it would make it to painful to
install.  And I don't think Windows quirks would really be practical
either, because it would take so long for a new Windows release
containing the quirk to appear that the OEM could never wait for it.

So I think that if we can make Linux parse _CRS in the same way Window
does, we should be able to handle most or all machines in the field
without a lot of quirks.

Of course, this is all pure speculation on my part; I've never worked on
Windows.

Bjorn

> >   3) Change the BIOS
> > 
> > The first two sound like such a hassle to me that I doubt they would be
> > practical.
> 
> I agree.
> For 1), we need OEM-specific driver, not chipset specific driver.
> 
> For 2), I thought it depends on how many machines with broken _CRS are
> there, and I didn't think there are so many, but...
> 
> > 
> > But it's clear that there are systems like this with what appear to be
> > _CRS defects.  It's quite possible that it's not really a defect, and we
> > just don't understand how to parse _CRS correctly yet.  Or, Windows
> > might have a few heuristics to clean up obvious errors.
> 
> Indeed, it might be true. Now I realize I need to change my opinion
> about 2).
> 
> > 
> > For example, I think Windows aligns host bridge windows, as documented
> > here: http://bugzilla.kernel.org/show_bug.cgi?id=14337
> > 
> > I think Windows also knows to ignore the Consumer/Producer bit in
> > Address Space Descriptors, and assume that all resources on bridges are
> > Producers.
> > 
> > Hmm, what we really need is a way to run Windows in a virtualized
> > environment where we could manipulate the _CRS method and see what
> > Windows does with it...
> > 
> > Anyway, I'd like to make Linux behave as much like Windows as possible
> > in this area so we can take advantage of all the testing that's done
> > with Windows.
> 
> Okey, thank you very much for explanation. I understood the background
> of your change.
> 
> Thanks,
> Kenji Kaneshige
> 
> 
> 

[PATCH v1 3/3] PCI: for address space collisions, show conflicting resource

[Bjorn Helgaas]

With request_resource_conflict(), we can learn what the actual conflict is,
so print that info for debugging purposes.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---

 drivers/pci/setup-res.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)


diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 7d678bb..17bed18 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -93,8 +93,7 @@ void pci_update_resource(struct pci_dev *dev, int resno)
 int pci_claim_resource(struct pci_dev *dev, int resource)
 {
 	struct resource *res = &dev->resource[resource];
-	struct resource *root;
-	int err;
+	struct resource *root, *conflict;
 
 	root = pci_find_parent_resource(dev, res);
 	if (!root) {
@@ -103,12 +102,15 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
 		return -EINVAL;
 	}
 
-	err = request_resource(root, res);
-	if (err)
+	conflict = request_resource_conflict(root, res);
+	if (conflict) {
 		dev_err(&dev->dev,
-			"address space collision: %pR already in use\n", res);
+			"address space collision: %pR conflicts with %s %pR\n",
+			res, conflict->name, conflict);
+		return -EBUSY;
+	}
 
-	return err;
+	return 0;
 }
 EXPORT_SYMBOL(pci_claim_resource);
 

Re: [PATCH v1 0/3] [RFC] resource, PCI: work around pci=use_crs conflicts

[Yanko Kaneti]

On Thu, 2010-03-11 at 17:01 -0700, Bjorn Helgaas wrote:
> These patches are to fix this pci=use_crs regression:
> 
>     http://bugzilla.kernel.org/show_bug.cgi?id=15480
> 
> The problem is that BIOS reported a PCI host bridge window that overlaps
> system RAM.  This workaround trims the window to avoid the overlap, which
> requires information about the conflicting resource.
> 
> Yanko, I reworked these a bit, so if you have a chance to retest them
> and collect another dmesg log, I'd appreciate it.
> 
> I'm still hoping for someone to find out how Windows deals with this, but
> haven't gotten any data yet.
> 
> ---
> 
> Bjorn Helgaas (3):
>       resources: add interfaces that return conflict information
>       x86/PCI: trim _CRS windows when they conflict with previous reservations
>       PCI: for address space collisions, show conflicting resource
> 
> 
>  arch/x86/pci/acpi.c     |   48 ++++++++++++++++++++++++++++++++++-------------
>  drivers/pci/setup-res.c |   14 ++++++++------
>  include/linux/ioport.h  |    2 ++
>  kernel/resource.c       |   44 ++++++++++++++++++++++++++++++++++++-------
>  4 files changed, 82 insertions(+), 26 deletions(-)

Applied on top of latest rawhide kernel. Boots without pci=  workarounds
and works fine so far.
Dmesg attached to the bug report.

Thanks

Re: [PATCH v1 0/3] [RFC] resource, PCI: work around pci=use_crs conflicts

[Bjorn Helgaas]

On Thursday 11 March 2010 05:01:03 pm Bjorn Helgaas wrote:
> These patches are to fix this pci=use_crs regression:
> 
>     http://bugzilla.kernel.org/show_bug.cgi?id=15480
> 
> The problem is that BIOS reported a PCI host bridge window that overlaps
> system RAM.  This workaround trims the window to avoid the overlap, which
> requires information about the conflicting resource.

Any opinions on these?  Yanko tested them and verified that they
fix the regression above.

Patch [3/3] only improves some debug, so that can wait until next
time around.

Bjorn