archive mirror
 help / color / mirror / Atom feed
From: Vikram Sethi <>
To: Will Deacon <>,
	Lorenzo Pieralisi <>,
	<>, <>
Cc: Jon Masters <>,
	Jeremy Linton <>, <>,
	<>, <>,
	<>, <>,
	<>, <>,
	<>, <>
Subject: Re: [PATCH] arm64: PCI: Enable SMC conduit
Date: Tue, 26 Jan 2021 11:08:31 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20210122194829.GE25471@willie-the-truck>

Hi Will, Lorenzo, Bjorn,

On 1/22/2021 1:48 PM, Will Deacon wrote:
> Hi Lorenzo,
> On Fri, Jan 08, 2021 at 10:32:16AM +0000, Lorenzo Pieralisi wrote:
>> On Thu, Jan 07, 2021 at 04:05:48PM -0500, Jon Masters wrote:
>>> On 1/7/21 1:14 PM, Will Deacon wrote:
>>>> On Mon, Jan 04, 2021 at 10:57:35PM -0600, Jeremy Linton wrote:
>>>>> Given that most arm64 platform's PCI implementations needs quirks
>>>>> to deal with problematic config accesses, this is a good place to
>>>>> apply a firmware abstraction. The ARM PCI SMMCCC spec details a
>>>>> standard SMC conduit designed to provide a simple PCI config
>>>>> accessor. This specification enhances the existing ACPI/PCI
>>>>> abstraction and expects power, config, etc functionality is handled
>>>>> by the platform. It also is very explicit that the resulting config
>>>>> space registers must behave as is specified by the pci specification.
>>>>> Lets hook the normal ACPI/PCI config path, and when we detect
>>>>> missing MADT data, attempt to probe the SMC conduit. If the conduit
>>>>> exists and responds for the requested segment number (provided by the
>>>>> ACPI namespace) attach a custom pci_ecam_ops which redirects
>>>>> all config read/write requests to the firmware.
>>>>> This patch is based on the Arm PCI Config space access document @
>>>> Why does firmware need to be involved with this at all? Can't we just
>>>> quirk Linux when these broken designs show up in production? We'll need
>>>> to modify Linux _anyway_ when the firmware interface isn't implemented
>>>> correctly...
>>> I agree with Will on this. I think we want to find a way to address some
>>> of the non-compliance concerns through quirks in Linux. However...
>> I understand the concern and if you are asking me if this can be fixed
>> in Linux it obviously can. The point is, at what cost for SW and
>> maintenance - in Linux and other OSes, I think Jeremy summed it up
>> pretty well:
>> The issue here is that what we are asked to support on ARM64 ACPI is a
>> moving target and the target carries PCI with it.
>> This potentially means that all drivers in:
>> drivers/pci/controller
>> may require an MCFG quirk and to implement it we may have to:
>> - Define new ACPI bindings (that may need AML and that's already a
>>   showstopper for some OSes)
>> - Require to manage clocks in the kernel (see link-up checks)
>> - Handle PCI config space faults in the kernel
>> Do we really want to do that ? I don't think so. Therefore we need
>> to have a policy to define what constitutes a "reasonable" quirk and
>> that's not objective I am afraid, however we slice it (there is no
>> such a thing as eg 90% ECAM).
> Without a doubt, I would much prefer to see these quirks and workarounds
> in Linux than hidden behind a firmware interface. Every single time.

In that case, can you please comment on/apply Tegra194 ECAM quirk that was rejected

a year ago, and was the reason we worked with Samer/ARM to define this common


The T194 ECAM is from widely used Root Port IP from a IP vendor. That is one reason so many

*existing* SOCs have ECAM quirks. ARM is only now working with the Root port IP vendors

to test ECAM, MSI etc, but the reality is there were deficiencies in industry IP that is widely

used. If this common quirk is not the way to go, then please apply the T194 specific quirk which was

