From: Bjorn Helgaas <helgaas@kernel.org>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Tomasz Nowicki <tn@semihalf.com>,
David Daney <ddaney@caviumnetworks.com>,
Will Deacon <will.deacon@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Rafael Wysocki <rafael@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
Hanjun Guo <hanjun.guo@linaro.org>,
Sinan Kaya <okaya@codeaurora.org>,
Jayachandran C <jchandra@broadcom.com>,
Christopher Covington <cov@codeaurora.org>,
Duc Dang <dhdang@apm.com>,
Robert Richter <robert.richter@caviumnetworks.com>,
Marcin Wojtas <mw@semihalf.com>,
Liviu Dudau <Liviu.Dudau@arm.com>,
Yijing Wang <wangyijing@huawei.com>,
Mark Salter <msalter@redhat.com>,
linux-pci@vger.kernel.org,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Linaro ACPI Mailman List <linaro-acpi@lists.linaro.org>,
Jon Masters <jcm@redhat.com>,
Andrea Gallo <andrea.gallo@linaro.org>,
Jeremy Linton <jeremy.linton@arm.com>,
Dongdong Liu <liudongdong3@huawei.com>,
Gabriele Paoloni <gabriele.paoloni@huawei.com>,
Jeff Hugo <jhugo@codeaurora.org>,
"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
Date: Wed, 21 Sep 2016 13:04:57 -0500 [thread overview]
Message-ID: <20160921180457.GB20006@localhost> (raw)
In-Reply-To: <20160921140549.GA11968@red-moon>
On Wed, Sep 21, 2016 at 03:05:49PM +0100, Lorenzo Pieralisi wrote:
> On Tue, Sep 20, 2016 at 02:17:44PM -0500, Bjorn Helgaas wrote:
> > On Tue, Sep 20, 2016 at 04:09:25PM +0100, Ard Biesheuvel wrote:
>
> [...]
>
> > > None of these platforms can be fixed entirely in software, and given
> > > that we will not be adding quirks for new broken hardware, we should
> > > ask ourselves whether having two versions of a quirk, i.e., one for
> > > broken hardware + currently shipping firmware, and one for the same
> > > broken hardware with fixed firmware is really an improvement over what
> > > has been proposed here.
> >
> > We're talking about two completely different types of quirks:
> >
> > 1) MCFG quirks to use memory-mapped config space that doesn't quite
> > conform to the ECAM model in the PCIe spec, and
> >
> > 2) Some yet-to-be-determined method to describe address space
> > consumed by a bridge.
> >
> > The first two patches of this series are a nice implementation for 1).
> > The third patch (ThunderX-specific) is one possibility for 2), but I
> > don't like it because there's no way for generic software like the
> > ACPI core to discover these resources.
>
> Ok, so basically this means that to implement (2) we need to assign
> some sort of _HID to these quirky PCI bridges (so that we know what
> device they represent and we can retrieve their _CRS). I take from
> this discussion that the goal is to make sure that all non-config
> resources have to be declared through _CRS device objects, which is
> fine but that requires a FW update (unless we can fabricate ACPI
> devices and corresponding _CRS in the kernel whenever we match a
> given MCFG table signature).
All resources consumed by ACPI devices should be declared through
_CRS. If you want to fabricate ACPI devices or _CRS via kernel
quirks, that's fine with me. This could be triggered via MCFG
signature, DMI info, host bridge _HID, etc.
> We discussed this already and I think we should make a decision:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/414722.html
>
> > > > I'd like to step back and come up with some understanding of how
> > > > non-broken firmware *should* deal with this issue. Then, if we *do*
> > > > work around this particular broken firmware in the kernel, it would be
> > > > nice to do it in a way that fits in with that understanding.
> > > >
> > > > For example, if a companion ACPI device is the preferred solution, an
> > > > ACPI quirk could fabricate a device with the required resources. That
> > > > would address the problem closer to the source and make it more likely
> > > > that the rest of the system will work correctly: /proc/iomem could
> > > > make sense, things that look at _CRS generically would work (e.g,
> > > > /sys/, an admittedly hypothetical "lsacpi", etc.)
> > > >
> > > > Hard-coding stuff in drivers is a point solution that doesn't provide
> > > > any guidance for future platforms and makes it likely that the hack
> > > > will get copied into even more drivers.
> > > >
> > >
> > > OK, I see. But the guidance for future platforms should be 'do not
> > > rely on quirks', and what I am arguing here is that the more we polish
> > > up this code and make it clean and reusable, the more likely it is
> > > that will end up getting abused by new broken hardware that we set out
> > > to reject entirely in the first place.
> > >
> > > So of course, if the quirk involves claiming resources, let's make
> > > sure that this occurs in the cleanest and most compliant way possible.
> > > But any factoring/reuse concerns other than for the current crop of
> > > broken hardware should be avoided imo.
> >
> > If future hardware is completely ECAM-compliant and we don't need any
> > more MCFG quirks, that would be great.
>
> Yes.
>
> > But we'll still need to describe that memory-mapped config space
> > somewhere. If that's done with PNP0C02 or similar devices (as is done
> > on my x86 laptop), we'd be all set.
>
> I am not sure I understand what you mean here. Are you referring
> to MCFG regions reported as PNP0c02 resources through its _CRS ?
Yes. PCI Firmware Spec r3.0, Table 4-2, note 2 says address ranges
reported via MCFG or _CBA should be reserved by _CRS of a PNP0C02
device.
> IIUC PNP0C02 is a reservation mechanism, but it does not help us
> associate its _CRS to a specific PCI host bridge instance, right ?
Gab proposed a hierarchy that *would* associate a PNP0C02 device with
a PCI bridge:
Device (PCI1) {
Name (_HID, "HISI0080") // PCI Express Root Bridge
Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
Method (_CRS, 0, Serialized) { // Root complex resources (windows) }
Device (RES0) {
Name (_HID, "HISI0081") // HiSi PCIe RC config base address
Name (_CID, "PNP0C02") // Motherboard reserved resource
Name (_CRS, ResourceTemplate () { ... }
}
}
That's a possibility. The PCI Firmware Spec suggests putting RES0 at
the root (under \_SB), but I don't know why.
Putting it at the root means we couldn't generically associate it with
a bridge, although I could imagine something like this:
Device (RES1) {
Name (_HID, "HISI0081") // HiSi PCIe RC config base address
Name (_CID, "PNP0C02") // Motherboard reserved resource
Name (_CRS, ResourceTemplate () { ... }
Method (BRDG) { "PCI1" } // hand-wavy ASL
}
Device (PCI1) {
Name (_HID, "HISI0080") // PCI Express Root Bridge
Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
Method (_CRS, 0, Serialized) { // Root complex resources (windows) }
}
Where you could search PNP0C02 devices for a cookie that matched the
host bridge.
> > If we need to work around firmware in the field that doesn't do that,
> > one possibility is a PNP quirk along the lines of
> > quirk_amd_mmconfig_area().
>
> You mean matching PNP0C01/PNP0c02 and create a resource (that has to
> hardcoded in a static array in the kernel anyway, there is no way to
> retrieve it otherwise) in the corresponding PNP quirk handler ?
Right. On some hardware we can read the resource out of a
device-specific register, as we do in quirk_intel_mch(). But if
that's not possible, it would have to be hard-coded.
> And it is not a given we can match against PNP0c01/PNP0c02.
>
> So it looks like the only solution is allocating an _HID for each
> host bridge that is not ECAM compliant to add resources to its _CRS
> (unless the MCFG quirk does not need any additional data/resource,
> eg "use different set of PCI accessorsi 32-bit vs byte-access").
It doesn't matter whether it's ECAM-compliant or not. Any
memory-mapped config space should be reported via some device's _CRS.
The existing x86 practice is to use PNP0C02 devices for this purpose,
and I think we should just follow that practice.
> For FW that is immutable I really do not see what we can do apart
> from hardcoding the non-config resources (consumed by a bridge),
> somehow.
Right. Well, I assume you mean we should hard-code "non-window
resources consumed directly by a bridge". If firmware in the field is
broken, we should work around it, and that may mean hard-coding some
resources.
My point is that the hard-coding should not be buried in a driver
where it's invisible to the rest of the kernel. If we hard-code it in
a quirk that adds _CRS entries, then the kernel will work just like it
would if the firmware had been correct in the first place. The
resource will appear in /sys/devices/pnp*/*/resources and /proc/iomem,
and if we ever used _SRS to assign or move ACPI devices, we would know
to avoid the bridge resource.
Bjorn
next prev parent reply other threads:[~2016-09-21 18:05 UTC|newest]
Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-09 19:24 [PATCH V6 0/5] ECAM quirks handling for ARM64 platforms Tomasz Nowicki
2016-09-09 19:24 ` [PATCH V6 1/5] PCI/ACPI: Extend pci_mcfg_lookup() responsibilities Tomasz Nowicki
2016-09-09 19:24 ` [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Tomasz Nowicki
2016-09-12 22:24 ` Duc Dang
2016-09-12 22:47 ` Duc Dang
2016-09-13 5:58 ` Tomasz Nowicki
2016-09-13 6:37 ` Tomasz Nowicki
2016-09-13 2:36 ` Dongdong Liu
2016-09-13 6:32 ` Tomasz Nowicki
2016-09-13 11:38 ` Dongdong Liu
2016-09-14 12:40 ` Lorenzo Pieralisi
2016-09-15 10:58 ` Lorenzo Pieralisi
2016-09-16 9:02 ` Gabriele Paoloni
2016-09-16 12:27 ` Christopher Covington
2016-09-16 13:42 ` Gabriele Paoloni
2016-09-09 19:24 ` [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case Tomasz Nowicki
2016-09-19 18:09 ` Bjorn Helgaas
2016-09-20 7:23 ` Tomasz Nowicki
2016-09-20 13:33 ` Bjorn Helgaas
2016-09-20 13:40 ` Ard Biesheuvel
2016-09-20 14:05 ` Bjorn Helgaas
2016-09-20 15:09 ` Ard Biesheuvel
2016-09-20 19:17 ` Bjorn Helgaas
2016-09-21 14:05 ` Lorenzo Pieralisi
2016-09-21 18:04 ` Bjorn Helgaas [this message]
2016-09-21 18:58 ` Duc Dang
2016-09-21 19:18 ` Bjorn Helgaas
2016-09-23 10:53 ` Tomasz Nowicki
2016-09-22 9:49 ` Lorenzo Pieralisi
2016-09-22 11:10 ` Gabriele Paoloni
2016-09-22 12:44 ` Lorenzo Pieralisi
2016-09-22 18:31 ` Bjorn Helgaas
2016-09-22 22:10 ` Bjorn Helgaas
2016-09-23 10:11 ` Lorenzo Pieralisi
2016-09-23 10:58 ` Gabriele Paoloni
2017-09-14 14:06 ` Ard Biesheuvel
2017-09-26 8:23 ` Gabriele Paoloni
2016-09-22 14:20 ` Christopher Covington
2016-09-21 14:10 ` Gabriele Paoloni
2016-09-21 18:59 ` Bjorn Helgaas
2016-09-22 11:12 ` Gabriele Paoloni
2016-09-09 19:24 ` [PATCH V6 4/5] PCI: thunder: Enable ACPI PCI controller for ThunderX pass2.x silicon version Tomasz Nowicki
2016-09-19 15:45 ` Bjorn Helgaas
2016-09-20 7:06 ` Tomasz Nowicki
2016-09-20 13:08 ` Bjorn Helgaas
2016-09-21 8:05 ` Tomasz Nowicki
2016-09-09 19:24 ` [PATCH V6 5/5] PCI: thunder: Enable ACPI PCI controller for ThunderX pass1.x " Tomasz Nowicki
2016-09-09 19:30 ` [PATCH V6 0/5] ECAM quirks handling for ARM64 platforms Tomasz Nowicki
2016-09-20 19:26 ` Bjorn Helgaas
2016-09-21 1:15 ` cov
2016-09-21 13:11 ` Bjorn Helgaas
2016-09-21 14:07 ` Sinan Kaya
2016-09-21 17:31 ` Bjorn Helgaas
2016-09-21 17:34 ` Sinan Kaya
2016-09-21 22:38 ` [PATCHv2] PCI: QDF2432 32 bit config space accessors Christopher Covington
2016-10-31 21:48 ` Bjorn Helgaas
2016-11-01 13:06 ` cov
2016-11-02 16:08 ` Bjorn Helgaas
2016-11-02 16:36 ` Sinan Kaya
2016-11-03 14:00 ` Bjorn Helgaas
2016-11-03 16:58 ` Sinan Kaya
2016-11-03 17:06 ` Sinan Kaya
2016-11-03 20:43 ` Bjorn Helgaas
2016-11-03 23:49 ` Sinan Kaya
2016-12-02 4:58 ` Jon Masters
2016-11-02 16:41 ` Bjorn Helgaas
2016-11-09 19:25 ` Christopher Covington
2016-11-09 20:06 ` Bjorn Helgaas
2016-11-09 20:29 ` Ard Biesheuvel
2016-11-09 22:49 ` Bjorn Helgaas
2016-11-10 10:25 ` Ard Biesheuvel
2016-11-10 13:57 ` Lorenzo Pieralisi
2016-11-10 17:42 ` Bjorn Helgaas
2016-12-02 5:12 ` Jon Masters
2016-09-21 22:40 ` [PATCH V6 0/5] ECAM quirks handling for ARM64 platforms Christopher Covington
2016-09-22 23:08 ` Bjorn Helgaas
2016-09-23 18:41 ` Christopher Covington
2016-09-23 19:17 ` Bjorn Helgaas
2016-09-23 19:22 ` Christopher Covington
2016-11-24 11:05 ` [PATCH V6 1/1] ARM64/PCI: Manage controller-specific information on the host controller basis Tomasz Nowicki
2016-11-29 23:40 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160921180457.GB20006@localhost \
--to=helgaas@kernel.org \
--cc=Liviu.Dudau@arm.com \
--cc=andrea.gallo@linaro.org \
--cc=ard.biesheuvel@linaro.org \
--cc=arnd@arndb.de \
--cc=catalin.marinas@arm.com \
--cc=cov@codeaurora.org \
--cc=ddaney@caviumnetworks.com \
--cc=dhdang@apm.com \
--cc=gabriele.paoloni@huawei.com \
--cc=hanjun.guo@linaro.org \
--cc=jchandra@broadcom.com \
--cc=jcm@redhat.com \
--cc=jeremy.linton@arm.com \
--cc=jhugo@codeaurora.org \
--cc=linaro-acpi@lists.linaro.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=liudongdong3@huawei.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=msalter@redhat.com \
--cc=mw@semihalf.com \
--cc=okaya@codeaurora.org \
--cc=rafael@kernel.org \
--cc=rjw@rjwysocki.net \
--cc=robert.richter@caviumnetworks.com \
--cc=tn@semihalf.com \
--cc=wangyijing@huawei.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).