diff mbox

agp/intel: can't ioremap flush page - no chipset flushing

Message ID 20140310182431.GA11424@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bjorn Helgaas March 10, 2014, 6:24 p.m. UTC 
[+cc linux-pci]

On Sat, Mar 08, 2014 at 03:44:37PM +0100, Paul Bolle wrote:
> Bjorn Helgaas schreef op za 08-03-2014 om 07:12 [-0700]:
> > I assume you have CONFIG_PHYS_ADDR_T_64BIT=n (which is perfectly
> > legal); let me know if otherwise.
> 
> $ grep CONFIG_PHYS_ADDR_T_64BIT /boot/config-3.14.0-0.rc5.1.local2.fc20.i686 
> # CONFIG_PHYS_ADDR_T_64BIT is not set
> 
> So, yes, CONFIG_PHYS_ADDR_T_64BIT=n here.

Thanks.  Can you try the patch below?  I think it should fix the problem.


PCI: Don't check resource_size() in pci_bus_alloc_resource()

From: Bjorn Helgaas <bhelgaas@google.com>

When resource_size_t is 32 bits wide, e.g., when CONFIG_PHYS_ADDR_T_64BIT
is not defined, resource_size() on [mem 0x00000000-0xffffffff] returns 0
because (r->end - r->start + 1) overflows.

Therefore, we can't use "resource_size() == 0" to decide that allocation
from this resource will fail.  allocate_resource() should fail anyway if it
can't satisfy the address constraints, so we should just depend on that.

A [mem 0x00000000-0xffffffff] bus resource is obviously not really valid,
but we do fall back to it as a default when we don't have information about
host bridge apertures.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=71611
Fixes: f75b99d5a77d PCI: Enforce bus address limits in resource allocation
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/bus.c |    2 --
 1 file changed, 2 deletions(-)

Comments

