linux-kernel - Re: [RFC 1/2] arm64: PCI: Allow use arch-specific pci sysdata

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a2HVMiqJWc3kGi9j1CNNzVT5OFaeZaXxpAY42yu8Q-hvQ@mail.gmail.com>
Date:   Sat, 20 Mar 2021 13:52:10 +0100
From:   Arnd Bergmann <arnd@...nel.org>
To:     Boqun Feng <boqun.feng@...il.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux on Hyper-V List <linux-hyperv@...r.kernel.org>,
        linux-pci <linux-pci@...r.kernel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Rob Herring <robh@...nel.org>, Clint Sbisa <csbisa@...zon.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Sunil Muthuswamy <sunilmut@...rosoft.com>
Subject: Re: [RFC 1/2] arm64: PCI: Allow use arch-specific pci sysdata

On Fri, Mar 19, 2021 at 5:22 PM Boqun Feng <boqun.feng@...il.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@...rosoft.com>
> Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@...il.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