NAK'd a year ago, or suggest how to improve that quirk.

The ECAM issue has been fixed on future Tegra chips and is validated preSilicon with BSA

tests, so it is not going to be a recurrent issue for us.

> This isn't like the usual fragmentation problems, where firmware swoops in
> to save the day; CPU onlining, spectre mitigations, early entropy etc. All
> of these problems exist because there isn't a standard method to implement
> them outside of firmware, and so adding a layer of abstraction there makes
> sense.
> But PCIe is already a standard!
> We shouldn't paper over hardware designers' inability to follow a ~20 year
> old standard by hiding it behind another standard that is hot off the press.
> Seriously.
> There is not a scrap of evidence to suggest that the firmware
> implementations will be any better, but they will certainly be harder to
> debug and maintain.  I have significant reservations about Arm's interest in
> maintaining the spec as both more errata appear and the PCIe spec evolves
> (after all, this is outside of SBSA, no?). The whole thing stinks of "if all
> you have is a hammer, then everything looks like a nail". But this isn't the
> sort of problem that is solved with yet another spec -- instead, how about
> encouraging vendors to read the specs that already exist?
>> The SMC is an olive branch and just to make sure it is crystal clear
>> there won't be room for adding quirks if the implementation turns out
>> to be broken, if a line in the sand is what we want here it is.
> I appreciate the sentiment, but you're not solving the problem here. You're
> moving it somewhere else. Somewhere where you don't have to deal with it
> (and I honestly can't blame you for that), but also somewhere where you
> _can't_ necessarily deal with it. The inevitable outcome is an endless
> succession of crappy, non-compliant machines which only appear to operate
> correctly with particularly kernel/firmware combinations. Imagine trying to
> use something like that?
> The approach championed here actively discourages vendors from building
> spec-compliant hardware and reduces our ability to work around problems
> on such hardware at the same time.
> So I won't be applying these patches, sorry.
> Will

  parent reply	other threads:[~2021-01-26 23:49 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-05  4:57 [PATCH] arm64: PCI: Enable SMC conduit Jeremy Linton
2021-01-07 15:28 ` Rob Herring
2021-01-07 16:23   ` Jeremy Linton
2021-01-07 17:36     ` Rob Herring
2021-01-07 19:45       ` Jeremy Linton
2021-01-07 20:35         ` Rob Herring
2021-01-07 18:14 ` Will Deacon
2021-01-07 19:18   ` Jeremy Linton
2021-01-07 21:05   ` Jon Masters
2021-01-07 21:49     ` Rob Herring
2021-01-08 10:32     ` Lorenzo Pieralisi
2021-01-22 19:48       ` Will Deacon
2021-01-26 16:46         ` Jeremy Linton
2021-01-26 22:54           ` Will Deacon
2021-01-28 18:50             ` Jeremy Linton
2021-01-28 23:31           ` Bjorn Helgaas
     [not found]             ` <>
2021-02-25  9:30               ` Lorenzo Pieralisi
2021-02-25 22:31                 ` Jeremy Linton
2021-01-26 17:08         ` Vikram Sethi [this message]
2021-01-26 22:53           ` Will Deacon
2021-03-25 13:12             ` Lorenzo Pieralisi
2021-03-25 20:45               ` Marcin Wojtas
2021-03-25 21:12                 ` Jon Masters
2021-03-26  9:27                   ` Marcin Wojtas
2021-06-16 17:36               ` Jason Gunthorpe
     [not found]                 ` <>
2021-06-18 14:05                   ` Jason Gunthorpe
2021-06-19 16:34                     ` Jon Masters
2021-06-19 16:38                       ` Jon Masters
2021-06-20  0:26                       ` Jason Gunthorpe
2021-06-18 15:10               ` Jeremy Linton
2021-01-12 16:16 ` Vidya Sagar
2021-01-12 16:57   ` Jeremy Linton

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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).