Paul Bolle March 10, 2014, 11:45 p.m. UTC | #1 
Bjorn Helgaas schreef op ma 10-03-2014 om 12:24 [-0600]:
> Thanks.  Can you try the patch below?  I think it should fix the problem.
> 
> 
> PCI: Don't check resource_size() in pci_bus_alloc_resource()
> 
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> When resource_size_t is 32 bits wide, e.g., when CONFIG_PHYS_ADDR_T_64BIT
> is not defined, resource_size() on [mem 0x00000000-0xffffffff] returns 0
> because (r->end - r->start + 1) overflows.
> 
> Therefore, we can't use "resource_size() == 0" to decide that allocation
> from this resource will fail.  allocate_resource() should fail anyway if it
> can't satisfy the address constraints, so we should just depend on that.
> 
> A [mem 0x00000000-0xffffffff] bus resource is obviously not really valid,
> but we do fall back to it as a default when we don't have information about
> host bridge apertures.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=71611
> Fixes: f75b99d5a77d PCI: Enforce bus address limits in resource allocation
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/bus.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 00660cc502c5..38901665c770 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -162,8 +162,6 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
>  
>  		avail = *r;
>  		pci_clip_resource_to_region(bus, &avail, region);
> -		if (!resource_size(&avail))
> -			continue;
>  
>  		/*
>  		 * "min" is typically PCIBIOS_MIN_IO or PCIBIOS_MIN_MEM to

I've applied your patch on top of v3.14-rc6. The boot warning is gone.
And /proc/iomem once again has the lines:
    [...]
    80000000-801fffff : PCI Bus 0000:02
    80200000-8027ffff : 0000:00:02.1
    80280000-80280fff : Intel Flush Page
    [...]

Which is what we want, don't we?

A bit of doubt is caused by two new boot time messages:
    pnp 00:00: unknown resource type 10 in _CRS
    pnp 00:00: can't evaluate _CRS: 1

But I haven't yet tried v3.14-rc6 without your patch, so these might be
unrelated. They're unclear to me, anyway.

Thanks,


Paul Bolle
Bjorn Helgaas March 11, 2014, 12:07 a.m. UTC | #2 
[+cc Rafael]

On Mon, Mar 10, 2014 at 5:45 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> Bjorn Helgaas schreef op ma 10-03-2014 om 12:24 [-0600]:
>> Thanks.  Can you try the patch below?  I think it should fix the problem.
>>
>>
>> PCI: Don't check resource_size() in pci_bus_alloc_resource()
>>
>> From: Bjorn Helgaas <bhelgaas@google.com>
>>
>> When resource_size_t is 32 bits wide, e.g., when CONFIG_PHYS_ADDR_T_64BIT
>> is not defined, resource_size() on [mem 0x00000000-0xffffffff] returns 0
>> because (r->end - r->start + 1) overflows.
>>
>> Therefore, we can't use "resource_size() == 0" to decide that allocation
>> from this resource will fail.  allocate_resource() should fail anyway if it
>> can't satisfy the address constraints, so we should just depend on that.
>>
>> A [mem 0x00000000-0xffffffff] bus resource is obviously not really valid,
>> but we do fall back to it as a default when we don't have information about
>> host bridge apertures.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=71611
>> Fixes: f75b99d5a77d PCI: Enforce bus address limits in resource allocation
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>>  drivers/pci/bus.c |    2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
>> index 00660cc502c5..38901665c770 100644
>> --- a/drivers/pci/bus.c
>> +++ b/drivers/pci/bus.c
>> @@ -162,8 +162,6 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
>>
>>               avail = *r;
>>               pci_clip_resource_to_region(bus, &avail, region);
>> -             if (!resource_size(&avail))
>> -                     continue;
>>
>>               /*
>>                * "min" is typically PCIBIOS_MIN_IO or PCIBIOS_MIN_MEM to
>
> I've applied your patch on top of v3.14-rc6. The boot warning is gone.
> And /proc/iomem once again has the lines:
>     [...]
>     80000000-801fffff : PCI Bus 0000:02
>     80200000-8027ffff : 0000:00:02.1
>     80280000-80280fff : Intel Flush Page
>     [...]
>
> Which is what we want, don't we?

Yep, that part looks good.

> A bit of doubt is caused by two new boot time messages:
>     pnp 00:00: unknown resource type 10 in _CRS
>     pnp 00:00: can't evaluate _CRS: 1
>
> But I haven't yet tried v3.14-rc6 without your patch, so these might be
> unrelated. They're unclear to me, anyway.

Hmm, this is definitely concerning.  Resource type 10 is
ACPI_RESOURCE_TYPE_FIXED_MEMORY32, which we do handle (unless it's
length 0).  And your previous dmesg logs (at
https://bugzilla.kernel.org/show_bug.cgi?id=71611) don't show anything
like that.

Can you attach an acpidump and the dmesg log with my patch to the bugzilla?

Bjorn
Paul Bolle March 11, 2014, 12:15 a.m. UTC | #3 
On Mon, 2014-03-10 at 18:07 -0600, Bjorn Helgaas wrote:
> On Mon, Mar 10, 2014 at 5:45 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> > A bit of doubt is caused by two new boot time messages:
> >     pnp 00:00: unknown resource type 10 in _CRS
> >     pnp 00:00: can't evaluate _CRS: 1
> >
> > But I haven't yet tried v3.14-rc6 without your patch, so these might be
> > unrelated. They're unclear to me, anyway.
> 
> Hmm, this is definitely concerning.  Resource type 10 is
> ACPI_RESOURCE_TYPE_FIXED_MEMORY32, which we do handle (unless it's
> length 0).  And your previous dmesg logs (at
> https://bugzilla.kernel.org/show_bug.cgi?id=71611) don't show anything
> like that.
> 
> Can you attach an acpidump and the dmesg log with my patch to the bugzilla?

It's getting rather late over here. So I'll try (quite) a few hours
later. But acpidump doesn't ring any bells right now. Any hints?

Thanks,


Paul Bolle
Bjorn Helgaas March 11, 2014, 2:07 a.m. UTC | #4 
On Mon, Mar 10, 2014 at 6:15 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Mon, 2014-03-10 at 18:07 -0600, Bjorn Helgaas wrote:
>> On Mon, Mar 10, 2014 at 5:45 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
>> > A bit of doubt is caused by two new boot time messages:
>> >     pnp 00:00: unknown resource type 10 in _CRS
>> >     pnp 00:00: can't evaluate _CRS: 1
>> >
>> > But I haven't yet tried v3.14-rc6 without your patch, so these might be
>> > unrelated. They're unclear to me, anyway.
>>
>> Hmm, this is definitely concerning.  Resource type 10 is
>> ACPI_RESOURCE_TYPE_FIXED_MEMORY32, which we do handle (unless it's
>> length 0).  And your previous dmesg logs (at
>> https://bugzilla.kernel.org/show_bug.cgi?id=71611) don't show anything
>> like that.
>>
>> Can you attach an acpidump and the dmesg log with my patch to the bugzilla?
>
> It's getting rather late over here. So I'll try (quite) a few hours
> later. But acpidump doesn't ring any bells right now. Any hints?

There's acpidump info here: https://01.org/linux-acpi/utilities .  You
don't need to extract the binary tables or anything; just attach the
text dump to the bugzilla.

It would be very interesting to try v3.14-rc6 without my patch.  I
hope it has the same "unknown resource type" messages, because I don't
see how my patch could be related to them.  There were no recent
PNP-related changes, so then I start to worry about some sort of
memory corruption from an unrelated patch.  But I hope that's not the
case.

Bjorn
Paul Bolle March 11, 2014, 9:20 a.m. UTC | #5 
Bjorn Helgaas schreef op ma 10-03-2014 om 20:07 [-0600]:
> On Mon, Mar 10, 2014 at 6:15 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> > On Mon, 2014-03-10 at 18:07 -0600, Bjorn Helgaas wrote:
> >> On Mon, Mar 10, 2014 at 5:45 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> >> > A bit of doubt is caused by two new boot time messages:
> >> >     pnp 00:00: unknown resource type 10 in _CRS
> >> >     pnp 00:00: can't evaluate _CRS: 1
> >> >
> >> > But I haven't yet tried v3.14-rc6 without your patch, so these might be
> >> > unrelated. They're unclear to me, anyway.
> >>
> >> Hmm, this is definitely concerning.  Resource type 10 is
> >> ACPI_RESOURCE_TYPE_FIXED_MEMORY32, which we do handle (unless it's
> >> length 0).  And your previous dmesg logs (at
> >> https://bugzilla.kernel.org/show_bug.cgi?id=71611) don't show anything
> >> like that.
> >>
> >> Can you attach an acpidump and the dmesg log with my patch to the bugzilla?
> >
> > It's getting rather late over here. So I'll try (quite) a few hours
> > later. But acpidump doesn't ring any bells right now. Any hints?
> 
> There's acpidump info here: https://01.org/linux-acpi/utilities .  You
> don't need to extract the binary tables or anything; just attach the
> text dump to the bugzilla.

(On Fedora it's shipped in acpica-tools. But since acpidump is a symlink
to /usr/bin/acpidump-acpica that is created at install time it won't
show up when one does "repoquery -f "*/acpidump". Add to this that "yum
list pmtools" returns an error, and one is guaranteed roughly 30 minutes
of sheer fun. Unsurprisingly, stubbornly trying "yum install pmtools"
will actually install acpica-tools.)

At least this all is written down somewhere now.)

> It would be very interesting to try v3.14-rc6 without my patch.  I
> hope it has the same "unknown resource type" messages, because I don't
> see how my patch could be related to them.  There were no recent
> PNP-related changes, so then I start to worry about some sort of
> memory corruption from an unrelated patch.  But I hope that's not the
> case.

lkml.org is returning some error page, but Markus Trippelsdorf just
reported seeing this message too. Markus is unlikely to be also trying
this patch.

I've just added you to the CC's of my reply to Markus. I won't update
the bugzilla report for now, because those messages appear unrelated to
your patch.

Thanks,


Paul Bolle
diff mbox

Patch

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 00660cc502c5..38901665c770 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -162,8 +162,6 @@  static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
 
 		avail = *r;
 		pci_clip_resource_to_region(bus, &avail, region);
-		if (!resource_size(&avail))
-			continue;
 
 		/*
 		 * "min" is typically PCIBIOS_MIN_IO or PCIBIOS_MIN_MEM to