From: Arnd Bergmann <arnd@kernel.org> To: Boqun Feng <boqun.feng@gmail.com> Cc: Bjorn Helgaas <bhelgaas@google.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Linux on Hyper-V List <linux-hyperv@vger.kernel.org>, linux-pci <linux-pci@vger.kernel.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, "K. Y. Srinivasan" <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Stephen Hemminger <sthemmin@microsoft.com>, Wei Liu <wei.liu@kernel.org>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Rob Herring <robh@kernel.org>, Clint Sbisa <csbisa@amazon.com>, Ard Biesheuvel <ardb@kernel.org>, Sunil Muthuswamy <sunilmut@microsoft.com> Subject: Re: [RFC 1/2] arm64: PCI: Allow use arch-specific pci sysdata Date: Sat, 20 Mar 2021 13:52:10 +0100 [thread overview] Message-ID: <CAK8P3a2HVMiqJWc3kGi9j1CNNzVT5OFaeZaXxpAY42yu8Q-hvQ@mail.gmail.com> (raw) In-Reply-To: <20210319161956.2838291-2-boqun.feng@gmail.com> On Fri, Mar 19, 2021 at 5:22 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > Currently, if an architecture selects CONFIG_PCI_DOMAINS_GENERIC, the > ->sysdata in bus and bridge will be treated as struct pci_config_window, > which is created by generic ECAM using the data from acpi. > > However, for a virtualized PCI bus, there might be no enough data in of > or acpi table to create a pci_config_window. This is similar to the case > where CONFIG_PCI_DOMAINS_GENERIC=n, IOW, architectures use their own > structure for sysdata, so no apci table lookup is required. > > In order to enable Hyper-V's virtual PCI (which doesn't have acpi table > entry for PCI) on ARM64 (which selects CONFIG_PCI_DOMAINS_GENERIC), we > introduce arch-specific pci sysdata (similar to the one for x86) for > ARM64, and allow the core PCI code to detect the type of sysdata at the > runtime. The latter is achieved by adding a pci_ops::use_arch_sysdata > field. > > Originally-by: Sunil Muthuswamy <sunilmut@microsoft.com> > Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com> I think this takes it in the opposite direction of where it should be going. > --- > arch/arm64/include/asm/pci.h | 29 +++++++++++++++++++++++++++++ > arch/arm64/kernel/pci.c | 15 ++++++++++++--- > include/linux/pci.h | 3 +++ > 3 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h > index b33ca260e3c9..dade061a0658 100644 > --- a/arch/arm64/include/asm/pci.h > +++ b/arch/arm64/include/asm/pci.h > @@ -22,6 +22,16 @@ > > extern int isa_dma_bridge_buggy; > > +struct pci_sysdata { > + int domain; /* PCI domain */ > + int node; /* NUMA Node */ > +#ifdef CONFIG_ACPI > + struct acpi_device *companion; /* ACPI companion device */ > +#endif > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > + void *fwnode; /* IRQ domain for MSI assignment */ > +#endif > +}; I think none of these members belong into sysdata or architecture specific code. The fact that a pci_host_bridge belongs to a particular NUMA node or i associated with a firmware description is neither specific to a host bridge implementation nor a CPU instruction set! Moreover, you cannot assume that all PCI host bridges on any given architecture can share the pci_sysdata pointer, it is purely specific to the bridge driver. A good start would be to move the members (one at a time) into struct pci_host_bridge and out of the sysdata of individual host bridge drivers. > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c > index 1006ed2d7c60..63d420d57e63 100644 > --- a/arch/arm64/kernel/pci.c > +++ b/arch/arm64/kernel/pci.c > @@ -74,15 +74,24 @@ struct acpi_pci_generic_root_info { > int acpi_pci_bus_find_domain_nr(struct pci_bus *bus) > { > struct pci_config_window *cfg = bus->sysdata; > - struct acpi_device *adev = to_acpi_device(cfg->parent); > - struct acpi_pci_root *root = acpi_driver_data(adev); > + struct pci_sysdata *sd = bus->sysdata; > + struct acpi_device *adev; > + struct acpi_pci_root *root; There should be no reason to add even most code to this file, it should in fact become empty as it gets generalized more. Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@kernel.org> To: Boqun Feng <boqun.feng@gmail.com> Cc: Bjorn Helgaas <bhelgaas@google.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Linux on Hyper-V List <linux-hyperv@vger.kernel.org>, linux-pci <linux-pci@vger.kernel.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, "K. Y. Srinivasan" <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Stephen Hemminger <sthemmin@microsoft.com>, Wei Liu <wei.liu@kernel.org>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Rob Herring <robh@kernel.org>, Clint Sbisa <csbisa@amazon.com>, Ard Biesheuvel <ardb@kernel.org>, Sunil Muthuswamy <sunilmut@microsoft.com> Subject: Re: [RFC 1/2] arm64: PCI: Allow use arch-specific pci sysdata Date: Sat, 20 Mar 2021 13:52:10 +0100 [thread overview] Message-ID: <CAK8P3a2HVMiqJWc3kGi9j1CNNzVT5OFaeZaXxpAY42yu8Q-hvQ@mail.gmail.com> (raw) In-Reply-To: <20210319161956.2838291-2-boqun.feng@gmail.com> On Fri, Mar 19, 2021 at 5:22 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > Currently, if an architecture selects CONFIG_PCI_DOMAINS_GENERIC, the > ->sysdata in bus and bridge will be treated as struct pci_config_window, > which is created by generic ECAM using the data from acpi. > > However, for a virtualized PCI bus, there might be no enough data in of > or acpi table to create a pci_config_window. This is similar to the case > where CONFIG_PCI_DOMAINS_GENERIC=n, IOW, architectures use their own > structure for sysdata, so no apci table lookup is required. > > In order to enable Hyper-V's virtual PCI (which doesn't have acpi table > entry for PCI) on ARM64 (which selects CONFIG_PCI_DOMAINS_GENERIC), we > introduce arch-specific pci sysdata (similar to the one for x86) for > ARM64, and allow the core PCI code to detect the type of sysdata at the > runtime. The latter is achieved by adding a pci_ops::use_arch_sysdata > field. > > Originally-by: Sunil Muthuswamy <sunilmut@microsoft.com> > Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com> I think this takes it in the opposite direction of where it should be going. > --- > arch/arm64/include/asm/pci.h | 29 +++++++++++++++++++++++++++++ > arch/arm64/kernel/pci.c | 15 ++++++++++++--- > include/linux/pci.h | 3 +++ > 3 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h > index b33ca260e3c9..dade061a0658 100644 > --- a/arch/arm64/include/asm/pci.h > +++ b/arch/arm64/include/asm/pci.h > @@ -22,6 +22,16 @@ > > extern int isa_dma_bridge_buggy; > > +struct pci_sysdata { > + int domain; /* PCI domain */ > + int node; /* NUMA Node */ > +#ifdef CONFIG_ACPI > + struct acpi_device *companion; /* ACPI companion device */ > +#endif > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > + void *fwnode; /* IRQ domain for MSI assignment */ > +#endif > +}; I think none of these members belong into sysdata or architecture specific code. The fact that a pci_host_bridge belongs to a particular NUMA node or i associated with a firmware description is neither specific to a host bridge implementation nor a CPU instruction set! Moreover, you cannot assume that all PCI host bridges on any given architecture can share the pci_sysdata pointer, it is purely specific to the bridge driver. A good start would be to move the members (one at a time) into struct pci_host_bridge and out of the sysdata of individual host bridge drivers. > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c > index 1006ed2d7c60..63d420d57e63 100644 > --- a/arch/arm64/kernel/pci.c > +++ b/arch/arm64/kernel/pci.c > @@ -74,15 +74,24 @@ struct acpi_pci_generic_root_info { > int acpi_pci_bus_find_domain_nr(struct pci_bus *bus) > { > struct pci_config_window *cfg = bus->sysdata; > - struct acpi_device *adev = to_acpi_device(cfg->parent); > - struct acpi_pci_root *root = acpi_driver_data(adev); > + struct pci_sysdata *sd = bus->sysdata; > + struct acpi_device *adev; > + struct acpi_pci_root *root; There should be no reason to add even most code to this file, it should in fact become empty as it gets generalized more. Arnd _______________________________________________ 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:[~2021-03-20 12:53 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-19 16:19 [RFC 0/2] PCI: Introduce pci_ops::use_arch_sysdata Boqun Feng 2021-03-19 16:19 ` Boqun Feng 2021-03-19 16:19 ` [RFC 1/2] arm64: PCI: Allow use arch-specific pci sysdata Boqun Feng 2021-03-19 16:19 ` Boqun Feng 2021-03-19 21:12 ` Bjorn Helgaas 2021-03-19 21:12 ` Bjorn Helgaas 2021-03-20 12:54 ` Marc Zyngier 2021-03-20 12:54 ` Marc Zyngier 2021-03-20 13:03 ` Arnd Bergmann 2021-03-20 13:03 ` Arnd Bergmann 2021-03-20 13:23 ` Marc Zyngier 2021-03-20 13:23 ` Marc Zyngier 2021-03-20 14:24 ` Arnd Bergmann 2021-03-20 14:24 ` Arnd Bergmann 2021-03-20 17:14 ` Marc Zyngier 2021-03-20 17:14 ` Marc Zyngier 2021-03-20 12:54 ` Arnd Bergmann 2021-03-20 12:54 ` Arnd Bergmann 2021-03-20 16:09 ` Arnd Bergmann 2021-03-20 16:09 ` Arnd Bergmann 2021-03-29 14:32 ` Boqun Feng 2021-03-29 14:32 ` Boqun Feng 2021-03-29 14:43 ` Arnd Bergmann 2021-03-29 14:43 ` Arnd Bergmann 2021-03-20 12:52 ` Arnd Bergmann [this message] 2021-03-20 12:52 ` Arnd Bergmann 2021-03-19 16:19 ` [RFC 2/2] PCI: hv: Tell PCI core arch-specific sysdata is used Boqun Feng 2021-03-19 16:19 ` Boqun Feng 2021-03-19 19:04 ` [RFC 0/2] PCI: Introduce pci_ops::use_arch_sysdata Bjorn Helgaas 2021-03-19 19:04 ` Bjorn Helgaas
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=CAK8P3a2HVMiqJWc3kGi9j1CNNzVT5OFaeZaXxpAY42yu8Q-hvQ@mail.gmail.com \ --to=arnd@kernel.org \ --cc=ardb@kernel.org \ --cc=bhelgaas@google.com \ --cc=boqun.feng@gmail.com \ --cc=catalin.marinas@arm.com \ --cc=csbisa@amazon.com \ --cc=haiyangz@microsoft.com \ --cc=kys@microsoft.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-hyperv@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=robh@kernel.org \ --cc=sthemmin@microsoft.com \ --cc=sunilmut@microsoft.com \ --cc=wei.liu@kernel.org \ --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: 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.