From: Benjamin Herrenschmidt <email@example.com> To: Bjorn Helgaas <firstname.lastname@example.org> Cc: Ard Biesheuvel <email@example.com>, Sinan Kaya <firstname.lastname@example.org>, Lorenzo Pieralisi <email@example.com>, firstname.lastname@example.org, email@example.com 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 Message-ID: <firstname.lastname@example.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.
next prev parent 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 [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 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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 \ email@example.com firstname.lastname@example.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