lkml.org 
[lkml]   [2008]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 0/4, v14] PCI, ACPI: Physical PCI slot objects
    Alex Chiang wrote:
    > Hi Jesse, Ben, Kenji-san,
    >
    > This is v14 of the physical PCI slot series.
    >
    > This patchset has been kicking around -mm for the past few
    > months, and they were getting clobbered on a continual basis, so
    > let's say I'm quite motivated to get them into mainline. ;)
    >
    > This mail describes two things:
    >
    > - an update for handling pSeries
    > - explanation of how I did not regress Kenji-san's latest
    > changes
    >
    > Review comments are much appreciated.
    >
    > -----------------------
    > pSeries-related changes
    > -----------------------
    > The most recent outstanding issue with this series was breakage
    > with pSeries. In a nutshell, the problem was:
    >
    > - pci_hp_register() interface changed to require a devfn
    > when registering a slot
    >
    > - pSeries doesn't necessarily know the devfn of an
    > unpopulated slot
    >
    > There are more details, of course, and they are in the archives.
    > I can dig them up if people want more context.
    >
    > After working offline with BenH and Willy, we thought that the
    > best way forward was for the new infrastructure to provide a new
    > API, pci_update_slot_number(), which can be used by callers to
    > modify the slot number after slot creation.
    >
    > This change goes hand in hand with modifying the semantics of
    > pci_hp_register() where callers are now allowed to pass -1 for
    > slot_nr to create a 'placeholder' slot.
    >
    > The third related change is that the infrastructure will only
    > display an 'address' value of 'dddd:bb' (domain:bus) when the
    > device is -1. In the normal case, the full address of dddd:bb:dd
    > is displayed.
    >
    > I did fold an earlier -mm fixup patch into these changes to
    > improve future bisectability.
    >
    > I added kerneldoc to explain the APIs.
    >
    > -----------------------------
    > maintaining Kenji-san's fixes
    > -----------------------------
    > Finally, this patch series slightly changes the logic introduced
    > by Kenji-san's patches:
    >
    > 9e4f2e8d4ddb04ad16a3828cd9a369a5a5287009
    > a86161b3134465f072d965ca7508ec9c1e2e52c7
    >
    > For a86161b31344, the check against registering a slot with the
    > same name multiple times is removed. That check prevents a
    > scenario where multiple pcihp drivers try to claim the same slot.
    >
    > The check is removed because the new code allows multiple callers
    > of pci_create_slot(). One callsite is pci_hp_register(), the
    > other is in the ACPI slot detection driver provided in patch 4/4.
    > In the case of multiple legitimate callers, the correct thing to
    > do is refcount the struct pci_slot's kobj.
    >
    > In the case of two pcihp drivers attempting to claim the same
    > slot, pci_hp_register() returns -EBUSY to indicate it has already
    > been claimed. This logic has been part of the patch series from
    > the beginning.
    >
    > Thus, the end behavior introduced by a86161b31344 is preserved,
    > although in a slightly different implementation.
    >
    > The firmware defect that Kenji-san is trying to fix with
    > 9e4f2e8d4d is the case where broken firmware will present the
    > kernel with slots like bb1:dd1, name "A" and bb2:dd2, name "A".
    >
    > In other words, two different busses or two different devices on
    > the same bus, but both have the same name.
    >
    > In this case, pci_create_slot() thinks they are two different
    > physical slots (which is true), and tries to register them with
    > the kobject core, which will then complain about registering two
    > objects with the same name. -EEXIST will be returned back up
    > through the pcihp core and back to pciehp, which will then printk
    > the message added by 9e4f2e8d4d.
    >
    > Thus, the condition Kenji-san is trying to warn about with
    > 9e4f2e8d4d is preserved.
    >

    I tried your patches and I have a comment (question) about the behavior.

    To emulate the (broken?) platform that assigns the same name to multiple
    slots, I made a debug patch for pciehp driver to assign same name ('1000')
    to all slots (my environment has two PCIe slots). With this patch, I
    noticed that the behavior or pci_hp_register() (or pci_create_slot())
    varies depending on whether pci_slot driver is loaded or not. See below.

    (a) With pci_slot driver loaded
    I got the following error message when I loaded pciehp driver.

    pciehp: pci_hp_register failed with error -17
    pciehp: Failed to register slot because of name collision. Try
    'pciehp_slot_with_bus' module option.
    pciehp: pciehp: slot initialization failed

    (b) Without pci_slot driver loaded
    I got the kernel stack dump and error messages when I loaded pciehp
    driver.

    kobject_add_internal failed for 1000 with -EEXIST, don't try to
    register things with the same name in the same directory.

    Call Trace:
    [<a000000100015180>] show_stack+0x40/0xa0
    sp=e0000040a086fb80 bsp=e0000040a0861158
    [<a000000100015210>] dump_stack+0x30/0x60
    sp=e0000040a086fd50 bsp=e0000040a0861140
    [<a0000001003b3910>] kobject_add_internal+0x330/0x400
    sp=e0000040a086fd50 bsp=e0000040a0861100
    [<a0000001003b3bd0>] kobject_add_varg+0x90/0xc0
    sp=e0000040a086fd50 bsp=e0000040a08610c8
    [<a0000001003b3c90>] kobject_init_and_add+0x90/0xc0
    sp=e0000040a086fd50 bsp=e0000040a0861068
    [<a0000001003d69b0>] pci_create_slot+0x150/0x260
    sp=e0000040a086fd80 bsp=e0000040a0861030
    [<a000000200b71870>] pci_hp_register+0x130/0x880 [pci_hotplug]
    sp=e0000040a086fd80 bsp=e0000040a0860ff0
    [<a000000200ec1a60>] pciehp_probe+0x720/0xca0 [pciehp]

    (snip...)

    Unable to register kobject 1000
    pciehp: pci_hp_register failed with error -17
    pciehp: Failed to register slot because of name collision. Try
    'pciehp_slot_with_bus' module option.
    pciehp: pciehp: slot initialization failed

    Could you tell me why that difference happen? And my expectation is
    the result should be (a) above regardless of whether pci_slot driver
    is loaded or not.

    Thanks,
    Kenji Kaneshige




    \
     
     \ /
      Last update: 2008-06-09 10:21    [W:0.032 / U:62.144 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site