linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Jeremy Linton <jeremy.linton@arm.com>
To: Will Deacon <will@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: mark.rutland@arm.com, Jon Masters <jcm@jonmasters.org>,
	linux-pci@vger.kernel.org, sudeep.holla@arm.com,
	linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
	bhelgaas@google.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: PCI: Enable SMC conduit
Date: Tue, 26 Jan 2021 10:46:04 -0600	[thread overview]
Message-ID: <4c2db08d-ccc4-05eb-6b7b-5fd7d07dd11e@arm.com> (raw)
In-Reply-To: <20210122194829.GE25471@willie-the-truck>

Hi,

On 1/22/21 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 @
>>>>> https://developer.arm.com/documentation/den0115/latest
>>>>
>>>> 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:
>>
>> https://lore.kernel.org/linux-pci/61558f73-9ac8-69fe-34c1-2074dec5f18a@arm.com
>>
>> 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.
> 
> 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.

There are a lot of parallels with PSCI here because there were existing 
standards for cpu online.

> 
> But PCIe is already a standard!

And it says that ECAM is optional, particularly if there are 
firmware/platform standardized ways of accessing the config space.

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

No disagreement, but its been more than half a decade and there are some 
high (millions!) volume parts, that still don't have kernel support.

> 
> 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?

PSCI, isn't a good example of a firmware interface that works?

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

Does that mean its open season for ECAM quirks, and we can expect them 
to start being merged now?

Thanks.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-01-26 16:47 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 [this message]
2021-01-26 22:54           ` Will Deacon
2021-01-28 18:50             ` Jeremy Linton
2021-01-28 23:31           ` Bjorn Helgaas
     [not found]             ` <CACCGGCc3zULqHgUh3Q9wA5WtPBnQ4eq_v2+1qA8bOBCQZJ5YoQ@mail.gmail.com>
2021-02-25  9:30               ` Lorenzo Pieralisi
2021-02-25 22:31                 ` Jeremy Linton
2021-01-26 17:08         ` Vikram Sethi
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]                 ` <CA+kK7ZijdNERQSauEvAffR7JLbfZ512na2-9cJrU0vFbNnDGwQ@mail.gmail.com>
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:
  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=4c2db08d-ccc4-05eb-6b7b-5fd7d07dd11e@arm.com \
    --to=jeremy.linton@arm.com \
    --cc=bhelgaas@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=jcm@jonmasters.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=sudeep.holla@arm.com \
    --cc=will@kernel.org \
    /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).