linux-kernel - Re: [BUG] WARN_ON(!context) in drivers/pci/hotplug/acpiphp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAErSpo7MWaJpY3pw1Db0J6vLZd22LcxReRxs3i2CUZb6qMexXw@mail.gmail.com>
Date:	Fri, 11 Oct 2013 09:08:40 -0600
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux PCI <linux-pci@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Subject: Re: [BUG] WARN_ON(!context) in drivers/pci/hotplug/acpiphp_glue.c

On Fri, Oct 11, 2013 at 7:01 AM, Steven Rostedt <rostedt@...dmis.org> wrote:
> On Fri, 11 Oct 2013 13:13:47 +0200
> "Rafael J. Wysocki" <rjw@...ysocki.net> wrote:
>>
>> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>> Subject: ACPI / hotplug / PCI: Accept coexistence with native PCIe hotplug
>>
>> Allow ACPIPHP (ACPI-based PCI hotplug) to handle event signaling for
>> devices that have already been claimed by the native PCIe hotplug
>> (pciehp).
>>
>> The ACPI hotplug events are essentially re-scan, remove and eject
>> requests.  Re-scan and remove should work regardless, because they
>> may be triggered by user space via sysfs and the ACPI eject (_EJ0)
>> should work if the BIOS wants us to use it.  There may be an issue
>> if the BIOS signals ACPI eject and wants us to use the native eject,
>> but that doesn't work without this change anyway.
>>
>> This change prevents the WARN_ON() in acpiphp_enumerate_slots() from
>> triggering unnecessarily for bridges whose parents are managed by
>> pciehp.
>>
>
> Reported-by: Steven Rostedt <rostedt@...dmis.org>
> Tested-by: Steven Rostedt <rostedt@...dmis.org>

I opened https://bugzilla.kernel.org/show_bug.cgi?id=62831 for this
issue because I don't think this question of how to coordinate acpiphp
and pciehp is completely resolved, and I think it might be interesting
to have the complete dmesg, acpidump, and "lspci -vv" output attached
there for future reference.

Steve, I tried to attach the dmesg you mentioned yesterday in IRC, but
the link didn't work for me.  Would you mind attaching this info?

Rafael, I assume you'll probably merge this through your tree.  Would
you mind adding a reference to this bugzilla in the changelog?  I do
have a "convert to dynamic debug" acpiphp patch in my "next" branch
(bd950799), but I suspect you have several more interesting ones in
your tree.

Bjorn

>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>> ---
>>  drivers/pci/hotplug/acpiphp_glue.c |   32 ++++++++++++++++++++++++++------
>>  1 file changed, 26 insertions(+), 6 deletions(-)
>>
>> Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
>> ===================================================================
>> --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
>> +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
>> @@ -259,6 +259,31 @@ static void acpiphp_dock_release(void *d
>>       put_bridge(context->func.parent);
>>  }
>>
>> +/**
>> + * slot_should_be_exposed - Check whether or not to expose a slot to userland.
>> + * @bridge: ACPIPHP bridge the slot belongs to.
>> + * @handle: ACPI handle of a device in the slot.
>> + */
>> +static inline bool slot_should_be_exposed(struct acpiphp_bridge *bridge,
>> +                                       acpi_handle handle)
>> +{
>> +     struct pci_bus *pbus = bridge->pci_bus;
>> +     struct pci_dev *pdev = bridge->pci_dev;
>> +
>> +     /*
>> +      * Do not expose slots whose bridges are managed by pciehp, because they
>> +      * will be exposed to user space by the pciehp driver.
>> +      */
>> +     if (pdev && device_is_managed_by_native_pciehp(pdev))
>> +             return false;
>> +
>> +     /*
>> +      * Expose slots for devices with either _EJ0 or _RMV and for devices
>> +      * on docking stations.
>> +      */
>> +     return acpi_pci_check_ejectable(pbus, handle) || is_dock_device(handle);
>> +}
>> +
>>  /* callback routine to register each ACPI PCI slot object */
>>  static acpi_status register_slot(acpi_handle handle, u32 lvl, void *data,
>>                                void **rv)
>> @@ -271,12 +296,8 @@ static acpi_status register_slot(acpi_ha
>>       unsigned long long adr;
>>       int device, function;
>>       struct pci_bus *pbus = bridge->pci_bus;
>> -     struct pci_dev *pdev = bridge->pci_dev;
>>       u32 val;
>>
>> -     if (pdev && device_is_managed_by_native_pciehp(pdev))
>> -             return AE_OK;
>> -
>>       status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
>>       if (ACPI_FAILURE(status)) {
>>               acpi_handle_warn(handle, "can't evaluate _ADR (%#x)\n", status);
>> @@ -325,8 +346,7 @@ static acpi_status register_slot(acpi_ha
>>
>>       list_add_tail(&slot->node, &bridge->slots);
>>
>> -     /* Register slots for ejectable funtions only. */
>> -     if (acpi_pci_check_ejectable(pbus, handle)  || is_dock_device(handle)) {
>> +     if (slot_should_be_exposed(bridge, handle)) {
>>               unsigned long long sun;
>>               int retval;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/