Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Bjorn Helgaas <helgaas@kernel.org>, Sinan Kaya <okaya@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	linux-arm-kernel <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:42:05 +0200
Message-ID: <CAKv+Gu95pQ7_OfLbEXHZ_bhYnqOgTBKCmTgqUY27un-Y708BgQ@mail.gmail.com> (raw)
In-Reply-To: <e6c7854ae360be513f6f43729ed6d4052e289376.camel@kernel.crashing.org>

On Fri, 14 Jun 2019 at 01:07, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> 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 ?
>

The original purpose was for firmware running on 64-bit hosts to not
create a PCI resource assignment that was incompatible with the OS
booting in 32-bit mode.

So the expectation was that a 32-bit OS would reuse whatever config
the firmware created, and the 64-bit version would be enlightened to
understand that it could reassign resource assignments to make better
use of the available resource windows, but this required a mechanism
to describe which resources should be left alone by the OS.

So I don't think anybody cares about that use case anymore, and I have
no idea how widespread its use was when people 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 ? :-)
>

It would help if you could explain what exactly is wrong with your
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.
>

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13  7:54 Benjamin Herrenschmidt
2019-06-13 19:02 ` Bjorn Helgaas
2019-06-13 21:59   ` Benjamin Herrenschmidt
2019-06-13 23:07   ` Benjamin Herrenschmidt
2019-06-14  7:42     ` Ard Biesheuvel [this message]
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 publically 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=CAKv+Gu95pQ7_OfLbEXHZ_bhYnqOgTBKCmTgqUY27un-Y708BgQ@mail.gmail.com \
    --to=ard.biesheuvel@linaro.org \
    --cc=benh@kernel.crashing.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

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org linux-pci@archiver.kernel.org
	public-inbox-index linux-pci


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox