linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Sinan Kaya <okaya@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH v2] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup
Date: Fri, 14 Jun 2019 09:07:35 +1000	[thread overview]
Message-ID: <e6c7854ae360be513f6f43729ed6d4052e289376.camel@kernel.crashing.org> (raw)
In-Reply-To: <20190613190248.GH13533@google.com>

On Thu, 2019-06-13 at 14:02 -0500, Bjorn Helgaas wrote:
> 
> PCI FW r3.2 says 0 means "the OS must not ignore config done by
> firmware."  That means we must keep the FW configuration intact.

So I tried implementing what you seem to want and it doesn't make sense
imho.

I think the wording of the spec is terrible and doesn't convey the
intent here.

What is it that _DSM #5 is about ? Is it about telling us that the FW
config shall not be trusted ? That seem to be its original intent based
on the existing wording and the fact that "1" says "may ignore".

Or was it always intended to be some kind of inverted logic with 0
meaning that we *must* preserve what FW did ?

But preserving what FW did was always the default for x86 and
Windows... It's just that we happen to do something wrong today on
Linux/ARM64 which is to always reassign everything.

The way Linux resource assignment works accross platforms has generally
been based on one of these 3 methods:

 - The standard x86 method, which is to claim what's there when it
doesn't look completely busted and has been assigned, then assign
what's left. This allows for FW doing partial assignment, and allows to
work around a number of BIOS bugs.

 - The "probe only" method. This was created independently on powerpc
and some other archs afaik. At least for powerpc, the reason for that
is some interesting virtualization cases where we just cannot touch or
change or move anything. The effect is to not reassign even what we
dont like, and not call pci_assign_unassign_resources().

 - The "reassign everything" method. This is used by almost all
embedded patforms accross archs. All arm32, all arm64 today (but we
agree that's wrong), all embedded powerpc etc... This is basically
meant for us not trusting whatever random uboot or other embedded FW,
if any, and do a full from-scratch assignment. There are issues in how
that is implemented accross the various platforms/archs, some for
example still honor existing bus numbers and some don't, but I doubt
it's intentional etc... but that method is there to stay.

Now, the questions are two fold

  - How do we map _DSM #5 to these, at least on arm64, maybe in the
long run we can also look at affecting x86, but that's less urgent.

  - How do I ensure the above fixes my Amazon platform ? :-)

There's one obvious thing here. If we don't want to break existing
things, then the absence of _DSM #5 must not change our existing
behaviour. I think we can all agree on this.

We might explore changing arm64 default behaviour as a second step
since we all agree it's not doing what it should, but we also know that
it will probably break some things so we need to be careful, understand
the issues and work around them. This isn't the scope of the initial
_DSM #5 patch.

That leaves us with the _DSM #5 present cases.

Now we have two values. What do they mean ? As I already said before,
the wording with "must not ignore" and "may ignore" is completely
useless and doesn't tell us a thing about the intention here. We don't
know why the FW folks may have put a given value here, and what they
expect us to do about it.

What we do know is that Seattle returns 1 and needs reassignment, and
we do know that the Amazon platforms return 0 and will want us to not
touch the existing setup.

However, does 1 means "business as usual" or does it mean "reassign
everything" ?

Does 0 means "probe only" ? Or do we still do an assignment pass for
things that the FW may have left unassigned ?

Today in Linux, the "probe only" logic tends to not call
pci_assign_unassigned_resources for example.

From a pure reading of the spec, there's an argument to be made that
both 0 and 1 values can lead to the same code that reads what's there
and reassign what's missing.

So this is a mess, a usual when it comes to specs written by
committees, but at this stage I'm at a loss as to what you want me to
do.

Cheers,
Ben.
 


  parent reply	other threads:[~2019-06-13 23:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13  7:54 [RFC PATCH v2] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup Benjamin Herrenschmidt
2019-06-13 19:02 ` Bjorn Helgaas
2019-06-13 21:59   ` Benjamin Herrenschmidt
2019-06-13 23:07   ` Benjamin Herrenschmidt [this message]
2019-06-14  7:42     ` Ard Biesheuvel
2019-06-14  8:36       ` Benjamin Herrenschmidt
2019-06-14  9:57         ` Lorenzo Pieralisi
2019-06-14 10:36           ` Benjamin Herrenschmidt
2019-06-14 10:43             ` Benjamin Herrenschmidt
2019-06-14 13:12               ` Bjorn Helgaas
2019-06-14 13:48                 ` Benjamin Herrenschmidt
2019-06-14 20:13                   ` Bjorn Helgaas
2019-06-15  1:18                     ` Benjamin Herrenschmidt
2019-06-14 13:09         ` Bjorn Helgaas
2019-06-14 13:46           ` Benjamin Herrenschmidt

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=e6c7854ae360be513f6f43729ed6d4052e289376.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=helgaas@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=okaya@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).