From: Benjamin Herrenschmidt <benh@kernel.crashing.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Bjorn Helgaas <helgaas@kernel.org>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Sinan Kaya <okaya@kernel.org>, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "Zilberman, Zeev" <zeev@amazon.com>, "Saidi, Ali" <alisaidi@amazon.com> Subject: Re: [PATCH/RESEND] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup Date: Wed, 12 Jun 2019 08:19:40 +1000 [thread overview] Message-ID: <5b5199b008d6c8831175018975f09599081dc5e4.camel@kernel.crashing.org> (raw) In-Reply-To: <20190611145832.GB11736@redmoon> On Tue, 2019-06-11 at 15:58 +0100, Lorenzo Pieralisi wrote: > > > if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) { > /* preserve existing resource assignment */ > pci_bus_claim_resources(bus); > } > > pci_bus_size_bridges(bus); > pci_bus_assign_resources(bus); So that makes me nervous... my understanding is that the pair pci_bus_size_bridges(bus); pci_bus_assign_resources(bus); Is intended for full reassignment. Now they will try to skip resources that already have a parent, but that's yet another code path. What's wrong with pci_unassigned_* ? That's what it's meant for... > That's how it should be I think: > > 1) we do not want pci_assign_unassigned_root_bus_resources(bus) to > reallocate resources already claimed (see realloc parameter), do we ? Well, realloc is useful to handle SR_IOV when the BIOS doesn't do it right (common case). That said, at this point, we should be able to honor IORESOURCE_PCI_FIXED for things that have _DSM #5 since they have been claimed. I don't see that realloc logic being a problem for us, and I want to avoid gratuitous differences with x86, but maybe I'm missing something here... > 2) pci_bus_size_bridges(bus) and pci_bus_assign_resources(bus) should > not interfere with resources already claimed so it *should* be safe > to call them anyway Sure, *should* and here we introduce yet another way of doing things though... Any reason we don't want to do what x86 does here ? > Most importantly: I want everyone to agree that claiming is equivalent > to making a resource immutable (except for realloc, see (1) above) > because that's what we are doing by claiming on _DSM #5 == 0. I think the combination of claiming *and* IORESOURCE_PCI_FIXED is what makes it *really* immutable. I'm a bit confused by the realloc logic right now, I'll need more quality time looking at it after ingesting more caffeing but I'm under the impression that it will honor the flag. > There are too many ways to make a resource immutable in the kernel > and this is confusing and prone to bugs. It is, but I don't want to create new ways of doing things, and what you seem to propose is a new way imho :-) Cheers, Ben. > Thanks, > Lorenzo > > > + ACPI_FREE(obj); > > > > list_for_each_entry(child, &bus->children, node) > > pcie_bus_configure_settings(child); > > diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h > > index 8082b612f561..62b7fdcc661c 100644 > > --- a/include/linux/pci-acpi.h > > +++ b/include/linux/pci-acpi.h > > @@ -107,9 +107,10 @@ static inline void acpiphp_check_host_bridge(struct acpi_device *adev) { } > > #endif > > > > extern const guid_t pci_acpi_dsm_guid; > > -#define DEVICE_LABEL_DSM 0x07 > > -#define RESET_DELAY_DSM 0x08 > > -#define FUNCTION_DELAY_DSM 0x09 > > +#define IGNORE_PCI_BOOT_CONFIG_DSM 0x05 > > +#define DEVICE_LABEL_DSM 0x07 > > +#define RESET_DELAY_DSM 0x08 > > +#define FUNCTION_DELAY_DSM 0x09 > > > > #else /* CONFIG_ACPI */ > > static inline void acpi_pci_add_bus(struct pci_bus *bus) { } > > > >
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>, linux-pci@vger.kernel.org, Sinan Kaya <okaya@kernel.org>, "Zilberman, Zeev" <zeev@amazon.com>, "Saidi, Ali" <alisaidi@amazon.com>, Bjorn Helgaas <helgaas@kernel.org>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH/RESEND] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup Date: Wed, 12 Jun 2019 08:19:40 +1000 [thread overview] Message-ID: <5b5199b008d6c8831175018975f09599081dc5e4.camel@kernel.crashing.org> (raw) In-Reply-To: <20190611145832.GB11736@redmoon> On Tue, 2019-06-11 at 15:58 +0100, Lorenzo Pieralisi wrote: > > > if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) { > /* preserve existing resource assignment */ > pci_bus_claim_resources(bus); > } > > pci_bus_size_bridges(bus); > pci_bus_assign_resources(bus); So that makes me nervous... my understanding is that the pair pci_bus_size_bridges(bus); pci_bus_assign_resources(bus); Is intended for full reassignment. Now they will try to skip resources that already have a parent, but that's yet another code path. What's wrong with pci_unassigned_* ? That's what it's meant for... > That's how it should be I think: > > 1) we do not want pci_assign_unassigned_root_bus_resources(bus) to > reallocate resources already claimed (see realloc parameter), do we ? Well, realloc is useful to handle SR_IOV when the BIOS doesn't do it right (common case). That said, at this point, we should be able to honor IORESOURCE_PCI_FIXED for things that have _DSM #5 since they have been claimed. I don't see that realloc logic being a problem for us, and I want to avoid gratuitous differences with x86, but maybe I'm missing something here... > 2) pci_bus_size_bridges(bus) and pci_bus_assign_resources(bus) should > not interfere with resources already claimed so it *should* be safe > to call them anyway Sure, *should* and here we introduce yet another way of doing things though... Any reason we don't want to do what x86 does here ? > Most importantly: I want everyone to agree that claiming is equivalent > to making a resource immutable (except for realloc, see (1) above) > because that's what we are doing by claiming on _DSM #5 == 0. I think the combination of claiming *and* IORESOURCE_PCI_FIXED is what makes it *really* immutable. I'm a bit confused by the realloc logic right now, I'll need more quality time looking at it after ingesting more caffeing but I'm under the impression that it will honor the flag. > There are too many ways to make a resource immutable in the kernel > and this is confusing and prone to bugs. It is, but I don't want to create new ways of doing things, and what you seem to propose is a new way imho :-) Cheers, Ben. > Thanks, > Lorenzo > > > + ACPI_FREE(obj); > > > > list_for_each_entry(child, &bus->children, node) > > pcie_bus_configure_settings(child); > > diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h > > index 8082b612f561..62b7fdcc661c 100644 > > --- a/include/linux/pci-acpi.h > > +++ b/include/linux/pci-acpi.h > > @@ -107,9 +107,10 @@ static inline void acpiphp_check_host_bridge(struct acpi_device *adev) { } > > #endif > > > > extern const guid_t pci_acpi_dsm_guid; > > -#define DEVICE_LABEL_DSM 0x07 > > -#define RESET_DELAY_DSM 0x08 > > -#define FUNCTION_DELAY_DSM 0x09 > > +#define IGNORE_PCI_BOOT_CONFIG_DSM 0x05 > > +#define DEVICE_LABEL_DSM 0x07 > > +#define RESET_DELAY_DSM 0x08 > > +#define FUNCTION_DELAY_DSM 0x09 > > > > #else /* CONFIG_ACPI */ > > static inline void acpi_pci_add_bus(struct pci_bus *bus) { } > > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-06-11 22:19 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-03 23:41 [RFC] ARM64 PCI resource survey issue(s) Benjamin Herrenschmidt 2019-06-03 23:41 ` Benjamin Herrenschmidt 2019-06-04 1:49 ` Bjorn Helgaas 2019-06-04 1:49 ` Bjorn Helgaas 2019-06-04 3:32 ` Benjamin Herrenschmidt 2019-06-04 3:32 ` Benjamin Herrenschmidt 2019-06-04 3:37 ` Benjamin Herrenschmidt 2019-06-04 3:37 ` Benjamin Herrenschmidt 2019-06-04 6:56 ` Benjamin Herrenschmidt 2019-06-04 6:56 ` Benjamin Herrenschmidt 2019-06-04 12:49 ` Bjorn Helgaas 2019-06-04 12:49 ` Bjorn Helgaas 2019-06-04 20:41 ` Benjamin Herrenschmidt 2019-06-04 20:41 ` Benjamin Herrenschmidt 2019-06-06 9:00 ` [PATCH/RESEND] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup Benjamin Herrenschmidt 2019-06-06 9:00 ` Benjamin Herrenschmidt 2019-06-06 9:13 ` Ard Biesheuvel 2019-06-06 9:13 ` Ard Biesheuvel 2019-06-06 10:55 ` Benjamin Herrenschmidt 2019-06-06 10:55 ` Benjamin Herrenschmidt 2019-06-11 14:31 ` Lorenzo Pieralisi 2019-06-11 14:31 ` Lorenzo Pieralisi 2019-06-11 22:09 ` Benjamin Herrenschmidt 2019-06-11 22:09 ` Benjamin Herrenschmidt 2019-06-11 22:34 ` Ard Biesheuvel 2019-06-11 22:34 ` Ard Biesheuvel 2019-06-11 22:40 ` Benjamin Herrenschmidt 2019-06-11 22:40 ` Benjamin Herrenschmidt 2019-06-12 10:21 ` Lorenzo Pieralisi 2019-06-12 10:21 ` Lorenzo Pieralisi 2019-06-12 22:05 ` Benjamin Herrenschmidt 2019-06-12 22:05 ` Benjamin Herrenschmidt 2019-06-11 14:58 ` Lorenzo Pieralisi 2019-06-11 14:58 ` Lorenzo Pieralisi 2019-06-11 22:19 ` Benjamin Herrenschmidt [this message] 2019-06-11 22:19 ` Benjamin Herrenschmidt 2019-06-12 10:08 ` Lorenzo Pieralisi 2019-06-12 10:08 ` Lorenzo Pieralisi 2019-06-12 10:58 ` Benjamin Herrenschmidt 2019-06-12 10:58 ` Benjamin Herrenschmidt 2019-06-11 23:39 ` Bjorn Helgaas 2019-06-11 23:39 ` Bjorn Helgaas 2019-06-12 0:06 ` Benjamin Herrenschmidt 2019-06-12 0:06 ` Benjamin Herrenschmidt 2019-06-12 13:27 ` Bjorn Helgaas 2019-06-12 13:27 ` Bjorn Helgaas 2019-06-12 21:46 ` Benjamin Herrenschmidt 2019-06-12 21:46 ` Benjamin Herrenschmidt 2019-06-12 23:58 ` Benjamin Herrenschmidt 2019-06-12 23:58 ` Benjamin Herrenschmidt 2019-06-10 10:11 ` [RFC] ARM64 PCI resource survey issue(s) Lorenzo Pieralisi 2019-06-10 10:11 ` Lorenzo Pieralisi 2019-06-11 5:46 ` Benjamin Herrenschmidt 2019-06-11 5: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=5b5199b008d6c8831175018975f09599081dc5e4.camel@kernel.crashing.org \ --to=benh@kernel.crashing.org \ --cc=alisaidi@amazon.com \ --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 \ --cc=zeev@amazon.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.