From: Mike Habeck on
Bjorn Helgaas wrote:
> [Re-added linux-pci, which got lost again somewhere.]
>
> On Monday, May 31, 2010 05:12:00 am Mike Travis wrote:
>> H. Peter Anvin wrote:
>>> On 05/28/2010 10:10 AM, Mike Travis wrote:
>>>> H. Peter Anvin wrote:
>>>>> On 05/28/2010 09:53 AM, Mike Travis wrote:
>>>>>> Any further consideration for this patch, or has it been rejected?
>
> I'm disappointed that you didn't rework this to make it generic,
> not x86-specific. That would be pretty easy and would remove
> the need for somebody else to come and clean it up later.
>
>>>>> Well, it's really up to Jesse, but as far as I can see, this patch is a
>>>>> net loss of functionality and doesn't actually add anything. Without
>>>>> this patch, some resources that were not assigned by BIOS will be
>>>>> unreachable. With this patch, *all* resources that were not assigned by
>>>>> BIOS will be unreachable...
>>>>>
>>>>> -hpa
>>>>>
>>>> Apparently you're missing the point of the patch? The patch is needed
>>>> because BIOS is purposely not assigning I/O BAR's to devices that won't
>>>> use them, freeing up the resource for devices that do need them. Where
>>>> is the "all" resources that are not reachable?
>>> No, the patch isn't needed for those.
>>>
>>> Without your patch:
>>>
>>> - Devices assigned by BIOS remain assigned;
>>> - Devices not assigned by BIOS get assigned until address space
>>> exhausted.
>>>
>>> With your patch:
>>>
>>> - Devices assigned by BIOS remain assigned;
>>> - Devices not assigned by BIOS never get assigned at all.
>>>
>>> What am I missing here?
>> BIOS still assigns the MMIO BAR's so the devices are alive.
>
> I'm sorry; I don't follow this. BIOS assigns MMIO BARs regardless
> of whether we have your patch.
>
> I'm still having trouble reconciling the stated purpose, i.e., the
> changelog, with the behavior. The changelog implies that the patch
> is required to make >16 devices with I/O BARs work at all, but per
> Mike Habeck, the patch just gets rid of some warnings and maybe helps
> with hot-add of devices using I/O space.

greaterthan 16 devices will still work if the BIOS doesn't assign
the I/O BARs and the kernel does (including the devices that don't
get assigned due to the kernel running out of I/O Port resources).
And when the kernel runs out of I/O Port space it will warn for
those devices it couldn't assign: (example):

pci 0002:03:00.0: BAR 5: can't allocate I/O resource [0x0-0x7f]

But if the kernel assigns all the I/O Port space to these devices
we know don't need it (thus the reason the BIOS didn't assign it)
then I believe hot-add of devices using I/O space will fail.
What the patch is attempting to do is allow the BIOS a way to
not assign resources it knows are not needed and thus make sure
the kernel doesn't override that.

There is also the issue that quirk_system_pci_resources() thinks
those unassigned I/O resources are using I/O port space 0x0-0xFF.
Since the BIOS never assigned the BAR the kernel reads it as
having a base of 0x0 and a limit of whatever the BAR size is when
it writes all 1's to obtain the size. So that results in
quirk_system_pci_resources() disabling pnp devices: (example):

pnp 00:11: io resource (0x92-0x92) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling
pnp 00:11: io resource (0x10-0x1f) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling
pnp 00:11: io resource (0x72-0x73) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling
pnp 00:11: io resource (0x80-0x80) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling
pnp 00:11: io resource (0x84-0x86) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling
pnp 00:11: io resource (0x88-0x88) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling
pnp 00:11: io resource (0x8c-0x8e) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling
pnp 00:11: io resource (0x90-0x9f) overlaps 0002:03:00.0 BAR 0 (0x0-0xff), disabling

One could argue this is a quirk_system_pci_resources() issue and
should be handled there rather than "zeroing out the resource if
the bios didn't assign it" as the patch does, but what the patch
is attempting to do (as stated above) is to allow the BIOS a way
to not assign resources it knows are not needed and thus make sure
the kernel doesn't override that... and in doing that the quirk
issue goes away too.

-mike



>
> Is there a deeper problem that happens if we exhaust I/O space?
> Are we releasing device resources in pci_assign_unassigned_bridge_resources()
> and then we fail to reassign even MMIO resources after we exhaust
> I/O space?
>
> Maybe a complete dmesg log showing the failure would be helpful. if
> so, you could open a kernel.org bugzilla and reference it in your
> changelog so we can take this issue into account in future PCI work.
>
> Bjorn

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on
On 06/02/2010 08:53 AM, Jesse Barnes wrote:
>>
>> That's what I thought, which I guess means my original question to Mike
>> still stands...
>
> I thought the whole reason for this was hotplug; we don't want to
> exhaust I/O space unnecessarily by allocating resources for BARs the
> BIOS didn't assign so we can keep them around for later hotplug
> activity.
>
> If there's some other issue, it's not too late to drop this patch.
>

Okay, now... this means that if a device that the BIOS doesn't know
about, but which needs I/O addresses, then it will work if hotplugged,
but not if it is plugged in on system boot?

-hpa


--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mike Habeck on
Jesse Barnes wrote:
> On Tue, 08 Jun 2010 17:53:19 -0700
> "H. Peter Anvin" <hpa(a)zytor.com> wrote:
>
>> On 06/02/2010 08:53 AM, Jesse Barnes wrote:
>>>> That's what I thought, which I guess means my original question to Mike
>>>> still stands...
>>> I thought the whole reason for this was hotplug; we don't want to
>>> exhaust I/O space unnecessarily by allocating resources for BARs the
>>> BIOS didn't assign so we can keep them around for later hotplug
>>> activity.
>>>
>>> If there's some other issue, it's not too late to drop this patch.
>>>
>> Okay, now... this means that if a device that the BIOS doesn't know
>> about, but which needs I/O addresses, then it will work if hotplugged,
>> but not if it is plugged in on system boot?
>
> Depends on the BIOS interactions on this platform; if the kernel ends
> up doing all the allocations itself, we'll allocate space for every BAR
> unconditionally, meaning that any hotplugged device should work.

Correct, our BIOS allocates I/O space for all devices except for a
few that it knows don't use it. On a hotplug attach the kernel
will be unconditionally allocating the I/O space for all devices..
The pci=nobar option strictly prevents the kernel from allocating
BAR resources to device BARs that the BIOS didn't assign (similar
to how the pci=norom option works for the device's ROM BAR)

>
> But really the SGI guys should comment here.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/