linux-kernel - Re: [PATCH 5/6] PCI: acpiphp: look _RMV method a bit deeper in the hierarhcy

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: <20130702174542.18099E0090@blue.fi.intel.com>
Date:	Tue,  2 Jul 2013 20:45:42 +0300 (EEST)
From:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Yinghai Lu <yinghai@...nel.org>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"Penner, Miles J" <miles.j.penner@...el.com>,
	Bruce Allan <bruce.w.allan@...el.com>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH 5/6] PCI: acpiphp: look _RMV method a bit deeper in the
 hierarhcy

Bjorn Helgaas wrote:
> On Tue, Jul 2, 2013 at 4:44 AM, Kirill A. Shutemov
> <kirill.shutemov@...ux.intel.com> wrote:
> > Mika Westerberg wrote:
> >> The acpiphp driver finds out whether the device is hotpluggable by checking
> >> whether it has _RMV method behind it (and if it returns 1). However, at
> >> least Acer Aspire S5 with Thunderbolt host router has this method placed
> >> behind device called EPUP (endpoint upstream port?) and not directly behind
> >> the root port as can be seen from the ASL code below:
> >>
> >> Device (RP05)
> >> {
> >>       ...
> >>       Device (HRUP)
> >>       {
> >>               Name (_ADR, Zero)
> >>               Name (_PRW, Package (0x02)
> >>               {
> >>                       0x09,
> >>                       0x04
> >>               })
> >>               Device (HRDN)
> >>               {
> >>                       Name (_ADR, 0x00040000)
> >>                       Name (_PRW, Package (0x02)
> >>                       {
> >>                               0x09,
> >>                               0x04
> >>                       })
> >>                       Device (EPUP)
> >>                       {
> >>                               Name (_ADR, Zero)
> >>                               Method (_RMV, 0, NotSerialized)
> >>                               {
> >>                                       Return (One)
> >>                               }
> >>                       }
> >>               }
> >>       }
> >>
> >> If we want to support such machines we must look for the _RMV method a bit
> >> deeper in the hierarchy. Fix this by changing pcihp_is_ejectable() to check
> >> few more devices down from the root port.
> >
> > We found that this approach is broken. We've got false positive: host
> > bridge itself was detected as hotplugable slot %) I think it's not
> > acceptable.
> >
> > Mika has tried few more approaches, but we haven't found anything better
> > then hardcoded path like in original workaround patch[1]. It's not generic
> > at all, but safe from false positives.
> >
> > Any thoughts?
> >
> > [1] http://article.gmane.org/gmane.linux.kernel.pci/19102
> 
> The changelog of this patch ([1]) says "Correct ACPI PCI hotplug
> implementation should have _RMV method in a PCI slot (device under pci
> bridge). In Acer Aspire S5 case we have it deeper in hierarchy ..."
> 
> This is exactly what I mean about acpiphp being brittle because of the
> assumptions it makes about the ACPI namespace.  Is there actually
> something in the spec that requires the _RMV method to be where
> pcihp_is_ejectable() expects it?

Spec says that _RMV indicates the device which can be removed.
Rest, I believe, are assumptions: if the device is removable, then parent
must be a hotpluggable slot.

It looks reasonable, but doesn't work in this case. And it's not obvious
how we can generalize the assumption to cover the case.

> I'm not 100% dead-set against merging the workaround with hard-coded
> path, but I still don't think it's a good idea.  It "fixes" it for one
> particular machine, but there will likely be other machines that
> require similar fixes in the future.  It makes it harder for somebody
> to clean up the design later, because that person won't have an Aspire
> S5 to test.  It makes it less likely that somebody *will* clean it up
> later, because "everything is already working."
> 
> That's why my preference (given infinite resources) would be to rework
> acpiphp now, while people are interested and can test it.  I'm sure
> this would be a major redesign of acpiphp and its interaction with
> pciehp, but I think it's something we're going to have to do at some
> point.

Basically, we run out of ideas. Any input is welcome! :)

For now we can post new version of patchset without the workaround and
come back later with workaround (or proper solution if we'll find any).

Does it work for you? Or you prefer everything in one patchset?

-- 
 Kirill A. Shutemov
--
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/